Skip to content

worker_threads: keep error.code when the thrown value cannot be cloned - #34509

Closed
cirospaciari wants to merge 1 commit into
ciro/worker-threads-node-testsfrom
ciro/worker-error-code-across-threads
Closed

cirospaciari wants to merge 1 commit into
ciro/worker-threads-node-testsfrom
ciro/worker-error-code-across-threads

Conversation

@cirospaciari

Copy link
Copy Markdown
Member

Stacked on #34424 — review that first; this PR is the last two commits only.

A worker that throws something structured-clone can't serialize falls back to sending only the message text, and the parent rebuilds a bare Error from it — losing code. Bun's own ResolveMessage is exactly that case: it carries the right code and isn't even an Error instance (isError: false), so it neither clones nor hits the ErrorInstance retry path.

new Worker(`require("node:internal/freelist")`, { eval: true })
// node: code ERR_UNKNOWN_BUILTIN_MODULE
// bun : code undefined

The code isn't missing — Bun sets it correctly, and the same require on the main thread reports ERR_UNKNOWN_BUILTIN_MODULE. It's lost only crossing the thread boundary. This affects every coded error a worker throws that isn't cloneable, not just this one.

What it does

Carries code alongside the message rather than replacing the value, and shares the read with the value path as Worker::errorCodeOf (under a top exception scope, since a code getter can run JS).

Replacing the value was my first attempt and it was wrong. Rebuilding an Error from ResolveMessage's own .message drops Bun's "error: ..." prefix, which support require in eval for a file that doesnt exist asserts on — it went from "error: Cannot find module..." to "Error: Cannot find module...". The gate caught it. The message text is now byte-identical to before; only code is added.

Verification

before after
test-worker-internal-modules.mjs 3 fail 3 pass (0/2 without the change)
resolve-error .code undefined ERR_UNKNOWN_BUILTIN_MODULE (= node)
thrown-error .code E_CUSTOM E_CUSTOM (unchanged)

Vendored test-worker*: 106 pass / 2 fail — both pre-existing (arraybuffer-zerofill needs bun test; on-process-exit is debug-only). Bun's worker_threads suite back to its 2 known failures. test-worker-error-stack-getter-throws still passes (same function).

Why stacked rather than in #34424

#34424 is already 22 source files / 820 insertions / 17 commits across the CLI parser, TLS/CA, cpu-prof, worker_threads and async_hooks. It's at the limit of what's reasonable to review in one pass, and landing is what actually delivers compatibility. This is a separate, self-contained bug with its own test, so it gets its own PR.

A worker that throws something structured-clone can't serialize falls back to
sending only the message text, and the parent rebuilds a bare Error from it —
losing `code`. Bun's own ResolveMessage is exactly that case: it carries the
right code and is not even an Error instance.

  new Worker(`require("node:internal/freelist")`, { eval: true })
  node: code ERR_UNKNOWN_BUILTIN_MODULE
  bun : code undefined        (the worker-side value has the right code)

Carry the code alongside the message instead of replacing the value. Replacing
it was the first attempt and it broke `support require in eval for a file that
doesnt exist`: rebuilding from ResolveMessage's own .message drops Bun's
"error: ..." prefix, which that test asserts on. The message text is now
untouched — only `code` is added.

The read is shared with the value path as Worker::errorCodeOf, under a top
exception scope since a `code` getter can run JS.

test-worker-internal-modules.mjs: 3 fail -> 3 pass, and 0/2 without the change.
Vendored test-worker* 106 pass/2 fail (both pre-existing: arraybuffer-zerofill
needs `bun test`, on-process-exit is debug-only). Thrown errors keep their code
as before, and test-worker-error-stack-getter-throws still passes.
@github-actions

Copy link
Copy Markdown
Contributor

This PR may be a duplicate of:

  1. worker_threads: preserve error own properties, surface parse diagnostics, report exit code 1 on terminate() #32867 - Also preserves error own properties (including code) across worker_threads thread boundaries when structured cloning fails; is a superset that preserves all enumerable own properties, not just code

🤖 Generated with Claude Code

@cirospaciari

Copy link
Copy Markdown
Member Author

Folding this into #34424 — three stacked PRs was more confusing than it was worth. Same commits, same tests, one review.

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.

2 participants