diff --git a/src/jsc/bindings/FormatStackTraceForJS.cpp b/src/jsc/bindings/FormatStackTraceForJS.cpp index b1734eb34d4b..553d526cd853 100644 --- a/src/jsc/bindings/FormatStackTraceForJS.cpp +++ b/src/jsc/bindings/FormatStackTraceForJS.cpp @@ -285,12 +285,14 @@ WTF::String formatStackTrace( sb.append(" at ("_s); - sb.append(remappedFrame.source_url.toWTFString()); + WTF::String remappedSourceURL = remappedFrame.source_url.toWTFString(); + sb.append(remappedSourceURL); if (remappedFrame.remapped) { errorInstance->putDirect(vm, builtinNames(vm).originalLinePublicName(), jsNumber(originalLine.oneBasedInt()), PropertyAttribute::DontEnum | 0); hasSet = true; line = remappedFrame.position.line(); + sourceURL = WTF::move(remappedSourceURL); } if (remappedFrame.remapped) { diff --git a/src/jsc/bindings/ZigException.cpp b/src/jsc/bindings/ZigException.cpp index 160fad449ee9..0fcaab51076f 100644 --- a/src/jsc/bindings/ZigException.cpp +++ b/src/jsc/bindings/ZigException.cpp @@ -800,18 +800,8 @@ void exceptionFromString(ZigException& except, JSC::JSValue value, JSC::JSGlobal } if (line) { if (line.isNumber()) { + // Not marked as remapped, even with an `originalLine`: Error.captureStackTrace() puts one on any object. except.stack.frames_ptr[0].position.line_zero_based = OrdinalNumber::fromOneBasedInt(JSC::toInt32(line.asNumber())).zeroBasedInt(); - - // TODO: don't sourcemap it twice - auto originalLine = obj->getIfPropertyExists(global, builtinNames(vm).originalLinePublicName()); - if (scope.exception()) [[unlikely]] { - scope.clearExceptionExceptTermination(); - } - if (originalLine) { - if (originalLine.isNumber()) { - except.stack.frames_ptr[0].position.line_zero_based = OrdinalNumber::fromOneBasedInt(JSC::toInt32(originalLine.asNumber())).zeroBasedInt(); - } - } except.stack.frames_len = 1; } } diff --git a/test/bundler/bundler_compile.test.ts b/test/bundler/bundler_compile.test.ts index ca33edde4b84..b7af9912e0a6 100644 --- a/test/bundler/bundler_compile.test.ts +++ b/test/bundler/bundler_compile.test.ts @@ -1398,6 +1398,33 @@ error: Hello World`, }, }, }); + // JavaScriptCore's parser is the one that rejects second.ts: the bundler passes a regular expression literal through. + // The source map names second.ts for the line of the error, and the module that is loaded is the embedded one. + itBundled("compile/SourceMapParserSyntaxErrorLocation", { + backend: "cli", + target: "bun", + compile: true, + files: { + "/entry.ts": /* js */ ` + const specifier = "./second" + ".js"; + try { + require(specifier); + } catch (e) { + console.log(JSON.stringify({ sourceURL: e.sourceURL, line: e.line, stack: e.stack.split("\\n")[1].trim() })); + } + `, + "/first.ts": "export const first = 1;\n", + "/second.ts": 'import { first } from "./first.ts";\nexport const second = first;\nexport const re = /(/;\n', + }, + entryPointsRaw: ["./entry.ts", "./second.ts"], + sourceMap: "external", + outfile: "dist/out", + run: { + stdout: JSON.stringify({ sourceURL: "second.ts", line: 3, stack: "at (second.ts:3)" }), + file: "dist/out", + setCwd: true, + }, + }); itBundled("compile/BunBeBunEnvVar", { compile: true, files: { diff --git a/test/js/bun/test/stack.test.ts b/test/js/bun/test/stack.test.ts index 5445e109bf8e..a4dbaa1039de 100644 --- a/test/js/bun/test/stack.test.ts +++ b/test/js/bun/test/stack.test.ts @@ -1,7 +1,7 @@ import { $ } from "bun"; -import { expect, test } from "bun:test"; -import { bunEnv, bunExe, bunRun, normalizeBunSnapshot } from "harness"; -import { join } from "node:path"; +import { describe, expect, test } from "bun:test"; +import { bunEnv, bunExe, bunRun, normalizeBunSnapshot, tempDir } from "harness"; +import { join, sep } from "node:path"; test("name property is used for function calls in Error.stack", () => { function WRONG() { @@ -85,6 +85,241 @@ test("err.line and err.column are set", async () => { ); }); +/** What the error printer shows of one error: the source lines above the message, and the frames below it. */ +function shown(printed: string, root: string) { + const lines = printed.replaceAll(root + sep, "").split("\n"); + let sourceLines = 0; + while (sourceLines < lines.length && /^\s*(\d+ \| .*|\^)$/.test(lines[sourceLines])) sourceLines++; + return { + lines: lines.slice(0, sourceLines), + at: lines.filter(line => line.trimStart().startsWith("at ")), + }; +} + +// JavaScriptCore's parser is the one that rejects second.ts: the bundler passes a regular expression literal through. +// The source map of a bundle names second.ts for the line of the error, and the file that is loaded is the bundle. +describe.concurrent("a SyntaxError from JavaScriptCore's parser in a file with a source map", () => { + const sources = { + "first.ts": "export const first = 1;\n", + "second.ts": "export const a = 1;\nexport const b = 2;\nexport const re = /(/;\n", + "entry.ts": 'import { first } from "./first.ts";\nimport { re } from "./second.ts";\nconsole.log(first, re);\n', + }; + // What an error says about where it is: its properties, and the "at " line of its stack if it has one. + const locate = /* js */ ` + const { relative } = require("node:path"); + function locate(e) { + const sourceURL = relative(__dirname, e.sourceURL).replaceAll("\\\\", "/"); + const parse = String(e.stack).split("\\n").find(line => line.includes("at ")); + return { sourceURL, line: e.line, originalLine: e.originalLine, stack: parse?.trim().replace(e.sourceURL, sourceURL) }; + } + function thrownBy(file) { + try { + require("./" + file); + } catch (e) { + return e; + } + } + `; + + type Options = Pick & { entry?: string }; + /** Builds entry.ts into the folder `name`. Returns the line of the bundle that has the regular expression. */ + async function bundle(root: string, name: string, { entry = "entry.ts", ...options }: Options) { + const { outputs } = await Bun.build({ + entrypoints: [join(root, entry)], + outdir: join(root, name), + target: "bun", + ...options, + }); + const output = await outputs.find(output => output.kind === "entry-point")!.text(); + return output.split("\n").findIndex(line => line.includes("/(/")) + 1; + } + + async function run(cwd: string) { + await using proc = Bun.spawn({ cmd: [bunExe(), "main.cjs"], env: bunEnv, cwd, stdout: "pipe", stderr: "pipe" }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + return { stdout, stderr, exitCode }; + } + + test("has the line and the sourceURL of one file", async () => { + using dir = tempDir("parser-error-location", { + ...sources, + "throws.ts": 'export const a = 1;\nexport const b = 2;\nthrow new TypeError("thrown");\n', + "transpiled.ts": + "type T = { a: number };\ninterface I { b: string }\nconst a: T = { a: 1 };\nconst re = /(/;\nexport { a, re };\n", + "unmapped.js": "// @bun\nconst a = 1;\nconst b = 2;\nconst re = /(/;\n", + // The source map has a mapping for line 2 and none for the line of the error. + "partly-mapped.js": "// @bun\nconst a = 1;\nconst b = 2;\nconst re = /(/;\n", + "partly-mapped.js.map": JSON.stringify({ + version: 3, + sources: ["other.ts"], + sourcesContent: [""], + names: [], + mappings: ";AAAA", + }), + "main.cjs": /* js */ ` + ${locate} + (async () => { + const locations = {}; + const files = ["linked/entry.js", "inline/entry.js", "cjs/entry.js", "unmapped.js", "partly-mapped.js", "transpiled.ts"]; + const errors = Object.fromEntries(files.map(file => [file, thrownBy(file)])); + for (const file of files) locations[file] = locate(errors[file]); + + // An error with frames has an originalLine only if no collection formats its stack first. + const { sourceURL, line } = locate(thrownBy("throws/throws.js")); + locations["throws/throws.js"] = { sourceURL, line }; + + const imported = await import("./imported/entry.js").catch(e => e); + locations["imported/entry.js"] = locate(imported); + // The frames are alive while this function runs, so no collection formats them first. + Error.captureStackTrace(imported); + locations["imported/entry.js, with frames"] = locate(imported); + + const clone = structuredClone(errors["linked/entry.js"]); + Error.captureStackTrace(clone); + locations["a clone of linked/entry.js, with frames"] = locate(clone); + + Error.prepareStackTrace = () => ""; + const prepared = thrownBy("prepared/entry.js"); + Error.prepareStackTrace = undefined; + locations["prepared/entry.js, under Error.prepareStackTrace"] = locate(prepared); + + console.log(JSON.stringify(locations)); + })(); + `, + }); + const root = String(dir); + // The line of each bundle that has the error. + const line = { + linked: await bundle(root, "linked", { sourcemap: "linked" }), + inline: await bundle(root, "inline", { sourcemap: "inline" }), + cjs: await bundle(root, "cjs", { sourcemap: "linked", format: "cjs" }), + imported: await bundle(root, "imported", { sourcemap: "linked" }), + prepared: await bundle(root, "prepared", { sourcemap: "linked" }), + }; + await bundle(root, "throws", { sourcemap: "linked", entry: "throws.ts" }); + + const { stdout, stderr, exitCode } = await run(root); + + // `line` is a line of second.ts, so `sourceURL` is second.ts. `originalLine` is the line before the source map. + const inSecond = (originalLine: number) => ({ + sourceURL: "second.ts", + line: 3, + originalLine, + stack: "at (second.ts:3)", + }); + // No source map gives another file for the line: both are the ones of the file that was loaded. + const inLoaded = (file: string, line: number) => ({ + sourceURL: file, + line, + originalLine: line, + stack: `at (${file}:${line})`, + }); + expect({ locations: stdout && JSON.parse(stdout), stderr, exitCode }).toEqual({ + locations: { + "linked/entry.js": inSecond(line.linked), + "inline/entry.js": inSecond(line.inline), + "cjs/entry.js": inSecond(line.cjs), + "unmapped.js": inLoaded("unmapped.js", 4), + "partly-mapped.js": inLoaded("partly-mapped.js", 4), + // Bun's own source map of a file that it transpiles names that file. + "transpiled.ts": { + sourceURL: "transpiled.ts", + line: 4, + originalLine: 2, + stack: "at (transpiled.ts:4)", + }, + // An error that has frames takes both from its first frame. + "throws/throws.js": { sourceURL: "throws.ts", line: 3 }, + // A module is parsed with no JS frame on the stack. Nothing maps its error until it has frames. + "imported/entry.js": { sourceURL: "imported/entry.js", line: line.imported }, + "imported/entry.js, with frames": inSecond(line.imported), + // A structured clone starts from the line and the sourceURL of the error. They are not mapped a second time. + "a clone of linked/entry.js, with frames": inSecond(3), + // A callable Error.prepareStackTrace makes the stack, and nothing maps the error. + "prepared/entry.js, under Error.prepareStackTrace": { sourceURL: "prepared/entry.js", line: line.prepared }, + }, + stderr: "", + exitCode: 0, + }); + }); + + // The line is looked up at column 0. A minified bundle has its modules on one line, and the first mapping of that + // line is in first.ts, so the error says first.ts:1. The parser knows the column: https://github.com/oven-sh/bun/issues/5192 + test.failing("names the module that has the error when the bundle is minified", async () => { + using dir = tempDir("parser-error-minified", { + ...sources, + "main.cjs": `${locate}\nconsole.log(JSON.stringify(locate(thrownBy("minified/entry.js"))));`, + }); + const root = String(dir); + const line = await bundle(root, "minified", { sourcemap: "linked", minify: true }); + + const { stdout, stderr, exitCode } = await run(root); + expect({ location: stdout && JSON.parse(stdout), stderr, exitCode }).toEqual({ + location: { sourceURL: "second.ts", line: 3, originalLine: line, stack: "at (second.ts:3)" }, + stderr: "", + exitCode: 0, + }); + }); + + test("is printed at that location when the printer has only its properties to read", async () => { + using dir = tempDir("parser-error-printed", { + ...sources, + "main.cjs": /* js */ ` + try { + require("./out/entry.js"); + } catch (e) { + // Neither has frames to print: the error has no stack now, and an object made from it is not an Error. + e.stack = ""; + console.log(Bun.inspect(e, { colors: false })); + throw Object.create(e); + } + `, + }); + const root = String(dir); + await bundle(root, "out", { sourcemap: "linked" }); + + const { stdout, stderr, exitCode } = await run(root); + expect({ error: shown(stdout, root), object: shown(stderr, root), exitCode }).toEqual({ + error: { + lines: ["1 | export const a = 1;", "2 | export const b = 2;", "3 | export const re = /(/;", " ^"], + at: [" at second.ts:3:1"], + }, + // The printer reads the file that an Error names, and not the file that another object names. + object: { lines: [], at: [" at second.ts:3"] }, + exitCode: 1, + }); + }); +}); + +test.concurrent("an object with the properties of an error is printed at the line of the error", async () => { + // The lines of types are not in the text that runs. The error is on line 6, which is line 2 before the source map. + using dir = tempDir("error-properties-location", { + "thrower.ts": `type A = { a: number }; +interface B { + b: string; +} +const value: A = { a: 1 }; +const error = new Error("boom " + value.a); +throw Object.create(Error.prototype, Object.getOwnPropertyDescriptors(error)); +`, + }); + const root = String(dir); + await using proc = Bun.spawn({ + cmd: [bunExe(), "thrower.ts"], + env: bunEnv, + cwd: root, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + expect({ stdout, shown: shown(stderr, root), exitCode }).toEqual({ + stdout: "", + shown: { lines: [], at: [" at thrower.ts:6"] }, + exitCode: 1, + }); +}); + test("throwing inside an error suppresses the error and prints the stack", async () => { $.throws(false); $.env(bunEnv);