error printer: don't invent a 1:1 position for frames parsed out of error.stack without one

[robobun]

Problem

• Once error.stack has been read or assigned, Bun.inspect(err), console.error(err) and the uncaught exception printout rebuild the frames by parsing that string. Frames the string prints without a position come back out with one invented for them: at foo (native) prints as at foo (native:1:1), at unknown as at unknown:1:1, and a frame with only a line, at bar (x.js:7), as at bar (x.js:7:1). Reproduces on 1.4.0 and on main:

  const e = new Error("X");
  e.stack = "Error: X\n    at foo (native)\n    at unknown\n    at bar (/a/b.js:7)";
  console.log(Bun.inspect(e));   // foo (native:1:1), unknown:1:1, bar (/a/b.js:7:1)

  Real errors hit it too: after .stack is read, a JSON.parse SyntaxError prints at parse (unknown:1:1), an indirect eval at eval (unknown:1:1), and the ERR_SOCKET_CLOSED error in test/regression/issue/23022-stack-trace-iterator.test.ts prints at unknown:1:1.

• The invented position is also acted on: the source preview is looked up for it, so a stack whose first frame is at first (native) followed by a real frame draws the caret at column 1 instead of under the real frame's column, a frame naming a file without a line gets line 1 of that file previewed, and under GITHUB_ACTIONS the annotation for such an error is ::error file=../workspace/bun/native,line=1,col=1,....

• Cause, C++ side: V8StackTraceIterator::StackFrame (src/jsc/bindings/ZigException.cpp:248) initializes lineNumber/columnNumber to zero-based 0, a valid line 1 / column 1, and the copy in fromErrorInstance (ZigException.cpp:612) stores them unconditionally. current = {} also leaves byte_position at 0, so even with the ordinals unset the position would not equal ZigStackFramePosition::INVALID, which is what the Rust side tests with is_invalid().

• Cause, Rust side (src/jsc/VirtualMachine.rs): remap_zig_exception builds the lookup for an already-remapped top frame with .zero_based().max(0) and writes it back, turning INVALID into 0:0 again (this dates back to the Zig version and was masked by the C++ default); print_stack_trace used !position.is_invalid() to decide whether a named frame has a file, so a frame with a file and no position would print as at foo with the file dropped; print_github_annotation printed col= whenever the position as a whole was set, which for a line-only frame is col=0.

Fix

• StackFrame defaults the ordinals to OrdinalNumber::beforeFirst() (-1, the convention CallSite.h and BunProcess.cpp already use) and the copy sets byte_position = -1, the same explicit -1, -1, -1 that FormatStackTraceForJS.cpp writes for frames it does not position. A frame whose location parsed to nothing is now exactly ZigStackFramePosition::INVALID; one that parsed a line only has a valid line and an invalid column.
• remap_zig_exception copies the top frame's ordinals into the lookup unchanged (both sides are bun_core::Ordinal, the same direct copy the write-back below it already does) and skips fetching the file for the preview when there is no line to preview. print_stack_trace prints at name (file...) when the frame has a file or a position. print_github_annotation keys the location on the line being valid and adds col= only when the column is.
• Why this is correct: the stack string is the only information these frames have, and it says there is no position, so the structured frame has to say the same thing with the sentinel every other producer uses (Holder initializes frames to it, the structured path leaves it for wasm frames, FormatStackTraceForJS.cpp writes it explicitly). The consumers already handle it: SourceURLFormatter prints file, file:line or file:line:col depending on which ordinals are valid, the JUnit reporter (test_command.rs record_failure) does the same, and the preview code skips frames whose position is invalid. The three places changed here were the ones that either used the sentinel as a proxy for "has a file" or clamped it away. The output now matches what error.stack itself says, and what Node prints for such frames (it never invents a position either).
• Blast radius beyond parsed frames: the only other producer of a frame with a file but no position is the dev server's browser error report (error_report_request.rs), and only when every frame is in the HMR runtime (otherwise those frames are dropped before printing); that case now prints at fn (Bun HMR Runtime) instead of at fn, which is what it already printed for such frames without a name. Structured frames are unaffected: populateStackTrace only keeps frames with a position or wasm frames, and wasm frames never get a file (Zig::sourceURL returns [wasm code], which populateStackFrameMetadata discards), so they still print at name through the unchanged branch; builtin frames with a position but no file (at require (51:24) in debug builds) print as before. The GitHub annotation only changes for frames without a line (no location instead of a bogus one) or without a column (no col= instead of col=0/col=1).
• Interaction with open PRs: error.stack: report frames at new X(...) at the new keyword #37396 makes error.stack always print :line:column for frames that have one (today a frame at column 1 prints as x.js:7, which round-trips through this change as x.js:7, and through main as x.js:7:1 by accident); once both land such frames round-trip exactly. error printer: keep parsing error.stack past frames without a function name #38308 and error printer: print the AggregateError header, label [cause]/[errors] blocks, and guard the .errors walk #36602 change the body of parseFrame and error printer: remap frames when the original source is unavailable, and not twice after error.stack #38296 changes remap_zig_exception a few lines below the hunks here; none of them touch these lines, and the shared preamble of inspect-error.test.js (harness import, normalizeError, shifted snapshot line numbers) is byte-identical to theirs so either order rebases cleanly.
• Tests: test/js/bun/util/inspect-error.test.js, describe("printing the frames parsed back out of error.stack without a position"): the five frame shapes above through Bun.inspect; a native frame as the only frame (the clamp); a file without a line (no preview, no :1:1); a native frame above a positioned one (the caret belongs to the positioned one, which is what byte_position = -1 buys); and two spawned uncaught errors under GITHUB_ACTIONS=true checking the printout and the annotation for a top frame without a position and with a line only. test/regression/issue/23022-stack-trace-iterator.test.ts now requires the unknown line to be exactly at unknown. All six fail on the release binary and on main's src/ under bun bd (the normalizeError update in the same file also makes the two minified-file tests pass under bun bd again, as in error printer: keep parsing error.stack past frames without a function name #38308), and pass with this change.
• Also run with this change, all passing: inspect.test.js, reportError.test.ts, test/js/bun/test/stack.test.ts, error-name-preservation.test.ts, circular-error-stack*.test.ts, fix-bindings-stack-trace.test.ts, prepare-stack-trace-crash.test.ts, test/js/node/v8/capture-stack-trace.test.js, test/cli/test/bun-test.test.ts (the existing annotation tests), test/js/bun/test/bun_test.test.ts.

Background

• error.stack is lazy in JSC: an ErrorInstance keeps the captured JSC::StackFrames until .stack is first read or written, formats them into the string, and drops them. Bun's native error printer (remap_zig_exception + print_error_instance_body in VirtualMachine.rs, behind Bun.inspect, console.*, uncaught exceptions and the test reporters) therefore has two sources of frames: the structured ones while they exist, otherwise the string, which fromErrorInstance parses back into ZigStackFrames with V8StackTraceIterator.
• ZigStackFrame / ZigStackFramePosition are the #[repr(C)] structs shared between the C++ that fills them and the Rust that prints them. A position is three i32s: zero-based line, zero-based column and a byte offset, each -1 when unknown; ZigStackFramePosition::INVALID is all three at -1 and is_invalid() compares the whole struct. bun_core::Ordinal and WTF::OrdinalNumber are the same representation of "zero-based index, -1 means none" on the two sides.
• V8-format stack strings print a frame's location as file, file:line or file:line:column. Native functions print as name (native), and frames with neither a name nor a file print as the bare word unknown; both are common in Bun's own stacks.

Outputs before and after

Release 1.4.0 / main, the repro from the test file:

error: boom
      at first (native:1:1)
      at second (node:child_process:1:1)
      at unknown:1:1
      at third (/fake/lib.js:3:4)
      at fourth (/fake/lib.js:7:1)

With this change:

error: boom
      at first (native)
      at second (node:child_process)
      at unknown
      at third (/fake/lib.js:3:4)
      at fourth (/fake/lib.js:7)

Real errors after .stack was read, with this change (at <parse> for JSON.parse previously printed as at <parse> (1:1); its presence at all is a separate, already tracked issue):

SyntaxError: JSON Parse error: Expected '}'
      at <parse>
      at parse (unknown)

error: from eval
      at file:///tmp/caret/main.js:1:16
      at eval (unknown)

Annotations on release for the two spawned test cases:

::error file=../workspace/bun/native,line=1,col=1,title=error: boom::...
::error file=lib.js,line=7,col=1,title=error: boom::...

With this change:

::error title=error: boom::...
::error file=lib.js,line=7,title=error: boom::...

[coderabbitai]

Warning

Review limit reached

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

Next review available in: 47 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: cb9d0fec-6722-4627-9324-f03f180df9c0

📥 Commits

Reviewing files that changed from the base of the PR and between e697804 and df7e291.

📒 Files selected for processing (4)

• src/jsc/VirtualMachine.rs
• src/jsc/bindings/ZigException.cpp
• test/js/bun/util/inspect-error.test.js
• test/regression/issue/23022-stack-trace-iterator.test.ts

---

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

[robobun]

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

❌ @robobun, your commit df7e291 has some failures in Build #95316 (All Failures)

---

🧪   To try this PR locally:

bunx bun-pr 38328

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

bun-38328 --bun

[robobun]

Status: fix pushed, waiting on CI.

Reproduced on the release binary (1.4.0) and on main's src/ under bun bd with the new tests in test/js/bun/util/inspect-error.test.js (describe("printing the frames parsed back out of error.stack without a position")) and the tightened assertion in test/regression/issue/23022-stack-trace-iterator.test.ts: frames parsed back out of error.stack print as at foo (native:1:1) / at unknown:1:1 / at bar (x.js:7:1), the caret and GitHub annotation follow the invented position. All six fail without the src/ changes of this PR and pass with them.

[claude]

I reviewed this PR and didn't find any bugs. The sentinel change is small and the consumers I traced already branch on per-ordinal validity, but because it shifts ZigStackFramePosition's "unknown" convention at the C++/Rust boundary and the description names blast radius beyond parsed frames (wasm frames with a named source, the HMR-runtime case), a human look at those output changes would still be worthwhile.

What was reviewed:

• bun_sourcemap::LineColumnOffset::{lines,columns} and ZigStackFramePosition::{line,column} are both bun_core::Ordinal, so the direct copy in remap_zig_exception type-checks and preserves -1.
• After the new !mapping.original.lines.is_valid() early break, the write-back at frames[top].position re-stores the same INVALID ordinals and code is empty, so get_lines_in_text and collect_source_lines see the same no-preview path as any other empty-source case.
• print_stack_trace's new !file.is_empty() arm still routes through source_url_formatter, which already handles file-only / file:line / file:line:col; the has_name && position invalid && file empty branch below is unchanged.
• The GitHub annotation change only alters output for frames whose line is invalid (drops the location) or whose column alone is invalid (drops col=); the fully-positioned path is byte-identical.

Extended reasoning...

Overview

The PR stops Bun's error printer from inventing :1:1 for stack frames that were parsed back out of error.stack without a position. It changes V8StackTraceIterator::StackFrame's default line/column from zero-based 0 to beforeFirst() (-1) and sets byte_position = -1 when copying into ZigStackFrame, so an unpositioned parsed frame equals ZigStackFramePosition::INVALID. On the Rust side it removes the .zero_based().max(0) clamp in remap_zig_exception (direct Ordinal copy instead), skips fetching a file for the source preview when there is no line, makes print_stack_trace print at name (file) when a frame has a file but no position, and makes print_github_annotation gate the location on line.is_valid() and emit col= only when the column is valid. Tests cover all five frame shapes through Bun.inspect, the source-preview/caret selection, and two spawned processes checking the uncaught printout and ::error annotation under GITHUB_ACTIONS=true.

Security risks

None. This is presentation-only (error message formatting and CI annotation strings). No parsing of untrusted input was added; the existing V8StackTraceIterator::parseFrame body is unchanged. No allocation, FFI ownership, or lifetime changes.

Level of scrutiny

Medium. The runtime effect is cosmetic, but the mechanism is a sentinel-value convention at the C++/Rust FFI boundary (ZigStackFramePosition). I verified the types line up (bun_sourcemap::Ordinal re-exports bun_core::Ordinal, matching ZigStackFramePosition::line/column), that is_invalid() compares all three fields so byte_position = -1 is required, and that the downstream write-back and get_lines_in_text path after the new early break degrade to the existing empty-source behaviour rather than doing anything new with a -1 line.

Other factors

The PR description explicitly enumerates blast radius beyond parsed-string frames — wasm frames with a named source and the dev server's all-HMR-frames case now print at name (file) instead of at name — and lists ten related test files that were re-run. That is the right diligence, but it is also a user-visible output change outside the direct bug that a maintainer should sign off on. The change is not a mechanical/config tweak, so per the approval guidelines I'm deferring rather than auto-approving.

[robobun]

Thanks. On the two output changes outside parsed frames, I re-checked and narrowed them in the description:

• wasm frames turn out to be unaffected: Zig::sourceURL returns [wasm code] for them, which populateStackFrameMetadata discards, so structured wasm frames never have a file and still go through the unchanged at name branch.
• The only other producer of a frame with a file but no position is the dev server's browser error report, and only when every frame is inside the HMR runtime (otherwise those frames are dropped before printing). That case goes from at fn to at fn (Bun HMR Runtime), matching what it already printed for the same frames when they have no name.

The description's blast radius bullet now says exactly that.