From 32e16a3d6a40635cee263823ad69b595042ecd99 Mon Sep 17 00:00:00 2001 From: Maciej Lesniewski Date: Fri, 15 May 2026 10:58:47 +0200 Subject: [PATCH 1/7] fix: add IDB backing store corruption healing mechanism Adds a Dexie-style heal pattern to createStore for Chromium's Internal error opening backing store error (884K errors/month). - isBackingStoreError() detects the Chromium-specific corruption - Shared healAttemptsRemaining counter (3, reset on success) - On backing store error: clear cached connection, retry once - Clear dbp on rejection so retries get fresh indexedDB.open() - 5 new tests: mid-session heal, init heal, budget exhaustion, budget reset, error classification No deleteDatabase(), no provider swap, no UI changes. Scoped to IDBKeyValProvider only -- SQLite provider untouched. Ref: https://github.com/Expensify/App/issues/90636 Co-Authored-By: Claude Opus 4.6 --- .../IDBKeyValProvider/createStore.ts | 82 ++++++--- .../unit/storage/providers/createStoreTest.ts | 168 ++++++++++++++++++ 2 files changed, 227 insertions(+), 23 deletions(-) diff --git a/lib/storage/providers/IDBKeyValProvider/createStore.ts b/lib/storage/providers/IDBKeyValProvider/createStore.ts index d7c6d0f8b..9124af652 100644 --- a/lib/storage/providers/IDBKeyValProvider/createStore.ts +++ b/lib/storage/providers/IDBKeyValProvider/createStore.ts @@ -2,12 +2,25 @@ import * as IDB from 'idb-keyval'; import type {UseStore} from 'idb-keyval'; import * as Logger from '../../../Logger'; +const HEAL_ATTEMPTS_MAX = 3; + +/** + * Detects the Chromium-specific IDB backing store corruption error. + * Fires when LevelDB files backing IndexedDB are corrupted and Chrome's + * internal recovery (RepairDB -> delete -> recreate) also fails. + * https://github.com/Expensify/App/issues/87862 + */ +function isBackingStoreError(error: unknown): boolean { + return error instanceof DOMException && error.name === 'UnknownError' && error.message.includes('Internal error opening backing store'); +} + // This is a copy of the createStore function from idb-keyval, we need a custom implementation // because we need to create the database manually in order to ensure that the store exists before we use it. // If the store does not exist, idb-keyval will throw an error // source: https://github.com/jakearchibald/idb-keyval/blob/9d19315b4a83897df1e0193dccdc29f78466a0f3/src/index.ts#L12 function createStore(dbName: string, storeName: string): UseStore { let dbp: Promise | undefined; + let healAttemptsRemaining = HEAL_ATTEMPTS_MAX; const attachHandlers = (db: IDBDatabase) => { // Browsers may close idle IDB connections at any time, especially Safari. @@ -36,11 +49,11 @@ function createStore(dbName: string, storeName: string): UseStore { request.onupgradeneeded = () => request.result.createObjectStore(storeName); dbp = IDB.promisifyRequest(request); - dbp.then( - attachHandlers, - // eslint-disable-next-line @typescript-eslint/no-empty-function - () => {}, - ); + dbp.then(attachHandlers, () => { + // Clear the cached rejected promise so the next operation retries + // with a fresh indexedDB.open() instead of returning the same rejection. + dbp = undefined; + }); return dbp; }; @@ -67,8 +80,9 @@ function createStore(dbName: string, storeName: string): UseStore { }; dbp = IDB.promisifyRequest(request); - // eslint-disable-next-line @typescript-eslint/no-empty-function - dbp.then(attachHandlers, () => {}); + dbp.then(attachHandlers, () => { + dbp = undefined; + }); return dbp; }; @@ -78,23 +92,45 @@ function createStore(dbName: string, storeName: string): UseStore { .then((db) => callback(db.transaction(storeName, txMode).objectStore(storeName))); } - // If the connection was closed between getDB() resolving and db.transaction() executing, - // the transaction throws InvalidStateError. We catch it and retry once with a fresh connection. + function resetHealBudget(result: T): T { + healAttemptsRemaining = HEAL_ATTEMPTS_MAX; + return result; + } + + // Handles two recoverable error classes: + // 1. InvalidStateError — connection closed between getDB() resolving and db.transaction(). + // Retry once with a fresh connection. + // 2. Backing store corruption (Chromium UnknownError) — close + reopen the IDB connection. + // Bounded by a shared heal budget (3 attempts, reset on success). + // Mirrors Dexie's PR1398_maxLoop pattern: https://github.com/dexie/Dexie.js/blob/master/src/functions/temp-transaction.ts return (txMode, callback) => - executeTransaction(txMode, callback).catch((error) => { - if (error instanceof DOMException && error.name === 'InvalidStateError') { - Logger.logAlert('IDB InvalidStateError, retrying with fresh connection', { - dbName, - storeName, - txMode, - errorMessage: error.message, - }); - dbp = undefined; - // Retry only once — this call is not wrapped, so if it also fails the error propagates normally. - return executeTransaction(txMode, callback); - } - throw error; - }); + executeTransaction(txMode, callback) + .then(resetHealBudget) + .catch((error) => { + if (error instanceof DOMException && error.name === 'InvalidStateError') { + Logger.logAlert('IDB InvalidStateError, retrying with fresh connection', { + dbName, + storeName, + txMode, + errorMessage: error.message, + }); + dbp = undefined; + return executeTransaction(txMode, callback).then(resetHealBudget); + } + + if (isBackingStoreError(error) && healAttemptsRemaining > 0) { + healAttemptsRemaining--; + Logger.logInfo('IDB heal: backing store error, attempting close + reopen', { + dbName, + storeName, + healAttemptsRemaining, + }); + dbp = undefined; + return executeTransaction(txMode, callback).then(resetHealBudget); + } + + throw error; + }); } export default createStore; diff --git a/tests/unit/storage/providers/createStoreTest.ts b/tests/unit/storage/providers/createStoreTest.ts index 231244706..0032fd19d 100644 --- a/tests/unit/storage/providers/createStoreTest.ts +++ b/tests/unit/storage/providers/createStoreTest.ts @@ -245,4 +245,172 @@ describe('createStore', () => { expect(result).toBe('value'); }); }); + + describe('backing store healing', () => { + /** + * Helper: creates a DOMException matching Chromium's backing store corruption error. + */ + function backingStoreError() { + return new DOMException('Internal error opening backing store for indexedDB.open.', 'UnknownError'); + } + + it('should heal mid-session by closing and reopening the connection', async () => { + const store = createStore(uniqueDBName(), STORE_NAME); + + await store('readwrite', (s) => { + s.put('value', 'key1'); + return IDB.promisifyRequest(s.transaction); + }); + + const original = IDBDatabase.prototype.transaction; + let callCount = 0; + jest.spyOn(IDBDatabase.prototype, 'transaction').mockImplementation(function (this: IDBDatabase, ...args) { + callCount++; + if (callCount === 1) { + throw backingStoreError(); + } + return original.apply(this, args); + }); + + const result = await store('readonly', (s) => IDB.promisifyRequest(s.get('key1'))); + expect(result).toBe('value'); + expect(callCount).toBe(2); + expect(logInfoSpy).toHaveBeenCalledWith('IDB heal: backing store error, attempting close + reopen', expect.objectContaining({healAttemptsRemaining: 2})); + }); + + it('should heal on init when indexedDB.open() rejects with UnknownError', async () => { + const dbName = uniqueDBName(); + + // Pre-create the DB with data + const setupStore = createStore(dbName, STORE_NAME); + await setupStore('readwrite', (s) => { + s.put('persisted', 'key1'); + return IDB.promisifyRequest(s.transaction); + }); + + // Fresh store instance (simulates app restart — dbp starts undefined) + const store = createStore(dbName, STORE_NAME); + + // Mock indexedDB.open to fail twice, then succeed. + // First failure: initial open in the first store() call. + // Second failure: the heal retry's open in that same call — propagates error. + // Third call (second store() invocation): succeeds, proving budget still has 1 left. + const realOpen = indexedDB.open.bind(indexedDB); + let openCallCount = 0; + jest.spyOn(indexedDB, 'open').mockImplementation((name: string, version?: number) => { + openCallCount++; + if (openCallCount <= 2) { + const req = {} as IDBOpenDBRequest; + Promise.resolve().then(() => { + Object.defineProperty(req, 'error', {value: backingStoreError(), configurable: true}); + req.onerror?.(new Event('error') as Event & {target: IDBOpenDBRequest}); + }); + return req; + } + return realOpen(name, version); + }); + + // First call: open fails, heal retry also fails — error propagates + await expect(store('readonly', (s) => IDB.promisifyRequest(s.get('key1')))).rejects.toThrow('Internal error opening backing store'); + + // Second call: open succeeds (mock exhausted), heals + const result = await store('readonly', (s) => IDB.promisifyRequest(s.get('key1'))); + expect(result).toBe('persisted'); + expect(openCallCount).toBeGreaterThanOrEqual(3); + }); + + it('should stop healing after budget exhausts across multiple operations', async () => { + const store = createStore(uniqueDBName(), STORE_NAME); + + // All opens fail permanently + jest.spyOn(indexedDB, 'open').mockImplementation(() => { + const req = {} as IDBOpenDBRequest; + Promise.resolve().then(() => { + Object.defineProperty(req, 'error', {value: backingStoreError(), configurable: true}); + req.onerror?.(new Event('error') as Event & {target: IDBOpenDBRequest}); + }); + return req; + }); + + // Each call consumes 1 heal attempt (initial fails, heal retry also fails, propagates) + await expect(store('readonly', (s) => IDB.promisifyRequest(s.get('k')))).rejects.toThrow('Internal error opening backing store'); + await expect(store('readonly', (s) => IDB.promisifyRequest(s.get('k')))).rejects.toThrow('Internal error opening backing store'); + await expect(store('readonly', (s) => IDB.promisifyRequest(s.get('k')))).rejects.toThrow('Internal error opening backing store'); + + // Budget exhausted — 4th call should NOT attempt healing + logInfoSpy.mockClear(); + await expect(store('readonly', (s) => IDB.promisifyRequest(s.get('k')))).rejects.toThrow('Internal error opening backing store'); + expect(logInfoSpy).not.toHaveBeenCalledWith(expect.stringContaining('IDB heal'), expect.anything()); + }); + + it('should reset heal budget after a successful operation', async () => { + const dbName = uniqueDBName(); + const store = createStore(dbName, STORE_NAME); + + await store('readwrite', (s) => { + s.put('value', 'key1'); + return IDB.promisifyRequest(s.transaction); + }); + + const original = IDBDatabase.prototype.transaction; + + // Drain budget to 1 remaining: fail twice, each heals successfully + for (let i = 0; i < 2; i++) { + let callCount = 0; + const spy = jest.spyOn(IDBDatabase.prototype, 'transaction').mockImplementation(function (this: IDBDatabase, ...args) { + callCount++; + if (callCount === 1) { + throw backingStoreError(); + } + spy.mockRestore(); + return original.apply(this, args); + }); + await store('readonly', (s) => IDB.promisifyRequest(s.get('key1'))); } + + // Clean success — resets budget to 3 + jest.restoreAllMocks(); + logInfoSpy = jest.spyOn(Logger, 'logInfo'); + await store('readonly', (s) => IDB.promisifyRequest(s.get('key1'))); + + // Now fail 3 more times — all should still heal (proving budget was reset) + for (let i = 0; i < 3; i++) { + let callCount = 0; + const spy = jest.spyOn(IDBDatabase.prototype, 'transaction').mockImplementation(function (this: IDBDatabase, ...args) { + callCount++; + if (callCount === 1) { + throw backingStoreError(); + } + spy.mockRestore(); + return original.apply(this, args); + }); + const result = await store('readonly', (s) => IDB.promisifyRequest(s.get('key1'))); expect(result).toBe('value'); + } + }); + + it('should not attempt healing for non-backing-store errors', async () => { + const store = createStore(uniqueDBName(), STORE_NAME); + + await store('readwrite', (s) => { + s.put('value', 'key1'); + return IDB.promisifyRequest(s.transaction); + }); + + // UnknownError but different message + jest.spyOn(IDBDatabase.prototype, 'transaction').mockImplementation(() => { + throw new DOMException('Some other unknown error', 'UnknownError'); + }); + await expect(store('readonly', (s) => IDB.promisifyRequest(s.get('key1')))).rejects.toThrow('Some other unknown error'); + + jest.restoreAllMocks(); + logInfoSpy = jest.spyOn(Logger, 'logInfo'); + + // QuotaExceededError + jest.spyOn(IDBDatabase.prototype, 'transaction').mockImplementation(() => { + throw new DOMException('Quota exceeded', 'QuotaExceededError'); + }); + await expect(store('readonly', (s) => IDB.promisifyRequest(s.get('key1')))).rejects.toThrow('Quota exceeded'); + + expect(logInfoSpy).not.toHaveBeenCalledWith(expect.stringContaining('IDB heal'), expect.anything()); + }); + }); }); From a36d270ec07a2bef36f32abc65f619409dda0154 Mon Sep 17 00:00:00 2001 From: Maciej Lesniewski Date: Fri, 15 May 2026 13:02:16 +0200 Subject: [PATCH 2/7] fix: guard stale dbp reject handler, document concurrent budget drain - Capture dbp reference before attaching reject handler; only clear if dbp hasn't been replaced by a concurrent heal/retry (prevents stale rejection handler from clearing a newer promise) - Add comment documenting concurrent store() budget drain behavior - Fix test formatting Co-Authored-By: Claude Opus 4.6 --- .../providers/IDBKeyValProvider/createStore.ts | 12 ++++++++++++ tests/unit/storage/providers/createStoreTest.ts | 6 ++++-- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/lib/storage/providers/IDBKeyValProvider/createStore.ts b/lib/storage/providers/IDBKeyValProvider/createStore.ts index 9124af652..06a9d4a6a 100644 --- a/lib/storage/providers/IDBKeyValProvider/createStore.ts +++ b/lib/storage/providers/IDBKeyValProvider/createStore.ts @@ -49,9 +49,14 @@ function createStore(dbName: string, storeName: string): UseStore { request.onupgradeneeded = () => request.result.createObjectStore(storeName); dbp = IDB.promisifyRequest(request); + const currentPromise = dbp; dbp.then(attachHandlers, () => { // Clear the cached rejected promise so the next operation retries // with a fresh indexedDB.open() instead of returning the same rejection. + // Guard: only clear if dbp hasn't been replaced by a concurrent heal/retry. + if (dbp !== currentPromise) { + return; + } dbp = undefined; }); return dbp; @@ -80,7 +85,11 @@ function createStore(dbName: string, storeName: string): UseStore { }; dbp = IDB.promisifyRequest(request); + const currentPromise = dbp; dbp.then(attachHandlers, () => { + if (dbp !== currentPromise) { + return; + } dbp = undefined; }); return dbp; @@ -103,6 +112,9 @@ function createStore(dbName: string, storeName: string): UseStore { // 2. Backing store corruption (Chromium UnknownError) — close + reopen the IDB connection. // Bounded by a shared heal budget (3 attempts, reset on success). // Mirrors Dexie's PR1398_maxLoop pattern: https://github.com/dexie/Dexie.js/blob/master/src/functions/temp-transaction.ts + // Note: concurrent store() calls share the budget. Under overlapping failures each caller + // decrements independently, so the budget may drain faster than one-per-incident. This is + // acceptable — same as Dexie's approach — and the budget resets on any success. return (txMode, callback) => executeTransaction(txMode, callback) .then(resetHealBudget) diff --git a/tests/unit/storage/providers/createStoreTest.ts b/tests/unit/storage/providers/createStoreTest.ts index 0032fd19d..4bfa015d2 100644 --- a/tests/unit/storage/providers/createStoreTest.ts +++ b/tests/unit/storage/providers/createStoreTest.ts @@ -365,7 +365,8 @@ describe('createStore', () => { spy.mockRestore(); return original.apply(this, args); }); - await store('readonly', (s) => IDB.promisifyRequest(s.get('key1'))); } + await store('readonly', (s) => IDB.promisifyRequest(s.get('key1'))); + } // Clean success — resets budget to 3 jest.restoreAllMocks(); @@ -383,7 +384,8 @@ describe('createStore', () => { spy.mockRestore(); return original.apply(this, args); }); - const result = await store('readonly', (s) => IDB.promisifyRequest(s.get('key1'))); expect(result).toBe('value'); + const result = await store('readonly', (s) => IDB.promisifyRequest(s.get('key1'))); + expect(result).toBe('value'); } }); From 4a134a1cd7e76ae2750aba35ed9d6a5e02e81f7e Mon Sep 17 00:00:00 2001 From: Maciej Lesniewski Date: Fri, 15 May 2026 13:06:12 +0200 Subject: [PATCH 3/7] fix: align comments and logs with actual behavior (drop cache, not close) The heal path clears the cached dbp and reopens via indexedDB.open(), but does not call db.close() on the old IDBDatabase. Updated comments and log messages from 'close + reopen' to 'drop cached connection and reopen' to match what the code actually does. Co-Authored-By: Claude Opus 4.6 --- lib/storage/providers/IDBKeyValProvider/createStore.ts | 4 ++-- tests/unit/storage/providers/createStoreTest.ts | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/storage/providers/IDBKeyValProvider/createStore.ts b/lib/storage/providers/IDBKeyValProvider/createStore.ts index 06a9d4a6a..19b37712f 100644 --- a/lib/storage/providers/IDBKeyValProvider/createStore.ts +++ b/lib/storage/providers/IDBKeyValProvider/createStore.ts @@ -109,7 +109,7 @@ function createStore(dbName: string, storeName: string): UseStore { // Handles two recoverable error classes: // 1. InvalidStateError — connection closed between getDB() resolving and db.transaction(). // Retry once with a fresh connection. - // 2. Backing store corruption (Chromium UnknownError) — close + reopen the IDB connection. + // 2. Backing store corruption (Chromium UnknownError) — drop cached connection and reopen the IDB connection. // Bounded by a shared heal budget (3 attempts, reset on success). // Mirrors Dexie's PR1398_maxLoop pattern: https://github.com/dexie/Dexie.js/blob/master/src/functions/temp-transaction.ts // Note: concurrent store() calls share the budget. Under overlapping failures each caller @@ -132,7 +132,7 @@ function createStore(dbName: string, storeName: string): UseStore { if (isBackingStoreError(error) && healAttemptsRemaining > 0) { healAttemptsRemaining--; - Logger.logInfo('IDB heal: backing store error, attempting close + reopen', { + Logger.logInfo('IDB heal: backing store error, attempting drop cached connection and reopen', { dbName, storeName, healAttemptsRemaining, diff --git a/tests/unit/storage/providers/createStoreTest.ts b/tests/unit/storage/providers/createStoreTest.ts index 4bfa015d2..56f445e42 100644 --- a/tests/unit/storage/providers/createStoreTest.ts +++ b/tests/unit/storage/providers/createStoreTest.ts @@ -254,7 +254,7 @@ describe('createStore', () => { return new DOMException('Internal error opening backing store for indexedDB.open.', 'UnknownError'); } - it('should heal mid-session by closing and reopening the connection', async () => { + it('should heal mid-session by dropping cached connection and reopening', async () => { const store = createStore(uniqueDBName(), STORE_NAME); await store('readwrite', (s) => { @@ -275,7 +275,7 @@ describe('createStore', () => { const result = await store('readonly', (s) => IDB.promisifyRequest(s.get('key1'))); expect(result).toBe('value'); expect(callCount).toBe(2); - expect(logInfoSpy).toHaveBeenCalledWith('IDB heal: backing store error, attempting close + reopen', expect.objectContaining({healAttemptsRemaining: 2})); + expect(logInfoSpy).toHaveBeenCalledWith('IDB heal: backing store error, attempting drop cached connection and reopen', expect.objectContaining({healAttemptsRemaining: 2})); }); it('should heal on init when indexedDB.open() rejects with UnknownError', async () => { From 6550d736bfb937d3588fa352e04d6ebdf0c7e253 Mon Sep 17 00:00:00 2001 From: Maciej Lesniewski Date: Mon, 18 May 2026 15:43:46 +0200 Subject: [PATCH 4/7] refactor: simplify error type checks per review feedback - isBackingStoreError: use Error instead of DOMException, drop .name check - InvalidStateError catch: same simplification - Remove issue link from JSDoc Co-Authored-By: Claude Opus 4.6 --- lib/storage/providers/IDBKeyValProvider/createStore.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/storage/providers/IDBKeyValProvider/createStore.ts b/lib/storage/providers/IDBKeyValProvider/createStore.ts index 19b37712f..0789c5420 100644 --- a/lib/storage/providers/IDBKeyValProvider/createStore.ts +++ b/lib/storage/providers/IDBKeyValProvider/createStore.ts @@ -8,10 +8,9 @@ const HEAL_ATTEMPTS_MAX = 3; * Detects the Chromium-specific IDB backing store corruption error. * Fires when LevelDB files backing IndexedDB are corrupted and Chrome's * internal recovery (RepairDB -> delete -> recreate) also fails. - * https://github.com/Expensify/App/issues/87862 */ function isBackingStoreError(error: unknown): boolean { - return error instanceof DOMException && error.name === 'UnknownError' && error.message.includes('Internal error opening backing store'); + return error instanceof Error && error.message.includes('Internal error opening backing store'); } // This is a copy of the createStore function from idb-keyval, we need a custom implementation @@ -119,7 +118,7 @@ function createStore(dbName: string, storeName: string): UseStore { executeTransaction(txMode, callback) .then(resetHealBudget) .catch((error) => { - if (error instanceof DOMException && error.name === 'InvalidStateError') { + if (error instanceof Error && error.name === 'InvalidStateError') { Logger.logAlert('IDB InvalidStateError, retrying with fresh connection', { dbName, storeName, From 6fad68efd61d1f3cfdb412b23393c12ebf5b8fb5 Mon Sep 17 00:00:00 2001 From: Maciej Lesniewski Date: Tue, 19 May 2026 15:06:42 +0200 Subject: [PATCH 5/7] feat: extend IDB heal to detect Safari connection lost error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add isConnectionLostError() to detect 'Connection to Indexed Database server lost' and 'Connection is closing' — Safari/WebKit errors that fire when the browser terminates IDB connections for backgrounded tabs. Uses the same heal-and-retry mechanism as backing store corruption: drop cached dbp, retry once with fresh indexedDB.open(), shared budget. Addresses Expensify/App#87864. Co-Authored-By: Claude Opus 4.6 --- .../IDBKeyValProvider/createStore.ts | 16 ++- .../unit/storage/providers/createStoreTest.ts | 130 ++++++++++++++++++ 2 files changed, 144 insertions(+), 2 deletions(-) diff --git a/lib/storage/providers/IDBKeyValProvider/createStore.ts b/lib/storage/providers/IDBKeyValProvider/createStore.ts index 0789c5420..a4d1e53b6 100644 --- a/lib/storage/providers/IDBKeyValProvider/createStore.ts +++ b/lib/storage/providers/IDBKeyValProvider/createStore.ts @@ -13,6 +13,17 @@ function isBackingStoreError(error: unknown): boolean { return error instanceof Error && error.message.includes('Internal error opening backing store'); } +/** + * Detects Safari/WebKit IDB connection termination errors. + * Fires when Safari kills the IDB server process for backgrounded tabs. + * WebKit bugs: https://bugs.webkit.org/show_bug.cgi?id=197050, https://bugs.webkit.org/show_bug.cgi?id=201483 + */ +function isConnectionLostError(error: unknown): boolean { + if (!(error instanceof Error)) return false; + const msg = error.message.toLowerCase(); + return msg.includes('connection to indexed database server lost') || msg.includes('connection is closing'); +} + // This is a copy of the createStore function from idb-keyval, we need a custom implementation // because we need to create the database manually in order to ensure that the store exists before we use it. // If the store does not exist, idb-keyval will throw an error @@ -129,9 +140,10 @@ function createStore(dbName: string, storeName: string): UseStore { return executeTransaction(txMode, callback).then(resetHealBudget); } - if (isBackingStoreError(error) && healAttemptsRemaining > 0) { + if ((isBackingStoreError(error) || isConnectionLostError(error)) && healAttemptsRemaining > 0) { healAttemptsRemaining--; - Logger.logInfo('IDB heal: backing store error, attempting drop cached connection and reopen', { + const errorType = isBackingStoreError(error) ? 'backing store' : 'connection lost'; + Logger.logInfo(`IDB heal: ${errorType} error, attempting drop cached connection and reopen`, { dbName, storeName, healAttemptsRemaining, diff --git a/tests/unit/storage/providers/createStoreTest.ts b/tests/unit/storage/providers/createStoreTest.ts index 56f445e42..8698ad1e8 100644 --- a/tests/unit/storage/providers/createStoreTest.ts +++ b/tests/unit/storage/providers/createStoreTest.ts @@ -415,4 +415,134 @@ describe('createStore', () => { expect(logInfoSpy).not.toHaveBeenCalledWith(expect.stringContaining('IDB heal'), expect.anything()); }); }); + + describe('connection lost healing', () => { + function connectionLostError() { + return new DOMException( + 'Connection to Indexed Database server lost. Refresh the page to try again', + 'UnknownError', + ); + } + + it('should heal by dropping cached connection and reopening', async () => { + const store = createStore(uniqueDBName(), STORE_NAME); + + await store('readwrite', (s) => { + s.put('value', 'key1'); + return IDB.promisifyRequest(s.transaction); + }); + + const original = IDBDatabase.prototype.transaction; + let callCount = 0; + jest.spyOn(IDBDatabase.prototype, 'transaction').mockImplementation(function (this: IDBDatabase, ...args) { + callCount++; + if (callCount === 1) { + throw connectionLostError(); + } + return original.apply(this, args); + }); + + const result = await store('readonly', (s) => IDB.promisifyRequest(s.get('key1'))); + expect(result).toBe('value'); + expect(callCount).toBe(2); + expect(logInfoSpy).toHaveBeenCalledWith( + 'IDB heal: connection lost error, attempting drop cached connection and reopen', + expect.objectContaining({healAttemptsRemaining: expect.any(Number)}), + ); + }); + + it('should stop healing after budget exhausts', async () => { + const store = createStore(uniqueDBName(), STORE_NAME); + + // All transaction calls fail permanently with connection lost. + // The heal path clears dbp and calls indexedDB.open() again — mock that to + // also fail so fake-indexeddb doesn't deadlock waiting for the old connection + // to close (same pattern as the backing store budget exhaustion test). + jest.spyOn(IDBDatabase.prototype, 'transaction').mockImplementation(() => { + throw connectionLostError(); + }); + jest.spyOn(indexedDB, 'open').mockImplementation(() => { + const req = {} as IDBOpenDBRequest; + Promise.resolve().then(() => { + Object.defineProperty(req, 'error', {value: connectionLostError(), configurable: true}); + req.onerror?.(new Event('error') as Event & {target: IDBOpenDBRequest}); + }); + return req; + }); + + // Each call drains 1 heal attempt (initial fails, heal retry also fails) + await expect(store('readonly', (s) => IDB.promisifyRequest(s.get('k')))).rejects.toThrow('Connection to Indexed Database server lost'); + await expect(store('readonly', (s) => IDB.promisifyRequest(s.get('k')))).rejects.toThrow('Connection to Indexed Database server lost'); + await expect(store('readonly', (s) => IDB.promisifyRequest(s.get('k')))).rejects.toThrow('Connection to Indexed Database server lost'); + + // Budget exhausted — 4th call should NOT attempt healing + logInfoSpy.mockClear(); + await expect(store('readonly', (s) => IDB.promisifyRequest(s.get('k')))).rejects.toThrow('Connection to Indexed Database server lost'); + expect(logInfoSpy).not.toHaveBeenCalledWith(expect.stringContaining('IDB heal'), expect.anything()); + }); + + it('should also heal "connection is closing" variant', async () => { + const store = createStore(uniqueDBName(), STORE_NAME); + + await store('readwrite', (s) => { + s.put('value', 'key1'); + return IDB.promisifyRequest(s.transaction); + }); + + const original = IDBDatabase.prototype.transaction; + let callCount = 0; + jest.spyOn(IDBDatabase.prototype, 'transaction').mockImplementation(function (this: IDBDatabase, ...args) { + callCount++; + if (callCount === 1) { + throw new DOMException('Connection is closing.', 'UnknownError'); + } + return original.apply(this, args); + }); + + const result = await store('readonly', (s) => IDB.promisifyRequest(s.get('key1'))); + expect(result).toBe('value'); + expect(callCount).toBe(2); + }); + + it('should share heal budget with backing store errors', async () => { + const store = createStore(uniqueDBName(), STORE_NAME); + + // All transaction calls fail permanently, alternating error types. + // The heal path clears dbp and calls indexedDB.open() again — mock that to + // also fail so fake-indexeddb doesn't deadlock waiting for the old connection + // to close. + const txErrors = [ + new DOMException('Internal error opening backing store for indexedDB.open.', 'UnknownError'), + connectionLostError(), + new DOMException('Internal error opening backing store for indexedDB.open.', 'UnknownError'), + ]; + let txErrorIndex = 0; + jest.spyOn(IDBDatabase.prototype, 'transaction').mockImplementation(() => { + const err = txErrors[Math.min(txErrorIndex, txErrors.length - 1)]; + txErrorIndex++; + throw err; + }); + jest.spyOn(indexedDB, 'open').mockImplementation(() => { + const req = {} as IDBOpenDBRequest; + Promise.resolve().then(() => { + Object.defineProperty(req, 'error', { + value: new DOMException('Internal error opening backing store for indexedDB.open.', 'UnknownError'), + configurable: true, + }); + req.onerror?.(new Event('error') as Event & {target: IDBOpenDBRequest}); + }); + return req; + }); + + // 3 calls drain the budget (each call: fail + heal retry fail = 1 budget) + await expect(store('readonly', (s) => IDB.promisifyRequest(s.get('k')))).rejects.toThrow(); + await expect(store('readonly', (s) => IDB.promisifyRequest(s.get('k')))).rejects.toThrow(); + await expect(store('readonly', (s) => IDB.promisifyRequest(s.get('k')))).rejects.toThrow(); + + // Budget exhausted — no more healing for either error type + logInfoSpy.mockClear(); + await expect(store('readonly', (s) => IDB.promisifyRequest(s.get('k')))).rejects.toThrow(); + expect(logInfoSpy).not.toHaveBeenCalledWith(expect.stringContaining('IDB heal'), expect.anything()); + }); + }); }); From d1f95a03443b7126e017bd14c84f49c7d47eb296 Mon Sep 17 00:00:00 2001 From: Maciej Lesniewski Date: Wed, 20 May 2026 12:46:32 +0200 Subject: [PATCH 6/7] refactor: DRY error classification and promise cleanup in createStore MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extract module-level helpers: isInvalidStateError, isBudgetedHealError, getBudgetedHealErrorLabel. Extract cacheOpenPromise to deduplicate rejected-promise cleanup in getDB and verifyStoreExists. Pure refactor — no behavior change. Co-Authored-By: Claude Opus 4.6 --- .../IDBKeyValProvider/createStore.ts | 69 +++++++++++-------- .../unit/storage/providers/createStoreTest.ts | 5 +- 2 files changed, 40 insertions(+), 34 deletions(-) diff --git a/lib/storage/providers/IDBKeyValProvider/createStore.ts b/lib/storage/providers/IDBKeyValProvider/createStore.ts index a4d1e53b6..f1a369470 100644 --- a/lib/storage/providers/IDBKeyValProvider/createStore.ts +++ b/lib/storage/providers/IDBKeyValProvider/createStore.ts @@ -24,6 +24,19 @@ function isConnectionLostError(error: unknown): boolean { return msg.includes('connection to indexed database server lost') || msg.includes('connection is closing'); } +function isInvalidStateError(error: unknown): boolean { + return error instanceof Error && error.name === 'InvalidStateError'; +} + +/** Errors that trigger a budgeted heal-and-retry in store(). */ +function isBudgetedHealError(error: unknown): boolean { + return isBackingStoreError(error) || isConnectionLostError(error); +} + +function getBudgetedHealErrorLabel(error: unknown): 'backing store' | 'connection lost' { + return isBackingStoreError(error) ? 'backing store' : 'connection lost'; +} + // This is a copy of the createStore function from idb-keyval, we need a custom implementation // because we need to create the database manually in order to ensure that the store exists before we use it. // If the store does not exist, idb-keyval will throw an error @@ -53,23 +66,27 @@ function createStore(dbName: string, storeName: string): UseStore { }; }; - const getDB = () => { - if (dbp) return dbp; - const request = indexedDB.open(dbName); - request.onupgradeneeded = () => request.result.createObjectStore(storeName); - dbp = IDB.promisifyRequest(request); - - const currentPromise = dbp; - dbp.then(attachHandlers, () => { - // Clear the cached rejected promise so the next operation retries - // with a fresh indexedDB.open() instead of returning the same rejection. - // Guard: only clear if dbp hasn't been replaced by a concurrent heal/retry. + // Cache the open promise and attach handlers + rejection cleanup. + // On rejection, clears dbp so the next operation retries with a fresh indexedDB.open() + // instead of returning the same rejected promise. + // Guard: only clear if dbp hasn't been replaced by a concurrent heal/retry. + function cacheOpenPromise(openPromise: Promise) { + dbp = openPromise; + const currentPromise = openPromise; + openPromise.then(attachHandlers, () => { if (dbp !== currentPromise) { return; } dbp = undefined; }); - return dbp; + return openPromise; + } + + const getDB = () => { + if (dbp) return dbp; + const request = indexedDB.open(dbName); + request.onupgradeneeded = () => request.result.createObjectStore(storeName); + return cacheOpenPromise(IDB.promisifyRequest(request)); }; // Ensures the store exists in the DB. If missing, bumps the version to trigger @@ -94,15 +111,7 @@ function createStore(dbName: string, storeName: string): UseStore { updatedDatabase.createObjectStore(storeName); }; - dbp = IDB.promisifyRequest(request); - const currentPromise = dbp; - dbp.then(attachHandlers, () => { - if (dbp !== currentPromise) { - return; - } - dbp = undefined; - }); - return dbp; + return cacheOpenPromise(IDB.promisifyRequest(request)); }; function executeTransaction(txMode: IDBTransactionMode, callback: (store: IDBObjectStore) => T | PromiseLike): Promise { @@ -116,11 +125,12 @@ function createStore(dbName: string, storeName: string): UseStore { return result; } - // Handles two recoverable error classes: + // Handles three recoverable error classes: // 1. InvalidStateError — connection closed between getDB() resolving and db.transaction(). - // Retry once with a fresh connection. - // 2. Backing store corruption (Chromium UnknownError) — drop cached connection and reopen the IDB connection. - // Bounded by a shared heal budget (3 attempts, reset on success). + // Retry once with a fresh connection. No budget limit (transient, always worth one reopen). + // 2. Backing store corruption (Chromium UnknownError) — drop cached connection and reopen. + // 3. Connection lost (Safari UnknownError) — IDB server terminated for backgrounded tabs. + // Both 2 and 3 share a heal budget (3 attempts, reset on success). // Mirrors Dexie's PR1398_maxLoop pattern: https://github.com/dexie/Dexie.js/blob/master/src/functions/temp-transaction.ts // Note: concurrent store() calls share the budget. Under overlapping failures each caller // decrements independently, so the budget may drain faster than one-per-incident. This is @@ -129,21 +139,20 @@ function createStore(dbName: string, storeName: string): UseStore { executeTransaction(txMode, callback) .then(resetHealBudget) .catch((error) => { - if (error instanceof Error && error.name === 'InvalidStateError') { + if (isInvalidStateError(error)) { Logger.logAlert('IDB InvalidStateError, retrying with fresh connection', { dbName, storeName, txMode, - errorMessage: error.message, + errorMessage: error instanceof Error ? error.message : String(error), }); dbp = undefined; return executeTransaction(txMode, callback).then(resetHealBudget); } - if ((isBackingStoreError(error) || isConnectionLostError(error)) && healAttemptsRemaining > 0) { + if (isBudgetedHealError(error) && healAttemptsRemaining > 0) { healAttemptsRemaining--; - const errorType = isBackingStoreError(error) ? 'backing store' : 'connection lost'; - Logger.logInfo(`IDB heal: ${errorType} error, attempting drop cached connection and reopen`, { + Logger.logInfo(`IDB heal: ${getBudgetedHealErrorLabel(error)} error, attempting drop cached connection and reopen`, { dbName, storeName, healAttemptsRemaining, diff --git a/tests/unit/storage/providers/createStoreTest.ts b/tests/unit/storage/providers/createStoreTest.ts index 8698ad1e8..43599c072 100644 --- a/tests/unit/storage/providers/createStoreTest.ts +++ b/tests/unit/storage/providers/createStoreTest.ts @@ -418,10 +418,7 @@ describe('createStore', () => { describe('connection lost healing', () => { function connectionLostError() { - return new DOMException( - 'Connection to Indexed Database server lost. Refresh the page to try again', - 'UnknownError', - ); + return new DOMException('Connection to Indexed Database server lost. Refresh the page to try again', 'UnknownError'); } it('should heal by dropping cached connection and reopening', async () => { From ca5c2b1434ea9a89bd78cb85838526dc4afe7d12 Mon Sep 17 00:00:00 2001 From: Maciej Lesniewski Date: Wed, 20 May 2026 13:53:59 +0200 Subject: [PATCH 7/7] fix: improve diagnostic logging for all heal and error paths - Heal attempts: logAlert with error type, action taken, remaining budget - Heal success: logInfo confirming recovery after each error type - Budget exhaustion: explicit logAlert when heal budget drains - Non-recoverable errors: logAlert with error details - Updated test assertions to match new log messages and levels Co-Authored-By: Claude Opus 4.6 --- .../IDBKeyValProvider/createStore.ts | 23 ++++++++-- .../unit/storage/providers/createStoreTest.ts | 43 ++++++++++++------- 2 files changed, 46 insertions(+), 20 deletions(-) diff --git a/lib/storage/providers/IDBKeyValProvider/createStore.ts b/lib/storage/providers/IDBKeyValProvider/createStore.ts index f1a369470..f6c64718d 100644 --- a/lib/storage/providers/IDBKeyValProvider/createStore.ts +++ b/lib/storage/providers/IDBKeyValProvider/createStore.ts @@ -140,7 +140,7 @@ function createStore(dbName: string, storeName: string): UseStore { .then(resetHealBudget) .catch((error) => { if (isInvalidStateError(error)) { - Logger.logAlert('IDB InvalidStateError, retrying with fresh connection', { + Logger.logAlert('IDB InvalidStateError — dropping cached connection and retrying', { dbName, storeName, txMode, @@ -152,15 +152,30 @@ function createStore(dbName: string, storeName: string): UseStore { if (isBudgetedHealError(error) && healAttemptsRemaining > 0) { healAttemptsRemaining--; - Logger.logInfo(`IDB heal: ${getBudgetedHealErrorLabel(error)} error, attempting drop cached connection and reopen`, { + const label = getBudgetedHealErrorLabel(error); + Logger.logAlert(`IDB heal: ${label} error detected — dropping cached connection and reopening (${healAttemptsRemaining} attempts left)`, { dbName, storeName, - healAttemptsRemaining, }); dbp = undefined; - return executeTransaction(txMode, callback).then(resetHealBudget); + return executeTransaction(txMode, callback).then((result) => { + Logger.logInfo(`IDB heal: successfully recovered after ${label} error`, {dbName, storeName}); + return resetHealBudget(result); + }); } + if (isBudgetedHealError(error)) { + Logger.logAlert(`IDB heal: ${getBudgetedHealErrorLabel(error)} error — heal budget exhausted, giving up`, { + dbName, + storeName, + }); + } else { + Logger.logAlert('IDB error is not recoverable, giving up', { + dbName, + storeName, + errorMessage: error instanceof Error ? error.message : String(error), + }); + } throw error; }); } diff --git a/tests/unit/storage/providers/createStoreTest.ts b/tests/unit/storage/providers/createStoreTest.ts index 43599c072..b9a71f57e 100644 --- a/tests/unit/storage/providers/createStoreTest.ts +++ b/tests/unit/storage/providers/createStoreTest.ts @@ -96,7 +96,8 @@ describe('createStore', () => { await expect(store('readonly', (s) => IDB.promisifyRequest(s.get('key1')))).rejects.toThrow(DOMException); expect(callCount).toBe(1); - expect(logAlertSpy).not.toHaveBeenCalled(); + expect(logAlertSpy).toHaveBeenCalledWith('IDB error is not recoverable, giving up', expect.objectContaining({errorMessage: 'Not found'})); + expect(logAlertSpy).not.toHaveBeenCalledWith(expect.stringContaining('dropping cached connection'), expect.anything()); }); it('should not retry on non-DOMException errors', async () => { @@ -115,7 +116,8 @@ describe('createStore', () => { await expect(store('readonly', (s) => IDB.promisifyRequest(s.get('key1')))).rejects.toThrow(TypeError); expect(callCount).toBe(1); - expect(logAlertSpy).not.toHaveBeenCalled(); + expect(logAlertSpy).toHaveBeenCalledWith('IDB error is not recoverable, giving up', expect.objectContaining({errorMessage: 'Something went wrong'})); + expect(logAlertSpy).not.toHaveBeenCalledWith(expect.stringContaining('dropping cached connection'), expect.anything()); }); it('should preserve data integrity after a successful retry', async () => { @@ -174,7 +176,7 @@ describe('createStore', () => { return IDB.promisifyRequest(s.transaction); }); - expect(logAlertSpy).toHaveBeenCalledWith('IDB InvalidStateError, retrying with fresh connection', { + expect(logAlertSpy).toHaveBeenCalledWith('IDB InvalidStateError — dropping cached connection and retrying', { dbName, storeName: STORE_NAME, txMode: 'readwrite', @@ -275,7 +277,8 @@ describe('createStore', () => { const result = await store('readonly', (s) => IDB.promisifyRequest(s.get('key1'))); expect(result).toBe('value'); expect(callCount).toBe(2); - expect(logInfoSpy).toHaveBeenCalledWith('IDB heal: backing store error, attempting drop cached connection and reopen', expect.objectContaining({healAttemptsRemaining: 2})); + expect(logAlertSpy).toHaveBeenCalledWith('IDB heal: backing store error detected — dropping cached connection and reopening (2 attempts left)', expect.objectContaining({dbName: expect.any(String)})); + expect(logInfoSpy).toHaveBeenCalledWith('IDB heal: successfully recovered after backing store error', expect.objectContaining({dbName: expect.any(String)})); }); it('should heal on init when indexedDB.open() rejects with UnknownError', async () => { @@ -337,10 +340,11 @@ describe('createStore', () => { await expect(store('readonly', (s) => IDB.promisifyRequest(s.get('k')))).rejects.toThrow('Internal error opening backing store'); await expect(store('readonly', (s) => IDB.promisifyRequest(s.get('k')))).rejects.toThrow('Internal error opening backing store'); - // Budget exhausted — 4th call should NOT attempt healing - logInfoSpy.mockClear(); + // Budget exhausted — 4th call should NOT attempt healing, but should log budget exhausted + logAlertSpy.mockClear(); await expect(store('readonly', (s) => IDB.promisifyRequest(s.get('k')))).rejects.toThrow('Internal error opening backing store'); - expect(logInfoSpy).not.toHaveBeenCalledWith(expect.stringContaining('IDB heal'), expect.anything()); + expect(logAlertSpy).toHaveBeenCalledWith(expect.stringContaining('heal budget exhausted'), expect.anything()); + expect(logAlertSpy).not.toHaveBeenCalledWith(expect.stringContaining('dropping cached connection and reopening'), expect.anything()); }); it('should reset heal budget after a successful operation', async () => { @@ -404,7 +408,7 @@ describe('createStore', () => { await expect(store('readonly', (s) => IDB.promisifyRequest(s.get('key1')))).rejects.toThrow('Some other unknown error'); jest.restoreAllMocks(); - logInfoSpy = jest.spyOn(Logger, 'logInfo'); + logAlertSpy = jest.spyOn(Logger, 'logAlert'); // QuotaExceededError jest.spyOn(IDBDatabase.prototype, 'transaction').mockImplementation(() => { @@ -412,7 +416,8 @@ describe('createStore', () => { }); await expect(store('readonly', (s) => IDB.promisifyRequest(s.get('key1')))).rejects.toThrow('Quota exceeded'); - expect(logInfoSpy).not.toHaveBeenCalledWith(expect.stringContaining('IDB heal'), expect.anything()); + expect(logAlertSpy).not.toHaveBeenCalledWith(expect.stringContaining('dropping cached connection and reopening'), expect.anything()); + expect(logAlertSpy).toHaveBeenCalledWith('IDB error is not recoverable, giving up', expect.objectContaining({errorMessage: 'Quota exceeded'})); }); }); @@ -442,9 +447,13 @@ describe('createStore', () => { const result = await store('readonly', (s) => IDB.promisifyRequest(s.get('key1'))); expect(result).toBe('value'); expect(callCount).toBe(2); + expect(logAlertSpy).toHaveBeenCalledWith( + expect.stringContaining('connection lost error detected — dropping cached connection and reopening'), + expect.objectContaining({dbName: expect.any(String)}), + ); expect(logInfoSpy).toHaveBeenCalledWith( - 'IDB heal: connection lost error, attempting drop cached connection and reopen', - expect.objectContaining({healAttemptsRemaining: expect.any(Number)}), + 'IDB heal: successfully recovered after connection lost error', + expect.objectContaining({dbName: expect.any(String)}), ); }); @@ -472,10 +481,11 @@ describe('createStore', () => { await expect(store('readonly', (s) => IDB.promisifyRequest(s.get('k')))).rejects.toThrow('Connection to Indexed Database server lost'); await expect(store('readonly', (s) => IDB.promisifyRequest(s.get('k')))).rejects.toThrow('Connection to Indexed Database server lost'); - // Budget exhausted — 4th call should NOT attempt healing - logInfoSpy.mockClear(); + // Budget exhausted — 4th call should NOT attempt healing, but should log budget exhausted + logAlertSpy.mockClear(); await expect(store('readonly', (s) => IDB.promisifyRequest(s.get('k')))).rejects.toThrow('Connection to Indexed Database server lost'); - expect(logInfoSpy).not.toHaveBeenCalledWith(expect.stringContaining('IDB heal'), expect.anything()); + expect(logAlertSpy).toHaveBeenCalledWith(expect.stringContaining('heal budget exhausted'), expect.anything()); + expect(logAlertSpy).not.toHaveBeenCalledWith(expect.stringContaining('dropping cached connection and reopening'), expect.anything()); }); it('should also heal "connection is closing" variant', async () => { @@ -537,9 +547,10 @@ describe('createStore', () => { await expect(store('readonly', (s) => IDB.promisifyRequest(s.get('k')))).rejects.toThrow(); // Budget exhausted — no more healing for either error type - logInfoSpy.mockClear(); + logAlertSpy.mockClear(); await expect(store('readonly', (s) => IDB.promisifyRequest(s.get('k')))).rejects.toThrow(); - expect(logInfoSpy).not.toHaveBeenCalledWith(expect.stringContaining('IDB heal'), expect.anything()); + expect(logAlertSpy).toHaveBeenCalledWith(expect.stringContaining('heal budget exhausted'), expect.anything()); + expect(logAlertSpy).not.toHaveBeenCalledWith(expect.stringContaining('dropping cached connection and reopening'), expect.anything()); }); }); });