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

Author: sheremet-va

packages/browser/src/node/projectParent.ts

@@ -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'
@@ -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,
@@ -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
       },
@@ -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]
+  }
 }

packages/utils/src/source-map.ts

@@ -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
 }
 
@@ -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)
@@ -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[] {

pnpm-lock.yaml

@@ -0,0 +1,6 @@
+{
+  "name": "@vitest/bundled-lib",
+  "type": "module",
+  "private": true,
+  "main": "./src/index.js"
+}

test/browser/bundled-lib/package.json

@@ -0,0 +1,3 @@
+export function a() {
+  return 'a'
+}

test/browser/bundled-lib/src/a.js

@@ -0,0 +1,3 @@
+export function b() {
+  throw new Error('error from b')
+}

test/browser/bundled-lib/src/b.js

@@ -0,0 +1 @@
+export declare function index(): string

test/browser/bundled-lib/src/index.d.ts

@@ -0,0 +1,6 @@
+import { a } from './a.js'
+import { b } from './b.js'
+
+export function index() {
+  return a() + b()
+}

test/browser/bundled-lib/src/index.js

@@ -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",

test/browser/package.json

@@ -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', () => {

test/browser/specs/runner.test.ts

@@ -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'
 
@@ -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()
+})

test/browser/test/failing.test.ts

@@ -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}'],