From c4ecd0221e2353331439e61eacebfafb0ab38cfe Mon Sep 17 00:00:00 2001 From: eliran goshen Date: Mon, 18 May 2026 15:22:40 +0200 Subject: [PATCH 1/2] Skip retries in retryOperation for non-retriable storage errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduce a NON_RETRIABLE_ERRORS classification alongside the existing STORAGE_ERRORS list, and short-circuit retryOperation for errors where retrying is provably futile (the underlying IDB connection/store is broken at the filesystem level). The healing path is responsible for recovery; retryOperation should defer to it, not compete with it. Scoped to "Internal error opening backing store" — the LevelDB backing-store class analyzed in Expensify/App#87862, which accounts for ~26% of storage failures and produces ~6× log volume due to the 5-attempt retry loop. Fixes Expensify/App#90633 Co-Authored-By: Claude Opus 4.7 (1M context) --- lib/OnyxUtils.ts | 15 +++++++++++++++ tests/unit/onyxUtilsTest.ts | 21 +++++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index 56dbb10fb..0c5f764f3 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -61,6 +61,14 @@ const SQLITE_STORAGE_ERRORS = [ const STORAGE_ERRORS = [...IDB_STORAGE_ERRORS, ...SQLITE_STORAGE_ERRORS]; +// IndexedDB errors where retrying is futile because the underlying connection/store is broken. +// The healing path (separate from retryOperation) is responsible for recovery. +const IDB_NON_RETRIABLE_ERRORS = [ + 'internal error opening backing store', // LevelDB backing store is broken at the filesystem level +] as const; + +const NON_RETRIABLE_ERRORS = [...IDB_NON_RETRIABLE_ERRORS]; + // Max number of retries for failed storage operations const MAX_STORAGE_OPERATION_RETRY_ATTEMPTS = 5; @@ -786,6 +794,7 @@ function reportStorageQuota(error?: Error): Promise { * Handles storage operation failures based on the error type: * - Storage capacity errors: evicts data and retries the operation * - Invalid data errors: logs an alert and throws an error + * - Non-retriable errors: logs an alert and resolves without retrying * - Other errors: retries the operation */ function retryOperation(error: Error, onyxMethod: TMethod, defaultParams: Parameters[0], retryAttempt: number | undefined): Promise { @@ -802,6 +811,12 @@ function retryOperation(error: Error, on const errorMessage = error?.message?.toLowerCase?.(); const errorName = error?.name?.toLowerCase?.(); const isStorageCapacityError = STORAGE_ERRORS.some((storageError) => errorName?.includes(storageError) || errorMessage?.includes(storageError)); + const isNonRetriableError = NON_RETRIABLE_ERRORS.some((nonRetriableError) => errorName?.includes(nonRetriableError) || errorMessage?.includes(nonRetriableError)); + + if (isNonRetriableError) { + Logger.logAlert(`Storage operation skipped retry for non-retriable error. Error: ${error}. onyxMethod: ${onyxMethod.name}.`); + return Promise.resolve(); + } if (nextRetryAttempt > MAX_STORAGE_OPERATION_RETRY_ATTEMPTS) { Logger.logAlert(`Storage operation failed after 5 retries. Error: ${error}. onyxMethod: ${onyxMethod.name}.`); diff --git a/tests/unit/onyxUtilsTest.ts b/tests/unit/onyxUtilsTest.ts index cc1568365..dcd2f0b77 100644 --- a/tests/unit/onyxUtilsTest.ts +++ b/tests/unit/onyxUtilsTest.ts @@ -788,6 +788,7 @@ describe('OnyxUtils', () => { const genericError = new Error('Generic storage error'); const invalidDataError = new Error("Failed to execute 'put' on 'IDBObjectStore': invalid data"); const diskFullError = new Error('database or disk is full'); + const nonRetriableIdbError = Object.assign(new Error('Internal error opening backing store for indexedDB.open.'), {name: 'UnknownError'}); it('should retry only one time if the operation is firstly failed and then passed', async () => { StorageMock.setItem = jest.fn(StorageMock.setItem).mockRejectedValueOnce(genericError).mockImplementation(StorageMock.setItem); @@ -822,6 +823,26 @@ describe('OnyxUtils', () => { expect(retryOperationSpy).toHaveBeenCalledTimes(1); }); + it('should not retry for non-retriable IndexedDB backing-store errors', async () => { + StorageMock.setItem = jest.fn().mockRejectedValue(nonRetriableIdbError); + + await Onyx.set(ONYXKEYS.TEST_KEY, {test: 'data'}); + + // Called once (initial attempt only) -- no recursion, unlike the 6 calls for generic errors + expect(retryOperationSpy).toHaveBeenCalledTimes(1); + }); + + it('should log a single skip alert for non-retriable errors', async () => { + const logAlertSpy = jest.spyOn(Logger, 'logAlert'); + StorageMock.setItem = jest.fn().mockRejectedValue(nonRetriableIdbError); + + await Onyx.set(ONYXKEYS.TEST_KEY, {test: 'data'}); + + expect(logAlertSpy).toHaveBeenCalledWith(`Storage operation skipped retry for non-retriable error. Error: ${nonRetriableIdbError}. onyxMethod: setWithRetry.`); + // Not paired with the "5 retries exhausted" alert + expect(logAlertSpy).toHaveBeenCalledTimes(1); + }); + it('should include the error in logAlert for IDBObjectStore invalid data errors', async () => { const logAlertSpy = jest.spyOn(Logger, 'logAlert'); StorageMock.setItem = jest.fn().mockRejectedValueOnce(invalidDataError); From 620b50e157ebb3bbc3ed287dabdb0b11a38d5e38 Mon Sep 17 00:00:00 2001 From: eliran goshen Date: Mon, 18 May 2026 15:42:45 +0200 Subject: [PATCH 2/2] Regenerate API-INTERNAL.md for new retryOperation JSDoc branch CI's "Verify API Docs Are Up To Date" step caught that the auto-generated docs missed the new "Non-retriable errors" branch added to retryOperation's JSDoc. Regenerated via `npm run build:docs`. Co-Authored-By: Claude Opus 4.7 (1M context) --- API-INTERNAL.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/API-INTERNAL.md b/API-INTERNAL.md index c84d29afb..b5a9f55be 100644 --- a/API-INTERNAL.md +++ b/API-INTERNAL.md @@ -83,6 +83,7 @@ If the requested key is a collection, it will return an object with all the coll
  • Storage capacity errors: evicts data and retries the operation
  • Invalid data errors: logs an alert and throws an error
    • +
  • Non-retriable errors: logs an alert and resolves without retrying
  • Other errors: retries the operation
@@ -323,6 +324,7 @@ Remove a key from Onyx and update the subscribers Handles storage operation failures based on the error type: - Storage capacity errors: evicts data and retries the operation - Invalid data errors: logs an alert and throws an error +- Non-retriable errors: logs an alert and resolves without retrying - Other errors: retries the operation **Kind**: global function