error printer: place the code frame caret by the excerpt, not by the top frame's column

[robobun]

Problem

• The caret under the source excerpt that Bun prints above an uncaught error (and above Bun.inspect(err) / console.error(err)) is drawn columnOffset columns too far right when the error is on the first line of a node:vm source compiled with a columnOffset. new vm.Script('throw new Error("x")', { filename: "s.js", columnOffset: 20 }).runInThisContext({ displayErrors: false }) prints 1 | throw new Error("x") with the ^ 20 columns to the right of new, past the end of the line. The frame line under it, at s.js:1:27, is correct: Node adds columnOffset to the first line's reported columns.
• Same mechanism with a builtin on top of the stack: for [].reduce((a, b) => a) in x.js, the excerpt is the user's line but the caret is drawn at the builtin's own column (11 on a release build, 9 on a debug build) instead of under reduce.
• Cause: the excerpt and the caret come from two different places. populateStackFramePosition (src/jsc/bindings/ZigException.cpp) cuts source_lines[0] out of the source text by byte offset, and remap_zig_exception (src/jsc/VirtualMachine.rs) rebuilds the lines from the file of the frame it picks; print_error_instance_body then indents the caret by the column of a frame it picks itself. A frame's column is what Node-style stack frames report, not an index into the printed line: on the first line of a source with a start column JSC adds that start column (CodeBlock::expressionInfoForBytecodeIndex adds firstLineColumnOffset() there), and the frame the printer picks is not always the frame the lines were taken from.

Fix

• ZigStackTrace gets a caret column that belongs to source_lines[0] and is written by whoever writes that line: populateStackFramePosition takes the position's byte offset minus the offset it cuts the line at; the lineText path in fromErrorInstance and the source-map path in remap_zig_exception store the column they built the line from. The printer indents the caret by it. Frame positions (at file:line:col, error.stack, prepareStackTrace) are unchanged.
• populateStackFramePosition used to cut the error line starting on the previous line's \n (its backward scan stops on that byte, and the lines-above loop depends on it staying there) and left the printer to trim the \n. The line is now cut from the byte after it, so the caret offset and the stored text have the same origin. Every printer trimmed that newline (SourceLine::trimmed_text), so output is unchanged; the only thing that sees the difference is the error line in the inspector's LifecycleReporter.error payload, whose source-map-built lines never had the newline anyway.
• Why this is right: the caret is an index into the text being printed, so it has to be derived from that text. The byte offset JSC records for a position is an offset into the same provider text the line is cut from, so byte_position - textStart is exact whatever the source's start line or column is, and it stays exact through the new X() walk-back in getAdjustedPositionForBytecode, which moves the byte offset along with the column. It is also what Node does for its own arrow header (GetErrorSource subtracts the script origin's column offset on the first line), and what node:vm's header in Bun already does (handleException in NodeVM.cpp); the printer was the remaining consumer doing otherwise. On the source-map path the stored column is the one the printer used before, so transpiled files print exactly as they did.
• The field is an int32_t in the C struct and a bun_core::Ordinal on the Rust side, the same split ZigStackFramePosition uses (a WTF::OrdinalNumber member would make ZigException non-C-compatible for the extern "C" declaration that returns it by value). It occupies existing padding, so the layout of the surrounding fields is unchanged on both sides.
• Verified with test/js/node/vm/vm.test.ts, "code frame of an error thrown from a source compiled with columnOffset": vm.Script first-line errors from a construct, a property access and an unresolvable identifier (caret equal to the un-offset run, frame column shifted by the offset), an error on the second line (identical to the un-offset run), compileFunction (the caret follows the text; this one passes today because compileFunction currently ignores columnOffset, and pins the printer for node:vm: report compileFunction body lines relative to lineOffset when it is 0 #38240, which makes it apply), and the uncaught printer in a subprocess through vm.runInThisContext. 4 of the 6 fail on the released 1.4.0 (caret at 26 instead of 6, frame column 27 in both), which prints the same output as a debug build of main for the repro; all 6 pass with the change.
• Also run on the debug build: inspect-error.test.js (its two minified-file snapshots fail without this change too, from a debug-only at require frame), inspect.test.js, reportError.test.ts, vm-sourceUrl.test.ts, the rest of vm.test.ts, bun/test/stack.test.ts, test-test.test.ts and the other bun test output tests, console-log.test.ts, the caret-asserting regression tests, and the source-mapped code frame in bundler_bun.test.ts (bun/TargetBunSourcemapInline, where new is walked back across a line).
• Overlaps with open PRs: GitHub Actions annotation and code frame caret: use the first frame that has a file, not a builtin frame on top #38335 fixes the builtin-on-top caret from the other direction (making the printer pick the frame remap_zig_exception picked) along with the GitHub Actions annotation; with this PR the caret no longer depends on the printer's frame choice and its caret test passes either way, the annotation half is independent, and the two diffs do not touch the same lines. error printer: fix the code frame lines above errors thrown from vm/eval sources #38244 rewrites the excerpt block in populateStackFramePosition; on top of it the new line is byte_position minus its line start. node:vm: report compileFunction body lines relative to lineOffset when it is 0 #38240 strips compileFunction's wrapper from the first excerpted line; the caret then has to be measured from that start too, which the compileFunction test here catches.

Background

• Code frame: the numbered source lines, the caret line and the name: message line Bun prints above a stack trace, for uncaught errors and for Bun.inspect / console.error of an Error. remap_zig_exception fills ZigStackTrace.source_lines (index 0 is the error's line, higher indices the lines above it), either from the original file through a source map or, when nothing maps (vm, eval, new Function, builtins), by calling populateStackFramePosition, which slices the lines out of JSC's copy of the source; print_error_instance_body renders them.
• ZigStackTrace / ZigException: C structs declared in headers-handwritten.h and mirrored by #[repr(C)] structs in src/jsc/; Rust allocates them (zig_exception::Holder) and C++ fills them in. The source lines and their line numbers live on the trace; each frame only has its own position.
• Frame position: for every bytecode JSC records a line/column and a character offset into the source provider's text (the "divot") marking where an error there is attributed. getAdjustedPositionForBytecode turns this into Bun's ZigStackFramePosition (line, column, byte offset), moving new X() positions back onto the new keyword. The column is the one Node-style frames print; for a SourceCode that does not start at column 0 (node:vm's columnOffset, which Node defines as added to the first line's columns only) JSC adds the start column to positions on the first line, so on that line the column is not an index into the physical text.

Before / after

$ cat caret.js
const vm = require("node:vm");
new vm.Script('throw new Error("x")', { filename: "/virtual/s.js", columnOffset: 20 })
  .runInThisContext({ displayErrors: false });

# before
1 | throw new Error("x")
                              ^
error: x
      at /virtual/s.js:1:27

# after
1 | throw new Error("x")
          ^
error: x
      at /virtual/s.js:1:27

$ echo '[].reduce((a, b) => a);' > reduce.js

# before
1 | [].reduce((a, b) => a);
              ^
TypeError: reduce of empty array with no initial value
      at reduce (1:11)
      at reduce.js:1:4

# after
1 | [].reduce((a, b) => a);
       ^
TypeError: reduce of empty array with no initial value
      at reduce (1:11)
      at reduce.js:1:4

With columnOffset: 0, or with the error on any line but the first, the output is the same before and after. displayErrors: false only keeps node:vm from replacing err.stack with its own header; the same excerpt is printed by Bun.inspect(err).

[coderabbitai]

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 2 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 1fa5375b-d1cd-4602-b1a3-0c7a830cd461

📥 Commits

Reviewing files that changed from the base of the PR and between 1cf8af0 and 067c8c7.

📒 Files selected for processing (6)

• src/jsc/VirtualMachine.rs
• src/jsc/ZigException.rs
• src/jsc/ZigStackTrace.rs
• src/jsc/bindings/ZigException.cpp
• src/jsc/bindings/headers-handwritten.h
• test/js/node/vm/vm.test.ts

---

_{Comment @coderabbitai help to get the list of available commands.}

[robobun]

Status

• Reproduced on the released 1.4.0 and on a debug build of main with new vm.Script('throw new Error("x")', { filename: "/virtual/s.js", columnOffset: 20 }).runInThisContext({ displayErrors: false }): the caret is printed 26 columns in while new is at column 6; the frame line says 1:27, as it should.
• Fix and tests are in this PR; test/js/node/vm/vm.test.ts ("code frame of an error thrown from a source compiled with columnOffset") has 4 cases failing without the src/ change and 6 passing with it.
• Review feedback so far is addressed (stdout drained in the spawn test; comments cut down, with the excerpt now cut from the same offset the caret is measured from). Waiting on CI.

[claude]

Beyond the inline nit, I checked the #[repr(C)] layout of the new ZigStackTrace field on both sides — bun_core::Ordinal is #[repr(transparent)] over c_int, and the field order/position matches headers-handwritten.h (the int32_t lands in the existing padding between the two u8s and frames_ptr). I also verified the else if let Some(top) = top_frame → else change is behavior-preserving (the preceding if already handles top_frame.is_none()), and that the new source_lines_caret_column out-param in populateStackFramePosition is only dereferenced on the is_top && OnlySourceLines path where it is non-null.

Extended reasoning...

The change is well-reasoned and the FFI layout checks out, but it modifies a #[repr(C)] struct shared across the Rust/C++ boundary and interacts with three other open PRs (#38335, #38244, #38240) per the description — a human should confirm merge ordering. The one finding is a harness-convention nit (undrained stdout on a child that writes nothing to it), not a functional issue.

[robobun]

^{Updated 2:05 AM PT - Aug 14th, 2026}

❌ @robobun, your commit 067c8c7 has some failures in Build #95420 (All Failures)

---

🧪   To try this PR locally:

bunx bun-pr 38349

That installs a local version of the PR into your bun-38349 executable, so you can run:

bun-38349 --bun

[claude]

I re-reviewed after 297bbcf and 067c8c7 and found no bugs; the earlier stdout-drain nit and the comment-cop notes are addressed. Because this changes the layout of a #[repr(C)] struct shared across the Rust/C++ boundary and reworks how the error printer derives its caret for every code frame, a human look is still worthwhile.

Checked: Ordinal is #[repr(transparent)] over c_int, so the new field matches int32_t and both sides place it in the same 4-byte slot between source_lines_to_collect and frames_ptr. The else if let Some(top) → else change is safe — the preceding if already guards top_frame.is_none(). col.max(0) on the indent handles the INVALID initializer. The textStart shift only drops a leading \n that every printer already trimmed via SourceLine::trimmed_text.

Extended reasoning...

Overview

Adds source_lines_caret_column (Rust Ordinal / C++ int32_t) to ZigStackTrace, filled by every code path that writes source_lines[0] — populateStackFramePosition (byte offset minus the excerpt's own start), the lineText fallback in fromErrorInstance, and the source-map path in remap_zig_exception. The printer now indents the ^ by that stored column instead of by the top frame's position.column. populateStackFramePosition also now cuts source_lines[0] starting after the previous line's \n so the excerpt and the caret share an origin. Six new tests cover vm.Script first-line/second-line, compileFunction, and the uncaught-error subprocess path.

Security risks

None. This is presentation-only (where the caret is drawn); frame positions, error.stack, and prepareStackTrace are unchanged. No untrusted input crosses a new boundary.

Level of scrutiny

Medium-high. The change itself is small and the mechanism is clearly correct (caret = index into the printed text, so derive it from the printed text), but it edits a hand-mirrored FFI struct and the error printer that runs for every uncaught error, Bun.inspect(err), and console.error(err). A layout mismatch or a missed producer of source_lines[0] would be visible everywhere. I verified the layout on both sides (Ordinal is #[repr(transparent)] c_int; the field lands at the same padded offset before frames_ptr), traced all three writers of source_lines[0] to confirm each also writes the new field, and confirmed the removed if let Some(top) binding was dead (the if arm above it already handles top_frame.is_none()).

Other factors

• Previous feedback is addressed: the spawn test now drains stdout and asserts it empty; the multi-paragraph comments were reduced after the excerpt origin was made to match the caret origin.
• The description notes overlap with three open PRs (#38335, #38244, #38240); the interactions look benign but merit a maintainer's ordering call.
• The tests assert behaviour relative to a columnOffset: 0 baseline rather than hard-coding columns, so they stay stable across debug/release column differences.