diff --git a/chrome/content/zotero/xpcom/db.js b/chrome/content/zotero/xpcom/db.js index d93c146990..d4bc0a880f 100644 --- a/chrome/content/zotero/xpcom/db.js +++ b/chrome/content/zotero/xpcom/db.js @@ -545,6 +545,12 @@ Zotero.DBConnection.prototype.executeTransaction = async function (func, options this._transactionID = null; } + // A corruption error from the transaction's own BEGIN or COMMIT is thrown by mozStorage + // instead of by one of our query methods, so it hasn't been checked yet + if (!e?.corruptionChecked) { + await this._checkException(e); + } + // Function to run once transaction has been committed but before any // permanent callbacks if (options.onRollback) { @@ -701,7 +707,7 @@ Zotero.DBConnection.prototype.queryAsync = async function (sql, params, options if (e.errors && e.errors[0]) { var eStr = e + ""; eStr = eStr.indexOf("Error: ") == 0 ? eStr.substr(7): e; - throw new Error(eStr + ' [QUERY: ' + sql + '] ' + let newError = new Error(eStr + ' [QUERY: ' + sql + '] ' + (params ? '[PARAMS: ' + (Array.isArray(params) @@ -710,6 +716,8 @@ Zotero.DBConnection.prototype.queryAsync = async function (sql, params, options ) + '] ' : '') + '[ERROR: ' + e.errors[0].message + ']'); + newError.corruptionChecked = e.corruptionChecked; + throw newError; } else { throw e; @@ -1922,6 +1930,11 @@ Zotero.DBConnection.prototype._revertWALHeader = async function (file) { * check, which detects index inconsistencies that quick_check misses) */ Zotero.DBConnection.prototype._checkException = async function (e, { mainConfirmedCorrupt } = {}) { + // Flag the error so that it isn't checked again as it propagates + if (e && typeof e == 'object') { + e.corruptionChecked = true; + } + if (this._externalDB || !this.isCorruptionError(e) || this._checkingCorruption || this._handlingCorruption) { return true; diff --git a/chrome/content/zotero/xpcom/zotero.js b/chrome/content/zotero/xpcom/zotero.js index d096ce1e51..87f054e06c 100644 --- a/chrome/content/zotero/xpcom/zotero.js +++ b/chrome/content/zotero/xpcom/zotero.js @@ -669,10 +669,14 @@ const { CommandLineOptions } = ChromeUtils.importESModule("chrome://zotero/conte throw e; } - let stack = e.stack ? Zotero.Utilities.Internal.filterStack(e.stack) : null; - Zotero.startupError = Zotero.getString('startupError.databaseUpgradeError') - + "\n\n" - + (stack || e); + // Report the error unless corruption recovery has already started a quit + // or restart + if (!Zotero.skipLoading) { + let stack = e.stack ? Zotero.Utilities.Internal.filterStack(e.stack) : null; + Zotero.startupError = Zotero.getString('startupError.databaseUpgradeError') + + "\n\n" + + (stack || e); + } throw e; } finally { @@ -847,7 +851,9 @@ const { CommandLineOptions } = ChromeUtils.importESModule("chrome://zotero/conte } catch (e) { Zotero.logError(e); - if (!Zotero.startupError) { + // Report a generic error unless a more specific one was set above or corruption + // recovery has already started a quit or restart + if (!Zotero.startupError && !Zotero.skipLoading) { Zotero.startupError = Zotero.getString('startupError', Zotero.appName) + "\n\n" + Zotero.getString('db.integrityCheck.reportInForums') + "\n\n" + e.message ? (e.message + "\n\n" + e.stack) : e; diff --git a/test/tests/dbTest.js b/test/tests/dbTest.js index 30477cdb2d..a8662c040b 100644 --- a/test/tests/dbTest.js +++ b/test/tests/dbTest.js @@ -354,6 +354,108 @@ describe("Zotero.DB", function () { assert.ok(callback1Ran); assert.ok(callback2Ran); }); + + // Corrupt a table's pages, leaving the header, the schema and both table roots + // readable so that the database still opens and bar can be written to + async function createDatabaseWithCorruptTable() { + let dir = await getTempDirectory(); + let dbPath = PathUtils.join(dir, 'test.sqlite'); + let db = new Zotero.DBConnection(dbPath); + await db.queryAsync("CREATE TABLE bar (a INTEGER PRIMARY KEY, b TEXT)"); + await db.queryAsync("CREATE TABLE foo (a INTEGER PRIMARY KEY, b TEXT)"); + await db.executeTransaction(async function () { + for (let i = 0; i < 500; i++) { + await db.queryAsync("INSERT INTO foo VALUES (?, ?)", [i, 'x'.repeat(300)]); + } + }); + let pageSize = await db.valueQueryAsync("PRAGMA page_size"); + await db.closeDatabase(); + for (let suffix of ['-wal', '-shm', '-journal']) { + await IOUtils.remove(dbPath + suffix, { ignoreAbsent: true }); + } + + let bytes = await IOUtils.read(dbPath); + for (let i = 4 * pageSize; i < bytes.length; i++) { + bytes[i] = 0xde; + } + await IOUtils.write(dbPath, bytes); + + db = new Zotero.DBConnection(dbPath); + // Corruption handling is skipped for external databases + db._externalDB = false; + return db; + } + + it("should detect corruption reported by the commit of a transaction", async function () { + let db = await createDatabaseWithCorruptTable(); + let quitStub = sinon.stub(Zotero.Utilities.Internal, 'quit'); + let promptService = Services.prompt; + // Decline the offer to recover + let promptStub = sinon.stub().returns(1); + Services.prompt = { confirmEx: promptStub }; + var e; + try { + e = await getPromiseError(db.executeTransaction(async function () { + await db.queryAsync("INSERT INTO bar VALUES (1, 'written')"); + // SQLite prohibits the commit once a statement in the transaction has + // hit the corrupt pages, so swallow that error the way a caller doing + // optional work would + try { + await db.valueQueryAsync("SELECT COUNT(*) FROM foo"); + } + catch {} + })); + } + finally { + quitStub.restore(); + Services.prompt = promptService; + Zotero.skipLoading = false; + try { + await db.closeDatabase(); + } + catch {} + Zotero.hideZoteroPaneOverlays(); + } + + // The commit is what failed, so the error arrives without the query details + // that queryAsync() adds to a statement error + assert.include(e.message, "database disk image is malformed"); + assert.notInclude(e.message, "[QUERY:"); + assert.equal(promptStub.callCount, 1); + assert.include( + promptStub.args[0][2], + Zotero.getString('db.dbCorrupted', [Zotero.appName, 'test.sqlite']) + ); + }); + + it("shouldn't prompt twice for one corruption error in a transaction", async function () { + let db = await createDatabaseWithCorruptTable(); + let quitStub = sinon.stub(Zotero.Utilities.Internal, 'quit'); + let promptService = Services.prompt; + let promptStub = sinon.stub().returns(1); + Services.prompt = { confirmEx: promptStub }; + var e; + try { + // queryAsync() checks the statement error itself, and the same error then + // propagates out of the transaction + e = await getPromiseError(db.executeTransaction(async function () { + await db.queryAsync("SELECT COUNT(*) FROM foo"); + })); + } + finally { + quitStub.restore(); + Services.prompt = promptService; + Zotero.skipLoading = false; + try { + await db.closeDatabase(); + } + catch {} + Zotero.hideZoteroPaneOverlays(); + } + + assert.include(e.message, "database disk image is malformed"); + assert.equal(promptStub.callCount, 1); + }); })