From 6defb3a6c2e6c56da1912793f39fd580b020f370 Mon Sep 17 00:00:00 2001
From: Stephen Sawchuk <sawchuk@gmail.com>
Date: Sat, 27 Sep 2014 12:09:16 -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  |  6 +-----
 test/datastore/transaction.js | 40 ++++++++++++++++++++++++-----------
 2 files changed, 29 insertions(+), 17 deletions(-)

diff --git a/lib/datastore/transaction.js b/lib/datastore/transaction.js
index db9db825a9d..30816ec5ae7 100644
--- a/lib/datastore/transaction.js
+++ b/lib/datastore/transaction.js
@@ -124,12 +124,8 @@ Transaction.prototype.rollback = function(callback) {
   var req = new pb.RollbackRequest({ transaction: this.id });
   var res = pb.RollbackResponse;
   this.makeReq('rollback', req, res, function(err) {
-    if (err) {
-      callback(err);
-      return;
-    }
     that.isFinalized = true;
-    callback(null);
+    callback(err || null);
   });
 };
 
diff --git a/test/datastore/transaction.js b/test/datastore/transaction.js
index c273c7a1e1d..c059c79354e 100644
--- a/test/datastore/transaction.js
+++ b/test/datastore/transaction.js
@@ -38,18 +38,34 @@ 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) {
+      transaction.makeReq = function(method, proto, respType, callback) {
+        var base64Id = new Buffer(transaction.id).toString('base64');
+        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();
+      });
     });
   });