Skip to content

Commit 62aa720

Browse files
authoredJun 30, 2024··
fix(browser): print correct stack trace in source files (#6003)
1 parent 6d8848e commit 62aa720

File tree

19 files changed

+147
-48
lines changed

19 files changed

+147
-48
lines changed
 

‎packages/browser/src/node/server.ts

+43
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,10 @@ import type {
99
WorkspaceProject,
1010
} from 'vitest/node'
1111
import { join, resolve } from 'pathe'
12+
import type { ErrorWithDiff } from '@vitest/utils'
1213
import { slash } from '@vitest/utils'
1314
import type { ResolvedConfig } from 'vitest'
15+
import { type StackTraceParserOptions, parseErrorStacktrace, parseStacktrace } from '@vitest/utils/source-map'
1416
import { BrowserServerState } from './state'
1517
import { getBrowserProvider } from './utils'
1618
import { BrowserServerCDPHandler } from './cdp'
@@ -33,10 +35,31 @@ export class BrowserServer implements IBrowserServer {
3335

3436
public vite!: Vite.ViteDevServer
3537

38+
private stackTraceOptions: StackTraceParserOptions
39+
3640
constructor(
3741
public project: WorkspaceProject,
3842
public base: string,
3943
) {
44+
this.stackTraceOptions = {
45+
frameFilter: project.config.onStackTrace,
46+
getSourceMap: (id) => {
47+
const result = this.vite.moduleGraph.getModuleById(id)?.transformResult
48+
return result?.map
49+
},
50+
getFileName: (id) => {
51+
const mod = this.vite.moduleGraph.getModuleById(id)
52+
if (mod?.file) {
53+
return mod.file
54+
}
55+
const modUrl = this.vite.moduleGraph.urlToModuleMap.get(id)
56+
if (modUrl?.file) {
57+
return modUrl.file
58+
}
59+
return id
60+
},
61+
}
62+
4063
this.state = new BrowserServerState()
4164

4265
const pkgRoot = resolve(fileURLToPath(import.meta.url), '../..')
@@ -139,6 +162,26 @@ export class BrowserServer implements IBrowserServer {
139162
})
140163
}
141164

165+
public parseErrorStacktrace(
166+
e: ErrorWithDiff,
167+
options: StackTraceParserOptions = {},
168+
) {
169+
return parseErrorStacktrace(e, {
170+
...this.stackTraceOptions,
171+
...options,
172+
})
173+
}
174+
175+
public parseStacktrace(
176+
trace: string,
177+
options: StackTraceParserOptions = {},
178+
) {
179+
return parseStacktrace(trace, {
180+
...this.stackTraceOptions,
181+
...options,
182+
})
183+
}
184+
142185
private cdpSessions = new Map<string, Promise<CDPSession>>()
143186

144187
async ensureCDPHandler(contextId: string, sessionId: string) {

‎packages/runner/src/collect.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ export async function collectTests(
2727
const config = runner.config
2828

2929
for (const filepath of paths) {
30-
const file = createFileTask(filepath, config.root, config.name)
30+
const file = createFileTask(filepath, config.root, config.name, runner.pool)
3131

3232
runner.onCollectStart?.(file)
3333

‎packages/runner/src/types/runner.ts

+4
Original file line numberDiff line numberDiff line change
@@ -148,4 +148,8 @@ export interface VitestRunner {
148148
* Publicly available configuration.
149149
*/
150150
config: VitestRunnerConfig
151+
/**
152+
* The name of the current pool. Can affect how stack trace is inferred on the server side.
153+
*/
154+
pool?: string
151155
}

‎packages/runner/src/types/tasks.ts

+1
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ export interface Suite extends TaskBase {
6363
}
6464

6565
export interface File extends Suite {
66+
pool?: string
6667
filepath: string
6768
projectName: string | undefined
6869
collectDuration?: number

‎packages/runner/src/utils/collect.ts

+2
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,7 @@ export function createFileTask(
118118
filepath: string,
119119
root: string,
120120
projectName: string,
121+
pool?: string,
121122
) {
122123
const path = relative(root, filepath)
123124
const file: File = {
@@ -130,6 +131,7 @@ export function createFileTask(
130131
meta: Object.create(null),
131132
projectName,
132133
file: undefined!,
134+
pool,
133135
}
134136
file.file = file
135137
return file

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

+5
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ export type { SourceMapInput } from '@jridgewell/trace-mapping'
1414
export interface StackTraceParserOptions {
1515
ignoreStackEntries?: (RegExp | string)[]
1616
getSourceMap?: (file: string) => unknown
17+
getFileName?: (id: string) => string
1718
frameFilter?: (error: Error, frame: ParsedStack) => boolean | void
1819
}
1920

@@ -192,6 +193,10 @@ export function parseStacktrace(
192193
)
193194
}
194195
return stacks.map((stack) => {
196+
if (options.getFileName) {
197+
stack.file = options.getFileName(stack.file)
198+
}
199+
195200
const map = options.getSourceMap?.(stack.file) as
196201
| SourceMapInput
197202
| null

‎packages/vitest/src/api/setup.ts

+9-7
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import { createBirpc } from 'birpc'
44
import { parse, stringify } from 'flatted'
55
import type { WebSocket } from 'ws'
66
import { WebSocketServer } from 'ws'
7-
import type { StackTraceParserOptions } from '@vitest/utils/source-map'
87
import type { ViteDevServer } from 'vite'
98
import { API_PATH } from '../constants'
109
import type { Vitest } from '../node'
@@ -182,15 +181,18 @@ export class WebSocketReporter implements Reporter {
182181

183182
packs.forEach(([taskId, result]) => {
184183
const project = this.ctx.getProjectByTaskId(taskId)
185-
186-
const parserOptions: StackTraceParserOptions = {
187-
getSourceMap: file => project.getBrowserSourceMapModuleById(file),
188-
}
184+
const task = this.ctx.state.idMap.get(taskId)
185+
const isBrowser = task && task.file.pool === 'browser'
189186

190187
result?.errors?.forEach((error) => {
191-
if (!isPrimitive(error)) {
192-
error.stacks = parseErrorStacktrace(error, parserOptions)
188+
if (isPrimitive(error)) {
189+
return
193190
}
191+
192+
const stacks = isBrowser
193+
? project.browser?.parseErrorStacktrace(error)
194+
: parseErrorStacktrace(error)
195+
error.stacks = stacks
194196
})
195197
})
196198

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

+10-19
Original file line numberDiff line numberDiff line change
@@ -4,41 +4,40 @@ import { Writable } from 'node:stream'
44
import { normalize, relative } from 'pathe'
55
import c from 'picocolors'
66
import cliTruncate from 'cli-truncate'
7-
import type { StackTraceParserOptions } from '@vitest/utils/source-map'
87
import { inspect } from '@vitest/utils'
98
import stripAnsi from 'strip-ansi'
109
import type { ErrorWithDiff, ParsedStack } from '../types'
1110
import {
1211
lineSplitRE,
13-
parseErrorStacktrace,
1412
positionToOffset,
1513
} from '../utils/source-map'
1614
import { F_POINTER } from '../utils/figures'
1715
import { TypeCheckError } from '../typecheck/typechecker'
1816
import { isPrimitive } from '../utils'
1917
import type { Vitest } from './core'
2018
import { divider } from './reporters/renderers/utils'
19+
import type { ErrorOptions } from './logger'
2120
import { Logger } from './logger'
2221
import type { WorkspaceProject } from './workspace'
2322

2423
interface PrintErrorOptions {
2524
type?: string
2625
logger: Logger
27-
fullStack?: boolean
2826
showCodeFrame?: boolean
2927
printProperties?: boolean
3028
screenshotPaths?: string[]
29+
parseErrorStacktrace: (error: ErrorWithDiff) => ParsedStack[]
3130
}
3231

33-
interface PrintErrorResult {
32+
export interface PrintErrorResult {
3433
nearest?: ParsedStack
3534
}
3635

3736
// use Logger with custom Console to capture entire error printing
3837
export function capturePrintError(
3938
error: unknown,
4039
ctx: Vitest,
41-
project: WorkspaceProject,
40+
options: ErrorOptions,
4241
) {
4342
let output = ''
4443
const writable = new Writable({
@@ -47,9 +46,10 @@ export function capturePrintError(
4746
callback()
4847
},
4948
})
50-
const result = printError(error, project, {
49+
const logger = new Logger(ctx, writable, writable)
50+
const result = logger.printError(error, {
5151
showCodeFrame: false,
52-
logger: new Logger(ctx, writable, writable),
52+
...options,
5353
})
5454
return { nearest: result?.nearest, output }
5555
}
@@ -59,7 +59,7 @@ export function printError(
5959
project: WorkspaceProject | undefined,
6060
options: PrintErrorOptions,
6161
): PrintErrorResult | undefined {
62-
const { showCodeFrame = true, fullStack = false, type, printProperties = true } = options
62+
const { showCodeFrame = true, type, printProperties = true } = options
6363
const logger = options.logger
6464
let e = error as ErrorWithDiff
6565

@@ -84,16 +84,7 @@ export function printError(
8484
return
8585
}
8686

87-
const parserOptions: StackTraceParserOptions = {
88-
// only browser stack traces require remapping
89-
getSourceMap: file => project.getBrowserSourceMapModuleById(file),
90-
frameFilter: project.config.onStackTrace,
91-
}
92-
93-
if (fullStack) {
94-
parserOptions.ignoreStackEntries = []
95-
}
96-
const stacks = parseErrorStacktrace(e, parserOptions)
87+
const stacks = options.parseErrorStacktrace(e)
9788

9889
const nearest
9990
= error instanceof TypeCheckError
@@ -195,9 +186,9 @@ export function printError(
195186
if (typeof e.cause === 'object' && e.cause && 'name' in e.cause) {
196187
(e.cause as any).name = `Caused by: ${(e.cause as any).name}`
197188
printError(e.cause, project, {
198-
fullStack,
199189
showCodeFrame: false,
200190
logger: options.logger,
191+
parseErrorStacktrace: options.parseErrorStacktrace,
201192
})
202193
}
203194

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

+24-6
Original file line numberDiff line numberDiff line change
@@ -2,22 +2,26 @@ import { Console } from 'node:console'
22
import type { Writable } from 'node:stream'
33
import { createLogUpdate } from 'log-update'
44
import c from 'picocolors'
5-
import type { ErrorWithDiff } from '../types'
5+
import { parseErrorStacktrace } from '@vitest/utils/source-map'
6+
import type { ErrorWithDiff, Task } from '../types'
67
import type { TypeCheckError } from '../typecheck/typechecker'
78
import { toArray } from '../utils'
89
import { highlightCode } from '../utils/colors'
910
import { divider } from './reporters/renderers/utils'
1011
import { RandomSequencer } from './sequencers/RandomSequencer'
1112
import type { Vitest } from './core'
13+
import type { PrintErrorResult } from './error'
1214
import { printError } from './error'
1315
import type { WorkspaceProject } from './workspace'
1416

15-
interface ErrorOptions {
17+
export interface ErrorOptions {
1618
type?: string
1719
fullStack?: boolean
1820
project?: WorkspaceProject
1921
verbose?: boolean
2022
screenshotPaths?: string[]
23+
task?: Task
24+
showCodeFrame?: boolean
2125
}
2226

2327
const ESC = '\x1B['
@@ -89,18 +93,32 @@ export class Logger {
8993
this.console.log(`${CURSOR_TO_START}${ERASE_DOWN}${log}`)
9094
}
9195

92-
printError(err: unknown, options: ErrorOptions = {}) {
96+
printError(err: unknown, options: ErrorOptions = {}): PrintErrorResult | undefined {
9397
const { fullStack = false, type } = options
9498
const project = options.project
9599
?? this.ctx.getCoreWorkspaceProject()
96100
?? this.ctx.projects[0]
97-
printError(err, project, {
98-
fullStack,
101+
return printError(err, project, {
99102
type,
100-
showCodeFrame: true,
103+
showCodeFrame: options.showCodeFrame ?? true,
101104
logger: this,
102105
printProperties: options.verbose,
103106
screenshotPaths: options.screenshotPaths,
107+
parseErrorStacktrace: (error) => {
108+
// browser stack trace needs to be processed differently,
109+
// so there is a separate method for that
110+
if (options.task?.file.pool === 'browser' && project.browser) {
111+
return project.browser.parseErrorStacktrace(error, {
112+
ignoreStackEntries: fullStack ? [] : undefined,
113+
})
114+
}
115+
116+
// node.js stack trace already has correct source map locations
117+
return parseErrorStacktrace(error, {
118+
frameFilter: project.config.onStackTrace,
119+
ignoreStackEntries: fullStack ? [] : undefined,
120+
})
121+
},
104122
})
105123
}
106124

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

+12-5
Original file line numberDiff line numberDiff line change
@@ -309,13 +309,15 @@ export abstract class BaseReporter implements Reporter {
309309
if (log.browser) {
310310
write('\n')
311311
}
312+
312313
const project = log.taskId
313314
? this.ctx.getProjectByTaskId(log.taskId)
314315
: this.ctx.getCoreWorkspaceProject()
315-
const stack = parseStacktrace(log.origin, {
316-
getSourceMap: file => project.getBrowserSourceMapModuleById(file),
317-
frameFilter: project.config.onStackTrace,
318-
})
316+
317+
const stack = log.browser
318+
? (project.browser?.parseStacktrace(log.origin) || [])
319+
: parseStacktrace(log.origin)
320+
319321
const highlight = task
320322
? stack.find(i => i.file === task.file.filepath)
321323
: null
@@ -605,7 +607,12 @@ export abstract class BaseReporter implements Reporter {
605607
}
606608
const screenshots = tasks.filter(t => t.meta?.failScreenshotPath).map(t => t.meta?.failScreenshotPath as string)
607609
const project = this.ctx.getProjectByTaskId(tasks[0].id)
608-
this.ctx.logger.printError(error, { project, verbose: this.verbose, screenshotPaths: screenshots })
610+
this.ctx.logger.printError(error, {
611+
project,
612+
verbose: this.verbose,
613+
screenshotPaths: screenshots,
614+
task: tasks[0],
615+
})
609616
errorDivider()
610617
}
611618
}

‎packages/vitest/src/node/reporters/github-actions.ts

+4-2
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ export class GithubActionsReporter implements Reporter {
1818
project: WorkspaceProject
1919
title: string
2020
error: unknown
21+
file?: File
2122
}>()
2223
for (const error of errors) {
2324
projectErrors.push({
@@ -40,14 +41,15 @@ export class GithubActionsReporter implements Reporter {
4041
project,
4142
title,
4243
error,
44+
file,
4345
})
4446
}
4547
}
4648
}
4749

4850
// format errors via `printError`
49-
for (const { project, title, error } of projectErrors) {
50-
const result = capturePrintError(error, this.ctx, project)
51+
for (const { project, title, error, file } of projectErrors) {
52+
const result = capturePrintError(error, this.ctx, { project, task: file })
5153
const stack = result?.nearest
5254
if (!stack) {
5355
continue

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -230,7 +230,7 @@ export class JUnitReporter implements Reporter {
230230
const result = capturePrintError(
231231
error,
232232
this.ctx,
233-
this.ctx.getProjectByTaskId(task.id),
233+
{ project: this.ctx.getProjectByTaskId(task.id), task },
234234
)
235235
await this.baseLog(
236236
escapeXML(stripAnsi(result.output.trim())),

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

+5-5
Original file line numberDiff line numberDiff line change
@@ -86,11 +86,11 @@ export class TapReporter implements Reporter {
8686
this.logger.indent()
8787

8888
task.result.errors.forEach((error) => {
89-
const stacks = parseErrorStacktrace(error, {
90-
getSourceMap: file =>
91-
project.getBrowserSourceMapModuleById(file),
92-
frameFilter: this.ctx.config.onStackTrace,
93-
})
89+
const stacks = task.file.pool === 'browser'
90+
? (project.browser?.parseErrorStacktrace(error) || [])
91+
: parseErrorStacktrace(error, {
92+
frameFilter: this.ctx.config.onStackTrace,
93+
})
9494
const stack = stacks[0]
9595

9696
this.logger.log('---')

‎packages/vitest/src/runtime/runners/test.ts

+2
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@ export class VitestTestRunner implements VitestRunner {
2828

2929
private assertionsErrors = new WeakMap<Readonly<Task>, Error>()
3030

31+
public pool = this.workerState.ctx.pool
32+
3133
constructor(public config: ResolvedConfig) {}
3234

3335
importFile(filepath: string, source: VitestRunnerImportSource): unknown {

‎packages/vitest/src/types/browser.ts

+4-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
1-
import type { Awaitable } from '@vitest/utils'
1+
import type { Awaitable, ErrorWithDiff, ParsedStack } from '@vitest/utils'
22
import type { ViteDevServer } from 'vite'
33
import type { CancelReason } from '@vitest/runner'
4+
import type { StackTraceParserOptions } from '@vitest/utils/source-map'
45
import type { WorkspaceProject } from '../node/workspace'
56
import type { ApiConfig } from './config'
67

@@ -190,6 +191,8 @@ export interface BrowserServer {
190191
provider: BrowserProvider
191192
close: () => Promise<void>
192193
initBrowserProvider: () => Promise<void>
194+
parseStacktrace: (stack: string) => ParsedStack[]
195+
parseErrorStacktrace: (error: ErrorWithDiff, options?: StackTraceParserOptions) => ParsedStack[]
193196
}
194197

195198
export interface BrowserCommand<Payload extends unknown[]> {

‎test/browser/specs/runner.test.ts

+4-1
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,10 @@ error with a stack
105105

106106
test(`stack trace points to correct file in every browser`, () => {
107107
// dependeing on the browser it references either `.toBe()` or `expect()`
108-
expect(stderr).toMatch(/test\/failing.test.ts:4:(12|17)/)
108+
expect(stderr).toMatch(/test\/failing.test.ts:5:(12|17)/)
109+
110+
// column is 18 in safari, 8 in others
111+
expect(stderr).toMatch(/throwError src\/error.ts:8:(18|8)/)
109112
})
110113

111114
test('popup apis should log a warning', () => {

‎test/browser/src/error.ts

+9
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
interface _SomeType {
2+
_unused: string
3+
}
4+
5+
// this should affect the line number
6+
7+
export function throwError(_opts?: _SomeType) {
8+
throw new Error('error')
9+
}

‎test/browser/test/failing.test.ts

+5
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,10 @@
11
import { expect, it } from 'vitest'
2+
import { throwError } from '../src/error'
23

34
it('correctly fails and prints a diff', () => {
45
expect(1).toBe(2)
56
})
7+
8+
it('correctly print error in another file', () => {
9+
throwError()
10+
})

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

+2
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ exports[`html reporter > resolves to "failing" status for test file "json-fail"
1313
"meta": {},
1414
"mode": "run",
1515
"name": "json-fail.test.ts",
16+
"pool": "forks",
1617
"prepareDuration": 0,
1718
"result": {
1819
"duration": 0,
@@ -126,6 +127,7 @@ exports[`html reporter > resolves to "passing" status for test file "all-passing
126127
"meta": {},
127128
"mode": "run",
128129
"name": "all-passing-or-skipped.test.ts",
130+
"pool": "forks",
129131
"prepareDuration": 0,
130132
"result": {
131133
"duration": 0,

0 commit comments

Comments
 (0)
Please sign in to comment.