Skip to content

Commit 6defb3a

Browse files
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
1 parent 50704b7 commit 6defb3a

File tree

2 files changed

+29
-17
lines changed

2 files changed

+29
-17
lines changed

lib/datastore/transaction.js

+1-5
Original file line numberDiff line numberDiff line change
@@ -124,12 +124,8 @@ Transaction.prototype.rollback = function(callback) {
124124
var req = new pb.RollbackRequest({ transaction: this.id });
125125
var res = pb.RollbackResponse;
126126
this.makeReq('rollback', req, res, function(err) {
127-
if (err) {
128-
callback(err);
129-
return;
130-
}
131127
that.isFinalized = true;
132-
callback(null);
128+
callback(err || null);
133129
});
134130
};
135131

test/datastore/transaction.js

+28-12
Original file line numberDiff line numberDiff line change
@@ -38,18 +38,34 @@ describe('Transaction', function() {
3838
transaction.begin(done);
3939
});
4040

41-
it('should rollback', function(done) {
42-
transaction.id = 'some-id';
43-
transaction.makeReq = function(method, proto, respType, callback) {
44-
assert.equal(method, 'rollback');
45-
assert.equal(
46-
proto.transaction.toBase64(),
47-
new Buffer('some-id').toString('base64'));
48-
callback();
49-
};
50-
transaction.rollback(function() {
51-
assert.equal(transaction.isFinalized, true);
52-
done();
41+
describe('rollback', function() {
42+
beforeEach(function() {
43+
transaction.id = 'transaction-id';
44+
});
45+
46+
it('should rollback', function(done) {
47+
transaction.makeReq = function(method, proto, respType, callback) {
48+
var base64Id = new Buffer(transaction.id).toString('base64');
49+
assert.equal(method, 'rollback');
50+
assert.equal(proto.transaction.toBase64(), base64Id);
51+
callback();
52+
};
53+
transaction.rollback(function() {
54+
assert.equal(transaction.isFinalized, true);
55+
done();
56+
});
57+
});
58+
59+
it('should mark as `finalized` when rollback errors', function(done) {
60+
var error = new Error('rollback error');
61+
transaction.makeReq = function(method, proto, respType, callback) {
62+
callback(error);
63+
};
64+
transaction.rollback(function(err) {
65+
assert.equal(err, error);
66+
assert.equal(transaction.isFinalized, true);
67+
done();
68+
});
5369
});
5470
});
5571

0 commit comments

Comments
 (0)