From 88c855eadcff7b59e175610c0f8a0b1e04a3ad12 Mon Sep 17 00:00:00 2001 From: fi3ework Date: Mon, 15 May 2023 18:20:23 +0800 Subject: [PATCH] fix(ssr): stacktrace uses abs path with or without sourcemap (#12902) Co-authored-by: sapphi-red --- packages/vite/src/node/ssr/ssrModuleLoader.ts | 2 +- packages/vite/src/node/ssr/ssrStacktrace.ts | 14 ++++-- packages/vite/src/node/ssr/ssrTransform.ts | 3 +- .../ssr-html/__tests__/ssr-html.spec.ts | 47 ++++++++++++------- playground/ssr-html/src/error.ts | 3 ++ playground/ssr-html/test-stacktrace.js | 29 ++++++++---- playground/test-utils.ts | 3 ++ 7 files changed, 67 insertions(+), 34 deletions(-) create mode 100644 playground/ssr-html/src/error.ts diff --git a/packages/vite/src/node/ssr/ssrModuleLoader.ts b/packages/vite/src/node/ssr/ssrModuleLoader.ts index 09a9b8817..3e498809f 100644 --- a/packages/vite/src/node/ssr/ssrModuleLoader.ts +++ b/packages/vite/src/node/ssr/ssrModuleLoader.ts @@ -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, diff --git a/packages/vite/src/node/ssr/ssrStacktrace.ts b/packages/vite/src/node/ssr/ssrStacktrace.ts index fe66b8803..5a2c55fdc 100644 --- a/packages/vite/src/node/ssr/ssrStacktrace.ts +++ b/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' @@ -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) { @@ -35,7 +36,8 @@ 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) { @@ -43,7 +45,9 @@ export function ssrRewriteStacktrace( } 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 { diff --git a/packages/vite/src/node/ssr/ssrTransform.ts b/packages/vite/src/node/ssr/ssrTransform.ts index 8a62094dc..549601950 100644 --- a/packages/vite/src/node/ssr/ssrTransform.ts +++ b/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 { @@ -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] diff --git a/playground/ssr-html/__tests__/ssr-html.spec.ts b/playground/ssr-html/__tests__/ssr-html.spec.ts index 2b9da5cad..f01565679 100644 --- a/playground/ssr-html/__tests__/ssr-html.spec.ts +++ b/playground/ssr-html/__tests__/ssr-html.spec.ts @@ -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) + }) + }) + } } }) diff --git a/playground/ssr-html/src/error.ts b/playground/ssr-html/src/error.ts new file mode 100644 index 000000000..fe8eeb20a --- /dev/null +++ b/playground/ssr-html/src/error.ts @@ -0,0 +1,3 @@ +export function error() { + throw new Error('e') +} diff --git a/playground/ssr-html/test-stacktrace.js b/playground/ssr-html/test-stacktrace.js index c3ce5e567..327a8b4c4 100644 --- a/playground/ssr-html/test-stacktrace.js +++ b/playground/ssr-html/test-stacktrace.js @@ -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+$/) @@ -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() diff --git a/playground/test-utils.ts b/playground/test-utils.ts index e7fcb93a4..26eaf5c13 100644 --- a/playground/test-utils.ts +++ b/playground/test-utils.ts @@ -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 }