Skip to content

fix(web): cancel superseded compression work and bound the ingest fan-out - #39

Closed
PunGrumpy wants to merge 6 commits into
advisor/004-web-test-environmentfrom
advisor/005-client-cancellation
Closed

fix(web): cancel superseded compression work and bound the ingest fan-out#39
PunGrumpy wants to merge 6 commits into
advisor/004-web-test-environmentfrom
advisor/005-client-cancellation

Conversation

@PunGrumpy

Copy link
Copy Markdown
Owner

Stacked on #38. Merge that one first; this PR targets its branch so the diff shows only this work.

Why

  • `compressWithApi`'s `fetch` took no `AbortSignal`. The generation map discarded stale results, but superseded requests still ran to completion, so dragging the quality slider stacked overlapping batches of uploads. A hung request never settled at all: the job stuck in "processing", Recompress stayed disabled, and four hung jobs deadlocked the queue until reload.
  • `ingestFiles` ran `Promise.all` over every accepted file. Dropping 50 large images started 50 simultaneous full-resolution `createImageBitmap` decodes before any compression began, and the pixel-limit check ran after the decode.
  • `addFiles` read the job count before an `await`, so a second drop during a slow ingest saw the old count and bypassed the 50-file ceiling.
  • `sanitizeCompressionOptions` clamped quality but passed `resizeWidth`/`resizeHeight` through raw, so a large typed value allocated a tab-killing canvas.

What changed

  • `compressWithApi` accepts an optional signal and always applies a 120s `AbortSignal.timeout` ceiling (the server's write timeout is 90s, so the client only gives up after the server certainly has).
  • The hook keeps one `AbortController` per job run. `invalidateJob` aborts the prior run before claiming the next generation, and `removeJob`/`clearAll` abort what they discard. A timeout surfaces as "The API did not respond in time."; a manual abort is always a stale run and stays silent. The browser encode path is untouched: canvas encoding has no abort API, and the stale check after the await is its cancellation.
  • Ingest runs through `runWithConcurrency` at `MAX_CONCURRENT_JOBS`, preserving outcome order by index.
  • `addFiles` claims its file count in a ref before awaiting, so concurrent drops see each other and the 50-file gate holds.
  • `resizeWidth`/`resizeHeight` are rounded and clamped to `[0, 16384]` in the sanitize funnel every run path already flows through.

Tests

The supersession and removal tests now also assert the first run's signal was aborted. A new test holds an ingest open and asserts the second `addFiles` sees the first drop's file count (it fails without the fix; verified by stashing). Three new cases cover the dimension clamp. 59 pass, 0 fail.

Notes for review

  • An `ingest.test.ts` was written and then removed: Bun's `mock.module` patches the module registry for the whole test process, and the use-optimizer suite replaces `@/lib/image/ingest` before other files import it, so the ingest test could only ever see the stub. The bounding change is covered by the `Promise.all` removal, the pool's own invariant tests, and the full suite.
  • `addFiles`'s try/finally wraps only the `await`, not the whole body as first drafted, because the wider shape tripped oxlint's react-compiler memo-dependency rule. Behavior is identical.

Each compress run gets its own AbortController; invalidateJob aborts the
prior run's controller before bumping the generation, and removeJob/clearAll
abort any run still in flight for jobs no longer in the queue.
Dropping many large files fanned out every createImageBitmap decode at
once; ingestFiles now runs through the same pool the compress queue uses,
so at most MAX_CONCURRENT_JOBS decode in parallel.
…le mock

Bun's mock.module patches the shared module registry for the whole test
run. use-optimizer.test.tsx replaces @/lib/image/ingest wholesale and only
restores it in its own afterAll, which runs after collection has already
resolved every file's imports; a separate file importing the real ingest.ts
gets the stub instead. runWithConcurrency's bounding and ordering are
already covered by lib/compress/__tests__/pool.test.ts.
jobsRef.current.length was read before the ingest await, so a second drop
during a slow ingest saw the pre-ingest count and could bypass MAX_FILES.
pendingIngestRef tracks files already claimed but not yet committed to
jobsRef, so the next addFiles call sees an accurate starting count.
quality and outputFormat were sanitized but resize dimensions passed through
raw; a large typed value reached the browser canvas path unclamped.
@changeset-bot

changeset-bot Bot commented Aug 30, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 78154e1

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@vercel

vercel Bot commented Aug 30, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
pigo Ready Ready Preview Aug 30, 2026 2:23am
1 Skipped Deployment
Project Deployment Actions Updated (UTC)
pigo-api Skipped Skipped Aug 30, 2026 2:23am

@github-actions

Copy link
Copy Markdown
Contributor

React Doctor found no new issues. 🎉

Reviewed by React Doctor for commit 78154e1.

@PunGrumpy
PunGrumpy deleted the branch advisor/004-web-test-environment August 30, 2026 03:16
@PunGrumpy PunGrumpy closed this Aug 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant