Skip to content

Commit 3bdf05d

Browse files
sheremet-vasheremet-va
authored
fix: ensure errors keep their message and stack after toJSON serialisation (#8053)
1 parent 4f50f25 commit 3bdf05d

File tree

15 files changed

+101
-43
lines changed

15 files changed

+101
-43
lines changed

‎packages/ui/client/components/views/ViewReport.vue

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -34,23 +34,23 @@ function collectFailed(task: Task, level: number): LeveledTask[] {
3434
function createHtmlError(filter: Convert, error: ErrorWithDiff) {
3535
let htmlError = ''
3636
if (error.message?.includes('\x1B')) {
37-
htmlError = `<b>${error.nameStr || error.name}</b>: ${filter.toHtml(
37+
htmlError = `<b>${error.name}</b>: ${filter.toHtml(
3838
escapeHtml(error.message),
3939
)}`
4040
}
4141
42-
const startStrWithX1B = error.stackStr?.includes('\x1B')
43-
if (startStrWithX1B || error.stack?.includes('\x1B')) {
42+
const startStrWithX1B = error.stack?.includes('\x1B')
43+
if (startStrWithX1B) {
4444
if (htmlError.length > 0) {
4545
htmlError += filter.toHtml(
46-
escapeHtml((startStrWithX1B ? error.stackStr : error.stack) as string),
46+
escapeHtml((error.stack) as string),
4747
)
4848
}
4949
else {
50-
htmlError = `<b>${error.nameStr || error.name}</b>: ${
50+
htmlError = `<b>${error.name}</b>: ${
5151
error.message
5252
}${filter.toHtml(
53-
escapeHtml((startStrWithX1B ? error.stackStr : error.stack) as string),
53+
escapeHtml((error.stack) as string),
5454
)}`
5555
}
5656
}

‎packages/ui/client/composables/error.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ export function parseError(e: unknown) {
4444
}
4545
}
4646

47-
error.stacks = parseStacktrace(error.stack || error.stackStr || '', {
47+
error.stacks = parseStacktrace(error.stack || '', {
4848
ignoreStackEntries: [],
4949
})
5050

‎packages/utils/src/error.ts

Lines changed: 30 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,25 @@ export function serializeValue(val: any, seen: WeakMap<WeakKey, any> = new WeakM
3232
if (!val || typeof val === 'string') {
3333
return val
3434
}
35+
if (val instanceof Error && 'toJSON' in val && typeof val.toJSON === 'function') {
36+
const jsonValue = val.toJSON()
37+
38+
if (jsonValue && jsonValue !== val && typeof jsonValue === 'object') {
39+
if (typeof val.message === 'string') {
40+
safe(() => jsonValue.message ??= val.message)
41+
}
42+
if (typeof val.stack === 'string') {
43+
safe(() => jsonValue.stack ??= val.stack)
44+
}
45+
if (typeof val.name === 'string') {
46+
safe(() => jsonValue.name ??= val.name)
47+
}
48+
if (val.cause != null) {
49+
safe(() => jsonValue.cause ??= serializeValue(val.cause, seen))
50+
}
51+
}
52+
return serializeValue(jsonValue, seen)
53+
}
3554
if (typeof val === 'function') {
3655
return `Function<${val.name || 'anonymous'}>`
3756
}
@@ -106,6 +125,15 @@ export function serializeValue(val: any, seen: WeakMap<WeakKey, any> = new WeakM
106125
}
107126
}
108127

128+
function safe(fn: () => void) {
129+
try {
130+
return fn()
131+
}
132+
catch {
133+
// ignore
134+
}
135+
}
136+
109137
export { serializeValue as serializeError }
110138

111139
function normalizeErrorMessage(message: string) {
@@ -122,15 +150,6 @@ export function processError(
122150
}
123151
const err = _err as TestError
124152

125-
// stack is not serialized in worker communication
126-
// we stringify it first
127-
if (typeof err.stack === 'string') {
128-
err.stackStr = String(err.stack)
129-
}
130-
if (typeof err.name === 'string') {
131-
err.nameStr = String(err.name)
132-
}
133-
134153
if (
135154
err.showDiff
136155
|| (err.showDiff === undefined
@@ -143,10 +162,10 @@ export function processError(
143162
})
144163
}
145164

146-
if (typeof err.expected !== 'string') {
165+
if ('expected' in err && typeof err.expected !== 'string') {
147166
err.expected = stringify(err.expected, 10)
148167
}
149-
if (typeof err.actual !== 'string') {
168+
if ('actual' in err && typeof err.actual !== 'string') {
150169
err.actual = stringify(err.actual, 10)
151170
}
152171

‎packages/utils/src/source-map.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -281,7 +281,7 @@ export function parseErrorStacktrace(
281281
return e.stacks
282282
}
283283

284-
const stackStr = e.stack || e.stackStr || ''
284+
const stackStr = e.stack || ''
285285
// if "stack" property was overwritten at runtime to be something else,
286286
// ignore the value because we don't know how to process it
287287
let stackFrames = typeof stackStr === 'string'

‎packages/utils/src/types.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,9 +55,7 @@ export interface ErrorWithDiff {
5555
message: string
5656
name?: string
5757
cause?: unknown
58-
nameStr?: string
5958
stack?: string
60-
stackStr?: string
6159
stacks?: ParsedStack[]
6260
showDiff?: boolean
6361
actual?: any

‎packages/vitest/src/node/printError.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,7 @@ function printErrorInner(
240240
})
241241
}
242242

243-
handleImportOutsideModuleError(e.stack || e.stackStr || '', logger)
243+
handleImportOutsideModuleError(e.stack || '', logger)
244244

245245
return { nearest }
246246
}
@@ -250,10 +250,8 @@ function printErrorType(type: string, ctx: Vitest) {
250250
}
251251

252252
const skipErrorProperties = new Set([
253-
'nameStr',
254253
'cause',
255254
'stacks',
256-
'stackStr',
257255
'type',
258256
'showDiff',
259257
'ok',
@@ -274,6 +272,7 @@ const skipErrorProperties = new Set([
274272
'VITEST_TEST_NAME',
275273
'VITEST_TEST_PATH',
276274
'VITEST_AFTER_ENV_TEARDOWN',
275+
'__vitest_rollup_error__',
277276
...Object.getOwnPropertyNames(Error.prototype),
278277
...Object.getOwnPropertyNames(Object.prototype),
279278
])
@@ -366,7 +365,7 @@ function printModuleWarningForSourceCode(logger: ErrorLogger, path: string) {
366365
}
367366

368367
function printErrorMessage(error: ErrorWithDiff, logger: ErrorLogger) {
369-
const errorName = error.name || error.nameStr || 'Unknown Error'
368+
const errorName = error.name || 'Unknown Error'
370369
if (!error.message) {
371370
logger.error(error)
372371
return

‎packages/vitest/src/node/reporters/base.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -585,9 +585,9 @@ export abstract class BaseReporter implements Reporter {
585585
task.result?.errors?.forEach((error) => {
586586
let previous
587587

588-
if (error?.stackStr) {
588+
if (error?.stack) {
589589
previous = errorsQueue.find((i) => {
590-
if (i[0]?.stackStr !== error.stackStr) {
590+
if (i[0]?.stack !== error.stack) {
591591
return false
592592
}
593593

‎packages/vitest/src/node/reporters/junit.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -250,7 +250,7 @@ export class JUnitReporter implements Reporter {
250250
'failure',
251251
{
252252
message: error?.message,
253-
type: error?.name ?? error?.nameStr,
253+
type: error?.name,
254254
},
255255
async () => {
256256
if (!error) {

‎packages/vitest/src/node/reporters/tap.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ export class TapReporter implements Reporter {
4141
}
4242

4343
private logErrorDetails(error: ErrorWithDiff, stack?: ParsedStack) {
44-
const errorName = error.name || error.nameStr || 'Unknown Error'
44+
const errorName = error.name || 'Unknown Error'
4545
this.logger.log(`name: ${yamlString(String(errorName))}`)
4646
this.logger.log(`message: ${yamlString(String(error.message))}`)
4747

‎packages/vitest/src/typecheck/typechecker.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -244,11 +244,9 @@ export class Typechecker {
244244
originalError: info,
245245
error: {
246246
name: error.name,
247-
nameStr: String(error.name),
248247
message: errMsg,
249248
stacks: error.stacks,
250249
stack: '',
251-
stackStr: '',
252250
},
253251
}
254252
})

‎test/cli/test/print-error.test.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ test('prints a custom error stack', async () => {
1313
1414
test('fails toJson', () => {
1515
class CustomError extends Error {
16+
name = 'CustomError'
1617
toJSON() {
1718
return {
1819
message: this.message,
@@ -35,7 +36,7 @@ Serialized Error: { stack: [ 'stack 1', 'stack 2' ] }
3536

3637
expect(stderr).toContain(`
3738
FAIL basic.test.ts > fails toJson
38-
Unknown Error: custom error
39+
CustomError: custom error
3940
⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
4041
Serialized Error: { stack: [ 'custom stack 1', 'custom stack 2' ] }
4142
`.trim())

‎test/core/test/error.test.ts

Lines changed: 52 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,5 +58,56 @@ test('Can correctly process error where cause leads to an infinite recursion', (
5858
configurable: true,
5959
})
6060

61-
expect(() => processError(err)).not.toThrow()
61+
const serialisedError = processError(err)
62+
63+
expect(serialisedError.name).toBeTypeOf('string')
64+
expect(serialisedError.stack).toBeTypeOf('string')
65+
expect(serialisedError.message).toBeTypeOf('string')
66+
67+
expect(serialisedError.cause.name).toBeTypeOf('string')
68+
expect(serialisedError.cause.stack).toBeTypeOf('string')
69+
expect(serialisedError.cause.message).toBeTypeOf('string')
70+
})
71+
72+
test('simple error has message, stack and name', () => {
73+
const error = new Error('My error')
74+
const serialisedError = processError(error)
75+
76+
expect(error.message).toBe(serialisedError.message)
77+
expect(error.name).toBe(serialisedError.name)
78+
expect(error.stack).toBe(serialisedError.stack)
79+
})
80+
81+
test('error with toJSON has message, stack and name', () => {
82+
class SerializableError extends Error {
83+
toJSON() {
84+
return { ...this }
85+
}
86+
}
87+
88+
const error = new SerializableError('My error')
89+
const serialisedError = processError(error)
90+
91+
expect(error.message).toBe(serialisedError.message)
92+
expect(error.name).toBe(serialisedError.name)
93+
expect(error.stack).toBe(serialisedError.stack)
94+
})
95+
96+
test('error with toJSON doesn\'t override nessage, stack and name if it\'s there already', () => {
97+
class SerializableError extends Error {
98+
toJSON() {
99+
return {
100+
name: 'custom',
101+
stack: 'custom stack',
102+
message: 'custom message',
103+
}
104+
}
105+
}
106+
107+
const error = new SerializableError('My error')
108+
const serialisedError = processError(error)
109+
110+
expect(serialisedError.name).toBe('custom')
111+
expect(serialisedError.stack).toBe('custom stack')
112+
expect(serialisedError.message).toBe('custom message')
62113
})

‎test/reporters/tests/__snapshots__/html.test.ts.snap

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,12 +57,10 @@ exports[`html reporter > resolves to "failing" status for test file "json-fail"
5757
"expected": "1",
5858
"message": "expected 2 to deeply equal 1",
5959
"name": "AssertionError",
60-
"nameStr": "AssertionError",
6160
"ok": false,
6261
"operator": "strictEqual",
6362
"showDiff": true,
6463
"stack": "AssertionError: expected 2 to deeply equal 1",
65-
"stackStr": "AssertionError: expected 2 to deeply equal 1",
6664
},
6765
],
6866
"repeatCount": 0,

‎test/reporters/tests/__snapshots__/junit.test.ts.snap

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -90,12 +90,7 @@ AssertionError: expected { hello: &apos;x&apos; } to deeply equal { hello: &apos
9090
</testcase>
9191
<testcase classname="error.test.ts" name="no name object" time="...">
9292
<failure>
93-
{
94-
noName: &apos;hi&apos;,
95-
expected: &apos;undefined&apos;,
96-
actual: &apos;undefined&apos;,
97-
stacks: []
98-
}
93+
{ noName: &apos;hi&apos;, stacks: [] }
9994
⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
10095
Serialized Error: { noName: &apos;hi&apos; }
10196
</failure>
@@ -112,7 +107,7 @@ Unknown Error: 1234
112107
</testcase>
113108
<testcase classname="error.test.ts" name="number name object" time="...">
114109
<failure type="1234">
115-
{ name: 1234, expected: &apos;undefined&apos;, actual: &apos;undefined&apos;, stacks: [] }
110+
{ name: 1234, stacks: [] }
116111
</failure>
117112
</testcase>
118113
<testcase classname="error.test.ts" name="xml" time="...">

‎test/reporters/tests/html.test.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,6 @@ describe('html reporter', async () => {
6565
task.result.startTime = 0
6666
expect(task.result.errors).toBeDefined()
6767
task.result.errors[0].stack = task.result.errors[0].stack.split('\n')[0]
68-
task.result.errors[0].stackStr = task.result.errors[0].stackStr.split('\n')[0]
6968
expect(task.logs).toBeDefined()
7069
expect(task.logs).toHaveLength(1)
7170
task.logs[0].taskId = 0

0 commit comments

Comments
 (0)