Skip retry for unrecoverable IndexedDB blob-write errors + instrument failing writes#806
Draft
elirangoshen wants to merge 1 commit into
Draft
Conversation
…ng writes Chromium "Failed to write blobs" (IOError / InvalidBlob) was unclassified, so retryOperation retried it 5x and logged once per attempt before dropping the write. Prod logs show ~99.7% of these exhaust all retries and still fail (it is a Chromium blob-file write fault, not disk/quota), so the retries are pure noise. - Add a new StorageErrorClass.UNRECOVERABLE for engine-level write faults / permanently invalid payloads that no retry, reopen, or eviction can fix and that must not crash the caller (unlike INVALID_DATA, which throws). - Classify "failed to write blobs" as UNRECOVERABLE so it is skipped quietly. - Add a log-only diagnostic (summarizeFailedWritePayload) that reports the failing key(s) and approximate serialized size (never the values), to confirm whether the >64KB value-wrapping path is the trigger and identify the culprit keys. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
43 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Details
Follow-up from the Onyx storage-error re-check (Expensify/App#87844). After the March fixes shipped, the March top-3 errors collapsed as intended — but
DataError: Failed to write blobsgrew from 1.9% → ~93% of all Onyx storage errors and is now the dominant one (Expensify/App#87871).Root cause. Chromium wraps any IndexedDB value whose serialized size exceeds 64 KB into an external blob file (MIME
application/vnd.blink-idb-value-wrapper). The IDB provider stores raw values (store.put(value, key)), so large Onyx writes take this path, andDataError: Failed to write blobsis thrown when that blob-file write fails —IOError(the file write failed) orInvalidBlob(dangling blob reference). Batch writes (multiSetWithRetry) dominate because they are the most likely to contain a >64 KB value, and one bad blob rejects the whole transaction.The classifier didn't recognize this error, so it fell into
UNKNOWNand was bounded-retried 5×, with theFailed to save to storage … retryAttempt N/5line logging once per attempt — ~6 log lines per failure before the write was silently dropped. Prod logs show this is futile: theretryAttempthistogram is flat from0/5to5/5(~99.7% exhaust all 5 retries and still fail), and it does not correlate with quota/disk-full — it's a Chromium blob-write fault, not storage pressure.Changes:
StorageErrorClass.UNRECOVERABLE(lib/storage/errors.ts) — an engine-level write fault or permanently-invalid payload that no retry, reopen, or eviction can fix, and that must not crash the caller (unlikeINVALID_DATA, which throws).failed to write blobs→UNRECOVERABLE(lib/storage/providers/IDBKeyValProvider/classifyError.ts). The connection layer (createStore) already rethrows any non-TRANSIENT/FATALclass quietly (no reopen), andretryOperationgets a branch that logs one skip line and resolves — no 5× retry, no throw, no eviction.summarizeFailedWritePayloadinlib/OnyxUtils.ts) — onUNRECOVERABLEblob failures, logs the failing key(s) and each value's approximate serialized size (never the values themselves):pairs: N. over64KB: M. largest: [key=bytesB, …]. This is diagnostic: it confirms whether the failing writes are the >64 KB ones and identifies which keys, so we can target follow-up work (e.g. chunking specific large collections).Net effect: log volume drops from ~6× to ~2× per failure, with zero lost successful writes (retries weren't succeeding anyway).
CAPACITYwas considered and rejected (no disk pressure to relieve; eviction would delete cached user data for a Chromium engine fault).FATAL/TRANSIENTwere rejected (they trigger wasteful DB reopens).INVALID_DATAwas rejected (it throws).Related Issues
Expensify/App#87871
Linked E/App PR
Test branch pinning E/App to this PR's HEAD:
callstack-internal/Expensify-App@eliran/2397-idb-blob-write-errors-test(onyx pinned togit+https://github.com/callstack-internal/react-native-onyx.git#9b9aba708a408af226563deff6add1e6035e9ea0). E/App PR: Expensify/App#95586Automated Tests
Added
tests/unit/storage/providers/classifyIDBErrorTest.tscoveringclassifyIDBError:Failed to write blobs (IOError)→UNRECOVERABLEFailed to write blobs (InvalidBlob)→UNRECOVERABLEQuotaExceededError→CAPACITYinternal error opening backing store→FATALUNKNOWNManual Tests
The underlying Chromium blob-write failure can't be reliably reproduced on demand, so verify the classification/handling behavior:
new DOMException('Failed to write blobs (IOError)', 'DataError')from an IDB write, or unit-simulate viaclassifyIDBError).Storage operation skipped retry; unrecoverable errors cannot be recovered by retrying …log line, followed by thepairs: … over64KB: … largest: […]payload summary.Attempted to set invalid data …throw, no eviction, and noIDB heal …reopen logs for this error.Author Checklist
### Related Issuessection above### Linked E/App PRsection above, and verified this change against it (E/App CI passed and manual testing completed)TestssectiontoggleReportand notonIconClick)myBool && <MyComponent />.STYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)/** comment above it */thisproperly so there are no scoping issues (i.e. foronClick={this.submit}the methodthis.submitshould be bound tothisin the constructor)thisare necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);ifthis.submitis never passed to a component event handler likeonClick)Avataris modified, I verified thatAvataris working as expected in all cases)mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari