Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(browser): improve source map handling for bundled files #7534

Merged
merged 4 commits into from
Feb 25, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 49 additions & 7 deletions packages/browser/src/node/projectParent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,10 @@ import type {
Vitest,
} from 'vitest/node'
import type { BrowserServerState } from './state'
import { readFileSync } from 'node:fs'
import { readFile } from 'node:fs/promises'
import { parseErrorStacktrace, parseStacktrace, type StackTraceParserOptions } from '@vitest/utils/source-map'
import { join, resolve } from 'pathe'
import { dirname, join, resolve } from 'pathe'
import { BrowserServerCDPHandler } from './cdp'
import builtinCommands from './commands/index'
import { distRoot } from './constants'
Expand Down Expand Up @@ -41,6 +42,9 @@ export class ParentBrowserProject {

public config: ResolvedConfig

// cache for non-vite source maps
private sourceMapCache = new Map<string, any>()

constructor(
public project: TestProject,
public base: string,
Expand All @@ -50,17 +54,39 @@ export class ParentBrowserProject {
this.stackTraceOptions = {
frameFilter: project.config.onStackTrace,
getSourceMap: (id) => {
if (this.sourceMapCache.has(id)) {
return this.sourceMapCache.get(id)
}
const result = this.vite.moduleGraph.getModuleById(id)?.transformResult
// this can happen for bundled dependencies in node_modules/.vite
if (result && !result.map) {
const sourceMapUrl = this.retrieveSourceMapURL(result.code)
if (!sourceMapUrl) {
return null
}
const filepathDir = dirname(id)
const sourceMapPath = resolve(filepathDir, sourceMapUrl)
const map = JSON.parse(readFileSync(sourceMapPath, 'utf-8'))
this.sourceMapCache.set(id, map)
return map
}
return result?.map
},
getFileName: (id) => {
getUrlId: (id) => {
const mod = this.vite.moduleGraph.getModuleById(id)
if (mod?.file) {
return mod.file
if (mod) {
return id
}
const resolvedPath = resolve(project.config.root, id.slice(1))
const modUrl = this.vite.moduleGraph.getModuleById(resolvedPath)
if (modUrl) {
return resolvedPath
}
const modUrl = this.vite.moduleGraph.urlToModuleMap.get(id)
if (modUrl?.file) {
return modUrl.file
// some browsers (looking at you, safari) don't report queries in stack traces
// the next best thing is to try the first id that this file resolves to
const files = this.vite.moduleGraph.getModulesByFile(resolvedPath)
if (files && files.size) {
return files.values().next().value!.id!
}
return id
},
Expand Down Expand Up @@ -235,4 +261,20 @@ export class ParentBrowserProject {
const decodedTestFile = decodeURIComponent(testFile)
return { sessionId, testFile: decodedTestFile }
}

private retrieveSourceMapURL(source: string): string | null {
const re
= /\/\/[@#]\s*sourceMappingURL=([^\s'"]+)\s*$|\/\*[@#]\s*sourceMappingURL=[^\s*'"]+\s*\*\/\s*$/gm
// Keep executing the search to find the *last* sourceMappingURL to avoid
// picking up sourceMappingURLs from comments, strings, etc.
let lastMatch, match
// eslint-disable-next-line no-cond-assign
while ((match = re.exec(source))) {
lastMatch = match
}
if (!lastMatch) {
return null
}
return lastMatch[1]
}
}
52 changes: 38 additions & 14 deletions packages/utils/src/source-map.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export type { SourceMapInput } from '@jridgewell/trace-mapping'
export interface StackTraceParserOptions {
ignoreStackEntries?: (RegExp | string)[]
getSourceMap?: (file: string) => unknown
getFileName?: (id: string) => string
getUrlId?: (id: string) => string
frameFilter?: (error: ErrorWithDiff, frame: ParsedStack) => boolean | void
}

Expand Down Expand Up @@ -62,7 +62,9 @@ function extractLocation(urlLike: string) {
}
if (url.startsWith('http:') || url.startsWith('https:')) {
const urlObj = new URL(url)
url = urlObj.pathname
urlObj.searchParams.delete('import')
urlObj.searchParams.delete('browserv')
url = urlObj.pathname + urlObj.hash + urlObj.search
}
if (url.startsWith('/@fs/')) {
const isWindows = /^\/@fs\/[a-zA-Z]:\//.test(url)
Expand Down Expand Up @@ -198,33 +200,55 @@ export function parseStacktrace(
options: StackTraceParserOptions = {},
): ParsedStack[] {
const { ignoreStackEntries = stackIgnorePatterns } = options
let stacks = !CHROME_IE_STACK_REGEXP.test(stack)
const stacks = !CHROME_IE_STACK_REGEXP.test(stack)
? parseFFOrSafariStackTrace(stack)
: parseV8Stacktrace(stack)
if (ignoreStackEntries.length) {
stacks = stacks.filter(
stack => !ignoreStackEntries.some(p => stack.file.match(p)),
)
}

return stacks.map((stack) => {
if (options.getFileName) {
stack.file = options.getFileName(stack.file)
if (options.getUrlId) {
stack.file = options.getUrlId(stack.file)
}

const map = options.getSourceMap?.(stack.file) as
| SourceMapInput
| null
| undefined
if (!map || typeof map !== 'object' || !map.version) {
return stack
return shouldFilter(ignoreStackEntries, stack.file) ? null : stack
}

const traceMap = new TraceMap(map)
const { line, column } = originalPositionFor(traceMap, stack)
const { line, column, source, name } = originalPositionFor(traceMap, stack)

let file: string = stack.file
if (source) {
const fileUrl = stack.file.startsWith('file://')
? stack.file
: `file://${stack.file}`
const sourceRootUrl = map.sourceRoot
? new URL(map.sourceRoot, fileUrl)
: fileUrl
file = new URL(source, sourceRootUrl).pathname
}

if (shouldFilter(ignoreStackEntries, file)) {
return null
}

if (line != null && column != null) {
return { ...stack, line, column }
return {
line,
column,
file,
method: name || stack.method,
}
}
return stack
})
}).filter(s => s != null)
}

function shouldFilter(ignoreStackEntries: (string | RegExp)[], file: string): boolean {
return ignoreStackEntries.some(p => file.match(p))
}

function parseFFOrSafariStackTrace(stack: string): ParsedStack[] {
Expand Down
3 changes: 3 additions & 0 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions test/browser/bundled-lib/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"name": "@vitest/bundled-lib",
"type": "module",
"private": true,
"main": "./src/index.js"
}
3 changes: 3 additions & 0 deletions test/browser/bundled-lib/src/a.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export function a() {
return 'a'
}
3 changes: 3 additions & 0 deletions test/browser/bundled-lib/src/b.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export function b() {
throw new Error('error from b')
}
1 change: 1 addition & 0 deletions test/browser/bundled-lib/src/index.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export declare function index(): string
6 changes: 6 additions & 0 deletions test/browser/bundled-lib/src/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import { a } from './a.js'
import { b } from './b.js'

export function index() {
return a() + b()
}
1 change: 1 addition & 0 deletions test/browser/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
"@types/react": "^18.2.79",
"@vitejs/plugin-basic-ssl": "^1.0.2",
"@vitest/browser": "workspace:*",
"@vitest/bundled-lib": "link:./bundled-lib",
"@vitest/cjs-lib": "link:./cjs-lib",
"@vitest/injected-lib": "link:./injected-lib",
"react": "^18.3.1",
Expand Down
12 changes: 8 additions & 4 deletions test/browser/specs/runner.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -163,15 +163,19 @@ error with a stack

test(`stack trace points to correct file in every browser`, () => {
// depending on the browser it references either `.toBe()` or `expect()`
expect(stderr).toMatch(/test\/failing.test.ts:10:(12|17)/)
expect(stderr).toMatch(/test\/failing.test.ts:11:(12|17)/)

// column is 18 in safari, 8 in others
expect(stderr).toMatch(/throwError src\/error.ts:8:(18|8)/)

expect(stderr).toContain('The call was not awaited. This method is asynchronous and must be awaited; otherwise, the call will not start to avoid unhandled rejections.')
expect(stderr).toMatch(/test\/failing.test.ts:18:(27|36)/)
expect(stderr).toMatch(/test\/failing.test.ts:19:(27|33)/)
expect(stderr).toMatch(/test\/failing.test.ts:20:(27|39)/)
expect(stderr).toMatch(/test\/failing.test.ts:19:(27|36)/)
expect(stderr).toMatch(/test\/failing.test.ts:20:(27|33)/)
expect(stderr).toMatch(/test\/failing.test.ts:21:(27|39)/)

expect(stderr).toMatch(/bundled-lib\/src\/b.js:2:(8|18)/)
expect(stderr).toMatch(/bundled-lib\/src\/index.js:5:(15|17)/)
expect(stderr).toMatch(/test\/failing.test.ts:25:(2|8)/)
})

test('popup apis should log a warning', () => {
Expand Down
5 changes: 5 additions & 0 deletions test/browser/test/failing.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { page } from '@vitest/browser/context'
import { index } from '@vitest/bundled-lib'
import { expect, it } from 'vitest'
import { throwError } from '../src/error'

Expand All @@ -19,3 +20,7 @@ it('several locator methods are not awaited', () => {
page.getByRole('button').click()
page.getByRole('button').tripleClick()
})

it('correctly prints error from a bundled file', () => {
index()
})
2 changes: 1 addition & 1 deletion test/browser/vitest.config.mts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export default defineConfig({
},
},
optimizeDeps: {
include: ['@vitest/cjs-lib', 'react/jsx-dev-runtime'],
include: ['@vitest/cjs-lib', '@vitest/bundled-lib', 'react/jsx-dev-runtime'],
},
test: {
include: ['test/**.test.{ts,js,tsx}'],
Expand Down