fix(vitest): render non-Error causes from env setup so the actual diagnostic is not dropped#10567
fix(vitest): render non-Error causes from env setup so the actual diagnostic is not dropped#10567spokodev wants to merge 3 commits into
Conversation
|
Hello @spokodev. Your PR has been labeled To keep your PR open, please follow these steps:
Please, do not generate or format the response with AI. If you do not speak English, reply in your native language or use translation software like Google Translate or Deepl. If the response is generated, the PR will be closed automatically. These measures help us reduce maintenance burden and keep the team's work efficient. See our AI contributions policy for more context. |
1 similar comment
|
Hello @spokodev. Your PR has been labeled To keep your PR open, please follow these steps:
Please, do not generate or format the response with AI. If you do not speak English, reply in your native language or use translation software like Google Translate or Deepl. If the response is generated, the PR will be closed automatically. These measures help us reduce maintenance burden and keep the team's work efficient. See our AI contributions policy for more context. |
|
Hi, this is Yarik. Spoko-dev. |
✅ Deploy Preview for vitest-dev ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
…gnostic is not dropped
The pool wrapper attaches the original failure via `{ cause: error }` when a
test environment's `setup()` rejects. The printError reporter then only
descended into `cause` when it was an object with a `name` property — i.e.
an Error-shaped value. Non-Error throws (`throw 'reason'`,
`throw { ... }`, or a wasm-bindgen `JsValue`) were silently dropped and the
user saw only the generic location-free
"[vitest-pool]: Failed to start <pool> worker for test files …".
Vitest 3.x rendered these as `Unknown Error: <value>`, so the silent drop
is a regression — likely a side-effect of the worker-start error wrapping
added in vitest-dev#9337 (which correctly closed the indefinite hang from vitest-dev#9271).
Coerce non-object/object-without-name causes into a printable shape before
delegating to `printErrorInner`. Real-world impact: native/wasm test
environments (e.g. `@stacks/clarinet-sdk`'s `vitest-environment-clarinet`)
reject from their wasm `setup()` with a JsValue; the actual contract
compilation diagnostic now surfaces instead of being lost.
Closes vitest-dev#10557
…or cause Promise.reject(3) hits the same non-Error cause path that vitest-dev#10557 was filed about: the pool/runtime wrapper attaches the rejected value (`3`) via `{ cause: 3 }` on the synthesised "intentional unhandled rejection" Error, and the printError change in this PR now renders it as `Caused by: 3` instead of silently dropping it. Snapshot updated to match. No code change; behaviour parity with the previous "3 is not an Error so nothing to render" outcome was the underlying bug.
f6598d5 to
834fbb9
Compare
What
When a custom test environment's
setup()rejects with a non-Errorvalue (a string, a plain object, or a wasm-bindgenJsValue), the actual diagnostic is silently dropped and the user only sees the generic, location-free[vitest-pool]: Failed to start <pool> worker for test files …from the pool wrapper.Vitest 3.x rendered the same non-
Errorthrow asUnknown Error: <value>, so the silent drop is a regression. After this PR the cause surfaces correctly for Error, plain-object, and primitive throws.Closes #10557.
Why / root cause
The pool wrapper in
packages/vitest/src/node/pools/pool.ts:127already attaches the original failure as{ cause: error }on the synthesised "Failed to start … worker" Error. The reporter inpackages/vitest/src/node/printError.tsthen descended intocauseonly when it satisfied a narrow shape check:That filter passes for
new Error(...)(object + truthy + hasname) but excludes:throw "string"—typeof "string" === "object"isfalse, so the branch is skipped entirely.throw { reason: '...' }— object + truthy, but nonameproperty, so'name' in causeisfalse.JsValue— object + truthy, but noname(and in some bindings,'name' in causethrows on the proxy).In all three cases the value reaches
printErrorvia the cause chain but never reachesprintErrorInner, so the actual diagnostic is lost.The regression most likely landed alongside the worker-start error wrapping in #9337 (which correctly fixed the indefinite-hang behaviour from #9271). The wrapper is well-formed; only the reporter's filter is too narrow.
The fix normalises the cause into a printable shape before calling
printErrorInner— which already knows how to handle primitives via itsisPrimitivebranch:{ name: 'Caused by', message: String(cause), stack: String(cause) }.name→ spread +name: 'Caused by'.name→ preserve the existing behaviour of prefixingnamewithCaused by:.printErrorInnerthen renders the message exactly as it would for a top-level non-Errorthrow, mirroring Vitest 3.x behaviour. Real-world impact: native/wasm environments (e.g.@stacks/clarinet-sdk'svitest-environment-clarinet) reject from their wasmsetup()with a JsValue when contract compilation fails — the failure reason now surfaces instead of being lost behind "Failed to start forks worker".Tests added
New e2e test at
test/e2e/test/env-setup-non-error-cause.test.tsusingrunInlineTestsfromtest/test-utils. Three cases, each booting Vitest against a fixture environment whosesetup()throws a different shape:non-Error string thrown from env setup surfaces in stderr— the bug case from A non-Errorvalue thrown from a test environment'ssetup()is dropped from the "Failed to start worker" report (regression from 3.x) #10557. Without this PR the string never appears in stderr; with it, thethe real reason setup failed (a non-Error value)substring shows up.non-Error plain object thrown from env setup surfaces in stderr— boundary H-case (wasm JsValue analogue). Without the PR the object is dropped entirely; with it, the object's keys / serialised value appear in stderr.Error thrown from env setup still renders with Caused by prefix— happy-path regression. Confirms the existingCaused by: Error: …rendering is unchanged for proper Error instances.Verified locally:
Before the fix: tests 1 and 2 fail with
Expected to contain '…the real reason…'/Expected to match /wasm-init-failed|reason/. Test 3 already passed on master.Repro
Minimal repro from the issue (https://stackblitz.com/edit/vitest-dev-vitest-ldzxobzz):
pnpm testshows only the genericFailed to start forks workermessage on master; with this PR the cause string appears underCaused by:.Validation
pnpm -F vitest build— greenCI=true pnpm test env-setup-non-error-cause— 3/3CI=true pnpm test print-error— 2/2 (no regression)CI=true pnpm test pool.test— 16/16 (no regression)pnpm lint:fix— cleanAI usage disclosure
Drafted with Claude assistance. The root-cause analysis (the
'name' in causenarrow filter and its three failure modes), the fix design, the test surface choice (runInlineTestsinstead of unit-testingprintError), and this PR text are mine. I verified the red baseline → fix → green baseline loop locally and have read every line of the diff. I will engage genuinely with any review feedback within the 3-day window described inAGENTS.md.