Skip to content

Commit

Permalink
datastore: mark failed rollbacks as finalized
Browse files Browse the repository at this point in the history
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
  • Loading branch information
stephenplusplus committed Sep 23, 2014
1 parent 50704b7 commit 56efd43
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 13 deletions.
2 changes: 1 addition & 1 deletion lib/datastore/transaction.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
};
Expand Down
41 changes: 29 additions & 12 deletions test/datastore/transaction.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
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();
});
});
});


it('should commit', function(done) {
transaction.id = 'some-id';
transaction.makeReq = function(method, proto, respType, callback) {
Expand Down

0 comments on commit 56efd43

Please sign in to comment.