Skip to content

Fix WebKit PDF-engine and storage failures, and catch them in cross-browser CI - #7366

Merged
EthanHealy01 merged 23 commits into
mainfrom
fix/webkit-engine-capabilities
Aug 13, 2026
Merged

Fix WebKit PDF-engine and storage failures, and catch them in cross-browser CI#7366
EthanHealy01 merged 23 commits into
mainfrom
fix/webkit-engine-capabilities

Conversation

@EthanHealy01

@EthanHealy01 EthanHealy01 commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Description of Changes

Follow-up to #7314, which fixed the IndexedDB blob rejection itself. This one fixes the remaining WebKit engine gaps, fixes the ways that class of failure surfaced to the user, and adds the cross-browser signal that would have caught them on the PR instead of six weeks later.

Why this exists

Two total WebKit outages sat on main for weeks:

  1. pdf.js reads its text stream with for await (… of readableStream), and WebKit has no ReadableStream[Symbol.asyncIterator]. All pdf.js text extraction threw TypeError: undefined is not a function — Compare, read-aloud and the PDF text editor were dead on Safari.
  2. IndexedDB in WebKit rejects Blob/File values with UnknownError: Error preparing Blob/File data to be stored in object store, so nothing persisted and every reload came back empty.

Neither was caught, because the existing specs never did the work. The Compare specs filled both slots and asserted the button was enabled; none of them clicked it. The persistence specs asserted a filename reappeared after a reload, which only needs the metadata record, not the bytes.

Every failure here looked like success — empty panes, blank thumbnails, a src that was set but empty. That shapes the tests more than the fixes.

WebKit engine gaps

  • ReadableStream[Symbol.asyncIterator], installed at the entry point before any PDF work starts. The lock discipline is the subtle part: releasing is idempotent, is not done after a successful read, and is done in the read's error steps — for await never calls return() when next() rejects, so nothing else would ever unlock an errored stream.
  • requestIdleCallback, installed once instead of guarded at each call site. This one wasn't broken, it was mistimed: the local fallbacks fired at 200ms and 1000ms, landing the pdfium WASM compile on top of the app's first renders. The shim honours the caller's full timeout, so {timeout: 2000} means 2000ms.
  • convertToBlob() does not fail on a format it can't encode. Per spec it silently serialises to PNG, so asking for WebP and getting PNG back looks like success. Canvas output now probes what the engine really produced (once per realm) and uses the best lossy format it honours. PNG of a rendered page is several times the size of the equivalent WebP or JPEG, held as object URLs for every page on screen, on the engine with the tightest renderer memory budget.

WebKit storage failures

These read as generic transaction hygiene. They aren't — a refused blob write aborts its transaction, which is the mechanism that turned a WebKit rejection into a hang.

  • Blob refusal is remembered from any write, not just the initial add. WebKit reports it when it can't write the blob's backing file, which is per-operation — an engine that accepted the add can still refuse the rewrite, and every read-modify-write rewrites the record with its body attached.
  • Aborted transactions no longer hang. Read-modify-write moves to a single updateRecord helper that owns its transaction, guards it once, and resolves on commit rather than on the put's onsuccess. The previous shape — two promises over one shared transaction, with an await between the get and the put — put the abort guard on the read, leaving the write with no handler at all. persistVersionedOutputs awaits that, and .catch can't rescue a promise that never settles, so tool outputs could silently stop persisting.
  • Stored blobs are no longer re-wrapped on read. Since Avoid renderer OOM when adding large PDFs to the workbench #7175 the record holds the File itself; wrapping it in new Blob([record.data]) can cost WebKit the backing handle, giving you an object that looks valid and reads as empty.
  • The file sidebar reaches a resting state when the library can't be read, instead of spinning forever on a rejection nobody observes. It carries on with the in-memory workbench files: an unreadable library should cost the user their history, not the file they're working on.
  • Thumbnail failures are logged. Three catch {} blocks returned "", and an empty thumbnail is indistinguishable from "this file has no preview" — which is how outage Frooodle patch 1 #1 hid as a cosmetic nicety.

CI

main now runs the whole stubbed suite once per engine (#7304), so the new @engine-capability specs get chromium, firefox and webkit for free. They assert the primitives actually work — a counted comparison, a raster thumbnail data URL with real payload, and a page rendered from a file restored by a reload — rather than that the UI rendered. Deliberately small: anything added there is paid for three times per PR, so add depth, not breadth. Run them alone with task e2e:cross-browser -- --grep @engine-capability.

The cross-browser projects now share the stubbed project's viewport. At the device presets' default 1280x720 a layout difference would fail these specs on Firefox/WebKit only, which reads as an engine outage.

vite.config.ts gains a worker.plugins entry so @app/* resolves inside worker bundles. Worker bundles are a separate Rollup pass and don't inherit plugins, so the alias worked in the app and failed in a worker — previously worked around with a relative import plus a lint exemption, which silently bypasses the layer cascade.

Verification

  • task frontend:check green: typecheck, oxlint, theme lint, stylelint, prettier, 215 test files / 1841 tests.
  • The @engine-capability suite passes on Chromium and WebKit locally.
  • Negative control: with the ReadableStream shim removed, the WebKit comparison spec fails at the Deletions/Additions assertion — the exact reported Safari symptom. Restored, and it passes. Both the fix and the test that guards it are load-bearing.
  • The worker alias change verified both ways: the build inlines the encoding probe into the worker chunk, and removing worker.plugins fails with Rollup failed to resolve import "@app/utils/canvasImageEncoding".
  • The abort regression test aborts the transaction mid-write and asserts markFileAsProcessed settles. Before the fix it never settles and the test times out.

Split out of this PR

Two things in earlier revisions of this branch were engine-agnostic — found via the same symptom, not the same cause — and now have their own PRs:

FileSidebar's try/catch appears in both this PR and #7416, identically: a WebKit rejection and a blocked-open rejection both have to stop stranding the spinner. Whichever merges second is a no-op for that file.

Known gaps

  • The blob-refused rewrite recovery in updateRecord isn't unit-tested. fake-indexeddb never returns Blob values from a read, so the branch that converts to a copy can't be reached there. Noted in the test file.
  • For the same reason, fileFromRecord's "hand the stored File back untouched" path is only covered on a real engine, by the reload spec.
  • Nothing asserts that src/index.tsx imports the shims. The unit suite installs the same module via setupTests.ts (jsdom has the same gaps WebKit does), so a future regression where the entry point drops the import would still be green under vitest.
  • FileSidebar's resting-state fix loses its E2E coverage until Stop a blocked IndexedDB upgrade from hanging the file library #7416 lands — forcing WebKit's blob refusal from a spec isn't practical, which is why that spec blocks the database instead.

Checklist

General

Documentation

@EthanHealy01
EthanHealy01 requested review from a team and Ludy87 as code owners August 8, 2026 19:49
@dosubot dosubot Bot added size:XL This PR changes 500-999 lines ignoring generated files. Bugfix Pull requests that fix bugs labels Aug 8, 2026
@stirlingbot stirlingbot Bot added Front End Issues or pull requests related to front-end development GitHub Issues or pull requests related to GitHub configuration and integrations and removed Bugfix Pull requests that fix bugs labels Aug 8, 2026
@EthanHealy01
EthanHealy01 force-pushed the fix/webkit-engine-capabilities branch from e1ecef0 to 04eb073 Compare August 8, 2026 19:56
Comment thread frontend/editor/src/core/utils/patchReadableStreamAsyncIterator.ts Outdated
@EthanHealy01
EthanHealy01 force-pushed the fix/webkit-engine-capabilities branch from 04eb073 to d90b1b7 Compare August 8, 2026 20:26
Comment thread frontend/editor/src/core/utils/patchReadableStreamAsyncIterator.ts Outdated
@EthanHealy01
EthanHealy01 force-pushed the fix/webkit-engine-capabilities branch 2 times, most recently from b4629c6 to 4aacd9d Compare August 8, 2026 21:35
…he app

Follow-up to #7314. That PR fixed the IndexedDB blob rejection itself; this
one fixes the ways the same failures surfaced as a permanent spinner, and adds
the cross-browser CI signal that would have caught them on the pull request
instead of six weeks later in a nightly run.

Storage no longer hangs:
- Blocked IndexedDB opens and deletes now time out with an actionable error
  instead of never settling, and the app sets `onversionchange` so an open tab
  yields its connection rather than blocking every other tab forever.
- The in-flight open promise is registered before the first await, so
  concurrent boot-time callers share one connection. Only the first request
  receives `blocked`; the others were getting no events at all.
- Transactions settle on abort. Read-modify-write moves to a single
  `updateRecord` helper that owns its transaction, guards it once, and resolves
  on commit - the previous two-promises-over-one-transaction shape left the
  write with no abort handler, which could hang output persistence silently.
- The blob-value refusal is remembered from any write, not just the initial
  add: WebKit reports it per-operation, so an engine that accepted the add can
  still refuse the rewrite.

WebKit engine gaps:
- ReadableStream async iteration, which pdf.js uses for all text extraction.
  Without it Compare, read-aloud and the text editor were dead on Safari.
- requestIdleCallback, installed once at the entry point instead of guarded at
  each call site.
- convertToBlob silently returns PNG for a format it can't encode, so canvas
  output now probes what the engine really produced and picks the best lossy
  format it honours.

CI:
- A small @engine-capability suite runs on Chromium, Firefox and WebKit on
  every pull request. It asserts the primitives actually work (a counted
  comparison, a raster thumbnail, a byte round-trip through a reload) rather
  than that the UI rendered, which is how two total WebKit outages passed.
- The cross-browser projects now share the stubbed project's viewport so a
  layout difference can't read as an engine outage.
@EthanHealy01
EthanHealy01 force-pushed the fix/webkit-engine-capabilities branch from 4aacd9d to d6168a5 Compare August 8, 2026 21:44
@stirlingbot stirlingbot Bot added the has conflicts Pull request has merge conflicts with the base branch label Aug 10, 2026
Resolves the e2e CI conflicts against #7304, which landed a per-browser
matrix that already runs the whole stubbed suite on chromium, firefox and
webkit for every PR.

- .github/workflows/e2e-stubbed.yml: take main's matrix. It supersedes this
  branch's dedicated @engine-capability step, which existed only to get
  cross-engine coverage into PRs without paying for the full suite.
- .taskfiles/e2e.yml: keep both tasks. `stubbed-project` is what the matrix
  calls; `capabilities` stays as the local shortcut for checking all three
  engines without the whole cross-browser run.
@stirlingbot stirlingbot Bot removed has conflicts Pull request has merge conflicts with the base branch GitHub Issues or pull requests related to GitHub configuration and integrations labels Aug 10, 2026
#7304 made CI run the whole stubbed suite once per engine, which removed the
only use of this task that e2e:cross-browser didn't already cover. Run the
capability specs alone with:

  task e2e:cross-browser -- --grep @engine-capability

The @engine-capability tag stays, so the specs are unchanged.
Worker bundles are a separate Rollup pass and do not inherit `plugins`, so
`@app/*` - provided by vite-tsconfig-paths - resolved in the app and failed
in a worker. The workaround was a relative import plus an oxlint exemption,
which left the next value import into a worker to rediscover the same thing.

Give the worker pass the same tsconfigPaths plugin. Verified both ways: the
build resolves the alias and inlines the probe into the worker chunk, and
removing the block fails with "Rollup failed to resolve @app/utils/
canvasImageEncoding from pixelCompareWorker.ts".
The multi-tab IndexedDB lifecycle fixes and the thumbnail TTL write
amplification fix are not WebKit bugs - they were found while chasing the same
symptom, not the same cause. They now live in their own PRs so each can be
reviewed against its own evidence:

- fix/indexeddb-multitab-lifecycle: blocked opens/deletes, the concurrent-open
  race, onversionchange, and the sidebar/picker resting state.
- perf/thumbnail-ttl-write-amplification: the once-a-day bump debounce.

What stays here is WebKit-caused, including the parts that look like generic
refactoring: a refused blob write aborts its transaction, so `settleOnAbort`
and the single-transaction `updateRecord` are the fix for tool outputs silently
failing to persist on Safari, not tidy-up.

FileSidebar's try/catch appears in both this branch and the lifecycle branch -
a WebKit rejection and a blocked-open rejection both have to stop stranding the
spinner. Whichever lands second is a no-op for that file.
@EthanHealy01 EthanHealy01 changed the title Stop WebKit storage and PDF-engine failures from hanging the app, and catch them in PR CI Fix WebKit PDF-engine and storage failures, and catch them in cross-browser CI Aug 10, 2026
@github-actions

github-actions Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

📦 Tauri Desktop Builds Ready!

The desktop applications have been built and are ready for testing.

Download Artifacts:

🍎 macOS Universal: Download Stirling-PDF-macos-universal (.dmg) - 340.5 MB
🪟 Windows x64: Download Stirling-PDF-windows-x86_64 (.exe, .msi) - 264.5 MB


Built from commit 8c98d24
Artifacts expire in 7 days

Storybook builds through its own Vite config, so the worker.plugins entry added
to editor/vite.config.ts didn't reach it: the app build resolved the alias and
the Storybook preview build failed on the same import.

Same root cause, same fix. The tsconfigPaths plugin is now built by a helper so
the main pass and the worker pass each get their own instance instead of the
alias map being restated.

Caught by CI rather than locally because `task frontend:check` stops at
lint/typecheck/test - storybook:build only runs under `check:all`.
WebKit refuses Blob/File values in IndexedDB outright - proven with raw
IndexedDB and no app code: the write fails with UnknownError and aborts its
transaction, where Chromium commits and reads back cleanly. It can also accept
one and later lose the backing store, leaving a record that looks valid and
whose bytes are gone.

- The "this browser loses blob values" verdict is now durable. Session-scoped,
  every reload re-decided optimistically and wrote another batch of files the
  engine would lose, so Safari never converged on a shape that works.
- Readability is reported, never awaited. The probe read of a lost backing store
  can stay pending forever in Safari, so awaiting it stalled EVERY file open
  rather than the one consumer that would have failed anyway. The store-time
  probe, which must run while the source file is still in hand to repair the
  record, gets a deadline instead.
- deleteStirlingFile resolves on commit with an abort guard: callers refresh
  their list as soon as it resolves, and an aborted delete put the row back.
- orphanedAncestorIds() collects the versions nothing else needs, keeping any
  ancestor a surviving leaf still descends from (split siblings).
`instantiateWasm` reports success by callback, so a rejection inside it is
invisible to emscripten: init() simply stays pending, and with it every
thumbnail, page parse, form read and policy delivery - silently, for the rest of
the session. The hand-rolled streaming fallback had no rejection handling at all.

Failures are now raced into the init promise, the fallback is gone (with no
override emscripten fetches the WASM itself and rejects properly), and a failed
load is no longer cached so the next call can retry.

The viewer was unaffected throughout because it uses embedpdf's own engine, which
is why this presented as "I can view files but see no thumbnails".
The workbench renders from hydrated bytes held in a ref, so a file is invisible
until hydration dispatches - and the only dispatch sat after a full pdfium parse
of the whole PDF. Loading also shared the parse queue's two slots, so a stalled
parse kept other files from loading at all. Observed in Chrome: nine seconds of
the upload drop zone on a cold engine, with the sidebar row showing as open.

- The File is published as soon as it loads; the parse is still queued and now
  only refines the stub (page metadata, thumbnail).
- The workbench shows progress instead of the drop zone while files are loading.
- A load that hasn't settled after 8s names the file in the console. Reporting
  only: the read is never abandoned, because large files legitimately take time.
Deleting a library file that was never in the workbench dispatches REMOVE_FILES
for an id the reducer doesn't hold. It rebuilt `files` and `ui` anyway, which the
dev identity guard flags: every file and UI consumer re-renders for nothing. The
console noise also buried the errors we were hunting.
Deleting a file removed one record; its older versions kept their full bytes and
were invisible, because listings filter on isLeaf. Observed live: three uploads
produced six records (a policy versioned each), and deleting one file left its v1
behind.

The lineage expansion belongs at the user-facing delete sites, NOT in
removeFiles: VersionHistoryModal deletes individual versions through the same
low-level path, so expanding there would wipe a whole chain when the user removes
one version.
getFiles() during render doesn't subscribe to the state it reads, so the panel
kept showing the pre-hydration (or pre-version) file. The app's own guard was
logging this while opening a file.
The enforcement overlay swallows clicks, and the card's had no dismiss - so a run
that never settles left the file permanently unusable with no way out. The
viewer's equivalent has always been dismissible.
A policy that changes nothing (redaction matching no text, say) completes with no
output file. The import effect skipped those runs entirely, so `imported` never
flipped - and the badge treats `imported` as the settle signal, so the file's
spinner and its blocking overlay ran forever.

Engine-agnostic: it needs a document the policy's patterns don't match, which is
why it looked Safari-specific. Confirmed from a backend log showing "Redaction
scan: 0 occurrences across 0 pages" for every spinning file.
@dosubot dosubot Bot added size:XXL This PR changes 1000+ lines ignoring generated files. and removed size:XL This PR changes 500-999 lines ignoring generated files. labels Aug 12, 2026
@EthanHealy01

Copy link
Copy Markdown
Contributor Author

/deploypr

Handing dead bytes over stopped the open path from stalling, but left the viewer
rendering a document that never loads - an endless spinner, reported on Safari and
the DuckDuckGo browser with the "File data is unavailable" toast alongside it.

- fileStorage notifies listeners when a record's bytes are confirmed unreadable,
  and refuses to hand that record out again for the rest of the session.
- FileContext subscribes and removes the file, so nothing keeps waiting on bytes
  that can't arrive.

The record itself is kept and the refusal is session-scoped, so a reload re-tests
it: WebKit throwing NotFoundError once isn't proof the file is gone forever, and
deleting on that evidence risks destroying recoverable data.
The stub/shadow pattern needs both layers to share an interface: core code passes
onDismiss, and in a core build the overlay resolves to the stub, which didn't
declare it. Broke `frontend:typecheck:core` - the proprietary typecheck passes
because the real overlay has always accepted it.
The library spinner ran forever with an empty console after uploading several
files and reloading. A blocked open fires `blocked` and then NOTHING - no success,
no error - so the open promise never settled and every caller hung. FileSidebar's
try/catch can't help: it guards a rejection, not a promise that never settles.

- `blockedGuard`: warn, wait out a grace period, then reject with something the
  user can act on. Rejecting doesn't cancel the request, so a connection that
  arrives late is closed rather than held - otherwise we become the next blocker.
- Registration hoisted above everything async. A map written after a yield point
  can't dedupe callers racing into it in the same tick, so every context that
  opened the files database during boot got its own connection, and per spec only
  the first request ever receives `blocked`. This only affected
  stirling-pdf-files: the one config with an await before registration.
- `onversionchange` closes and forgets our connection, so a release that bumps the
  schema no longer bricks every open tab. Forget before close: a cached but closed
  handle is worse than none, because every transaction on it throws.
- `deleteDatabase` gets the same guard; it blocks the same way and is awaited on
  the files open path.

Re-derived from #7416, which was closed unmerged and is in neither main nor this
branch. Whichever lands second is a no-op for the overlapping parts.
… rest

The library must tell the truth per row instead of listing files that pretend to
open. Timers and toasts were treating the symptom; this makes the sidebar
represent actual IndexedDB status, so legacy WebKit damage is visible and
actionable (re-upload), and stops the remaining data-loss vector.

- Listings audit every blob-backed record's bytes out of band (never awaited -
  the probe itself can hang on WebKit). A record whose bytes are gone flags
  `dataUnavailable` on its stub, renders a "Data lost" badge with a tooltip, and
  its click explains instead of failing.
- RESCUE: on a browser whose durable verdict is "blobs unsupported", a legacy
  blob record that is still readable today is rewritten as an ArrayBuffer copy
  while the bytes still exist - closing the loss vector for pre-verdict records
  instead of waiting for WebKit to lose them too.
- The v6/v7 version probe runs before the guarded open and a versionless open can
  be delayed indefinitely by another tab mid-versionchange; it now proceeds
  without an answer rather than hanging every storage consumer ahead of the
  blocked guard.

Records are never auto-deleted: one NotFoundError is not proof the bytes are
gone forever, and the flag is session-scoped so a reload re-tests.
@stirlingbot stirlingbot Bot added the Translation Issues or pull requests related to translation label Aug 12, 2026
The infinite "Loading files..." after a Safari reload, finally caught in the act:
the sidebar listing's thumbnail TTL bump opens a readwrite transaction that
re-reads every record - and in WebKit a `get` touching a blob-bodied record with
a damaged backing store HANGS rather than errors. One pending request keeps that
transaction alive forever, and every later transaction on the store queues behind
it: hydration reads, new uploads, policy-output persists. One wedge explained the
whole screen - files that won't open, fresh uploads spinning, runs that never
deliver.

Proof from the session log: after the TTL bump's refused-put warning, reads of
even a just-rescued (ArrayBuffer) record never settled - a healthy record's read
hanging means the store is blocked, not the record.

Maintenance now skips blob-bodied records on a browser whose durable verdict is
"blobs unsupported": their rewrite would be refused anyway, so they are all risk
and no value. Chrome (blobs genuinely supported) is unchanged.
Review pass, no behaviour change:

- Drop `isRecordUnreadable()`. Nothing in the app called it - listings read the
  set directly - so it was public API existing only for two test assertions.
  Those now assert the user-visible contract (`stub.dataUnavailable`) instead.
- Revert `createBlobUrl` to its previous shape. It has ZERO production callers,
  so rewriting it (and auditing inside it) was diff noise on a dead method.
- Remove a dead guard in `reportIfUnreadable`: every probed record is added to
  `auditedRecords` before probing, including ones that turn out unreadable, so
  the `unreadableRecords` check could never be the one to short-circuit.
EthanHealy01 added a commit that referenced this pull request Aug 12, 2026
Follow-up to #7366, which fixed the blocked-open hang itself. What's left here is
the UI resilience and the test that reproduces the failure for real:

- The saved-files picker cancels its storage read and scopes its spinner to the
  saved tab. The Workbench tab renders from memory, so a stuck storage read must
  not spin it too, and a read that outlives the popover must not set state.
- `file-library-resilience.spec.ts` doesn't mock the failure: an init script parks
  a connection on an older version and never yields it, then the spec asserts the
  workbench still renders, the sidebar spinner clears, and the reason reaches the
  console. Cleanup deletes the database explicitly, because WebKit keeps origin
  databases between browser contexts and a stray version would fail a later spec.
- `api-stubs` gains `storageEnabled`, which that spec needs to exercise the route.

Trimmed of everything #7366 now carries: the blocked guard, hoisted registration
and `onversionchange` in indexedDBManager, its unit tests, and FileSidebar's
try/catch (which was byte-identical).
EthanHealy01 added a commit that referenced this pull request Aug 12, 2026
Every listing bumped `thumbnailStoredAt` on every record it returned, and a bump
rewrites the WHOLE record - bytes included - so a 60-file library rewrote 60 full
records per refresh, several times per page load.

Debounced to once a day. On a 30-day TTL that is indistinguishable from bumping on
every read, and expiry is unchanged: a stale thumbnail is still cleared the moment
it's seen.

Rebased onto #7366 rather than main: that PR rewrote these exact call sites to keep
maintenance writes away from blob-bodied records (which can wedge the object store
on WebKit), so the two changes now compose - skip the risky records, and debounce
the rest.
@EthanHealy01
EthanHealy01 enabled auto-merge August 13, 2026 14:13
@EthanHealy01
EthanHealy01 added this pull request to the merge queue Aug 13, 2026
Merged via the queue into main with commit 9ef20dc Aug 13, 2026
49 checks passed
@EthanHealy01
EthanHealy01 deleted the fix/webkit-engine-capabilities branch August 13, 2026 17:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Front End Issues or pull requests related to front-end development size:XXL This PR changes 1000+ lines ignoring generated files. Translation Issues or pull requests related to translation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants