From 9cd5908c8f0dc6f905274c8f6f3ad9138d292ec5 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 13 Aug 2026 21:22:09 +0000 Subject: [PATCH 1/4] error printer: collect the code frame lines above a non-transpiled source correctly populateStackFramePosition builds the code frame for errors whose source has no source map (vm scripts, eval, new Function) straight from the source text JSC holds. Its line-start scan stopped on the newline that terminates the previous line, while the loop collecting the lines above still assumed it stopped on the first character of the line. As a result the line directly above the error was skipped, the remaining context lines were numbered one too high, errors on line 2 or 3 printed no context at all, a position on a line's terminating newline (where JSC puts a ReferenceError for an identifier ending a line) dropped the error line and caret, and a position at the end of the source read one byte past the string. Scan for line boundaries from the line's end instead, clamp the position to the source length, and strip a trailing CR so CRLF sources print clean lines. --- src/jsc/bindings/ZigException.cpp | 83 ++++++++-------- .../__snapshots__/vm-sourceUrl.test.ts.snap | 10 +- test/js/node/vm/vm.test.ts | 94 ++++++++++++++++++- 3 files changed, 138 insertions(+), 49 deletions(-) diff --git a/src/jsc/bindings/ZigException.cpp b/src/jsc/bindings/ZigException.cpp index ea84d276eda9..3cce270089df 100644 --- a/src/jsc/bindings/ZigException.cpp +++ b/src/jsc/bindings/ZigException.cpp @@ -161,22 +161,36 @@ static void populateStackFramePosition(const JSC::StackFrame& stackFrame, BunStr return; if (source_lines_count > 1 && source_lines != nullptr && sourceString.is8Bit()) { - // Search for the beginning of the line - unsigned int lineStart = location.byte_position; - while (lineStart > 0 && sourceString[lineStart] != '\n') { - lineStart--; - } + const std::span bytes = sourceString.span8(); + const unsigned length = sourceString.length(); + + // The divot of an expression may be its end offset: one past its last character, which + // for the last expression in the source is `length`, and otherwise is often the '\n' + // terminating its line. Both belong to the line the expression is on. + const unsigned divot = std::min(static_cast(std::max(location.byte_position, 0)), length); + + // Offset of the first character of the line whose terminating '\n' (or end of source) is at `end`. + auto startOfLineEndingAt = [&](unsigned end) -> unsigned { + unsigned start = end; + while (start > 0 && bytes[start - 1] != '\n') { + start--; + } + return start; + }; + + // The line's text without its terminator ("\r\n" included). + auto lineText = [&](unsigned start, unsigned end) -> BunString { + if (end > start && bytes[end - 1] == '\r') { + end--; + } + return Bun::toStringView(sourceString.substring(start, end - start)); + }; - // Search for the end of the line - unsigned int lineEnd = location.byte_position; - unsigned int maxSearch = sourceString.length(); - while (lineEnd < maxSearch && sourceString[lineEnd] != '\n') { + unsigned lineEnd = divot; + while (lineEnd < length && bytes[lineEnd] != '\n') { lineEnd++; } - - const unsigned char* bytes = sourceString.span8().data(); - - // Most of the time, when you look at a stack trace, you want a couple lines above. + unsigned lineStart = startOfLineEndingAt(lineEnd); // It is key to not clone this data because source code strings are large. // Usage of toStringView (non-owning) is safe as we ref the provider. @@ -185,41 +199,18 @@ static void populateStackFramePosition(const JSC::StackFrame& stackFrame, BunStr (*referenced_source_provider)->deref(); } *referenced_source_provider = provider; - source_lines[0] = Bun::toStringView(sourceString.substring(lineStart, lineEnd - lineStart)); + source_lines[0] = lineText(lineStart, lineEnd); source_line_numbers[0] = location.line(); - if (lineStart > 0) { - auto byte_offset_in_source_string = lineStart - 1; - uint8_t source_line_i = 1; - auto remaining_lines_to_grab = source_lines_count - 1; - - { - // This should probably be code points instead of newlines - while (byte_offset_in_source_string > 0 && bytes[byte_offset_in_source_string] != '\n') { - byte_offset_in_source_string--; - } - - byte_offset_in_source_string -= byte_offset_in_source_string > 0; - } - - while (byte_offset_in_source_string > 0 && remaining_lines_to_grab > 0) { - unsigned int end_of_line_offset = byte_offset_in_source_string; - - // This should probably be code points instead of newlines - while (byte_offset_in_source_string > 0 && bytes[byte_offset_in_source_string] != '\n') { - byte_offset_in_source_string--; - } - - // We are at the beginning of the line - source_lines[source_line_i] = Bun::toStringView(sourceString.substring(byte_offset_in_source_string, end_of_line_offset - byte_offset_in_source_string + 1)); - - source_line_numbers[source_line_i] = location.line().fromZeroBasedInt(location.line().zeroBasedInt() - source_line_i); - source_line_i++; - - remaining_lines_to_grab--; - - byte_offset_in_source_string -= byte_offset_in_source_string > 0; - } + // Most of the time, when you look at a stack trace, you want a couple lines above. + // While `lineStart` is not the start of the source, `bytes[lineStart - 1]` is the '\n' + // terminating the line above the one most recently collected. + for (uint8_t i = 1; i < source_lines_count && lineStart > 0; i++) { + const unsigned aboveEnd = lineStart - 1; + const unsigned aboveStart = startOfLineEndingAt(aboveEnd); + source_lines[i] = lineText(aboveStart, aboveEnd); + source_line_numbers[i] = OrdinalNumber::fromZeroBasedInt(location.line_zero_based - i); + lineStart = aboveStart; } } } diff --git a/test/js/node/vm/__snapshots__/vm-sourceUrl.test.ts.snap b/test/js/node/vm/__snapshots__/vm-sourceUrl.test.ts.snap index 650537102d0a..a9dbd738e57c 100644 --- a/test/js/node/vm/__snapshots__/vm-sourceUrl.test.ts.snap +++ b/test/js/node/vm/__snapshots__/vm-sourceUrl.test.ts.snap @@ -12,7 +12,10 @@ Error: hello `; exports[`can get sourceURL inside node:vm 1`] = ` -"4 | return Bun.inspect(new Error("hello")); +"1 | +2 | +3 | function hello() { +4 | return Bun.inspect(new Error("hello")); ^ error: hello at hello (hellohello.js:4:24) @@ -22,7 +25,10 @@ error: hello `; exports[`eval sourceURL is correct 1`] = ` -"4 | return Bun.inspect(new Error("hello")); +"1 | +2 | +3 | function hello() { +4 | return Bun.inspect(new Error("hello")); ^ error: hello at hello (hellohello.js:4:24) diff --git a/test/js/node/vm/vm.test.ts b/test/js/node/vm/vm.test.ts index d3d04239d179..e956350a7861 100644 --- a/test/js/node/vm/vm.test.ts +++ b/test/js/node/vm/vm.test.ts @@ -1,5 +1,5 @@ import { describe, expect, test } from "bun:test"; -import { bunEnv, bunExe, normalizeBunSnapshot } from "harness"; +import { bunEnv, bunExe, isWindows, normalizeBunSnapshot } from "harness"; import { compileFunction, constants, @@ -1484,3 +1484,95 @@ test("node:vm Object.defineProperty on the context global when the sandbox is an expect(stdout.trim()).toBe(JSON.stringify({ result: 1, sandboxArray: 1 })); expect(exitCode).toBe(0); }); + +// A vm script has no source map, so the code frame bun prints above an error +// thrown from it is cut out of the script text itself (the same path serves +// eval and new Function). Bun.inspect renders the same frame as the +// uncaught-error printer. +describe("code frame of an error thrown from a vm script", () => { + function codeFrame(source: string, options: { lineOffset?: number } = {}): string[] { + let error: unknown; + try { + // With displayErrors, node:vm replaces err.stack with node's own header. + runInThisContext(source, { filename: "frame.js", displayErrors: false, ...options }); + } catch (e) { + error = e; + } + return Bun.inspect(error).split("\n"); + } + + const throwLine = "throw new Error('x');"; + const numbered = (count: number) => Array.from({ length: count }, (_, i) => `'L${i + 1}';`); + // JSC positions a ReferenceError one past the end of the identifier. + const fooFrame = ["1 | 'L1';", "2 | foo", " ^", "ReferenceError: foo is not defined"]; + const fooAtEndOfSource = "'L1';\nfoo"; + + test.each([ + [ + "every line above the error, numbered", + [...numbered(4), throwLine].join("\n"), + ["1 | 'L1';", "2 | 'L2';", "3 | 'L3';", "4 | 'L4';", `5 | ${throwLine}`, " ^", "error: x"], + ], + ["error on line 2", ["'L1';", throwLine].join("\n"), ["1 | 'L1';", `2 | ${throwLine}`, " ^", "error: x"]], + [ + "at most five lines above the error", + [...numbered(9), throwLine].join("\n"), + [ + " 5 | 'L5';", + " 6 | 'L6';", + " 7 | 'L7';", + " 8 | 'L8';", + " 9 | 'L9';", + `10 | ${throwLine}`, + " ^", + "error: x", + ], + ], + [ + "blank lines, including a blank first line", + ["", "'L2';", "", throwLine].join("\n"), + ["1 | ", "2 | 'L2';", "3 | ", `4 | ${throwLine}`, " ^", "error: x"], + ], + [ + "CRLF line endings", + ["'L1';", "'L2';", throwLine, "'L4';", ""].join("\r\n"), + ["1 | 'L1';", "2 | 'L2';", `3 | ${throwLine}`, " ^", "error: x"], + ], + ["error positioned on the newline ending its line", "'L1';\nfoo\n'L3';", fooFrame], + ["error positioned at the end of the source", fooAtEndOfSource, fooFrame], + ])("%s", (_, source, expected) => { + expect(codeFrame(source).slice(0, expected.length)).toEqual(expected); + }); + + test("line numbers include lineOffset", () => { + const expected = ["11 | 'L1';", "12 | 'L2';", `13 | ${throwLine}`, " ^", "error: x"]; + expect(codeFrame([...numbered(2), throwLine].join("\n"), { lineOffset: 10 }).slice(0, expected.length)).toEqual( + expected, + ); + }); + + test("uncaught error", async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + `require("node:vm").runInThisContext(${JSON.stringify(fooAtEndOfSource)}, { filename: "frame.js", displayErrors: false })`, + ], + env: { + ...bunEnv, + // Malloc=1 routes JSC's allocations through the system allocator so ASAN + // sees a read past the end of the source string; detect_leaks=0 because + // that also exposes JSC's never-freed startup allocations to LSAN. + ...(isWindows ? {} : { Malloc: "1" }), + ASAN_OPTIONS: [bunEnv.ASAN_OPTIONS, "detect_leaks=0"].filter(Boolean).join(":"), + }, + stdout: "pipe", + stderr: "pipe", + }); + + const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); + + expect(stderr.split("\n").slice(0, fooFrame.length)).toEqual(fooFrame); + expect(exitCode).toBe(1); + }); +}); From 122eb09f35e61726001db818514863ce86e6d829 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 13 Aug 2026 21:35:45 +0000 Subject: [PATCH 2/4] test: drain stdout in the uncaught code frame test --- test/js/node/vm/vm.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/js/node/vm/vm.test.ts b/test/js/node/vm/vm.test.ts index e956350a7861..e040dc1936db 100644 --- a/test/js/node/vm/vm.test.ts +++ b/test/js/node/vm/vm.test.ts @@ -1570,8 +1570,9 @@ describe("code frame of an error thrown from a vm script", () => { stderr: "pipe", }); - const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stdout).toBe(""); expect(stderr.split("\n").slice(0, fooFrame.length)).toEqual(fooFrame); expect(exitCode).toBe(1); }); From aeb8ecbb6d3579f35eea31591ceb884c2ded9948 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 13 Aug 2026 21:38:45 +0000 Subject: [PATCH 3/4] error printer: shorten the line-scan comments --- src/jsc/bindings/ZigException.cpp | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/jsc/bindings/ZigException.cpp b/src/jsc/bindings/ZigException.cpp index 3cce270089df..947eabcca1f9 100644 --- a/src/jsc/bindings/ZigException.cpp +++ b/src/jsc/bindings/ZigException.cpp @@ -164,12 +164,10 @@ static void populateStackFramePosition(const JSC::StackFrame& stackFrame, BunStr const std::span bytes = sourceString.span8(); const unsigned length = sourceString.length(); - // The divot of an expression may be its end offset: one past its last character, which - // for the last expression in the source is `length`, and otherwise is often the '\n' - // terminating its line. Both belong to the line the expression is on. + // JSC may position an expression one past its end: on the '\n' ending its line, or at `length`. const unsigned divot = std::min(static_cast(std::max(location.byte_position, 0)), length); - // Offset of the first character of the line whose terminating '\n' (or end of source) is at `end`. + // `end` is the offset of a line's terminating '\n', or `length` for the last line. auto startOfLineEndingAt = [&](unsigned end) -> unsigned { unsigned start = end; while (start > 0 && bytes[start - 1] != '\n') { @@ -202,9 +200,7 @@ static void populateStackFramePosition(const JSC::StackFrame& stackFrame, BunStr source_lines[0] = lineText(lineStart, lineEnd); source_line_numbers[0] = location.line(); - // Most of the time, when you look at a stack trace, you want a couple lines above. - // While `lineStart` is not the start of the source, `bytes[lineStart - 1]` is the '\n' - // terminating the line above the one most recently collected. + // Lines above: `bytes[lineStart - 1]` is the '\n' ending the line above the one just collected. for (uint8_t i = 1; i < source_lines_count && lineStart > 0; i++) { const unsigned aboveEnd = lineStart - 1; const unsigned aboveStart = startOfLineEndingAt(aboveEnd); From e80e0b548e7c651c8654a5344a966813eaf037a6 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 14 Aug 2026 01:59:40 +0000 Subject: [PATCH 4/4] error printer: use StringView_slice for code frame lines; cover line 1 and blank CRLF lines Adds the two boundary cases the table was missing (an error on line 1 collects nothing above it; a blank line in a CRLF source prints empty) and asserts the spawned test's outputs as one object. --- src/jsc/bindings/ZigException.cpp | 2 +- test/js/node/vm/vm.test.ts | 21 +++++++++++++-------- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/src/jsc/bindings/ZigException.cpp b/src/jsc/bindings/ZigException.cpp index 947eabcca1f9..8e247c839ef6 100644 --- a/src/jsc/bindings/ZigException.cpp +++ b/src/jsc/bindings/ZigException.cpp @@ -181,7 +181,7 @@ static void populateStackFramePosition(const JSC::StackFrame& stackFrame, BunStr if (end > start && bytes[end - 1] == '\r') { end--; } - return Bun::toStringView(sourceString.substring(start, end - start)); + return Bun::toStringView(StringView_slice(sourceString, start, end)); }; unsigned lineEnd = divot; diff --git a/test/js/node/vm/vm.test.ts b/test/js/node/vm/vm.test.ts index e040dc1936db..dd49ce8ee9ed 100644 --- a/test/js/node/vm/vm.test.ts +++ b/test/js/node/vm/vm.test.ts @@ -1513,6 +1513,7 @@ describe("code frame of an error thrown from a vm script", () => { [...numbered(4), throwLine].join("\n"), ["1 | 'L1';", "2 | 'L2';", "3 | 'L3';", "4 | 'L4';", `5 | ${throwLine}`, " ^", "error: x"], ], + ["error on line 1", [throwLine, "'L2';"].join("\n"), [`1 | ${throwLine}`, " ^", "error: x"]], ["error on line 2", ["'L1';", throwLine].join("\n"), ["1 | 'L1';", `2 | ${throwLine}`, " ^", "error: x"]], [ "at most five lines above the error", @@ -1534,9 +1535,9 @@ describe("code frame of an error thrown from a vm script", () => { ["1 | ", "2 | 'L2';", "3 | ", `4 | ${throwLine}`, " ^", "error: x"], ], [ - "CRLF line endings", - ["'L1';", "'L2';", throwLine, "'L4';", ""].join("\r\n"), - ["1 | 'L1';", "2 | 'L2';", `3 | ${throwLine}`, " ^", "error: x"], + "CRLF line endings, including a blank line", + ["'L1';", "'L2';", "", throwLine, "'L5';", ""].join("\r\n"), + ["1 | 'L1';", "2 | 'L2';", "3 | ", `4 | ${throwLine}`, " ^", "error: x"], ], ["error positioned on the newline ending its line", "'L1';\nfoo\n'L3';", fooFrame], ["error positioned at the end of the source", fooAtEndOfSource, fooFrame], @@ -1561,8 +1562,10 @@ describe("code frame of an error thrown from a vm script", () => { env: { ...bunEnv, // Malloc=1 routes JSC's allocations through the system allocator so ASAN - // sees a read past the end of the source string; detect_leaks=0 because - // that also exposes JSC's never-freed startup allocations to LSAN. + // sees a read past the end of the source string (bmalloc's system heap + // is unimplemented on Windows, which has no ASAN lane anyway); + // detect_leaks=0 because it also exposes JSC's never-freed startup + // allocations to LSAN. ...(isWindows ? {} : { Malloc: "1" }), ASAN_OPTIONS: [bunEnv.ASAN_OPTIONS, "detect_leaks=0"].filter(Boolean).join(":"), }, @@ -1572,8 +1575,10 @@ describe("code frame of an error thrown from a vm script", () => { const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); - expect(stdout).toBe(""); - expect(stderr.split("\n").slice(0, fooFrame.length)).toEqual(fooFrame); - expect(exitCode).toBe(1); + expect({ stdout, frame: stderr.split("\n").slice(0, fooFrame.length), exitCode }).toEqual({ + stdout: "", + frame: fooFrame, + exitCode: 1, + }); }); });