scriptoriumadmin/zotero
1
0
Fork 0
mirror of https://github.com/zotero/zotero.git synced 2026-08-28 05:25:31 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 2

Don't treat error in commit callback as a rollback

Browse source 
An error thrown from a commit callback rejected executeTransaction()
even though the transaction had been committed, so callers would treat
saved data as rolled back, and rollback callbacks (e.g., the notifier
reset) ran against committed data. Commit-callback errors are now
logged instead, and any error after a successful commit skips rollback
callbacks and is marked with 'committed' on the error object.
This commit is contained in:
Dan Stillman 2026-08-09 11:26:40 -04:00
parent 46603ca4eb
commit 646fbfae65
2 changed files with 48 additions and 2 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

32
chrome/content/zotero/xpcom/db.js
View file

@ -439,6 +439,7 @@ Zotero.DBConnection.prototype.executeTransaction = async function (func, options
var resolve;
var startedTransaction = false;
var committed = false;
var id = Zotero.Utilities.randomString();
try {
@ -488,6 +489,7 @@ Zotero.DBConnection.prototype.executeTransaction = async function (func, options
}
result = await conn.executeTransaction(func);
committed = true;
this._commitCount++;
Zotero.debug(`Committed DB transaction ${id}`, 4);
}
@ -516,15 +518,28 @@ Zotero.DBConnection.prototype.executeTransaction = async function (func, options
this._callbacks.current.rollback = [];
// Run temporary commit callbacks
//
// The transaction is already committed, so errors in commit callbacks are logged
// rather than being treated as transaction failures
var f;
while (f = this._callbacks.current.commit.shift()) {
await Promise.resolve(f(id));
try {
await Promise.resolve(f(id));
}
catch (e) {
Zotero.logError(e);
}
}
// Run commit callbacks
for (var i=0; i<this._callbacks.commit.length; i++) {
if (this._callbacks.commit[i]) {
await this._callbacks.commit[i](id);
try {
await this._callbacks.commit[i](id);
}
catch (e) {
Zotero.logError(e);
}
}
}
@ -534,6 +549,10 @@ Zotero.DBConnection.prototype.executeTransaction = async function (func, options
if (e instanceof Zotero.DBConnection.TimeoutError) {
Zotero.debug(`Timed out waiting for transaction ${id}`, 1);
}
else if (committed) {
Zotero.debug(`Error after committing DB transaction ${id}`, 1);
Zotero.debug(e.message, 1);
}
else {
Zotero.debug(`Rolled back DB transaction ${id}`, 1);
Zotero.debug(e.message, 1);
@ -542,6 +561,15 @@ Zotero.DBConnection.prototype.executeTransaction = async function (func, options
this._transactionID = null;
}
// If the transaction was committed before the error, don't run rollback
// callbacks, since the data was saved
if (committed) {
e.committed = true;
this._callbacks.current.commit = [];
this._callbacks.current.rollback = [];
throw e;
}
// Discard commit callbacks from the rolled-back transaction
this._callbacks.current.commit = [];

18
test/tests/dbTest.js
View file

@ -297,6 +297,24 @@ describe("Zotero.DB", function () {
await Zotero.DB.queryAsync("DROP TABLE " + tmpTable);
});
it("shouldn't reject or roll back on an error in a commit callback", async function () {
var laterCallbackRan = false;
await Zotero.DB.executeTransaction(async function () {
await Zotero.DB.queryAsync("INSERT INTO " + tmpTable + " VALUES (1)");
Zotero.DB.addCurrentCallback('commit', function () {
throw new Error("Commit callback error -- ignore");
});
Zotero.DB.addCurrentCallback('commit', function () {
laterCallbackRan = true;
});
});
var count = await Zotero.DB.valueQueryAsync("SELECT COUNT(*) FROM " + tmpTable);
assert.equal(count, 1);
assert.ok(laterCallbackRan);
await Zotero.DB.queryAsync("DROP TABLE " + tmpTable);
});
it("should discard commit callbacks from a rolled-back transaction", async function () {
var callbackRan = false;
await executeTransactionWithForcedRollback(async function () {