From da2eaf984743d9adfdfe1d6e1c24df1aeaf51d19 Mon Sep 17 00:00:00 2001
From: Stephen Sawchuk <sawchuk@gmail.com>
Date: Tue, 23 Sep 2014 09:56:17 -0400
Subject: [PATCH] datastore: mark failed rollbacks as finalized

If an error occurs upstream while trying to rollback a
transaction, we can mark the transaction as `finalized`
regardless. By marking it `finalized`, we don't then
later make an accidental commit when `done` is
executed.

Fixes #237
---
 lib/datastore/transaction.js  |  2 +-
 test/datastore/transaction.js | 41 +++++++++++++++++++++++++----------
 2 files changed, 30 insertions(+), 13 deletions(-)

diff --git a/lib/datastore/transaction.js b/lib/datastore/transaction.js
index db9db825a9d2..52adc8f93509 100644
--- a/lib/datastore/transaction.js
+++ b/lib/datastore/transaction.js
@@ -124,11 +124,11 @@ Transaction.prototype.rollback = function(callback) {
   var req = new pb.RollbackRequest({ transaction: this.id });
   var res = pb.RollbackResponse;
   this.makeReq('rollback', req, res, function(err) {
+    that.isFinalized = true;
     if (err) {
       callback(err);
       return;
     }
-    that.isFinalized = true;
     callback(null);
   });
 };
diff --git a/test/datastore/transaction.js b/test/datastore/transaction.js
index c273c7a1e1de..7d44cd6349dc 100644
--- a/test/datastore/transaction.js
+++ b/test/datastore/transaction.js
@@ -38,21 +38,38 @@ describe('Transaction', function() {
     transaction.begin(done);
   });
 
-  it('should rollback', function(done) {
-    transaction.id = 'some-id';
-    transaction.makeReq = function(method, proto, respType, callback) {
-      assert.equal(method, 'rollback');
-      assert.equal(
-        proto.transaction.toBase64(),
-        new Buffer('some-id').toString('base64'));
-      callback();
-    };
-    transaction.rollback(function() {
-      assert.equal(transaction.isFinalized, true);
-      done();
+  describe('rollback', function() {
+    beforeEach(function() {
+      transaction.id = 'transaction-id';
+    });
+
+    it('should rollback', function(done) {
+      var base64id = new Buffer(transaction.id).toString('base64');
+      transaction.makeReq = function(method, proto, respType, callback) {
+        assert.equal(method, 'rollback');
+        assert.equal(proto.transaction.toBase64(), base64id);
+        callback();
+      };
+      transaction.rollback(function() {
+        assert.equal(transaction.isFinalized, true);
+        done();
+      });
+    });
+
+    it('should mark as `finalized` when rollback errors', function(done) {
+      var error = new Error('rollback error');
+      transaction.makeReq = function(method, proto, respType, callback) {
+       callback(error);
+      };
+      transaction.rollback(function(err) {
+        assert.equal(err, error);
+        assert.equal(transaction.isFinalized, true);
+        done();
+      });
     });
   });
 
+
   it('should commit', function(done) {
     transaction.id = 'some-id';
     transaction.makeReq = function(method, proto, respType, callback) {