Skip to content

Commit

Permalink
fix(ssr): stacktrace uses abs path with or without sourcemap (#12902)
Browse files Browse the repository at this point in the history
Co-authored-by: sapphi-red <green@sapphi.red>
  • Loading branch information
fi3ework and sapphi-red committed May 15, 2023
1 parent feef035 commit 88c855e
Show file tree
Hide file tree
Showing 7 changed files with 67 additions and 34 deletions.
2 changes: 1 addition & 1 deletion packages/vite/src/node/ssr/ssrModuleLoader.ts
Expand Up @@ -222,7 +222,7 @@ async function instantiateModule(
ssrExportAllKey,
'"use strict";' +
result.code +
`\n//# sourceURL=${mod.url}${sourceMapSuffix}`,
`\n//# sourceURL=${mod.id}${sourceMapSuffix}`,
)
await initModule(
context.global,
Expand Down
14 changes: 9 additions & 5 deletions packages/vite/src/node/ssr/ssrStacktrace.ts
@@ -1,3 +1,4 @@
import path from 'node:path'
import { TraceMap, originalPositionFor } from '@jridgewell/trace-mapping'
import type { ModuleGraph } from '../server/moduleGraph'

Expand All @@ -21,10 +22,10 @@ export function ssrRewriteStacktrace(
.map((line) => {
return line.replace(
/^ {4}at (?:(\S.*?)\s\()?(.+?):(\d+)(?::(\d+))?\)?/,
(input, varName, url, line, column) => {
if (!url) return input
(input, varName, id, line, column) => {
if (!id) return input

const mod = moduleGraph.urlToModuleMap.get(url)
const mod = moduleGraph.idToModuleMap.get(id)
const rawSourceMap = mod?.ssrTransformResult?.map

if (!rawSourceMap) {
Expand All @@ -35,15 +36,18 @@ export function ssrRewriteStacktrace(

const pos = originalPositionFor(traced, {
line: Number(line) - offset,
column: Number(column),
// stacktrace's column is 1-indexed, but sourcemap's one is 0-indexed
column: Number(column) - 1,
})

if (!pos.source || pos.line == null || pos.column == null) {
return input
}

const trimmedVarName = varName.trim()
const source = `${pos.source}:${pos.line}:${pos.column}`
const sourceFile = path.resolve(path.dirname(id), pos.source)
// stacktrace's column is 1-indexed, but sourcemap's one is 0-indexed
const source = `${sourceFile}:${pos.line}:${pos.column + 1}`
if (!trimmedVarName || trimmedVarName === 'eval') {
return ` at ${source}`
} else {
Expand Down
3 changes: 2 additions & 1 deletion packages/vite/src/node/ssr/ssrTransform.ts
@@ -1,3 +1,4 @@
import path from 'node:path'
import MagicString from 'magic-string'
import type { SourceMap } from 'rollup'
import type {
Expand Down Expand Up @@ -285,7 +286,7 @@ async function ssrTransformScript(
false,
) as SourceMap
} else {
map.sources = [url]
map.sources = [path.basename(url)]
// needs to use originalCode instead of code
// because code might be already transformed even if map is null
map.sourcesContent = [originalCode]
Expand Down
47 changes: 30 additions & 17 deletions playground/ssr-html/__tests__/ssr-html.spec.ts
Expand Up @@ -62,23 +62,36 @@ describe.runIf(isServe)('hmr', () => {
describe.runIf(isServe)('stacktrace', () => {
const execFileAsync = promisify(execFile)

for (const sourcemapsEnabled of [false, true]) {
test(`stacktrace is correct when sourcemaps is${
sourcemapsEnabled ? '' : ' not'
} enabled in Node.js`, async () => {
const testStacktraceFile = path.resolve(
__dirname,
'../test-stacktrace.js',
)
for (const ext of ['js', 'ts']) {
for (const sourcemapsEnabled of [false, true]) {
test(`stacktrace of ${ext} is correct when sourcemaps is${
sourcemapsEnabled ? '' : ' not'
} enabled in Node.js`, async () => {
const testStacktraceFile = path.resolve(
__dirname,
'../test-stacktrace.js',
)

const p = await execFileAsync('node', [
testStacktraceFile,
'' + sourcemapsEnabled,
])
const line = p.stdout
.split('\n')
.find((line) => line.includes('Module.error'))
expect(line.trim()).toMatch(/[\\/]src[\\/]error\.js:2:9/)
})
const p = await execFileAsync('node', [
testStacktraceFile,
'' + sourcemapsEnabled,
ext,
])
const lines = p.stdout
.split('\n')
.filter((line) => line.includes('Module.error'))

const reg = new RegExp(
path
.resolve(__dirname, '../src', `error.${ext}`)
.replace(/\\/g, '\\\\') + ':2:9',
'i',
)

lines.forEach((line) => {
expect(line.trim()).toMatch(reg)
})
})
}
}
})
3 changes: 3 additions & 0 deletions playground/ssr-html/src/error.ts
@@ -0,0 +1,3 @@
export function error() {
throw new Error('e')
}
29 changes: 19 additions & 10 deletions playground/ssr-html/test-stacktrace.js
Expand Up @@ -3,8 +3,10 @@ import { fileURLToPath } from 'node:url'
import { createServer } from 'vite'

const isSourceMapEnabled = process.argv[2] === 'true'
const ext = process.argv[3]
process.setSourceMapsEnabled(isSourceMapEnabled)
console.log('# sourcemaps enabled:', isSourceMapEnabled)
console.log('# source file extension:', ext)

const version = (() => {
const m = process.version.match(/^v(\d+)\.(\d+)\.\d+$/)
Expand All @@ -31,17 +33,24 @@ const vite = await createServer({
appType: 'custom',
})

const mod = await vite.ssrLoadModule('/src/error.js')
try {
mod.error()
} catch (e) {
// this should not be called
// when sourcemap support for `new Function` is supported and sourcemap is enabled
// because the stacktrace is already rewritten by Node.js
if (!(isSourceMapEnabled && isFunctionSourceMapSupported)) {
vite.ssrFixStacktrace(e)
const dir = path.dirname(fileURLToPath(import.meta.url))

const abs1 = await vite.ssrLoadModule(`/src/error.${ext}`)
const abs2 = await vite.ssrLoadModule(path.resolve(dir, `./src/error.${ext}`))
const relative = await vite.ssrLoadModule(`./src/error.${ext}`)

for (const mod of [abs1, abs2, relative]) {
try {
mod.error()
} catch (e) {
// this should not be called
// when sourcemap support for `new Function` is supported and sourcemap is enabled
// because the stacktrace is already rewritten by Node.js
if (!(isSourceMapEnabled && isFunctionSourceMapSupported)) {
vite.ssrFixStacktrace(e)
}
console.log(e)
}
console.log(e)
}

await vite.close()
3 changes: 3 additions & 0 deletions playground/test-utils.ts
Expand Up @@ -308,6 +308,9 @@ export const formatSourcemapForSnapshot = (map: any): any => {
delete m.file
delete m.names
m.sources = m.sources.map((source) => source.replace(root, '/root'))
if (m.sourceRoot) {
m.sourceRoot = m.sourceRoot.replace(root, '/root')
}
return m
}

Expand Down

0 comments on commit 88c855e

Please sign in to comment.