From cff6549a40340b01e8be7211af5be3ab34ee3ba6 Mon Sep 17 00:00:00 2001 From: EthanHealy01 <80844253+EthanHealy01@users.noreply.github.com> Date: Fri, 7 Aug 2026 14:36:46 +0100 Subject: [PATCH] fix(frontend): keep file persistence working when IndexedDB refuses blobs (#7314) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Description of Changes Fixes the WebKit nightly failures ([run 31067620195](https://github.com/Stirling-Tools/Stirling-PDF/actions/runs/31067620195/attempts/1)): 8 tests failed on `stubbed-webkit` only, and every one of them logs the same thing in its trace: ``` IndexedDB add error: UnknownError: Error preparing Blob/File data to be stored in object store ``` ## What broke `storeStirlingFile` stores the `File` itself in IndexedDB, so multi-GB uploads are persisted by reference and never materialize in JS memory. That came in with #7175 (`data: stirlingFile` replacing `data: await stirlingFile.arrayBuffer()`), which is a real memory win and worth keeping. WebKit refuses blob values whenever it can't write the blob's backing file, and rejects the request with the error above. The rejection was only `console.error`d, so on WebKit **no upload ever persisted**, and everything that reads the bytes back behaved as if the upload never happened: - `file-state-across-tools` — file gone after navigating; the sidebar shows "No files yet" - `compare` — `FileSelectorPicker: upload failed`, so the slot stays `data-slot-state="empty"` - `classification-grouping` / `classification-heuristic-upload` — the label backfill and thumbnails read from IDB (`not in IndexedDB (likely remote-only stub)`), so files land in "Recent" with no category headers Chromium and Firefox store blobs fine, and PR CI only runs the `stubbed` (chromium) project, so nightly was the only gate that could catch it. ## The fix Try the blob first, keep a fallback: - `storeStirlingFile`'s `add` is extracted into `addFileRecord` so it can run twice - if the value was a Blob and the failure is `UnknownError` / `DataCloneError`, re-add the record with an `ArrayBuffer` copy and set `blobValuesSupported = false`, so later files in that session go straight to the copy path instead of losing the blob attempt every time - deliberately narrow: `QuotaExceededError` and `ConstraintError` still propagate, because a copy would fail the same way and retrying would hide the real cause - dropped two internal `console.error`s: every caller already reports (`addFiles`, `FileSelectorPicker`, `zipFileService` collects into `result.errors`), so they were duplicate noise Every writer goes through `storeStirlingFile` (uploads, the file picker, zip extraction, folder automation, `IndexedDBContext`), so this one seam covers all of them. The read paths already accept either shape (`new Blob([record.data], ...)`). Net effect: Chromium and Firefox keep the no-copy path; engines that refuse blobs degrade to the pre-#7175 behaviour instead of silently losing files. On such an engine a very large file can still exhaust renderer memory — the fallback warns about exactly that. Fixing that properly means chunked storage, which is out of scope here. ## Verification Reproduced and confirmed the cause by A/B on a branch that predates #7175: as-is 8/8 pass on WebKit, and applying only #7175's `data: stirlingFile` line reproduces the exact CI failure set. | Check | Result | |---|---| | `stubbed-webkit`: the 8 nightly failures + `classification-heuristic-upload` | 9 passed | | `stubbed-webkit`: `files-page`, `page-editor-rotation`, `encrypted-pdf-unlock` | 32 passed, 1 skipped | | `stubbed` (chromium): the same specs + `files-page` | 35 passed, 1 skipped | | Frontend unit suite | 210 files, 1797 passed | | `typecheck:core`, `typecheck:proprietary`, eslint, prettier | clean | New unit coverage in `fileStorage.blobFallback.test.ts` pins the contract over `fake-indexeddb` with `add` instrumented to count blob vs copy attempts: blob path when accepted, blob-then-copy when refused (and readable back), one attempt only for later files, and quota not retried. --- ## Checklist ### General - [x] I have read the [Contribution Guidelines](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/CONTRIBUTING.md) - [x] I have read the [Stirling-PDF Developer Guide](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/DeveloperGuide.md) (if applicable) - [x] I have performed a self-review of my own code - [x] My changes generate no new warnings ### Testing (if applicable) - [x] Frontend typecheck (core + proprietary), eslint, prettier, the unit suite, and the affected Playwright specs on chromium and webkit all pass Co-authored-by: Anthony Stirling <77850077+Frooodle@users.noreply.github.com> --- .../services/fileStorage.blobFallback.test.ts | 146 ++++++++++++++++++ .../editor/src/core/services/fileStorage.ts | 57 +++++-- 2 files changed, 194 insertions(+), 9 deletions(-) create mode 100644 frontend/editor/src/core/services/fileStorage.blobFallback.test.ts diff --git a/frontend/editor/src/core/services/fileStorage.blobFallback.test.ts b/frontend/editor/src/core/services/fileStorage.blobFallback.test.ts new file mode 100644 index 0000000000..d8530af026 --- /dev/null +++ b/frontend/editor/src/core/services/fileStorage.blobFallback.test.ts @@ -0,0 +1,146 @@ +import { describe, expect, test, afterEach, beforeEach, vi } from "vitest"; +import "fake-indexeddb/auto"; +import { expectConsole } from "@app/tests/failOnConsole"; + +/** + * Regression test for the WebKit nightly breakage introduced with the + * large-file OOM fix (#7175): `storeStirlingFile` began putting the `File` + * itself into IndexedDB (persisted by reference, so multi-GB uploads never + * materialize in JS memory). WebKit refuses blob values whenever it can't write + * the blob's backing file and rejects the request with `UnknownError: Error + * preparing Blob/File data to be stored in object store`, so on WebKit every + * upload silently failed to persist: files vanished on navigation, Compare + * slots never filled, and the classification backfill had no bytes to read. + * + * The service now retries such a rejection with an ArrayBuffer copy and stops + * offering blobs for the rest of the session. + */ + +const nativeAdd = IDBObjectStore.prototype.add; + +/** What each `add` attempt carried in `data` — the blob path or the copy path. */ +let attempts: Array<"blob" | "copy"> = []; + +/** An IDBRequest that fails asynchronously, the way WebKit rejects blob puts. */ +class FailingRequest extends EventTarget { + onerror: ((event: Event) => void) | null = null; + onsuccess: ((event: Event) => void) | null = null; + + constructor(readonly error: DOMException) { + super(); + queueMicrotask(() => this.onerror?.(new Event("error"))); + } +} + +/** + * Record every add attempt, optionally failing the blob-valued ones the way an + * engine without blob storage does. + */ +function instrumentAdd(options: { rejectBlobs: boolean }) { + IDBObjectStore.prototype.add = function ( + this: IDBObjectStore, + value: unknown, + key?: IDBValidKey, + ) { + const isBlob = (value as { data?: unknown } | null)?.data instanceof Blob; + attempts.push(isBlob ? "blob" : "copy"); + if (isBlob && options.rejectBlobs) { + return new FailingRequest( + new DOMException( + "Error preparing Blob/File data to be stored in object store", + "UnknownError", + ), + ) as unknown as IDBRequest; + } + return key === undefined + ? nativeAdd.call(this, value) + : nativeAdd.call(this, value, key); + } as typeof IDBObjectStore.prototype.add; +} + +/** + * A fresh service per test: whether the engine accepts blobs is remembered for + * the process lifetime by design, so tests must not inherit that decision from + * each other. + */ +async function freshFileStorage() { + vi.resetModules(); + const [{ fileStorage }, { createStirlingFile, createNewStirlingFileStub }] = + await Promise.all([ + import("@app/services/fileStorage"), + import("@app/types/fileContext"), + ]); + const store = async (name: string) => { + const file = new File(["%PDF-1.7 stirling"], name, { + type: "application/pdf", + }); + const stub = createNewStirlingFileStub(file); + await fileStorage.storeStirlingFile( + createStirlingFile(file, stub.id), + stub, + ); + return stub.id; + }; + return { fileStorage, store }; +} + +beforeEach(() => { + attempts = []; +}); + +afterEach(() => { + IDBObjectStore.prototype.add = nativeAdd; +}); + +describe("storeStirlingFile — blob-value fallback", () => { + test("stores the File by reference when the engine accepts blob values", async () => { + const { fileStorage, store } = await freshFileStorage(); + instrumentAdd({ rejectBlobs: false }); + + const id = await store("by-reference.pdf"); + + expect(attempts).toEqual(["blob"]); + expect((await fileStorage.getStirlingFile(id))?.name).toBe( + "by-reference.pdf", + ); + }); + + test("falls back to a copy when the engine rejects blob values, and the file stays readable", async () => { + expectConsole.warn(/IndexedDB rejected a Blob value/); + const { fileStorage, store } = await freshFileStorage(); + instrumentAdd({ rejectBlobs: true }); + + const id = await store("webkit.pdf"); + + expect(attempts).toEqual(["blob", "copy"]); + // Readable back is what every downstream consumer depends on: rehydration + // after navigation, thumbnails, the classification backfill. + expect((await fileStorage.getStirlingFile(id))?.name).toBe("webkit.pdf"); + }); + + test("remembers the rejection, so later files skip the doomed blob attempt", async () => { + expectConsole.warn(/IndexedDB rejected a Blob value/); + const { fileStorage, store } = await freshFileStorage(); + instrumentAdd({ rejectBlobs: true }); + + await store("first.pdf"); + attempts = []; + const id = await store("second.pdf"); + + // Straight to the copy path — no repeated blob probe, and only the single + // warning expected above. + expect(attempts).toEqual(["copy"]); + expect((await fileStorage.getStirlingFile(id))?.name).toBe("second.pdf"); + }); + + test("does not retry a failure a copy can't fix (quota)", async () => { + const { store } = await freshFileStorage(); + IDBObjectStore.prototype.add = function (this: IDBObjectStore) { + attempts.push("blob"); + throw new DOMException("no space left", "QuotaExceededError"); + } as typeof IDBObjectStore.prototype.add; + + await expect(store("too-big.pdf")).rejects.toThrow(/no space left/); + expect(attempts).toEqual(["blob"]); + }); +}); diff --git a/frontend/editor/src/core/services/fileStorage.ts b/frontend/editor/src/core/services/fileStorage.ts index 4f82af3a8f..40f62642f0 100644 --- a/frontend/editor/src/core/services/fileStorage.ts +++ b/frontend/editor/src/core/services/fileStorage.ts @@ -63,9 +63,27 @@ export function legacyDerivedFromTool( return undefined; } +/** + * Can't persist a Blob/File value, so a copy would work? WebKit reports + * `UnknownError` ("Error preparing Blob/File data...") when it can't write the + * blob's backing file; a refused structured clone is `DataCloneError`. + * Narrow on purpose: retrying quota or duplicate-key failures would fail again + * and hide the real cause. + */ +function isBlobValueRejection(error: unknown): boolean { + const name = (error as DOMException | null)?.name; + return name === "UnknownError" || name === "DataCloneError"; +} + class FileStorageService { private readonly dbConfig = DATABASE_CONFIGS.FILES; private readonly storeName = "files"; + /** + * Whether this engine accepts Blob/File values in IndexedDB. Optimistic: the + * blob path avoids copying multi-GB files into JS memory, so we try it and + * remember the answer, rather than pre-emptively degrading everywhere. + */ + private blobValuesSupported = true; /** * Get database connection using centralized manager @@ -132,7 +150,10 @@ class FileStorageService { createdAt: stub.createdAt, // Store the File (a Blob) itself: IndexedDB persists it by reference and // streams to disk, so multi-GB files never materialize in JS memory. - data: stirlingFile, + // Engines that reject blob values fall back to a copy — see addFileRecord. + data: this.blobValuesSupported + ? stirlingFile + : await stirlingFile.arrayBuffer(), thumbnail: stub.thumbnailUrl, thumbnailStoredAt: stub.thumbnailUrl ? Date.now() : undefined, isLeaf: stub.isLeaf ?? true, @@ -160,6 +181,30 @@ class FileStorageService { classificationLabels: stub.classificationLabels, }; + try { + await this.addFileRecord(db, record); + } catch (error) { + // Recoverable: re-add as a copy, and stop offering blobs this session. + // Anything else is the caller's to report. + if (!(record.data instanceof Blob) || !isBlobValueRejection(error)) { + throw error; + } + this.blobValuesSupported = false; + console.warn( + "IndexedDB rejected a Blob value; falling back to in-memory copies for this session. " + + "Very large files may now exhaust renderer memory.", + error, + ); + record.data = await record.data.arrayBuffer(); + await this.addFileRecord(db, record); + } + } + + /** Single `add` of a file record. Rejects with the underlying IDB error. */ + private addFileRecord( + db: IDBDatabase, + record: StoredStirlingFileRecord, + ): Promise { return new Promise((resolve, reject) => { try { // Verify store exists before creating transaction @@ -174,15 +219,9 @@ class FileStorageService { const request = store.add(record); - request.onerror = () => { - console.error("IndexedDB add error:", request.error); - reject(request.error); - }; - request.onsuccess = () => { - resolve(); - }; + request.onerror = () => reject(request.error); + request.onsuccess = () => resolve(); } catch (error) { - console.error("Transaction error:", error); reject(error); } });