From f2a42bec150fa8b947ffde57afd72a22e805f085 Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Fri, 21 Aug 2026 16:16:38 -0400 Subject: [PATCH] Retry note saves that time out waiting for the database Any error while saving a note prompted the user to restart Zotero, even a transaction timeout caused by a long-running operation elsewhere. Nothing has been written when the wait times out, so retry, unless newer note content has been handed to the editor in the meantime. https://forums.zotero.org/discussion/133298/ --- chrome/content/zotero/xpcom/editorInstance.js | 29 +++++++++- test/tests/noteeditorTest.js | 56 +++++++++++++++++++ 2 files changed, 84 insertions(+), 1 deletion(-) diff --git a/chrome/content/zotero/xpcom/editorInstance.js b/chrome/content/zotero/xpcom/editorInstance.js index 559d32de51..add7e31708 100644 --- a/chrome/content/zotero/xpcom/editorInstance.js +++ b/chrome/content/zotero/xpcom/editorInstance.js @@ -50,10 +50,14 @@ const DOWNLOADED_IMAGE_TYPE = [ 'image/png' ]; +// Tries at saving a note when the database is busy, each waiting out the transaction timeout +const MAX_SAVE_ATTEMPTS = 3; + class EditorInstance { constructor() { this.instanceID = Zotero.Utilities.randomString(); this._undoRedoController = null; + this._lastSaveID = 0; } get itemID() { @@ -1140,7 +1144,9 @@ class EditorInstance { } } - async _save(noteData, skipDateModifiedUpdate) { + // saveID identifies this save among the editor's others, so that a retry can tell whether it + // still holds the newest content. Assigned below, once there's something to write. + async _save(noteData, skipDateModifiedUpdate, saveID, attempt = 1) { if (!noteData) return; let { state, html } = noteData; if (html === undefined) return; @@ -1158,6 +1164,13 @@ class EditorInstance { Zotero.debug('Note value not available -- not saving', 2); return; } + // Claim an ID now that there's something to write. saveSync() calls through with no + // data whenever the editor has no unsaved changes -- including once an update has been + // dispatched but not yet saved -- and such a save must not make a retry waiting below + // look superseded. + if (saveID === undefined) { + saveID = ++this._lastSaveID; + } // Update note if (this._item) { await Zotero.DB.executeTransaction(async () => { @@ -1199,6 +1212,20 @@ class EditorInstance { } } catch (e) { + // A long-running operation elsewhere (e.g., full-text index maintenance) can hold the + // database past the transaction wait timeout. Nothing was written, so try again rather + // than telling the user to restart. + if (e instanceof Zotero.DBConnection.TimeoutError && attempt < MAX_SAVE_ATTEMPTS) { + // Unless the editor has handed us newer content in the meantime, in which case + // this save is superseded and retrying it would undo the newer one + if (saveID != this._lastSaveID) { + Zotero.debug("Timed out saving note, but a newer save is pending -- skipping", 2); + return; + } + Zotero.debug("Timed out waiting for the database to save note -- retrying", 2); + await this._save(noteData, skipDateModifiedUpdate, saveID, attempt + 1); + return; + } Zotero.logError(e); Zotero.crash(true); throw e; diff --git a/test/tests/noteeditorTest.js b/test/tests/noteeditorTest.js index c821a6ba5c..8c2909c587 100644 --- a/test/tests/noteeditorTest.js +++ b/test/tests/noteeditorTest.js @@ -175,4 +175,60 @@ describe("Note Editor", function () { ); }); }); + + describe("Saving", function () { + it("should retry a save that times out waiting for the database", async function () { + let editorInstance = await openEditor(); + let crash = sinon.stub(Zotero, 'crash'); + let executeTransaction = sinon.stub(Zotero.DB, 'executeTransaction'); + executeTransaction.onFirstCall().rejects(new Zotero.DBConnection.TimeoutError()); + executeTransaction.callThrough(); + try { + await editorInstance._save({ state: {}, html: '

Retried note

' }); + } + finally { + executeTransaction.restore(); + crash.restore(); + } + assert.isTrue(crash.notCalled); + assert.equal(editorInstance._item.getNote(), '

Retried note

'); + }); + + it("shouldn't let a retried save overwrite newer note content", async function () { + let editorInstance = await openEditor(); + let crash = sinon.stub(Zotero, 'crash'); + let executeTransaction = sinon.stub(Zotero.DB, 'executeTransaction'); + executeTransaction.onFirstCall().rejects(new Zotero.DBConnection.TimeoutError()); + executeTransaction.callThrough(); + try { + let timedOut = editorInstance._save({ state: {}, html: '

Older note

' }); + await editorInstance._save({ state: {}, html: '

Newer note

' }); + await timedOut; + } + finally { + executeTransaction.restore(); + crash.restore(); + } + assert.equal(editorInstance._item.getNote(), '

Newer note

'); + }); + + it("shouldn't let a save with nothing to save supersede a pending retry", async function () { + let editorInstance = await openEditor(); + let crash = sinon.stub(Zotero, 'crash'); + let executeTransaction = sinon.stub(Zotero.DB, 'executeTransaction'); + executeTransaction.onFirstCall().rejects(new Zotero.DBConnection.TimeoutError()); + executeTransaction.callThrough(); + try { + let timedOut = editorInstance._save({ state: {}, html: '

Pending note

' }); + // What saveSync() passes through when the editor has no unsaved changes + await editorInstance._save(null); + await timedOut; + } + finally { + executeTransaction.restore(); + crash.restore(); + } + assert.equal(editorInstance._item.getNote(), '

Pending note

'); + }); + }); });