Classify "Failed to write blobs" IDB errors as INVALID_DATA (never retry) - #814
Conversation
A File/Blob whose backing bytes are gone (source file modified or deleted after being picked) fails structured clone at write time as InvalidBlob (Windows) or IOError (macOS). Retrying re-reads the same dead blob, so the UNKNOWN bounded-retry path can never succeed — promote the error into the never-retriable INVALID_DATA class. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… HEAD Revert to the published version once the Onyx PR is merged and released. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. Delightful! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
|
Hi @elirangoshen is this okay to merge, I'm not sure that the linked App PR was successful or not |
|
I think it's because it's including Onyx changes (Onyx |
Hi I forgot to add screenshot to the linked pr, now i added and yes it was successful. |
…ify-write-blobs-invalid-data Classify "Failed to write blobs" IDB errors as INVALID_DATA (never retry)
Details
DataError: Failed to write blobs (InvalidBlob)(Windows Chromium) /(IOError)(macOS Chromium) is thrown by IndexedDB when aFile/Blobin the written value references backing bytes that no longer exist — in practice, a picked file that was modified, deleted, or renamed on disk between being picked and the first persist of the request queue. Production logs show ~46k such failures over 3 days, all with the same signature: the write fails on the first attempt and on every one of the 5 bounded retries (retryAttempt: 0/5→5/5), because retrying re-reads the same dead blob.Today this error falls through to
UNKNOWN, which the operation layer handles with bounded retries — provably futile for this error class. This PR classifiesfailed to write blobsasINVALID_DATA("non-serializable payload, never retriable — the same data will always fail"), which is exactly the taxonomy's contract for it. This kills the 5×-per-write retry storm; the UNKNOWN branch's own doc asks for recurring cases to be promoted into a named class, which is what this does.This is a storage-layer defense: it stops the futile retries for any value that carries a dead File/Blob, regardless of which app flow produced it.
Related Issues
Expensify/App#97050
Linked E/App PR
Expensify/App#96957
Automated Tests
Added
tests/unit/storage/providers/classifyErrorTest.tscovering the new branch (bothInvalidBlobandIOErrordialects →INVALID_DATA) plus the existing classes (put-clone failure, quota, backing-store corruption, transient connection failures, and theUNKNOWNfallthrough).Manual Tests
invalidDatastorage failure with noretryAttempt: 1/5–5/5retry sequence (previously:class: unknownwith 5 futile retries per write).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