From a4ebed86110175652b6eaa3ed3c7a18abe61d4e9 Mon Sep 17 00:00:00 2001 From: Maciej Lesniewski Date: Wed, 20 May 2026 12:55:58 +0200 Subject: [PATCH 1/6] feat: add visibilitychange probe for proactive IDB health check Register a visibilitychange listener inside createStore() that runs a lightweight readonly probe when the tab returns to foreground. If the probe detects a dead IDB connection (connection lost, backing store error, or InvalidStateError), it drops the cached dbp so the next real operation opens a fresh connection instead of failing. This prevents the ReconnectApp write storm from hitting a dead IDB connection after Safari backgrounds a tab. Addresses Expensify/App#87864. Co-Authored-By: Claude Opus 4.6 --- .../IDBKeyValProvider/createStore.ts | 43 +++++++ .../unit/storage/providers/createStoreTest.ts | 112 ++++++++++++++++++ 2 files changed, 155 insertions(+) diff --git a/lib/storage/providers/IDBKeyValProvider/createStore.ts b/lib/storage/providers/IDBKeyValProvider/createStore.ts index 9b137c2da..dff3da203 100644 --- a/lib/storage/providers/IDBKeyValProvider/createStore.ts +++ b/lib/storage/providers/IDBKeyValProvider/createStore.ts @@ -37,6 +37,11 @@ function getBudgetedHealErrorLabel(error: unknown): 'backing store' | 'connectio return isBackingStoreError(error) ? 'backing store' : 'connection lost'; } +/** Union of all error types indicating a stale/dead IDB connection. Used by the visibilitychange probe. */ +function isStaleConnectionError(error: unknown): boolean { + return isInvalidStateError(error) || isBackingStoreError(error) || isConnectionLostError(error); +} + // 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 @@ -125,6 +130,44 @@ function createStore(dbName: string, storeName: string): UseStore { return result; } + // Proactive IDB health check when tab returns to foreground. + // Safari kills IDB connections for backgrounded tabs. By probing before + // the ReconnectApp write storm hits, we drop the stale dbp early so the + // first real operation opens a fresh connection instead of failing. + if (typeof document !== 'undefined') { + document.addEventListener('visibilitychange', () => { + if (document.visibilityState !== 'visible' || !dbp) { + return; + } + + const probePromise = dbp; + + const dropCacheIfStale = (error: unknown) => { + if (dbp !== probePromise || !isStaleConnectionError(error)) { + return; + } + Logger.logInfo('IDB visibilitychange probe: connection lost, dropping cached connection', {dbName, storeName}); + dbp = undefined; + }; + + probePromise.then((db) => { + if (dbp !== probePromise) { + return; + } + try { + const tx = db.transaction(storeName, 'readonly'); + const probeStore = tx.objectStore(storeName); + const req = probeStore.count(); + req.onerror = () => { + dropCacheIfStale(req.error); + }; + } catch (error) { + dropCacheIfStale(error); + } + }); + }); + } + // Handles three recoverable error classes: // 1. InvalidStateError — connection closed between getDB() resolving and db.transaction(). // Retry once with a fresh connection. No budget limit (transient, always worth one reopen). diff --git a/tests/unit/storage/providers/createStoreTest.ts b/tests/unit/storage/providers/createStoreTest.ts index bb6530c3b..8b1d9c899 100644 --- a/tests/unit/storage/providers/createStoreTest.ts +++ b/tests/unit/storage/providers/createStoreTest.ts @@ -553,4 +553,116 @@ describe('createStore', () => { expect(logAlertSpy).not.toHaveBeenCalledWith(expect.stringContaining('dropping cached connection and reopening'), expect.anything()); }); }); + + describe('visibilitychange probe', () => { + function simulateVisibilityChange(state: string) { + Object.defineProperty(document, 'visibilityState', {value: state, writable: true, configurable: true}); + document.dispatchEvent(new Event('visibilitychange')); + } + + afterEach(() => { + Object.defineProperty(document, 'visibilityState', {value: 'visible', writable: true, configurable: true}); + }); + + it('should drop stale dbp when probe detects connection lost on foreground', async () => { + const store = createStore(uniqueDBName(), STORE_NAME); + + await store('readwrite', (s) => { + s.put('value', 'key1'); + return IDB.promisifyRequest(s.transaction); + }); + + simulateVisibilityChange('hidden'); + + const original = IDBDatabase.prototype.transaction; + let probeIntercepted = false; + jest.spyOn(IDBDatabase.prototype, 'transaction').mockImplementation(function (this: IDBDatabase, ...args) { + if (!probeIntercepted) { + probeIntercepted = true; + throw new DOMException('Connection to Indexed Database server lost. Refresh the page to try again', 'UnknownError'); + } + return original.apply(this, args); + }); + + simulateVisibilityChange('visible'); + + await new Promise((resolve) => { + setTimeout(resolve, 0); + }); + + jest.restoreAllMocks(); + + const result = await store('readonly', (s) => IDB.promisifyRequest(s.get('key1'))); + expect(result).toBe('value'); + expect(logInfoSpy).toHaveBeenCalledWith('IDB visibilitychange probe: connection lost, dropping cached connection', expect.objectContaining({dbName: expect.any(String)})); + }); + + it('should not probe when no connection exists yet', async () => { + const dbName = uniqueDBName(); + createStore(dbName, STORE_NAME); + + simulateVisibilityChange('hidden'); + simulateVisibilityChange('visible'); + + await new Promise((resolve) => { + setTimeout(resolve, 0); + }); + + // No probe log for this specific store (dbp was never set) + expect(logInfoSpy).not.toHaveBeenCalledWith(expect.stringContaining('visibilitychange probe'), expect.objectContaining({dbName})); + }); + + it('should keep connection when probe succeeds', async () => { + const store = createStore(uniqueDBName(), STORE_NAME); + + await store('readwrite', (s) => { + s.put('value', 'key1'); + return IDB.promisifyRequest(s.transaction); + }); + + simulateVisibilityChange('hidden'); + simulateVisibilityChange('visible'); + + await new Promise((resolve) => { + setTimeout(resolve, 0); + }); + + const result = await store('readonly', (s) => IDB.promisifyRequest(s.get('key1'))); + expect(result).toBe('value'); + expect(logInfoSpy).not.toHaveBeenCalledWith(expect.stringContaining('visibilitychange probe'), expect.anything()); + }); + + it('should drop dbp when probe throws InvalidStateError', async () => { + const store = createStore(uniqueDBName(), STORE_NAME); + + await store('readwrite', (s) => { + s.put('value', 'key1'); + return IDB.promisifyRequest(s.transaction); + }); + + simulateVisibilityChange('hidden'); + + 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('The database connection is closing.', 'InvalidStateError'); + } + return original.apply(this, args); + }); + + simulateVisibilityChange('visible'); + + await new Promise((resolve) => { + setTimeout(resolve, 0); + }); + + jest.restoreAllMocks(); + + const result = await store('readonly', (s) => IDB.promisifyRequest(s.get('key1'))); + expect(result).toBe('value'); + expect(logInfoSpy).toHaveBeenCalledWith('IDB visibilitychange probe: connection lost, dropping cached connection', expect.objectContaining({dbName: expect.any(String)})); + }); + }); }); From 74d04d6010fde2c7a8055bc8567917dae56938a0 Mon Sep 17 00:00:00 2001 From: Maciej Lesniewski Date: Wed, 20 May 2026 13:54:18 +0200 Subject: [PATCH 2/6] fix: improve diagnostic logging for probe and all heal paths - Probe start: logInfo when tab becomes visible and probe begins - Probe healthy: logInfo confirming connection is healthy - Probe stale: logAlert with error details when stale connection detected - Heal attempts/success/exhaustion/non-recoverable: same as #780 - Updated test assertions to match new log messages and levels Co-Authored-By: Claude Opus 4.6 --- lib/storage/providers/IDBKeyValProvider/createStore.ts | 9 ++++++++- tests/unit/storage/providers/createStoreTest.ts | 8 +++++--- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/lib/storage/providers/IDBKeyValProvider/createStore.ts b/lib/storage/providers/IDBKeyValProvider/createStore.ts index dff3da203..feaff688c 100644 --- a/lib/storage/providers/IDBKeyValProvider/createStore.ts +++ b/lib/storage/providers/IDBKeyValProvider/createStore.ts @@ -140,13 +140,19 @@ function createStore(dbName: string, storeName: string): UseStore { return; } + Logger.logInfo('IDB visibilitychange probe: tab became visible, checking connection health', {dbName, storeName}); + const probePromise = dbp; const dropCacheIfStale = (error: unknown) => { if (dbp !== probePromise || !isStaleConnectionError(error)) { return; } - Logger.logInfo('IDB visibilitychange probe: connection lost, dropping cached connection', {dbName, storeName}); + Logger.logAlert('IDB visibilitychange probe: stale connection detected, dropping cached connection', { + dbName, + storeName, + errorMessage: error instanceof Error ? error.message : String(error), + }); dbp = undefined; }; @@ -161,6 +167,7 @@ function createStore(dbName: string, storeName: string): UseStore { req.onerror = () => { dropCacheIfStale(req.error); }; + Logger.logInfo('IDB visibilitychange probe: connection is healthy', {dbName, storeName}); } catch (error) { dropCacheIfStale(error); } diff --git a/tests/unit/storage/providers/createStoreTest.ts b/tests/unit/storage/providers/createStoreTest.ts index 8b1d9c899..9fd05a604 100644 --- a/tests/unit/storage/providers/createStoreTest.ts +++ b/tests/unit/storage/providers/createStoreTest.ts @@ -594,7 +594,7 @@ describe('createStore', () => { const result = await store('readonly', (s) => IDB.promisifyRequest(s.get('key1'))); expect(result).toBe('value'); - expect(logInfoSpy).toHaveBeenCalledWith('IDB visibilitychange probe: connection lost, dropping cached connection', expect.objectContaining({dbName: expect.any(String)})); + expect(logAlertSpy).toHaveBeenCalledWith(expect.stringContaining('IDB visibilitychange probe: stale connection detected'), expect.objectContaining({dbName: expect.any(String)})); }); it('should not probe when no connection exists yet', async () => { @@ -629,7 +629,9 @@ describe('createStore', () => { const result = await store('readonly', (s) => IDB.promisifyRequest(s.get('key1'))); expect(result).toBe('value'); - expect(logInfoSpy).not.toHaveBeenCalledWith(expect.stringContaining('visibilitychange probe'), expect.anything()); + // Probe ran but found healthy connection — no stale connection alert + expect(logAlertSpy).not.toHaveBeenCalledWith(expect.stringContaining('stale connection detected'), expect.anything()); + expect(logInfoSpy).toHaveBeenCalledWith(expect.stringContaining('connection is healthy'), expect.anything()); }); it('should drop dbp when probe throws InvalidStateError', async () => { @@ -662,7 +664,7 @@ describe('createStore', () => { const result = await store('readonly', (s) => IDB.promisifyRequest(s.get('key1'))); expect(result).toBe('value'); - expect(logInfoSpy).toHaveBeenCalledWith('IDB visibilitychange probe: connection lost, dropping cached connection', expect.objectContaining({dbName: expect.any(String)})); + expect(logAlertSpy).toHaveBeenCalledWith(expect.stringContaining('IDB visibilitychange probe: stale connection detected'), expect.objectContaining({dbName: expect.any(String)})); }); }); }); From b63de8b39939b413c67fa3949c1169c028b08de1 Mon Sep 17 00:00:00 2001 From: Maciej Lesniewski Date: Wed, 20 May 2026 14:12:55 +0200 Subject: [PATCH 3/6] fix: move probe healthy log to req.onsuccess to avoid false positive The "connection is healthy" log was emitted synchronously after count(), before the IDB request completed. If the request later failed via onerror, both healthy and stale logs would fire for the same visibility event. Now only logs on actual success. Co-Authored-By: Claude Opus 4.6 --- lib/storage/providers/IDBKeyValProvider/createStore.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/storage/providers/IDBKeyValProvider/createStore.ts b/lib/storage/providers/IDBKeyValProvider/createStore.ts index feaff688c..6b4e629b8 100644 --- a/lib/storage/providers/IDBKeyValProvider/createStore.ts +++ b/lib/storage/providers/IDBKeyValProvider/createStore.ts @@ -164,10 +164,12 @@ function createStore(dbName: string, storeName: string): UseStore { const tx = db.transaction(storeName, 'readonly'); const probeStore = tx.objectStore(storeName); const req = probeStore.count(); + req.onsuccess = () => { + Logger.logInfo('IDB visibilitychange probe: connection is healthy', {dbName, storeName}); + }; req.onerror = () => { dropCacheIfStale(req.error); }; - Logger.logInfo('IDB visibilitychange probe: connection is healthy', {dbName, storeName}); } catch (error) { dropCacheIfStale(error); } From 74a7370453052511bfdf984dd4e523d866364ba6 Mon Sep 17 00:00:00 2001 From: Maciej Lesniewski Date: Fri, 29 May 2026 12:18:12 +0200 Subject: [PATCH 4/6] fix: tighten error detection to handle DOMException not extending Error In some environments DOMException does not inherit from Error. Use (error instanceof Error || error instanceof DOMException) for all three detection functions to avoid missing IDB errors in those envs. Co-Authored-By: Claude Opus 4.6 --- lib/storage/providers/IDBKeyValProvider/createStore.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/storage/providers/IDBKeyValProvider/createStore.ts b/lib/storage/providers/IDBKeyValProvider/createStore.ts index 6b4e629b8..764403bed 100644 --- a/lib/storage/providers/IDBKeyValProvider/createStore.ts +++ b/lib/storage/providers/IDBKeyValProvider/createStore.ts @@ -10,7 +10,7 @@ const HEAL_ATTEMPTS_MAX = 3; * internal recovery (RepairDB -> delete -> recreate) also fails. */ function isBackingStoreError(error: unknown): boolean { - return error instanceof Error && error.message.includes('Internal error opening backing store'); + return (error instanceof Error || error instanceof DOMException) && (error as Error).message.includes('Internal error opening backing store'); } /** @@ -19,13 +19,13 @@ function isBackingStoreError(error: unknown): boolean { * 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(); + if (!(error instanceof Error || error instanceof DOMException)) return false; + const msg = (error as Error).message.toLowerCase(); 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'; + return (error instanceof Error || error instanceof DOMException) && (error as Error).name === 'InvalidStateError'; } /** Errors that trigger a budgeted heal-and-retry in store(). */ From 00e8667b1c51aa1d9b9d6de12e2b98ed8eb78469 Mon Sep 17 00:00:00 2001 From: Maciej Lesniewski Date: Fri, 29 May 2026 16:06:08 +0200 Subject: [PATCH 5/6] fix: add fallback label in getBudgetedHealErrorLabel Avoid silently mislabeling future error categories as connection lost. Co-Authored-By: Claude Opus 4.6 --- lib/storage/providers/IDBKeyValProvider/createStore.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/storage/providers/IDBKeyValProvider/createStore.ts b/lib/storage/providers/IDBKeyValProvider/createStore.ts index 764403bed..b543e7bc5 100644 --- a/lib/storage/providers/IDBKeyValProvider/createStore.ts +++ b/lib/storage/providers/IDBKeyValProvider/createStore.ts @@ -33,8 +33,10 @@ 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'; +function getBudgetedHealErrorLabel(error: unknown): string { + if (isBackingStoreError(error)) return 'backing store'; + if (isConnectionLostError(error)) return 'connection lost'; + return 'unknown'; } /** Union of all error types indicating a stale/dead IDB connection. Used by the visibilitychange probe. */ From e3a8cc5d00f097657649e1ebaa88cf538ec755d0 Mon Sep 17 00:00:00 2001 From: eliran goshen Date: Fri, 5 Jun 2026 11:29:00 +0200 Subject: [PATCH 6/6] fix: address review - remove document guard and ReconnectApp mention - Remove the `typeof document !== 'undefined'` guard; IDBKeyValProvider runs on web only, where document is always defined. - Drop the App-specific "ReconnectApp" reference from the probe comment. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../IDBKeyValProvider/createStore.ts | 78 +++++++++---------- 1 file changed, 38 insertions(+), 40 deletions(-) diff --git a/lib/storage/providers/IDBKeyValProvider/createStore.ts b/lib/storage/providers/IDBKeyValProvider/createStore.ts index b543e7bc5..80f0c6c32 100644 --- a/lib/storage/providers/IDBKeyValProvider/createStore.ts +++ b/lib/storage/providers/IDBKeyValProvider/createStore.ts @@ -133,51 +133,49 @@ function createStore(dbName: string, storeName: string): UseStore { } // Proactive IDB health check when tab returns to foreground. - // Safari kills IDB connections for backgrounded tabs. By probing before - // the ReconnectApp write storm hits, we drop the stale dbp early so the - // first real operation opens a fresh connection instead of failing. - if (typeof document !== 'undefined') { - document.addEventListener('visibilitychange', () => { - if (document.visibilityState !== 'visible' || !dbp) { - return; - } + // Safari kills IDB connections for backgrounded tabs. By probing as soon as + // the tab becomes visible, we drop the stale dbp early so the first real + // operation opens a fresh connection instead of failing. + document.addEventListener('visibilitychange', () => { + if (document.visibilityState !== 'visible' || !dbp) { + return; + } - Logger.logInfo('IDB visibilitychange probe: tab became visible, checking connection health', {dbName, storeName}); + Logger.logInfo('IDB visibilitychange probe: tab became visible, checking connection health', {dbName, storeName}); - const probePromise = dbp; + const probePromise = dbp; - const dropCacheIfStale = (error: unknown) => { - if (dbp !== probePromise || !isStaleConnectionError(error)) { - return; - } - Logger.logAlert('IDB visibilitychange probe: stale connection detected, dropping cached connection', { - dbName, - storeName, - errorMessage: error instanceof Error ? error.message : String(error), - }); - dbp = undefined; - }; - - probePromise.then((db) => { - if (dbp !== probePromise) { - return; - } - try { - const tx = db.transaction(storeName, 'readonly'); - const probeStore = tx.objectStore(storeName); - const req = probeStore.count(); - req.onsuccess = () => { - Logger.logInfo('IDB visibilitychange probe: connection is healthy', {dbName, storeName}); - }; - req.onerror = () => { - dropCacheIfStale(req.error); - }; - } catch (error) { - dropCacheIfStale(error); - } + const dropCacheIfStale = (error: unknown) => { + if (dbp !== probePromise || !isStaleConnectionError(error)) { + return; + } + Logger.logAlert('IDB visibilitychange probe: stale connection detected, dropping cached connection', { + dbName, + storeName, + errorMessage: error instanceof Error ? error.message : String(error), }); + dbp = undefined; + }; + + probePromise.then((db) => { + if (dbp !== probePromise) { + return; + } + try { + const tx = db.transaction(storeName, 'readonly'); + const probeStore = tx.objectStore(storeName); + const req = probeStore.count(); + req.onsuccess = () => { + Logger.logInfo('IDB visibilitychange probe: connection is healthy', {dbName, storeName}); + }; + req.onerror = () => { + dropCacheIfStale(req.error); + }; + } catch (error) { + dropCacheIfStale(error); + } }); - } + }); // Handles three recoverable error classes: // 1. InvalidStateError — connection closed between getDB() resolving and db.transaction().