From 34e200986e8a3dd16c2828913e6f721feed23a07 Mon Sep 17 00:00:00 2001
From: Stefan Penner <stefan.penner@gmail.com>
Date: Thu, 25 May 2017 10:27:33 -0700
Subject: [PATCH] [BUGFIX #484] Improve NestedTransaction handling * ensure
 running `commit` always clears the current transaction * if we begin a
 transaction with an existing transaction warn, commit it, then start begin
 the next.

---
 packages/@glimmer/runtime/lib/environment.ts |  5 ++--
 packages/@glimmer/runtime/test/env-test.ts   | 31 ++++++++++++++++++++
 2 files changed, 34 insertions(+), 2 deletions(-)
 create mode 100644 packages/@glimmer/runtime/test/env-test.ts

diff --git a/packages/@glimmer/runtime/lib/environment.ts b/packages/@glimmer/runtime/lib/environment.ts
index bacfd64067..90a7f395c1 100644
--- a/packages/@glimmer/runtime/lib/environment.ts
+++ b/packages/@glimmer/runtime/lib/environment.ts
@@ -330,7 +330,7 @@ export abstract class Environment {
   }
 
   begin() {
-    assert(!this._transaction, 'Cannot start a nested transaction');
+    assert(!this._transaction, 'a glimmer transaction was begun, but one already exists. You may have a nested transaction');
     this._transaction = new Transaction();
   }
 
@@ -359,8 +359,9 @@ export abstract class Environment {
   }
 
   commit() {
-    this.transaction.commit();
+    let transaction = this.transaction;
     this._transaction = null;
+    transaction.commit();
   }
 
   attributeFor(element: Simple.Element, attr: string, isTrusting: boolean, namespace?: string): AttributeManager {
diff --git a/packages/@glimmer/runtime/test/env-test.ts b/packages/@glimmer/runtime/test/env-test.ts
new file mode 100644
index 0000000000..924a903845
--- /dev/null
+++ b/packages/@glimmer/runtime/test/env-test.ts
@@ -0,0 +1,31 @@
+import { TestEnvironment } from "@glimmer/test-helpers";
+
+QUnit.module('env');
+
+QUnit.test('assert against nested transactions', assert => {
+  let env = new TestEnvironment();
+  env.begin();
+  assert.throws(() => env.begin(), 'a glimmer transaction was begun, but one already exists. You may have a nested transaction');
+});
+
+QUnit.test('ensure commit cleans up when it can', assert => {
+  let env = new TestEnvironment();
+  env.begin();
+
+  // ghetto stub
+  Object.defineProperty(env, 'transaction', {
+    get() {
+      return {
+        scheduledInstallManagers() : void { },
+        commit() : void {
+          throw new Error('something failed');
+        }
+      };
+    }
+  });
+
+  assert.throws(() => env.commit(), 'something failed'); // commit failed
+
+  // but a previous commit failing, does not cause a future transaction to fail to start
+  env.begin();
+});