From 646fbfae65c4497e32f871d431e99629e4e24574 Mon Sep 17 00:00:00 2001
From: Dan Stillman <dstillman@zotero.org>
Date: Sun, 9 Aug 2026 11:26:40 -0400
Subject: [PATCH] Don't treat error in commit callback as a rollback

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.
---
 chrome/content/zotero/xpcom/db.js | 32 +++++++++++++++++++++++++++++--
 test/tests/dbTest.js              | 18 +++++++++++++++++
 2 files changed, 48 insertions(+), 2 deletions(-)

diff --git a/chrome/content/zotero/xpcom/db.js b/chrome/content/zotero/xpcom/db.js
index 92e105af10..44770a0736 100644
--- a/chrome/content/zotero/xpcom/db.js
+++ b/chrome/content/zotero/xpcom/db.js
@@ -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 = [];
 		
diff --git a/test/tests/dbTest.js b/test/tests/dbTest.js
index 32a20eb64d..6d0eb7e9d0 100644
--- a/test/tests/dbTest.js
+++ b/test/tests/dbTest.js
@@ -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 () {