diff --git a/src/jsc/bindings/ZigException.cpp b/src/jsc/bindings/ZigException.cpp index ea84d276eda9..e16de4301a6c 100644 --- a/src/jsc/bindings/ZigException.cpp +++ b/src/jsc/bindings/ZigException.cpp @@ -261,6 +261,25 @@ class V8StackTraceIterator { { } + // The "(" opening the location of "name (location)"; notFound for a bare "location", which is how frames without a function name are printed. + static size_t locationOpeningParenthesis(StringView line) + { + if (!line.endsWith(')')) + return WTF::notFound; + + // Balance the final ")" so that a "(group)" inside the location is skipped. + unsigned depth = 0; + for (unsigned i = line.length(); i-- > 0;) { + if (line[i] == ')') + depth++; + else if (line[i] == '(' && --depth == 0) + return i; + } + + // More ")" than "(". + return line.find('('); + } + bool parseFrame(StackFrame& frame) { @@ -286,32 +305,23 @@ class V8StackTraceIterator { return false; } - StringView line = stack.substring(start, end - start); + StringView line = stack.substring(start, end - start).trim(isASCIIWhitespace); offset = end; - // the proper singular spelling is parenthesis - auto openingParentheses = line.reverseFind('('); - auto closingParentheses = line.reverseFind(')'); - - if (openingParentheses > closingParentheses) - openingParentheses = WTF::notFound; - - if (openingParentheses == WTF::notFound || closingParentheses == WTF::notFound) { - // Special case: "unknown" frames don't have parentheses but are valid - // These appear in stack traces from certain error paths - if (line == "unknown"_s) { - frame.sourceURL = line; - frame.functionName = StringView(); - return true; - } - - // For any other frame without parentheses, terminate parsing as before - offset = stack.length(); - return false; + StringView functionName; + StringView lineInner = line; + auto openingParentheses = locationOpeningParenthesis(line); + if (openingParentheses != WTF::notFound) { + lineInner = StringView_slice(line, openingParentheses + 1, line.length() - 1); + functionName = line.substring(0, openingParentheses); + if (functionName.endsWith(' ')) + functionName = functionName.substring(0, functionName.length() - 1); + } else if (line.startsWith("async "_s)) { + // V8 prints async module code as "at async file:///path/to/module.mjs:1:2" + frame.isAsync = true; + lineInner = line.substring(6); } - auto lineInner = StringView_slice(line, openingParentheses + 1, closingParentheses); - { auto marker1 = 0; auto marker2 = lineInner.find(':', marker1); @@ -383,8 +393,6 @@ class V8StackTraceIterator { } done_block: - StringView functionName = line.substring(0, openingParentheses - 1); - if (functionName == "global code"_s) { functionName = StringView(); frame.isGlobalCode = true; diff --git a/test/js/bun/util/inspect-error.test.js b/test/js/bun/util/inspect-error.test.js index 4d3f488dac81..daddc7ad7dd2 100644 --- a/test/js/bun/util/inspect-error.test.js +++ b/test/js/bun/util/inspect-error.test.js @@ -1,4 +1,5 @@ import { describe, expect, jest, test } from "bun:test"; +import { bunEnv, bunExe, tempDir } from "harness"; test("error.cause", () => { const err = new Error("error 1"); @@ -9,21 +10,23 @@ test("error.cause", () => { .replaceAll(import.meta.dir.replaceAll("\\", "/"), "[dir]"), ).toMatchInlineSnapshot(` "1 | import { describe, expect, jest, test } from "bun:test"; -2 | -3 | test("error.cause", () => { -4 | const err = new Error("error 1"); -5 | const err2 = new Error("error 2", { cause: err }); +2 | import { bunEnv, bunExe, tempDir } from "harness"; +3 | +4 | test("error.cause", () => { +5 | const err = new Error("error 1"); +6 | const err2 = new Error("error 2", { cause: err }); ^ error: error 2 - at ([dir]/inspect-error.test.js:5:20) + at ([dir]/inspect-error.test.js:6:20) 1 | import { describe, expect, jest, test } from "bun:test"; -2 | -3 | test("error.cause", () => { -4 | const err = new Error("error 1"); +2 | import { bunEnv, bunExe, tempDir } from "harness"; +3 | +4 | test("error.cause", () => { +5 | const err = new Error("error 1"); ^ error: error 1 - at ([dir]/inspect-error.test.js:4:19) + at ([dir]/inspect-error.test.js:5:19) " `); }); @@ -35,15 +38,15 @@ test("Error", () => { .replaceAll("\\", "/") .replaceAll(import.meta.dir.replaceAll("\\", "/"), "[dir]"), ).toMatchInlineSnapshot(` -"27 | " -28 | \`); -29 | }); -30 | -31 | test("Error", () => { -32 | const err = new Error("my message"); +"30 | " +31 | \`); +32 | }); +33 | +34 | test("Error", () => { +35 | const err = new Error("my message"); ^ error: my message - at ([dir]/inspect-error.test.js:32:19) + at ([dir]/inspect-error.test.js:35:19) " `); }); @@ -71,22 +74,13 @@ note: "duplicateConstDecl" was originally declared here } }); -const normalizeError = str => { - // remove debug-only stack trace frames - // like "at require (:1:21)" - if (str.includes(" (:")) { - const splits = str.split("\n"); - for (let i = 0; i < splits.length; i++) { - if (splits[i].includes(" (:")) { - splits.splice(i, 1); - i--; - } - } - return splits.join("\n"); - } - - return str; -}; +const normalizeError = str => + // remove debug-only stack trace frames of bun's own builtins, which have a + // position but no file, like "at require (51:24)" + str + .split("\n") + .filter(line => !/^\s*at \S+ \(:?\d+:\d+\)$/.test(line)) + .join("\n"); test("Error inside minified file (no color) ", () => { try { @@ -111,7 +105,7 @@ test("Error inside minified file (no color) ", () => { error: error inside long minified file! at ([dir]/inspect-error-fixture.min.js:26:2850) at ([dir]/inspect-error-fixture.min.js:26:2890) - at ([dir]/inspect-error.test.js:92:7)" + at ([dir]/inspect-error.test.js:86:7)" `); } }); @@ -140,7 +134,7 @@ test("Error inside minified file (color) ", () => { error: error inside long minified file! at ([dir]/inspect-error-fixture.min.js:26:2850) at ([dir]/inspect-error-fixture.min.js:26:2890) - at ([dir]/inspect-error.test.js:120:7)" + at ([dir]/inspect-error.test.js:114:7)" `); } }); @@ -154,7 +148,7 @@ test("Inserted originalLine and originalColumn do not appear in node:util.inspec .replaceAll(import.meta.path.replaceAll("\\", "/"), "[file]"), ).toMatchInlineSnapshot(` "Error: my message - at ([file]:149:19)" + at ([file]:143:19)" `); }); @@ -188,3 +182,126 @@ test("error.stack throwing an error doesn't lead to a crash", () => { throw err; }).toThrow(); }); + +// Once error.stack has been read (or assigned), JSC no longer has the frames of +// the error and the printer parses the " at ..." lines of that string instead. +describe("printing the frames parsed back out of error.stack", () => { + const printedFrames = text => + text + .split("\n") + .map(line => line.trim()) + .filter(line => line.startsWith("at ")); + + const printStack = lines => { + const err = new Error("boom"); + err.stack = lines.join("\n"); + return printedFrames(Bun.inspect(err)); + }; + + test("frames without a function name, as bun prints module top-level code and native frames", () => { + expect( + printStack([ + "Error: boom", + " at thrower (/fake/lib.mjs:2:13)", + " at /fake/lib.mjs:4:1", + " at unknown", + " at load (/fake/main.mjs:4:3)", + " at /fake/main.mjs:7", + ]), + ).toEqual([ + "at thrower (/fake/lib.mjs:2:13)", + "at /fake/lib.mjs:4:1", + expect.stringMatching(/^at unknown\b/), + "at load (/fake/main.mjs:4:3)", + expect.stringMatching(/^at \/fake\/main\.mjs:7\b/), + ]); + }); + + test("frames without a function name, as node prints ES module top-level code", () => { + expect( + printStack([ + "Error: boom", + " at thrower (file:///fake/lib.mjs:2:13)", + " at file:///fake/lib.mjs:4:1", + " at async file:///fake/main.mjs:3:7", + " at node:internal/main/run_main_module:36:49", + ]), + ).toEqual([ + "at thrower (file:///fake/lib.mjs:2:13)", + "at file:///fake/lib.mjs:4:1", + "at file:///fake/main.mjs:3:7", + "at node:internal/main/run_main_module:36:49", + ]); + }); + + test("parentheses inside the path or the function name", () => { + expect( + printStack([ + "Error: boom", + " at render (/fake/app/(group)/page.js:5:3)", + " at /fake/app/(group)/page.js:9:1", + " at method (with parens) (/fake/lib.js:2:3)", // { "method (with parens)"() {} } + ]), + ).toEqual([ + "at render (/fake/app/(group)/page.js:5:3)", + "at /fake/app/(group)/page.js:9:1", + "at method (with parens) (/fake/lib.js:2:3)", + ]); + }); + + test("CRLF line endings", () => { + expect( + printStack(["Error: boom\r", " at thrower (/fake/lib.mjs:2:13)\r", " at /fake/lib.mjs:4:1\r"]), + ).toEqual(["at thrower (/fake/lib.mjs:2:13)", "at /fake/lib.mjs:4:1"]); + }); + + test.concurrent("Bun.inspect and the uncaught exception printout show every frame of error.stack", async () => { + // require() puts the top-level frame of lib.mjs in the middle of the stack, + // with load() and the top-level frame of main.mjs below it. + using dir = tempDir("inspect-error-stack-string", { + "lib.mjs": ["function thrower() {", ' throw new Error("boom");', "}", "thrower();", ""].join("\n"), + "main.mjs": [ + 'import { createRequire } from "node:module";', + "const require = createRequire(import.meta.url);", + "function load() {", + ' require("./lib.mjs");', + "}", + "try {", + " load();", + "} catch (e) {", + " console.log(JSON.stringify({ stack: e.stack, inspect: Bun.inspect(e) }));", + " throw e;", + "}", + "", + ].join("\n"), + }); + await using proc = Bun.spawn({ + cmd: [bunExe(), "main.mjs"], + cwd: String(dir), + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + const { stack, inspect } = JSON.parse(stdout); + + // The frames of require() itself (between lib.mjs and load) are native and + // vary between debug and release builds, so only the frames of the two + // files are compared. + const prefix = String(dir).replaceAll("\\", "/") + "/"; + const ownFrames = text => + printedFrames(text.replaceAll("\\", "/").replaceAll(prefix, "")).filter(line => line.includes(".mjs")); + const expected = [ + expect.stringMatching(/^at thrower \(lib\.mjs:\d+:\d+\)$/), + expect.stringMatching(/^at lib\.mjs:\d+(:\d+)?$/), + expect.stringMatching(/^at load \(main\.mjs:\d+:\d+\)$/), + expect.stringMatching(/^at main\.mjs:\d+(:\d+)?$/), + ]; + expect({ stack: ownFrames(stack), inspect: ownFrames(inspect), uncaught: ownFrames(stderr) }).toEqual({ + stack: expected, + inspect: expected, + uncaught: expected, + }); + expect(exitCode).toBe(1); + }); +});