From 3bdf05d19c507d030fe4a8653af268b8c9b813ce Mon Sep 17 00:00:00 2001 From: Vladimir Date: Fri, 30 May 2025 13:12:32 +0200 Subject: [PATCH] fix: ensure errors keep their message and stack after `toJSON` serialisation (#8053) --- .../ui/client/components/views/ViewReport.vue | 12 ++--- packages/ui/client/composables/error.ts | 2 +- packages/utils/src/error.ts | 41 ++++++++++---- packages/utils/src/source-map.ts | 2 +- packages/utils/src/types.ts | 2 - packages/vitest/src/node/printError.ts | 7 ++- packages/vitest/src/node/reporters/base.ts | 4 +- packages/vitest/src/node/reporters/junit.ts | 2 +- packages/vitest/src/node/reporters/tap.ts | 2 +- packages/vitest/src/typecheck/typechecker.ts | 2 - test/cli/test/print-error.test.ts | 3 +- test/core/test/error.test.ts | 53 ++++++++++++++++++- .../tests/__snapshots__/html.test.ts.snap | 2 - .../tests/__snapshots__/junit.test.ts.snap | 9 +--- test/reporters/tests/html.test.ts | 1 - 15 files changed, 101 insertions(+), 43 deletions(-) diff --git a/packages/ui/client/components/views/ViewReport.vue b/packages/ui/client/components/views/ViewReport.vue index 6b4b27746..894a3914c 100644 --- a/packages/ui/client/components/views/ViewReport.vue +++ b/packages/ui/client/components/views/ViewReport.vue @@ -34,23 +34,23 @@ function collectFailed(task: Task, level: number): LeveledTask[] { function createHtmlError(filter: Convert, error: ErrorWithDiff) { let htmlError = '' if (error.message?.includes('\x1B')) { - htmlError = `${error.nameStr || error.name}: ${filter.toHtml( + htmlError = `${error.name}: ${filter.toHtml( escapeHtml(error.message), )}` } - const startStrWithX1B = error.stackStr?.includes('\x1B') - if (startStrWithX1B || error.stack?.includes('\x1B')) { + const startStrWithX1B = error.stack?.includes('\x1B') + if (startStrWithX1B) { if (htmlError.length > 0) { htmlError += filter.toHtml( - escapeHtml((startStrWithX1B ? error.stackStr : error.stack) as string), + escapeHtml((error.stack) as string), ) } else { - htmlError = `${error.nameStr || error.name}: ${ + htmlError = `${error.name}: ${ error.message }${filter.toHtml( - escapeHtml((startStrWithX1B ? error.stackStr : error.stack) as string), + escapeHtml((error.stack) as string), )}` } } diff --git a/packages/ui/client/composables/error.ts b/packages/ui/client/composables/error.ts index 34cdbc79a..7a0956378 100644 --- a/packages/ui/client/composables/error.ts +++ b/packages/ui/client/composables/error.ts @@ -44,7 +44,7 @@ export function parseError(e: unknown) { } } - error.stacks = parseStacktrace(error.stack || error.stackStr || '', { + error.stacks = parseStacktrace(error.stack || '', { ignoreStackEntries: [], }) diff --git a/packages/utils/src/error.ts b/packages/utils/src/error.ts index 801916ad4..6605f1ef3 100644 --- a/packages/utils/src/error.ts +++ b/packages/utils/src/error.ts @@ -32,6 +32,25 @@ export function serializeValue(val: any, seen: WeakMap = new WeakM if (!val || typeof val === 'string') { return val } + if (val instanceof Error && 'toJSON' in val && typeof val.toJSON === 'function') { + const jsonValue = val.toJSON() + + if (jsonValue && jsonValue !== val && typeof jsonValue === 'object') { + if (typeof val.message === 'string') { + safe(() => jsonValue.message ??= val.message) + } + if (typeof val.stack === 'string') { + safe(() => jsonValue.stack ??= val.stack) + } + if (typeof val.name === 'string') { + safe(() => jsonValue.name ??= val.name) + } + if (val.cause != null) { + safe(() => jsonValue.cause ??= serializeValue(val.cause, seen)) + } + } + return serializeValue(jsonValue, seen) + } if (typeof val === 'function') { return `Function<${val.name || 'anonymous'}>` } @@ -106,6 +125,15 @@ export function serializeValue(val: any, seen: WeakMap = new WeakM } } +function safe(fn: () => void) { + try { + return fn() + } + catch { + // ignore + } +} + export { serializeValue as serializeError } function normalizeErrorMessage(message: string) { @@ -122,15 +150,6 @@ export function processError( } const err = _err as TestError - // stack is not serialized in worker communication - // we stringify it first - if (typeof err.stack === 'string') { - err.stackStr = String(err.stack) - } - if (typeof err.name === 'string') { - err.nameStr = String(err.name) - } - if ( err.showDiff || (err.showDiff === undefined @@ -143,10 +162,10 @@ export function processError( }) } - if (typeof err.expected !== 'string') { + if ('expected' in err && typeof err.expected !== 'string') { err.expected = stringify(err.expected, 10) } - if (typeof err.actual !== 'string') { + if ('actual' in err && typeof err.actual !== 'string') { err.actual = stringify(err.actual, 10) } diff --git a/packages/utils/src/source-map.ts b/packages/utils/src/source-map.ts index 0354477ef..7da9c8a4c 100644 --- a/packages/utils/src/source-map.ts +++ b/packages/utils/src/source-map.ts @@ -281,7 +281,7 @@ export function parseErrorStacktrace( return e.stacks } - const stackStr = e.stack || e.stackStr || '' + const stackStr = e.stack || '' // if "stack" property was overwritten at runtime to be something else, // ignore the value because we don't know how to process it let stackFrames = typeof stackStr === 'string' diff --git a/packages/utils/src/types.ts b/packages/utils/src/types.ts index 8e3c3c229..c7ed18a96 100644 --- a/packages/utils/src/types.ts +++ b/packages/utils/src/types.ts @@ -55,9 +55,7 @@ export interface ErrorWithDiff { message: string name?: string cause?: unknown - nameStr?: string stack?: string - stackStr?: string stacks?: ParsedStack[] showDiff?: boolean actual?: any diff --git a/packages/vitest/src/node/printError.ts b/packages/vitest/src/node/printError.ts index 13d4428a9..eb4a6220e 100644 --- a/packages/vitest/src/node/printError.ts +++ b/packages/vitest/src/node/printError.ts @@ -240,7 +240,7 @@ function printErrorInner( }) } - handleImportOutsideModuleError(e.stack || e.stackStr || '', logger) + handleImportOutsideModuleError(e.stack || '', logger) return { nearest } } @@ -250,10 +250,8 @@ function printErrorType(type: string, ctx: Vitest) { } const skipErrorProperties = new Set([ - 'nameStr', 'cause', 'stacks', - 'stackStr', 'type', 'showDiff', 'ok', @@ -274,6 +272,7 @@ const skipErrorProperties = new Set([ 'VITEST_TEST_NAME', 'VITEST_TEST_PATH', 'VITEST_AFTER_ENV_TEARDOWN', + '__vitest_rollup_error__', ...Object.getOwnPropertyNames(Error.prototype), ...Object.getOwnPropertyNames(Object.prototype), ]) @@ -366,7 +365,7 @@ function printModuleWarningForSourceCode(logger: ErrorLogger, path: string) { } function printErrorMessage(error: ErrorWithDiff, logger: ErrorLogger) { - const errorName = error.name || error.nameStr || 'Unknown Error' + const errorName = error.name || 'Unknown Error' if (!error.message) { logger.error(error) return diff --git a/packages/vitest/src/node/reporters/base.ts b/packages/vitest/src/node/reporters/base.ts index 5917ca42a..7b7016ced 100644 --- a/packages/vitest/src/node/reporters/base.ts +++ b/packages/vitest/src/node/reporters/base.ts @@ -585,9 +585,9 @@ export abstract class BaseReporter implements Reporter { task.result?.errors?.forEach((error) => { let previous - if (error?.stackStr) { + if (error?.stack) { previous = errorsQueue.find((i) => { - if (i[0]?.stackStr !== error.stackStr) { + if (i[0]?.stack !== error.stack) { return false } diff --git a/packages/vitest/src/node/reporters/junit.ts b/packages/vitest/src/node/reporters/junit.ts index e31d2c8db..9b9fe527e 100644 --- a/packages/vitest/src/node/reporters/junit.ts +++ b/packages/vitest/src/node/reporters/junit.ts @@ -250,7 +250,7 @@ export class JUnitReporter implements Reporter { 'failure', { message: error?.message, - type: error?.name ?? error?.nameStr, + type: error?.name, }, async () => { if (!error) { diff --git a/packages/vitest/src/node/reporters/tap.ts b/packages/vitest/src/node/reporters/tap.ts index f9fa0816c..6de86c521 100644 --- a/packages/vitest/src/node/reporters/tap.ts +++ b/packages/vitest/src/node/reporters/tap.ts @@ -41,7 +41,7 @@ export class TapReporter implements Reporter { } private logErrorDetails(error: ErrorWithDiff, stack?: ParsedStack) { - const errorName = error.name || error.nameStr || 'Unknown Error' + const errorName = error.name || 'Unknown Error' this.logger.log(`name: ${yamlString(String(errorName))}`) this.logger.log(`message: ${yamlString(String(error.message))}`) diff --git a/packages/vitest/src/typecheck/typechecker.ts b/packages/vitest/src/typecheck/typechecker.ts index 53966d9e2..c5331ad43 100644 --- a/packages/vitest/src/typecheck/typechecker.ts +++ b/packages/vitest/src/typecheck/typechecker.ts @@ -244,11 +244,9 @@ export class Typechecker { originalError: info, error: { name: error.name, - nameStr: String(error.name), message: errMsg, stacks: error.stacks, stack: '', - stackStr: '', }, } }) diff --git a/test/cli/test/print-error.test.ts b/test/cli/test/print-error.test.ts index 78e9c6cd0..e9708b095 100644 --- a/test/cli/test/print-error.test.ts +++ b/test/cli/test/print-error.test.ts @@ -13,6 +13,7 @@ test('prints a custom error stack', async () => { test('fails toJson', () => { class CustomError extends Error { + name = 'CustomError' toJSON() { return { message: this.message, @@ -35,7 +36,7 @@ Serialized Error: { stack: [ 'stack 1', 'stack 2' ] } expect(stderr).toContain(` FAIL basic.test.ts > fails toJson -Unknown Error: custom error +CustomError: custom error ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯ Serialized Error: { stack: [ 'custom stack 1', 'custom stack 2' ] } `.trim()) diff --git a/test/core/test/error.test.ts b/test/core/test/error.test.ts index fe2b9e6e3..2f97ed9d2 100644 --- a/test/core/test/error.test.ts +++ b/test/core/test/error.test.ts @@ -58,5 +58,56 @@ test('Can correctly process error where cause leads to an infinite recursion', ( configurable: true, }) - expect(() => processError(err)).not.toThrow() + const serialisedError = processError(err) + + expect(serialisedError.name).toBeTypeOf('string') + expect(serialisedError.stack).toBeTypeOf('string') + expect(serialisedError.message).toBeTypeOf('string') + + expect(serialisedError.cause.name).toBeTypeOf('string') + expect(serialisedError.cause.stack).toBeTypeOf('string') + expect(serialisedError.cause.message).toBeTypeOf('string') +}) + +test('simple error has message, stack and name', () => { + const error = new Error('My error') + const serialisedError = processError(error) + + expect(error.message).toBe(serialisedError.message) + expect(error.name).toBe(serialisedError.name) + expect(error.stack).toBe(serialisedError.stack) +}) + +test('error with toJSON has message, stack and name', () => { + class SerializableError extends Error { + toJSON() { + return { ...this } + } + } + + const error = new SerializableError('My error') + const serialisedError = processError(error) + + expect(error.message).toBe(serialisedError.message) + expect(error.name).toBe(serialisedError.name) + expect(error.stack).toBe(serialisedError.stack) +}) + +test('error with toJSON doesn\'t override nessage, stack and name if it\'s there already', () => { + class SerializableError extends Error { + toJSON() { + return { + name: 'custom', + stack: 'custom stack', + message: 'custom message', + } + } + } + + const error = new SerializableError('My error') + const serialisedError = processError(error) + + expect(serialisedError.name).toBe('custom') + expect(serialisedError.stack).toBe('custom stack') + expect(serialisedError.message).toBe('custom message') }) diff --git a/test/reporters/tests/__snapshots__/html.test.ts.snap b/test/reporters/tests/__snapshots__/html.test.ts.snap index c576b433d..5ccdf0c51 100644 --- a/test/reporters/tests/__snapshots__/html.test.ts.snap +++ b/test/reporters/tests/__snapshots__/html.test.ts.snap @@ -57,12 +57,10 @@ exports[`html reporter > resolves to "failing" status for test file "json-fail" "expected": "1", "message": "expected 2 to deeply equal 1", "name": "AssertionError", - "nameStr": "AssertionError", "ok": false, "operator": "strictEqual", "showDiff": true, "stack": "AssertionError: expected 2 to deeply equal 1", - "stackStr": "AssertionError: expected 2 to deeply equal 1", }, ], "repeatCount": 0, diff --git a/test/reporters/tests/__snapshots__/junit.test.ts.snap b/test/reporters/tests/__snapshots__/junit.test.ts.snap index 918b43f7e..915f4a8b6 100644 --- a/test/reporters/tests/__snapshots__/junit.test.ts.snap +++ b/test/reporters/tests/__snapshots__/junit.test.ts.snap @@ -90,12 +90,7 @@ AssertionError: expected { hello: 'x' } to deeply equal { hello: &apos -{ - noName: 'hi', - expected: 'undefined', - actual: 'undefined', - stacks: [] -} +{ noName: 'hi', stacks: [] } ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯ Serialized Error: { noName: 'hi' } @@ -112,7 +107,7 @@ Unknown Error: 1234 -{ name: 1234, expected: 'undefined', actual: 'undefined', stacks: [] } +{ name: 1234, stacks: [] } diff --git a/test/reporters/tests/html.test.ts b/test/reporters/tests/html.test.ts index 78b206f04..f44b6d9f8 100644 --- a/test/reporters/tests/html.test.ts +++ b/test/reporters/tests/html.test.ts @@ -65,7 +65,6 @@ describe('html reporter', async () => { task.result.startTime = 0 expect(task.result.errors).toBeDefined() task.result.errors[0].stack = task.result.errors[0].stack.split('\n')[0] - task.result.errors[0].stackStr = task.result.errors[0].stackStr.split('\n')[0] expect(task.logs).toBeDefined() expect(task.logs).toHaveLength(1) task.logs[0].taskId = 0 -- 2.51.2