fix: use ErrorPlugin when formatting errors in display.ts and snapshot plugins (fix #8697) - #10885
fix: use ErrorPlugin when formatting errors in display.ts and snapshot plugins (fix #8697)#10885arjun2075 wants to merge 13 commits into
Conversation
✅ Deploy Preview for vitest-dev ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
sheremet-va
added
the
maybe automated
|
Hello @arjun2075. 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. |
|
arjun2075
changed the title
ac73155 to
fe1d2e2
Compare
display.ts's and snapshot/src/port/plugins.ts's PLUGINS lists omitted the Error plugin, so Error instances (including AssertionError and custom subclasses) printed only their message, dropping cause and any custom properties. Two errors with the same message but different extra properties were indistinguishable in a diff or snapshot. diff/index.ts already includes prettyFormatPlugins.Error (added in vitest-dev#5876), so the diff computation was already correct — only the display and snapshot paths lagged behind. This changes how errors are printed by toMatchInlineSnapshot, toThrowErrorMatchingInlineSnapshot, and plain inspect() of an error. Updated 81 existing inline/file snapshots that pinned the old bracketed format, and added a migration guide entry for Vitest 5.0. Added 3 regression tests to pretty-format.test.ts. BREAKING CHANGE: errors (including AssertionError) are now printed with their full properties instead of just their message when inspected via display.ts/snapshot plugins — see the Vitest 5.0 migration guide. Fixes vitest-dev#8697
fe1d2e2 to
7182c26
Compare
|
I dug into these failures a bit more and I think the two are separate. The vi-when.test.ts one is the Symbol(nodejs.dispose) vs Symbol(Symbol.dispose) mismatch. That fails the same way in all three pools (threads, forks, and vmThreads), so I don't think it's related to this PR. It looks more like a Node version/environment difference in CI vs what I was running locally. The error-mock.spec.ts failure is different. That only fails in vmThreads, and that one does seem to be caused by this change. I can reproduce it locally every time with: vitest run test/mocking/error-mock.spec.ts --project=vmThreads I also tried clearing caches / rebuilding and it still reproduces, so it doesn't seem cache-related. What's interesting is that the new tests I added in pretty-format.test.ts pass under vmThreads. Those call prettyInspect directly, so it doesn't look like adding ErrorPlugin to display.ts is broken in vmThreads by itself. The main difference I can see is the path the error takes in error-mock.spec.ts. The error is thrown inside a vi.mock() factory, then gets wrapped by createHelpfulError() in packages/mocker/src/registry.ts, and eventually comes back through a rejected dynamic import(). So unlike a normal throw new Error(), this error is crossing the vmThreads VM / message-passing boundary somewhere along the way. I tried adding some temporary logging and a file write directly around the PLUGINS construction in display.ts to see what was happening with ErrorPlugin. For this test under vmThreads, that instrumentation never fired, even after clean rebuilds. That makes me wonder if the error that eventually reaches the formatting code has been reconstructed while crossing the VM boundary, and isn't quite a normal Error anymore — e.g. different prototype, missing properties, etc. Does that sound like a known thing with vmThreads? More specifically, is there somewhere in the VM/RPC path where these errors get serialized/deserialized in a way that would make ErrorPlugin's instanceof Error check (or its property handling) not work as expected? And if so, is there already another error formatting/serialization path in Vitest that handles errors coming out of the VM that display.ts should be using here? Happy to dig further, I just might need a pointer to where the vmThreads error reconstruction/deserialization happens. I looked around packages/mocker and the pool code but haven't found the obvious place yet. Also just to keep the CI failures separate: I don't think the Symbol.dispose failure is part of this. Since that one happens identically in every pool, it looks like a separate CI/Node-version issue. |
sheremet-va
removed
the
maybe automated
`ErrorPlugin.test` used `instanceof Error`, which is realm-bound. Errors created inside a VM context (the `vmThreads` pool) are not instances of the outer realm's `Error`, so the plugin did not match them and they fell back to the collapsed `[Error: ...]` form while every other pool printed the expanded object with its `cause` chain. Fall back to the `[object Error]` brand check already used by `printBasicValue`, which is realm-independent. Snapshots updated accordingly: - `wasm.test.ts` — the `isVm` branches captured the collapsed form purely because of this bug, so they now match the other pools. The branches themselves remain, since the vm runner really does throw a plain Error where other pools throw a CompileError. - `vi-when.test.ts` — assert on the assertion *message* rather than the whole AssertionError. Its `actual` property holds the mock wrapper, whose keys include a realm-specific `Symbol.dispose` that is absent under `vmThreads`, so no single snapshot of the object can hold for all pools. The message carries the behaviour listing these tests exist to verify.
Errors now serialize through ErrorPlugin as an expanded object rather than the collapsed `[Error: ...]` form, so the remaining snapshots that captured the old shape are updated to match. Regenerated with `vitest -u`: - coverage-test/threshold-auto-update - e2e/snapshots (soft, soft-inline, inline-multiple-calls, domain-poll) - e2e/benchmarking The e2e `snapshots/fixtures/*` files still contain the collapsed form on purpose: those fixtures are rewritten by the meta-tests at runtime. `browser/findElement` is converted by hand to the same shape, since running that suite locally needs Playwright browsers.
|
Both fixes are pushed and CI is re-running |
Two browser assertions still expected the collapsed `[Error: ...]` form:
- `findElement` — `expect.element` rejects with an error that carries a
`cause` ("Matcher did not succeed in time."), which ErrorPlugin now
surfaces. The other three snapshots in this file were already correct.
- `errors.test.ts` — assert on the message rather than the bracketed form,
so the check holds regardless of how the error is rendered.
`stack` is a non-enumerable own property in V8, so the object rest in `serialize` skipped it. In SpiderMonkey and JavaScriptCore it is enumerable, so it was spread into the output — printing a machine-specific list of absolute URLs, ports and content hashes into every serialized error. That made the `findElement` snapshots pass under chromium but fail under firefox and webkit. Destructure `stack` out alongside `message` and `cause`. Other own properties, including custom ones, are still printed.
`expect.element` polls through a matcher that attaches a "Matcher did not succeed in time." cause in Chromium and Firefox, but rejects with the bare strict-mode error in WebKit. A single inline snapshot cannot describe both, so the assertion failed with "toThrowErrorMatchingInlineSnapshot with different snapshots cannot be called at the same location". Assert on the strict-mode message, which is what the test is verifying and is identical in every browser. The sibling `findElement` tests keep their snapshots — those throw directly and have no cause anywhere.
The test runs in watch mode over a directory created by `runInlineTests`. It never stopped watching, so teardown removed the inline test files while the watcher was still running, and the resulting `ENOENT` surfaced as an unhandled error that failed the e2e job even though every test passed. Close the context at the end of the test, matching `global-setup-rerun`.
|
I dug into both failures and pushed fixes for them. For error-mock, the issue was ErrorPlugin.test using I verified this with That also explains why the existing That change also meant updating snapshots across the unit, coverage, e2e, and browser suites, since these errors now serialize in their expanded form consistently. For vi-when, I was wrong in my earlier comment about it failing across all pools. The failure is specific to vmThreads. In the Node 22 environment I tested locally, Symbol.dispose is undefined inside the VM realm, which changes the shape of the mocked object. I couldn't verify the exact same runtime behavior locally on Node 24/26, but it matches the realm-specific CI failure. That also means my earlier comment saying the Symbol.dispose difference wasn't caused by this PR was incorrect. It is a consequence of the expanded AssertionError serialization introduced here. There isn't one object snapshot that works across all pools: a snapshot generated under vmThreads differs from the one produced under threads/forks. I changed those four assertions to snapshot the error message instead, which is the part those tests are actually trying to verify and is stable across the pools. Both fixes are pushed now, jobs are green. |
`jest-expect` and `vi-fn` gained assertions on thrown errors after the previous snapshot pass, so they still expected the collapsed `[Error: ...]` form. Regenerated. `artifacts` normalises the version out of reporter output, but its pattern only allowed a `-beta.N` prerelease. main is now on `v5.0.0-rc.1`, so the suffix leaked into the snapshot as `v<version>-rc.1`. Widened the pattern to any prerelease tag; this is independent of the error formatting change.
Closing the context was not enough to stop the unhandled ENOENT: the run started by the last restart can still be populating the file stats cache, so its `stat` lands after `runInlineTests` has removed the directory. The e2e job then failed on the unhandled error even though every test passed. Await `runningPromise` before `close()` so no run is in flight at teardown.
|
All checks are green after rebase :) |
2 participants
Description
Resolves #8697
display.ts's andsnapshot/src/port/plugins.ts'sPLUGINSlists both omit theErrorplugin from@vitest/pretty-format. As a result,Errorinstances (includingAssertionErrorand customErrorsubclasses) print only their message, droppingcauseand any custom properties. Two errors with the same message but different extra properties are indistinguishable in a diff or snapshot:
ts const a = Object.assign(new Error('boom'), { code: 123 }) const b = Object.assign(new Error('boom'), { code: 456 }) // before: both print as `[Error: boom]` — identical, the mismatch is invisible diff/index.tsalready includesprettyFormatPlugins.Error(added in #5876), so the diff computation is already correct — only the display path (stringify/inspect, used byexpect().toThrow()and plain object inspection) and the snapshot path lagged behind. Credit to @bwyard, who diagnosed this exact root cause and fix location in the issue back in March.Fix: add the
Errorplugin to bothPLUGINSlists indisplay.ts(including thefilterNodebranch, for consistency) and tosnapshot/src/port/plugins.ts.Breaking change: this changes how errors are printed by
toMatchInlineSnapshot,toThrowErrorMatchingInlineSnapshot, and plaininspect()of an error. Added an entry to the Vitest 5.0 migration guide (docs/guide/migration.md).Scope heads-up:
AssertionErroralso gets theErrorplugin treatment, so any snapshot of a thrownAssertionErrornow additionally shows itsactual/expected/showDiff/operatorfields, not just the message. Updated all 81 affected snapshots across 20 test files — mechanical changes only, no test's actual behavior changed. Did not special-caseAssertionErrorto suppress those fields, since that's a separate design decision beyond what the issue asked for.Please don't delete this checklist! Before submitting the PR, please make sure you do the following:
pnpm-lock.yamlunless you introduce a new test example.Tests
pnpm test:ci.Documentation
pnpm run docscommand.Changesets
feat:,fix:,perf:,docs:, orchore:.