Worker threads: reject pending RPC calls on worker failure or termination

[adamsilverstein]

Fixes #79953

What?

Makes worker RPC promises in @wordpress/worker-threads settle when the underlying Web Worker fails or is terminated, instead of hanging forever.

Why?

comctx (the RPC layer used by wrap()) only settles a call promise when a response message arrives from the worker, and wrap() runs with heartbeatCheck: false. If the vips worker dies, fails to initialize, or is terminated while an operation is in flight, the promise for that operation never settles. The upload queue item then stays in its current operation forever:

• The image block shows the blob: placeholder and a progress spinner indefinitely.
• No error is surfaced to the user (the onError path never runs).
• isUploading() stays true, so the editor's upload save lock keeps the post unsaveable.

The worker is terminated and recreated frequently (removeItem terminates it whenever the queue empties, and it is recycled every 50 operations), so failure windows occur many times per editing session. See #79953 for the full analysis.

How?

In packages/worker-threads/src/main-thread.ts, wrap() now tracks the rejecter of every in-flight call:

• terminate() rejects all pending calls, as its docblock already promised ("any pending calls will be rejected") but the implementation never did.
• error / messageerror listeners on the worker reject all pending calls when the worker crashes, fails script load/WASM instantiation, or delivers an undeserializable message.
• Once a worker has failed or been terminated, subsequent calls on that remote reject immediately instead of hanging.

This closes the failure loop with the existing queue logic: a rejected vips call cancels the queue item and surfaces onError, the queue drains, the empty-queue cleanup terminates the dead worker, and the next upload lazily creates a fresh one (getWorkerAPI() in @wordpress/vips). Both the vips and video-conversion workers benefit.

Per review feedback, cancelItem in @wordpress/upload-media also needed a change: it awaits vipsCancelOperations() / cancelGifToVideoOperations(), and with a dead worker those cancel calls themselves now reject, which previously aborted the cancellation flow before onError fired and the item was removed. Those calls are now best-effort (.catch( () => {} )), which keeps the recovery loop intact end to end.

The remaining hardening ideas from #79953 (per-call timeout for a wedged-but-alive worker, debounced empty-queue termination, awaiting mediaDelete in cancelItem) are intentionally left as follow-ups to keep this focused.

Testing Instructions

1. npm run test:unit packages/worker-threads

The 5 new test cases hang (5s timeout each) without the fix and pass with it: worker error event rejects pending calls, messageerror rejects pending calls, calls after a worker error reject immediately, terminate() rejects pending calls, and calls after termination reject immediately.

2. npm run test:unit packages/upload-media — includes two new tests asserting cancelItem still fires onError and removes the item when the worker cancel calls reject (both fail without the best-effort change).

3. npm run test:e2e -- test/e2e/specs/editor/various/client-side-media-processing.spec.js — includes a new test that crashes the vips worker mid-upload and asserts the error snackbar appears, the queue empties, the save lock releases, and a follow-up upload succeeds with a fresh worker.

Manual verification of the end-to-end behavior:

1. Open the post editor and start uploading a large image.
2. Tip: use network throttling to slow down the upload and give more time to kill the worker.
3. While the upload spinner is showing, kill the vips worker: in DevTools, go to Sources → Threads (or chrome://inspect/#workers) and terminate the blob worker, or simulate a crash.
4. Without this PR the block is stuck on the blob: placeholder forever and the post can't be saved. With this PR the upload fails with an error notice and the save lock releases.

[github-actions]

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: adamsilverstein <adamsilverstein@git.wordpress.org>
Co-authored-by: ramonjd <ramonopoly@git.wordpress.org>
Co-authored-by: andrewserong <andrewserong@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

[github-actions]

Size Change: +667 B (+0.01%)

Total Size: 7.72 MB

📦 View Changed

Filename	Size	Change
build/modules/video-conversion/worker.min.js	83.2 kB	+269 B (+0.32%)
build/modules/vips/worker.min.js	3.69 MB	+398 B (+0.01%)

_{compressed-size-action}

[github-actions]

Flaky tests detected in 3e63024.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/28952781567
📝 Reported issues:

• [Flaky Test] Should navigate inner blocks with arrow keys #39607 in /test/e2e/specs/editor/various/writing-flow.spec.js

[andrewserong]

I also wasn't able to manually test this one for some reason (I could pause workers, but Chrome didn't let me terminate any 🤷)

But, smoke testing uploading works as expected, and I didn't run into any issues looking over the code, and everything I got from passing this through Claude seemed minor/like nits. E.g.

• That the failWorker state means subsequent calls for a failed worker are rejected even if the worker is still running (i.e. it'll run until it's terminated). That seems like the intention of this PR, so doesn't sound like an issue?
• removeListeners() is only called when terminating the worker. Is that expected?
• And "Wrapping the delegate in Promise.resolve().then( () => value( ...args ) )" adds a microtask of latency. This also sounds like a non-issue to me

Given we're close to the feature freeze, I'll give this a tentative approval. But let me know if you wanted someone to be able to manually reproduce the testing steps, since it seems neither Ramon or I have been able to yet.

[adamsilverstein]

Some findings from the #80259 investigation (fix in #80260) that are relevant here:

The hang in #80259 turned out to be a wasm deadlock, not a crash: a libjpeg failure killed a libvips worker pthread without unwinding, and the coordinating thread blocked forever in Atomics.wait. The worker never fired error or messageerror and never terminated - so the event-based rejection in this PR wouldn't have caught that class of hang. Not a flaw in this PR (it correctly covers crashes, failed script loads, and termination), but probably worth a sentence in the description so it isn't assumed to be complete hang-proofing.

One concrete gap in the upload-media change: await vipsCancelOperations( id ).catch( () => {} ) handles a rejecting cancel, but against a deadlocked worker the promise never settles, so .catch never runs and cancelItem itself hangs at the await - cancelling a stuck upload still wouldn't unwind. A timeout race would cover it:

await Promise.race( [
	vipsCancelOperations( id ).catch( () => {} ),
	new Promise( ( resolve ) => setTimeout( resolve, 3000 ) ),
] );

Silver lining: the terminate()-rejects-pending behavior added here is exactly the primitive a real watchdog needs. A natural follow-up once this lands: when a vips call exceeds a generous deadline (or cancellation times out as above), call terminateVipsWorker() - pending calls then reject immediately and the queue's existing error handling unwinds. Today maybeRecycleVipsWorker only recycles on op-count, which never triggers for a stuck worker.

Also FYI: #80260 touches the worker side of this package (expose() now normalizes non-Error throwables so in-worker failures reject instead of resolving undefined) - complementary to this PR, only overlap is a trivial CHANGELOG.md conflict for whichever merges second.

[andrewserong]

So, sounds like we merge this PR now(ish) and then follow-up in #80260?

[github-actions]

There was a conflict while trying to cherry-pick the commit to the wp/7.1 branch. Please resolve the conflict manually and create a PR to the wp/7.1 branch.

PRs to wp/7.1 are similar to PRs to trunk, but you should base your PR on the wp/7.1 branch instead of trunk.

# Checkout the wp/7.1 branch instead of trunk.
git checkout wp/7.1

# Create a new branch for your PR.
git checkout -b my-branch

# Cherry-pick the commit.
git cherry-pick 70f9a9594d6532301fa31c3313f38275c60d26b9

# Check which files have conflicts.
git status

# Resolve the conflict...
# Add the resolved files to the staging area.
git status
git add .
git cherry-pick --continue

# Push the branch to the repository
git push origin my-branch

# Create a PR and set the base to the wp/7.1 branch.
# See https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/changing-the-base-branch-of-a-pull-request.

[adamsilverstein]

It looks like this was missing the 7.1 backport label and was never added to the 7.1 branch.

[adamsilverstein]

It looks like this was missing the 7.1 backport label and was never added to the 7.1 branch.

Going to stack the PR on #80420 which touches the same files. cc: @t-hamano

[t-hamano]

Removing the Backport to Gutenberg RC label from this PR and add it to #80421 instead. This is because cherry-picking the manual backport PR created for wp/7.1 is less likely to cause conflicts than cherry-picking the PR that caused the conflict.

[t-hamano]

This PR was manually backported by #80421.