Violent crash on stack-trace parsing
Describe the bug
Description
I'm trying to evaluate migration to vitest from mocha
All the projects of this codebase use a module that adds Error.prototype.toJSON that represents stack as an array, (the string stack split by \n).
We're getting it from packages whose purpose is to make sure that when errors are serialized when sent to ELK and it's like - they comes with all data, and it makes the stacks represented as arrays - which:
- lets the user filter them before they are serialized (much like
vitest: options.frameFilterdoes) - forces them to be displayed nicer on the web UI
- forces them to be displayed nicer when dumped as yaml to console or files in local-dev mode or troubleshoot mode. (There are more benefits, but they all are related to the concrete project).
However, when I add a setup file that calls this aforementioned module - I get these violent crashes that hide the real error:
e.g.:
I tried to do a quick PR https://github.com/vitest-dev/vitest/pull/6407, but this does not seem to be a one-liner fix, so I fall back to the full procedure 🤷
I really hope we can find a solution for this in the time I have left for this migration POC task, or all these projects will have to continue with mocha ...
The project looks amazing - Keep up the good work!
Reproduction
The minimalist form I could find. Please note we don't do it this way, we're using packages that do that and more (stated above).
Steps:
- use
globals: trueinvitest.config.js - use a setup file:
Error.prototype.toJSON = function() {
return {
name: this.name,
message: this.message,
...this,
stack: this.stack?.split('\n'),
}
}
- Use a test that fails
describe('stack trace parse failure', () => {
it('fails to parse', () => {
throw new Error('stack it up')
})
})
System Info
System:
OS: Linux 5.15 Fedora Linux 39 (Container Image)
CPU: (12) x64 13th Gen Intel(R) Core(TM) i7-1355U
Memory: 11.93 GB / 15.46 GB
Container: Yes
Shell: 5.9 - /usr/bin/zsh
Binaries:
Node: 22.6.0 - ~/.volta/tools/image/node/22.6.0/bin/node
Yarn: 4.1.1 - ~/.volta/tools/image/yarn/4.1.1/bin/yarn
npm: 10.8.2 - ~/.volta/tools/image/npm/10.8.2/bin/npm
pnpm: 9.9.0 - ~/.volta/bin/pnpm
npmPackages:
vitest: ^2.0.5 => 2.0.5
If it matters - the diagnosis is partial - it's actually a WSL on win11Pro :P
Used Package Manager
npm
Validations
- [X] Follow our Code of Conduct
- [X] Read the Contributing Guidelines.
- [X] Read the docs.
- [X] Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
- [X] Check that this is a concrete bug. For Q&A open a GitHub Discussion or join our Discord Chat Server.
- [X] The provided reproduction is a minimal reproducible example of the bug.
I do not think it makes sense to support stack as an array as it is a very obscure edge case. But Vitest shouldn't crash here at least. To do that, we need to ignore all non-string stack traces found on Errors.
I also do not think we should add options to bypass this for the same reason - the use case is too obscure to support. But maybe we can print non-string stacks as a property in that case:
https://github.com/vitest-dev/vitest/blob/83638eb9ad58b9addfe74121f7ef2952518aeb96/packages/vitest/src/node/error.ts#L359
I can agree with that :)
The basic "contract" with a test-runner is that the true error is visible. That's a must. All the "flying colors" and formatting can be done under SLA of best-effort.
As log as the information get to the screen - that would be good enough :)
If you could give me in the PR a hint on how you expect it be done in the PR - I'll be grateful :) Also - there's the macos check that I suspect to be flaky. If there's anything I can do to mend it - LMK, I'll include it.
Seems like this isn't that obscure -- happening to me fairly regularly with different packages, all with array-based stacks.
happening to me fairly regularly with different packages, all with array-based stacks.
@mattkindy Can you list such packages? I don't think we are familiar with this convention, so knowing concrete examples would help if you want to push the PR.
happening to me fairly regularly with different packages, all with array-based stacks.
@mattkindy Can you list such packages? I don't think we are familiar with this convention, so knowing concrete examples would help if you want to push the PR.
@hi-ogawa Apologies, as it seems like I was a bit off.
TL;DR: jsonify-error was being pulled in by a dependency and patching in the toJSON() method for Error. I added an entry to setupFiles to undo this.
/**
* This file removes the Error.prototype.toJSON method that gets added by
* breaking-dep via the jsonify-error package's overrideErrorMethods().
*
* Background:
* - jsonify-error modifies Error.prototype to add a toJSON method for better error serialization
* - This causes conflicts with Vitest's own error handling/reporting mechanisms
* - When both are active, error objects end up with an array-based stack trace instead
* of the standard string format, breaking error handling assumptions
*
* This setup file ensures we get standard Node.js error behavior in our tests by
* removing the custom toJSON method before any tests run.
*/
import 'breaking-dep';
if (Error.prototype.hasOwnProperty('toJSON')) {
delete Error.prototype['toJSON'];
}
Take the example of the openai package. It throws an error if it's missing an environment variable for the API key. That error is just
export class OpenAIError extends Error {}
I had caught and logged that error and saw an array stack, but it seems like that was just a quirk of my logger config. console.log / console.dir on the same object gave me a string stack. So, great, right?
However, I was still seeing
TypeError: stack.split is not a function
❯ parseFFOrSafariStackTrace node_modules/@vitest/utils/dist/source-map.js:936:16
❯ parseStacktrace node_modules/@vitest/utils/dist/source-map.js:912:54
❯ parseErrorStacktrace node_modules/@vitest/utils/dist/source-map.js:955:21
❯ Object.parseErrorStacktrace node_modules/vitest/dist/chunks/index.DsZFoqi9.js:3089:16
❯ printError node_modules/vitest/dist/chunks/index.DsZFoqi9.js:2732:26
❯ Logger.printError node_modules/vitest/dist/chunks/index.DsZFoqi9.js:3077:12
❯ DefaultReporter.printTaskErrors node_modules/vitest/dist/chunks/index.DsZFoqi9.js:3761:23
❯ DefaultReporter.printErrorsSummary node_modules/vitest/dist/chunks/index.DsZFoqi9.js:3695:12
❯ DefaultReporter.reportSummary node_modules/vitest/dist/chunks/index.DsZFoqi9.js:3619:10
❯ DefaultReporter.onFinished node_modules/vitest/dist/chunks/index.DsZFoqi9.js:3423:10
I started to dig around in node_modules/@vitest/utils/dist/source-map.js, and noticed this code:
function parseErrorStacktrace(e, options = {}) {
if (!e || isPrimitive(e)) {
return [];
}
if (e.stacks) {
return e.stacks;
}
const stackStr = e.stack || e.stackStr || "";
let stackFrames = parseStacktrace(stackStr, options);
if (options.frameFilter) {
stackFrames = stackFrames.filter(
(f) => options.frameFilter(e, f) !== false
);
}
e.stacks = stackFrames;
return stackFrames;
}
I logged out e here, and got the following
{
name: 'Error',
className: 'OpenAIError',
message: "The OPENAI_API_KEY environment variable is missing or empty; either provide it, or instantiate the OpenAI client with an apiKey option, like new OpenAI({ apiKey: 'My API Key' }).",
superclasses: [ 'Error', 'Object' ],
enumerableFields: {
stackStr: "Error: The OPENAI_API_KEY environment variable is missing or empty; either provide it, or instantiate the OpenAI client with an apiKey option, like new OpenAI({ apiKey: 'My API Key' }).\n" +
' at new OpenAI (file:///Users/mattkindy/code/node_modules/openai/src/index.ts:122:13)\n' +
' at /Users/mattkindy/code/src/server/service/openai:9:12\n' +
' at processTicksAndRejections (node:internal/process/task_queues:95:5)\n' +
' at VitestExecutor.runModule (file:///Users/mattkindy/code/node_modules/vite-node/dist/client.mjs:399:5)\n' +
' at VitestExecutor.directRequest (file:///Users/mattkindy/code/node_modules/vite-node/dist/client.mjs:381:5)\n' +
' at VitestExecutor.cachedRequest (file:///Users/mattkindy/code/node_modules/vite-node/dist/client.mjs:206:14)\n' +
' at VitestExecutor.dependencyRequest (file:///Users/mattkindy/code/node_modules/vite-node/dist/client.mjs:259:12)\n' +
' at /Users/mattkindy/code/src/server/service/tools/conversation/util.test.ts:2:31\n' +
' at VitestExecutor.runModule (file:///Users/mattkindy/code/node_modules/vite-node/dist/client.mjs:399:5)\n' +
' at VitestExecutor.directRequest (file:///Users/mattkindy/code/node_modules/vite-node/dist/client.mjs:381:5)\n' +
' at VitestExecutor.cachedRequest (file:///Users/mattkindy/code/node_modules/vite-node/dist/client.mjs:206:14)\n' +
' at VitestExecutor.executeId (file:///Users/mattkindy/code/node_modules/vite-node/dist/client.mjs:173:12)\n' +
' at collectTests (file:///Users/mattkindy/code/node_modules/@vitest/runner/dist/index.js:851:7)\n' +
' at startTests (file:///Users/mattkindy/code/node_modules/@vitest/runner/dist/index.js:1268:17)\n' +
' at file:///Users/mattkindy/code/node_modules/vitest/dist/chunks/runBaseTests.3qpJUEJM.js:126:11\n' +
' at withEnv (file:///Users/mattkindy/code/node_modules/vitest/dist/chunks/runBaseTests.3qpJUEJM.js:90:5)',
nameStr: 'Error',
expected: 'undefined',
actual: 'undefined'
},
stack: [
"Error: The OPENAI_API_KEY environment variable is missing or empty; either provide it, or instantiate the OpenAI client with an apiKey option, like new OpenAI({ apiKey: 'My API Key' }).",
'at new OpenAI (file:///Users/mattkindy/code/node_modules/openai/src/index.ts:122:13)',
'at /Users/mattkindy/code/src/server/service/openai:9:12',
'at processTicksAndRejections (node:internal/process/task_queues:95:5)',
'at VitestExecutor.runModule (file:///Users/mattkindy/code/node_modules/vite-node/dist/client.mjs:399:5)',
// ...
]
}
Of course, at this point, we know that const stackStr = e.stack || e.stackStr || ""; will eventually end up as an array... but how?
So in processError:
// stack is not serialized in worker communication
// we stringify it first
if (err.stack) {
err.stackStr = String(err.stack)
}
However, in my local logging, err.stack is already a string, so this is redundant at worst. I found
return serializeValue(err);
serializeValue: which has
if (typeof val.toJSON === 'function') {
return serializeValue(val.toJSON(), seen)
}
And I found that somehow this error class does in fact have a toJSON which turns the stack into an array... I tried with just throw new Error(), and that error, when .toJSON() is invoked, has the same default.
After more digging, I found that I had a dependency that was augmenting the Error class via jsonify-error, adding thetoJSONmethod. So that's the root cause at the end of the day.
Closed by https://github.com/vitest-dev/vitest/pull/7995