mirror of
https://github.com/Stirling-Tools/Stirling-PDF.git
synced 2026-09-03 05:10:16 +03:00
fix(frontend): keep file persistence working when IndexedDB refuses blobs (#7314)
# 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>
This commit is contained in:
co-authored by
Anthony Stirling
parent
1867c8f285
commit
cff6549a40
@@ -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<IDBValidKey>;
|
||||
}
|
||||
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"]);
|
||||
});
|
||||
});
|
||||
@@ -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<void> {
|
||||
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);
|
||||
}
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user