Skip to content

Commit

Permalink
fix: missing await in rollback (#671)
Browse files Browse the repository at this point in the history
  • Loading branch information
laljikanjareeya authored May 27, 2020
1 parent 74e157c commit 7cb353e
Show file tree
Hide file tree
Showing 5 changed files with 50 additions and 6 deletions.
4 changes: 2 additions & 2 deletions samples/concepts.js
Original file line number Diff line number Diff line change
Expand Up @@ -1102,15 +1102,15 @@ class Transaction extends TestHelper {
const [task] = await transaction.get(taskKey);
if (task) {
// The task entity already exists.
transaction.rollback();
await transaction.rollback();
} else {
// Create the task entity.
transaction.save(taskEntity);
transaction.commit();
}
return taskEntity;
} catch (err) {
transaction.rollback();
await transaction.rollback();
}
}
// [END datastore_transactional_get_or_create]
Expand Down
2 changes: 1 addition & 1 deletion samples/tasks.js
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ async function markDone(taskId) {
await transaction.commit();
console.log(`Task ${taskId} updated successfully.`);
} catch (err) {
transaction.rollback();
await transaction.rollback();
}
}
// [END datastore_update_entity]
Expand Down
2 changes: 1 addition & 1 deletion samples/tasks.markdone.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ async function main(taskId) {
await transaction.commit();
console.log(`Task ${taskId} updated successfully.`);
} catch (err) {
transaction.rollback();
await transaction.rollback();
throw err;
}
}
Expand Down
16 changes: 14 additions & 2 deletions src/request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1301,7 +1301,13 @@ class DatastoreRequest {
const transaction = this.datastore.transaction();
transaction.run(async err => {
if (err) {
transaction.rollback();
try {
await transaction.rollback();
} catch (error) {
// Provide the error & API response from the failed run to the user.
// Even a failed rollback should be transparent.
// RE: https://github.com/GoogleCloudPlatform/gcloud-node/pull/1369#discussion_r66833976
}
callback!(err);
return;
}
Expand All @@ -1321,7 +1327,13 @@ class DatastoreRequest {
const [response] = await transaction.commit();
callback!(null, response);
} catch (err) {
transaction.rollback();
try {
await transaction.rollback();
} catch (error) {
// Provide the error & API response from the failed commit to the user.
// Even a failed rollback should be transparent.
// RE: https://github.com/GoogleCloudPlatform/gcloud-node/pull/1369#discussion_r66833976
}
callback!(err);
}
});
Expand Down
32 changes: 32 additions & 0 deletions test/request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2251,6 +2251,38 @@ describe('Request', () => {
done();
});
});

it('should avoid the rollback exception in transaction.run', done => {
sandbox
.stub(transaction, 'run')
.callsFake((gaxOption, callback?: Function) => {
callback = typeof gaxOption === 'function' ? gaxOption : callback!;
callback(new Error('Error.'));
});

sandbox
.stub(transaction, 'rollback')
.rejects(new Error('Rollback Error.'));

request.merge({key, data: null}, (err: Error) => {
assert.strictEqual(err.message, 'Error.');
done();
});
});

it('should avoid the rollback exception in transaction.get/commit', done => {
sandbox.restore();
sandbox.stub(transaction, 'get').rejects(new Error('Error.'));

sandbox
.stub(transaction, 'rollback')
.rejects(new Error('Rollback Error.'));

request.merge({key, data: null}, (err: Error) => {
assert.strictEqual(err.message, 'Error.');
done();
});
});
});

describe('request_', () => {
Expand Down

0 comments on commit 7cb353e

Please sign in to comment.