Merge pull request #505 from glimmerjs/maybe-fix

[BUGFIX #484] ensure transactions are always.

Author: stefanpenner

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 {

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();
+});