[vitest-pool-workers] Report unavailable built-ins as module errors instead of crashing workerd - #14882
[vitest-pool-workers] Report unavailable built-ins as module errors instead of crashing workerd#14882petebacondarwin wants to merge 1 commit into
Conversation
…nstead of crashing workerd A Worker whose module graph statically reached a compatibility-gated built-in that wasn't enabled (e.g. `import "node:child_process"` without `nodejs_compat`) took down the runtime with `*** Received signal #11: Segmentation fault` before any test ran, reporting only "Worker exited unexpectedly" and naming no module. The module fallback service answered these specifiers with a redirect to the modules root, but `workerd` already resolves `node:`/`cloudflare:`/`workerd:` specifiers there and strips the leading `/` before querying, so the redirect pointed back at the specifier `workerd` was resolving. Its module registry caches that redirect and re-enters resolution with the identical path, with no self-redirect check and no recursion bound, so it recursed until the stack overflowed. Such a specifier only reaches the fallback service once `workerd`'s own registry has missed, so report it as not found instead. `workerd` then raises `No such module "node:child_process"`, matching `wrangler dev`. Fixes #14590
🦋 Changeset detectedLatest commit: 89dd105 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
|
Codeowners approval required for this PR:
Show detailed file reviewers
|
|
✅ All changesets look good |
|
That's a noted future improvement, out of scope for this PR. I've reviewed the code thoroughly:
No logic bugs, security issues, or backward-compatibility violations found. The implementation matches its documented reasoning. LGTM |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
@cloudflare/autoconfig
@cloudflare/config
create-cloudflare
@cloudflare/deploy-helpers
@cloudflare/kv-asset-handler
miniflare
@cloudflare/pages-shared
@cloudflare/unenv-preset
@cloudflare/vite-plugin
@cloudflare/vitest-pool-workers
@cloudflare/workers-auth
@cloudflare/workers-editor-shared
@cloudflare/workers-utils
wrangler
commit: |
2 participants
Fixes #14590.
A Worker whose module graph statically reached a compatibility-gated built-in that wasn't enabled — for example
import "node:child_process"withoutnodejs_compat— took downworkerdwith*** Received signal #11: Segmentation fault: 11at pool startup, before any test ran. Vitest reported onlyWorker exited unexpectedly, naming neither the module nor the file that imported it. The import didn't even have to be called; being reachable from the entrypoint was enough. The same Worker underwrangler devfails cleanly withNo such module "node:child_process".Root cause
The fallback service answered these specifiers with a
301to/${specifier}, on the assumption thatworkerdresolves them relative to the referrer and needs redirecting to the modules root. It doesn't:workerdrootsnode:/cloudflare:/workerd:specifiers itself (kj::Path::parse(spec)injsg/modules.c++) and strips the leading/before querying us. The redirect therefore pointed straight back at the specifierworkerdwas already resolving.workerd's legacy module registry caches that redirect and re-entersresolve()with the identical path, with no self-redirect check and no recursion bound, so it recursed until the stack overflowed. Traced withNODE_DEBUG=vitest-pool-workers:module-fallback, the crash follows the redirect immediately, with no second fallback request:Both guards for this already exist in the new module registry (
jsg/modules-new.c++, including a self-alias check commented "a buggy fallback resolver could end up in an infinite loop of aliasing"), but were never backported to the legacy one, which is still the default.Approach
Such a specifier only reaches the fallback service once
workerd's own registry has missed, so it is by definition unavailable at this Worker's compatibility date and flags. It's now reported as not found, andworkerdraises its ownNo such module "node:child_process"— matchingwrangler dev.The guard sits in
load(), the single place a redirect is emitted, so it covers theworkerdBuiltinModulesbranch inresolve()and both ofviteResolve()'s external branches at once, and applies torequire()as well asimport. It needs no compatibility-flag table mirrored into the pool, and it covers gatedcloudflare:*modules (cloudflare:sockets,cloudflare:email, …) as well asnode:*.The accompanying pool error now names the module and points at compatibility flags, instead of suggesting you bundle it with Vite — advice that can't help for a module built into the runtime.
Verified against the reporter's repro:
Error: No such module "node:child_process".and no crash.Notes for reviewers
workerd— a local dev service returning a301should not be able to SIGSEGV the runtime, and any other self-redirecting fallback service still can. Worth porting themodules-new.c++guards toModuleRegistryImpl::resolve. I've left the analysis on vitest-pool-workers: workerd segfaults (signal 11) at pool startup when the Worker's module graph has an unresolvable node:* import, instead of a clean "No such module" error #14590.miniflare'swaitForExitdiscards(code, signal), which is why the crash surfaced asWorker exited unexpectedlyrather than naming SIGSEGV. Capturing the signal would make failures like this much easier to diagnose.301without it; the negative control (a prefixed specifier that resolves to a real file must still redirect) passes both ways.packages/vitest-pool-workershas 5 pre-existing failures onmain(chunking.test.ts×1,console.test.ts×3,filtering.test.ts×1). I confirmed they're identical on a clean baseline and unrelated to this change.vitest-pool-workers— a crash becomes a clear error. No user-facing API or documented behaviour changes.Note
This is a contribution from an AI agent: OpenCode, claude-opus-5. Worked under direct human supervision.