Skip to content

feat(vitest-pool-workers): tolerate post-stop() socket error events when configured - #14794

Open
trafgals wants to merge 1 commit into
cloudflare:mainfrom
trafgals:fix/pool-worker-tolerate-shutdown-socket-errors
Open

feat(vitest-pool-workers): tolerate post-stop() socket error events when configured#14794
trafgals wants to merge 1 commit into
cloudflare:mainfrom
trafgals:fix/pool-worker-tolerate-shutdown-socket-errors

Conversation

@trafgals

@trafgals trafgals commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

What

When workerd segfaults during shutdown (cloudflare/workerd#6763), miniflare's WebSocket dispatches an error event after every test in the file has already been reported as passing. vitest then surfaces this as [vitest-pool]: Worker cloudflare-pool emitted error and fails the run with exit 1.

This change adds a new opt-in tolerateWorkerShutdownErrors: boolean option to the cloudflare-pool worker configuration. When enabled, error events received after stop() has begun are logged via NODE_DEBUG=vitest-pool-workers and not propagated to vitest as fatal errors. The default behavior is unchanged.

Relationship to #14793

This PR is intentionally distinct from #14793 (which wraps Miniflare#dispose() and the remote-proxy session dispose() with .catch()). The two changes tolerate different amplifiers in the same root-cause chain:

Both are useful. They can be enabled together or independently. Neither fixes the underlying workerd SIGSEGV — that needs to be done in workerd itself.

Root-cause recap

The workerd SIGSEGV is the trigger; the test-side non-zero exit is three layers deep:

  1. workerd subprocess dies during shutdown (upstream bug).
  2. The dispose promises in CloudflarePoolWorker.stop() reject with that segfault (mitigated by fix(vitest-pool-workers): tolerate dispose rejections in pool worker stop() #14793).
  3. The WebSocket dispatches an error event AFTER stop() begins, which vitest surfaces as fatal. (mitigated by this PR)
  4. tinypool's ProcessWorker.onUnexpectedExit also emits Worker exited unexpectedly for forks that exit while idle — this is mitigated by the new allowWorkerIdleExit opt-in in feat: add allowWorkerIdleExit option to suppress idle-worker error events tinylibs/tinypool#133.

Reproduction

Tests  794 passed (794)
❌ vitest exit 1
[vitest-pool]: Worker cloudflare-pool emitted error.
Caused by: Worker exited unexpectedly

With tolerateWorkerShutdownErrors: true, the same suite reports 794 passed and exits 0. The segfault is still printed to stderr; only the test-side exit suppression changes.

Implementation notes

  • The stopping flag is set at the very start of stop() so any error event that fires after that point — including ones that fire synchronously from socket.close() — sees the same state.
  • Errors raised while a task is in flight are still propagated unchanged. Only post-stop() error events are inspected.
  • The debug logging uses the same util.debuglog(vitest-pool-workers) channel the rest of the package uses for diagnostic logs.
  • Marks the message with the upstream issue URL so anyone seeing it during teardown can immediately find context.
  • Adds a patch-level changeset.

Related

🤖 Generated with Claude Code


Open in Devin Review

workerd can segfault during shutdown on linux-x64 / WSL2 and macOS-arm64
(cloudflare/workerd#6763). After every test in a file has already
reported as passing, the segfault propagates as a WebSocket `error`
event which vitest surfaces as `[vitest-pool]: Worker cloudflare-pool
emitted error`, failing an otherwise-passing run with exit 1.

Add an opt-in `tolerateWorkerShutdownErrors: boolean` option (default
`false`). When enabled, `error` events received after `stop()` has
begun are logged via `NODE_DEBUG=vitest-pool-workers` and not
forwarded to vitest as fatal. Errors received while the pool worker is
still running are propagated unchanged.

This complements the dispose-side fix in cloudflare#14793 (which wraps the
Miniflare and remote-proxy session `dispose()` calls with
`catch()`): the dispose fix tolerates teardown promise rejections,
while this option tolerates the post-teardown WebSocket `error`
event. Both opt-in; both default `false`.

Reproduction (matches the linked upstream issue):

    Tests  794 passed (794)
    ❌ vitest exit 1
    [vitest-pool]: Worker cloudflare-pool emitted error.
    Caused by: Worker exited unexpectedly

With `tolerateWorkerShutdownErrors: true`, the same suite reports
`794 passed` and exits 0. The segfault is still printed to stderr
under `NODE_DEBUG=vitest-pool-workers`; only the test-side exit
suppression changes.

Scope is a test-side mitigation only — the upstream `Received
signal cloudflare#11` still needs to be fixed in workerd itself.
@changeset-bot

changeset-bot Bot commented Jul 21, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 0f8d2c8

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@cloudflare/vitest-pool-workers Patch

Not sure what this means? Click here to learn what changesets are.

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

@workers-devprod
workers-devprod requested review from a team and petebacondarwin and removed request for a team July 21, 2026 21:57
@workers-devprod

Copy link
Copy Markdown
Contributor

Codeowners approval required for this PR:

  • @cloudflare/wrangler
Show detailed file reviewers
  • .changeset/pool-worker-tolerate-shutdown-socket-errors.md: [@cloudflare/wrangler]
  • packages/vitest-pool-workers/src/pool/cloudflare-pool-worker.ts: [@cloudflare/wrangler]
  • packages/vitest-pool-workers/src/pool/config.ts: [@cloudflare/wrangler]

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 2 potential issues.

Open in Devin Review

"@cloudflare/vitest-pool-workers": patch
---

fix: tolerate post-`stop()` socket `error` events when configured

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changeset title uses a forbidden conventional-commit prefix

The changeset headline starts with a fix: type prefix (.changeset/pool-worker-tolerate-shutdown-socket-errors.md:5), which the repository's review guidelines explicitly forbid for changeset titles.
Impact: The changelog entry will be formatted against the project's stated conventions and may require rework before release.

Rule source in REVIEW.md

REVIEW.md's Changesets section states: "Do NOT prefix changeset titles with a 'type' (e.g. fix:, feat:, chore:). The changeset title should be a plain description without conventional commit prefixes." The title on .changeset/pool-worker-tolerate-shutdown-socket-errors.md:5 begins with fix:.

Suggested change
fix: tolerate post-`stop()` socket `error` events when configured
Tolerate post-`stop()` socket `error` events when configured
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment thread .changeset/pool-worker-tolerate-shutdown-socket-errors.md
@edmundhung
edmundhung requested review from edmundhung and removed request for petebacondarwin July 24, 2026 10:25
Comment on lines +7 to +43
workerd can segfault (`Received signal #11`) during shutdown on linux-x64 /
WSL2 and macOS-arm64 (see
[cloudflare/workerd#6763](https://github.com/cloudflare/workerd/issues/6763)).
After all tests in a file have already reported as passing, the segfault causes
miniflare's WebSocket to dispatch an `error` event; vitest then surfaces that
as `[vitest-pool]: Worker cloudflare-pool emitted error` and fails the run
with exit 1.

This change adds an opt-in `tolerateWorkerShutdownErrors: boolean` option
(default `false`) to the cloudflare-pool worker configuration. When enabled,
`error` events received after `stop()` has begun are logged via
`NODE_DEBUG=vitest-pool-workers` but not forwarded to vitest as fatal. Errors
received while the pool worker is still running are still propagated
unchanged.

This change does **not** alter the behavior of `stop()` itself — for the
dispose-side fix, see #14793 (which wraps `Miniflare#dispose()` and the
remote-proxy session `dispose()` with `.catch()` handlers). Pairing both
options (`tolerateWorkerShutdownErrors: true` + the dispose fix) is what
produces a fully-shutdown-tolerant run.

Scope is a test-side mitigation only — the upstream `Received signal #11`
itself still needs to be fixed in workerd itself.

Reproduction (matches
[cloudflare/workerd#6763](https://github.com/cloudflare/workerd/issues/6763)):

```
Tests 794 passed (794)
❌ vitest exit 1
[vitest-pool]: Worker cloudflare-pool emitted error.
Caused by: Worker exited unexpectedly
```

With `tolerateWorkerShutdownErrors: true`, the same suite reports `794 passed`
and exits 0. The segfault is still printed to stderr; only the test-side exit
suppression changes.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changeset should focus more on user impact. You can use #14793 for reference.

Comment on lines +39 to +42
// Used to log non-fatal socket errors (e.g. workerd shutdown segfaults —
// see https://github.com/cloudflare/workerd/issues/6763) without surfacing
// them to vitest when configured to tolerate them.
private readonly debug = util.debuglog("vitest-pool-workers");

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you added this in #14793 already? Please rebase and resolve the conflict.

Comment on lines +48 to +50
// from the runner socket should be suppressed (see
// https://github.com/cloudflare/workerd/issues/6763 and the
// `tolerateWorkerShutdownErrors` config option).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no need to refer to the workerd issue in the comment. I will just treat this as a generic hardening.

This applies to the rest of the comments too.

Comment on lines +316 to +321
this.debug(
"ignoring socket `error` during teardown (tolerateWorkerShutdownErrors=true; see cloudflare/workerd#6763): %s",
errorValue instanceof Error
? errorValue.stack ?? errorValue.message
: errorValue
);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
this.debug(
"ignoring socket `error` during teardown (tolerateWorkerShutdownErrors=true; see cloudflare/workerd#6763): %s",
errorValue instanceof Error
? errorValue.stack ?? errorValue.message
: errorValue
);
this.debug(
"socket error event ignored during teardown: %O",
errorValue
);

Comment on lines +76 to +90
/**
* Tolerate `error` events on the runner WebSocket after the pool worker
* has begun shutting down. Workerd can segfault during teardown on
* linux-x64 / WSL2 and macOS-arm64 (see
* https://github.com/cloudflare/workerd/issues/6763); without this flag,
* those errors reach vitest as fatal `[vitest-pool]: Worker cloudflare-pool
* emitted error`s, failing an otherwise-passing run with exit 1.
*
* Errors raised while a task is in flight are still forwarded normally;
* only post-`stop()` `error` events are suppressed (and logged via
* `NODE_DEBUG=vitest-pool-workers`).
*
* @default false
*/
tolerateWorkerShutdownErrors: z.boolean().default(false),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no need to add an option for this.

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.

4 participants