error printer: print the AggregateError header, label [cause]/[errors] blocks, and guard the .errors walk

[robobun]

Consolidates the open AggregateError printing PRs into one change: the header/ordering fix (#35174, #36580 by @TheRodzz, which independently found the same early return), and the .errors walk hardening (#35825, which superseded #31988, #34892 and #35820). Those are closed in favor of this PR.

Problem

• console.error(agg), Bun.inspect(agg), an uncaught throw agg and an unhandled rejection of an AggregateError printed only the member errors. The aggregate's own AggregateError: <message> line, its stack and its cause were dropped, so a Promise.any() failure or a multi-error module build showed a list of unrelated-looking errors (print AggregateError more distinctly #21528, Unsupported proper logging of AggregateError, Error.cause, modified/accessed Error.stack #1352).
• Cause: VirtualMachine::print_errorlike_object special-cased is_aggregate_error, iterated .errors with for_each and returned before the normal printing path ran (src/jsc/VirtualMachine.rs, the removed block at the top of that function).
• The same walk had no cycle guard, no stack check and did not check what .errors currently holds, so each of these killed the process on every sink (console.log, Bun.inspect, uncaught throw, unhandled rejection): .errors containing the aggregate itself (SIGSEGV), two aggregates containing each other (SIGSEGV), a 3000-deep .errors chain (SIGSEGV), delete agg.errors (null JSCell call, panic: Segmentation fault at address 0x5 in release), .errors redefined as an accessor (ASSERTION FAILED: isSymbol() in debug, bogus TypeError in release).
• delete agg.errors happens without user code: when a second test file imports a module that failed to build, JSC's module loader replays the cached failure via JSModuleLoader::duplicateError, which creates an AggregateError-typed instance with no errors property, so bun test crashed at the second file (bun testrunner crash #36963, reproduces on 1.4.0).
• A deep cause chain crashed the uncaught-exception and unhandled-rejection reporters too: print_error_instance_js has a stack check, but the formatter built for those two sinks never initialized it (Formatter::new leaves stack_check in its always-passes state).
• Appended cause blocks were printed with no label, so a chain read as separate errors (improves Bun prints multi-level Error.cause chain as separate errors instead of preserving Node-style nested stack trace #32343).
• Two stack-string problems surfaced by the Unsupported proper logging of AggregateError, Error.cause, modified/accessed Error.stack #1352 repro: V8StackTraceIterator::parseFrame stopped at the first at /path:line:col frame (no function name, no parentheses), which Bun itself emits, so a reassigned err.stack was truncated; and remap_zig_exception re-applied the source map to frames that the .stack parse had already marked remapped, shifting their line numbers (Incorrect line numbers in error stack for TypeScript code #15859).

Fix

• print_errorlike_object no longer special-cases AggregateError. print_error_instance_body appends the .errors members to the same errors_to_append list the cause chain already uses, so the aggregate prints its own source preview, header and stack first, and an AggregateError reached through a cause also shows its members.
• .errors is read with getDirect and only walked when it is still an array (delete, accessor and reassigned cases print just the header), element reads that throw stop the walk, at most 100 members are appended and the rest are reported as ... N more errors.
• Each appended block is preceded by a dim [cause]: / [errors]: / [<property>]: label. The error being printed is registered in the formatter's visited set while its members print, so any chain leading back to it prints [Circular] once instead of printing the error again. Only Error-valued members are tracked this way: a plain-object member (what Promise.any produces when the inputs reject with non-errors) is handed to the regular formatter, which tracks it in the same set itself, so pre-registering it would have printed [Circular] in place of the object. The loop stops as soon as the formatter reports failure (stack check tripped or writer failed).
• The exception list that Bun.serve's development error page collects is now passed on to the nested errors as well (print_error_instance_js used to hand it to remap_zig_exception and give the body None), so the page lists the thrown error, then its cause and members, in print order; on main it listed only the members of a thrown AggregateError and nothing for a cause chain. BuildMessage members of a multi-error module reach the page's build log through the same list, as they did on main.
• Both uncaught sinks now go through VirtualMachine::print_exception, which is the one place that builds their formatter and seats StackCheck::init(), so the existing depth guard is live for them. Nested members are printed through print_errorlike_object, which routes every level through that guard.
• The guard's headroom on non-Windows goes from the 128 KB default to 384 KB. With the default, a debug+ASAN build still overflowed in about one run in four: gdb shows the SIGSEGV in bmalloc's deallocation-log flush about 130 KB below the last passing check (one print cycle runs the transpiler for the source preview). After the change, 23 runs under gdb with ASLR off and varied environment sizes plus 84 normal runs across all six sink/shape combinations had no crash. Windows already reserved more than this and is unchanged.
• parseFrame accepts frames with no parentheses at all (empty function name, optional async prefix); a frame with exactly one parenthesis, or ) before (, still stops parsing as before. remap_zig_exception skips frames already marked remapped.
• Why this is the right shape: it matches what util.inspect already does for these errors in Bun and Node (header first, [cause] and [errors] labeled, [Circular] on cycles) while keeping the native printer's layout, and it reuses the guards the cause chain already had instead of adding a second set to a separate walk.
• Verified with test/js/bun/util/inspect-error.test.js (header/labels on all four sinks and via a cause chain, exact uncaught layout snapshot for the print AggregateError more distinctly #21528 program, 9 cycle/tampering shapes x {console.error, uncaught throw} (including plain-object members, with and without a reference back to the aggregate), member cap with 101 and 103 members (... 1 more error / ... 3 more errors), deep .errors and cause chains on console/throw/reject that assert fewer levels than the chain has were printed (3000 levels under debug/ASAN, 50000 on release builds: a Windows release build printed all 3000, and main's release build on Linux segfaults after about 1450 levels of an uncaught cause chain), the bun testrunner crash #36963 two-test-file repro, reassigned V8-format stack ending in a frame with an unbalanced parenthesis, and a source-mapped TypeScript fixture whose printed frames must equal error.stack after it was read, which is the Incorrect line numbers in error stack for TypeScript code #15859 case). The existing development-error-page test in test/js/bun/http/serve.test.ts gained a module with 4 build errors (one AggregateError exception, 4 build-log entries) and a thrown AggregateError with a cause (exceptions listed as aggregate, cause, member, member); it fails on main too. 37 of the 46 tests in the inspect-error file plus test/regression/issue/jsx-template-string-crash.test.ts fail on main's src/ (crash or missing header), all pass with this branch; the two build-error snapshots in the jsx test now start with the AggregateError: 2 errors building ... header.
• Also run: bun-inspect.test.ts, inspect.test.js, reportError.test.ts, console-log.test.ts, console-recursive.test.ts (pass); inspect-error-leak.test.js RSS delta is the same with and without the change (349-357 MB vs 350 MB under ASAN; the test itself only times out in this container); cargo clippy on bun_jsc/bun_runtime and cargo fmt --check are clean.

Example, console.error(new AggregateError([new Error("Error 1")], "Aggregate error message.", { cause: new Error("Cause") })):

AggregateError: Aggregate error message.
      at ...

[cause]:
error: Cause
      at ...

[errors]:
error: Error 1
      at ...

Background

• Native error printer: the Rust code in src/jsc/VirtualMachine.rs that renders an error as source preview + name: message + own properties + stack. It is used by console.* and Bun.inspect (through ConsoleObject's formatter), and by the uncaught-exception / unhandled-rejection reporters (through jsc_hooks::print_exception). util.inspect is a separate JS implementation and was already correct.
• errors_to_append: the list of error objects print_error_instance_body prints as separate blocks after the main one. Before this PR it held error-valued own properties and the non-enumerable cause; .errors is also non-enumerable (DontEnum), which is why it needs explicit handling.
• formatter.map: the console formatter's visited set, keyed by JS object, used to print [Circular]. Entries are added before recursing into an object and removed afterwards, so it tracks the current path, not everything printed.
• StackCheck: compares the current stack pointer against the thread's stack bound; is_safe_to_recurse_with_extra(n) requires a platform threshold (128 KB on Linux/macOS, 256 KB on Windows) plus n bytes to remain. Formatter::new deliberately leaves it uninitialized (always passes); StackCheck::init() arms it. The reserve has to cover everything one print cycle does before the next check, which here includes a transpiler call for the source preview.
• getDirect: reads an own data property slot without running getters or walking the prototype chain; it returns the empty value when the property is gone and the GetterSetter cell when it was redefined as an accessor, so the array check rejects both.
• JSModuleLoader::duplicateError: when a module's fetch/parse fails, JSC caches the error and, for later importers, throws a fresh copy that keeps the error type but not the errors property.

Superseded description (before the hardening was folded in) and the "missing getError frame" note

The first version of this PR only removed the early return, added the labels and fixed the two stack-string problems; #35825's cycle/depth/tampering guards and tests were folded in afterwards, along with a test for #36963, so that one change can replace #35174, #35825, #36580, #31988, #34892 and #35820.

The #1352 repro's function getError(msg) { return new Error(msg); } frame is absent from err.stack itself, not just from the printed output (wrapping the return in try { } finally { } makes it appear in both), so it is a stack capture matter and this PR does not change it.

Fixes #21528
Fixes #1352
Fixes #36963
Fixes #15859

---

[review] gate passed · iteration 0 · 4 files touched

fails on main (without fix)

ASAN without fix: 11 FAILED
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/js/bun/util/inspect-error.test.js test/regression/issue/jsx-template-string-crash.test.ts
bun test v1.4.0 (1f67f056e)

test/regression/issue/jsx-template-string-crash.test.ts:
13 |     stderr: "pipe",
14 |     stdout: "pipe",
15 |   });
16 | 
17 |   expect(exitCode).toBe(1);
18 |   expect(normalizeBunSnapshot(stderr.toString().replace(/(Bun v.*)$/gm, ""))).toMatchInlineSnapshot(`
                                                                                   ^
error: expect(received).toMatchInlineSnapshot(expected)

  
- "AggregateError: 2 errors building "<cwd>/[eval]"
- 
- [errors]:
- 
- 1 | export function x(){return<div a=``/>}
+ "1 | export function x(){return<div a=``/>}
                                       ^
  error: Expected "{" but found "`"
      at <cwd>/[eval]:1:34
- 
- [errors]:
  
  1 | export function x(){return<div a=``/>}
                                        ^
  error: Unterminated string literal
      at <cwd>/[eval]:1:35"
  

- Expected  - 7
+ Received  + 1

      at <anonymous> (/workspace/bun/test/regr
... (truncated)

release without fix: all passed
bun test v1.4.0-canary.1 (7cc8524af)

test/regression/issue/jsx-template-string-crash.test.ts:
(pass) JSX lexer should not crash with slice bounds issues [28.27ms]
(pass) #30959 JSX attribute with invalid '(' value parses cleanly in debug builds [19.75ms]

test/js/bun/util/inspect-error.test.js:
(pass) error.cause [0.37ms]
(pass) Error [0.18ms]
(pass) BuildMessage [2.48ms]
(pass) Error inside minified file (no color)  [4.44ms]
(pass) Error inside minified file (color)  [2.19ms]
(pass) Inserted originalLine and originalColumn do not appear in node:util.inspect [20.22ms]
(pass) observable properties > sourceURL is observable [1.53ms]
(pass) observable properties > line is observable [0.13ms]
(pass) observable properties > column is observable [0.08ms]
(pass) error.stack throwing an error doesn't lead to a crash [0.14ms]
(pass) #1352 native error printer > AggregateError reached via a cause chain prints its members [11.07ms]
(pass) #1352 native error printer > AggregateError via console.error prints header, [cause] and each [errors] member [23.84ms]
(pass) #1352 native error printer > error.cause is labeled with [cause]: [13.41ms]
(pass) #1352 native error printer > Ag
... (truncated)

passes on PR (with fix)

ASAN with fix: all passed
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/js/bun/util/inspect-error.test.js test/regression/issue/jsx-template-string-crash.test.ts
bun test v1.4.0 (1f67f056e)

test/regression/issue/jsx-template-string-crash.test.ts:
(pass) JSX lexer should not crash with slice bounds issues [346.97ms]
(pass) #30959 JSX attribute with invalid '(' value parses cleanly in debug builds [459.39ms]

test/js/bun/util/inspect-error.test.js:
(pass) error.cause [6.77ms]
(pass) Error [4.44ms]
(pass) BuildMessage [13.43ms]
(pass) Error inside minified file (no color)  [572.89ms]
(pass) Error inside minified file (color)  [68.87ms]
(pass) Inserted originalLine and originalColumn do not appear in node:util.inspect [388.71ms]
(pass) observable properties > sourceURL is observable [6.59ms]
(pass) observable properties > line is observable [4.66ms]
(pass) observable properties > column is observable [2.25ms]
(pass) error.stack throwing an error doesn't lead to a crash [4.51ms]
(pass) #1352 native error printer > AggregateError via uncaught throw prints header, [cause] and each [errors] member [289.45ms]
... (truncated)

release with fix: all passed
$ bun scripts/build.ts --profile=release
[configured] bun-profile → bun (stripped) in 1664ms (unchanged)
ninja: Entering directory `/workspace/bun/build/release'
[1/8] gen cpp.rs (cppbind)
[2/8] gen generated_host_exports.rs
generated_host_exports.rs: 94 exports (host=3, lazy=10, generic=81, rust=0); 239 extern-C blocks audited
[2/8] cargo bun_bin → libbun_rust.a (--target x86_64-unknown-linux-gnu)

  nightly-2026-07-20-x86_64-unknown-linux-gnu unchanged - rustc 1.99.0-nightly (9f36de775 2026-07-19)

^[[1m^[[92m   Compiling^[[0m bun_core v0.0.0 (/workspace/bun/src/bun_core)
^[[1m^[[92m   Compiling^[[0m bun_errno v0.0.0 (/workspace/bun/src/errno)
^[[1m^[[92m   Compiling^[[0m bun_ptr v0.0.0 (/workspace/bun/src/ptr)
^[[1m^[[92m   Compiling^[[0m bun_boringssl_sys v0.0.0 (/workspace/bun/src/boringssl_sys)
^[[1m^[[92m   Compiling^[[0m bun_safety v0.0.0 (/workspace/bun/src/safety)
^[[1m^[[92m   Compiling^[[0m bun_zlib_sys v0.0.0 (/workspace/bun/src/zlib_sys)
^[[1m^[[92m   Compiling^[[0m bun_cares_sys v0.0.0 (/workspace/bun/src/cares_sys)
^[[1m^[[92m   Compiling^[[0m bun_zstd v0.0.0 (/workspace/bun/src/zstd)
^[[1m^[[92m   Compiling^[[0m bun_picohttp v0.0.0 (/workspace/bun/src/picohttp)
^[[1m^[[92m 
... (truncated)

diff hotspot

src/jsc/VirtualMachine.rs                          | 124 ++++++---------
 src/jsc/bindings/ZigException.cpp                  |  19 ++-
 test/js/bun/util/inspect-error.test.js             | 167 ++++++++++++++++-----
 .../issue/jsx-template-string-crash.test.ts        |  16 +-
 4 files changed, 206 insertions(+), 120 deletions(-)

gate history · 2 passed · 0 rejected · iteration 0

evidence per changed file

file                                                     reads  edits  tests
src/jsc/VirtualMachine.rs                                   12     17      0
src/jsc/bindings/ZigException.cpp                            4      4      0
test/js/bun/util/inspect-error.test.js                       6      8      0
test/regression/issue/jsx-template-string-crash.test.ts      0      0      0

[coderabbitai]

Warning

Review limit reached

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

Next review available in: 5 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: f064ac84-3167-49fe-b2c0-0f2e231a43c9

📥 Commits

Reviewing files that changed from the base of the PR and between 7c9b285 and d77ccd9.

📒 Files selected for processing (5)

• src/jsc/VirtualMachine.rs
• src/jsc/bindings/ZigException.cpp
• src/runtime/jsc_hooks.rs
• test/js/bun/http/serve.test.ts
• test/js/bun/util/inspect-error.test.js

Walkthrough

Changes

Error formatting

Layer / File(s)	Summary
Unified error entry and stack handling src/jsc/VirtualMachine.rs, src/runtime/jsc_hooks.rs	print_exception now accepts a value and optional exception metadata. Runtime hooks use this unified path for wrapped and ordinary values.
Nested error traversal and output src/jsc/VirtualMachine.rs	Nested causes and AggregateError members retain labels. Circular references, formatter failures, and omitted members receive explicit handling.
Location-only stack frame parsing src/jsc/bindings/ZigException.cpp	Stack parsing accepts frames without parentheses and recognizes asynchronous location-only frames.
Error formatting regression coverage test/js/bun/util/inspect-error.test.js, test/regression/issue/jsx-template-string-crash.test.ts	Tests cover AggregateError, causes, circular values, deep nesting, stack changes, repeated build failures, and updated JSX output.

Suggested reviewers: jarred-sumner, cirospaciari

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The changes address the coding objectives for AggregateError and cause output [#1352, #21528] and stack preservation and remapping [#15859].
Out of Scope Changes check	✅ Passed	The code and tests remain focused on native error formatting, stack parsing, source-map remapping, and the linked issue regressions.
Title check	✅ Passed	The title clearly summarizes the primary AggregateError printing and safety changes.
Description check	✅ Passed	The description explains the problem, implementation, affected behavior, verification steps, and linked issues in sufficient detail.

---

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

[github-actions]

Found 6 issues this PR may fix:

1. print AggregateError more distinctly #21528 - AggregateError header/message not printed distinctly; this PR removes the short-circuit and adds [errors]: labels
2. Bun prints multi-level Error.cause chain as separate errors instead of preserving Node-style nested stack trace #32343 - Multi-level .cause chain printed as separate errors instead of nested; this PR adds [cause]: labels to preserve the chain
3. Uncaught exception reporting ignores custom Error.stack value #32390 - Uncaught exception reporting ignores custom Error.stack value; this PR fixes parseFrame for parenthesis-less frames
4. Rethrowing inside process.on('uncaughtException') loses original Error stack #30504 - Rethrowing inside uncaughtException handler loses original stack; this PR fixes frame parsing so reassigned .stack strings are preserved
5. Incorrect line numbers in error stack for TypeScript code #15859 - Incorrect line numbers in error stack for TypeScript code; this PR fixes double source-map remapping of already-remapped frames
6. Bun's stack trace contains repeated lines that makes it different from Node.js #22339 - Stack trace contains repeated/duplicate lines; this PR fixes parseFrame mishandling certain frame formats

If this is helpful, copy the block below into the PR description to auto-close these issues on merge.

Fixes #21528
Fixes #32343
Fixes #32390
Fixes #30504
Fixes #15859
Fixes #22339

🤖 Generated with Claude Code

[github-actions]

This PR may be a duplicate of:

1. fix(jsc): print AggregateError message and stack before its children #36580 - Also removes the early-exit in print_errorlike_object to print AggregateError's own header/stack before its children
2. Print the AggregateError header before iterating its members #35174 - Also moves .errors iteration after the header so the AggregateError prints its own message and stack first

🤖 Generated with Claude Code

[robobun]

^{Updated 5:44 AM PT - Aug 13th, 2026}

✅ @robobun, your commit d77ccd9230b8f1dc12262c52567e2cd1a984bb21 passed in Build #94292! 🎉

---

🧪   To try this PR locally:

bunx bun-pr 36602

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

bun-36602 --bun

[claude]

I didn't find new issues — all four earlier findings (normalizeError snapshot leak, async prefix on paren-less frames, exit-code assertion ordering, exception discipline on get_index/get_length, plus the 100-member cap) are addressed. Deferring because this changes the user-visible native error format ([cause]: / repeated [errors]: labels, AggregateError header on build errors) and reworks the core error-printer recursion path; a maintainer should sign off on the output shape and the overlap with #36580/#35174.

What was reviewed:

• errors_to_append label lifecycle — dupe_ref/static_ paired with deref() in UnprotectAll::drop, and protect/unprotect balanced on every push.
• Recursion switch from print_error_instance_js? to print_errorlike_object — cycle guard via formatter.map still applies; the stack-safety guard on print_error_instance_js remains in the chain.
• remap_zig_exception skip on frames[i].remapped — matches the existing frames[top] handling; the .stack-parse path is the only setter.
• V8StackTraceIterator paren-less branch — hasParens computed before line is mutated for async, so openingParentheses indices stay valid.

Extended reasoning...

Overview

Four coordinated fixes to the native error printer: (1) removes the ~70-line is_aggregate_error short-circuit in print_errorlike_object and re-implements .errors iteration inside print_error_instance_body so the aggregate's own header/stack print first; (2) threads a label bun_core::String through errors_to_append and emits dim [cause]: / [errors]: markers; (3) makes V8StackTraceIterator::parseFrame accept at /path:l:c and at async /path:l:c frames instead of terminating; (4) skips secondary source-map remapping in remap_zig_exception for frames already marked remapped. Tests: an 8-case subprocess suite in inspect-error.test.js, three regenerated inline snapshots (with a widened normalizeError filter for internal-builtin frames), and two updated build-error snapshots in jsx-template-string-crash.test.ts.

Security risks

None. No untrusted external input reaches the changed code — the printer operates on the user's own thrown/logged values. The .errors iteration is now capped at 100, and get_length/get_index failures unconditionally clear_exception() (matching the removed for_each semantics). No auth/crypto/network surface.

Level of scrutiny

High. print_errorlike_object / print_error_instance_body run on every uncaught throw, unhandled rejection, console.error(Error), and Bun.inspect(Error). The change is behavior-visible to every user (new labels, AggregateError headers now appear on build-error output), reworks a JSC-interop path with protect/unprotect and string-refcount pairing, and changes source-map remapping and stack-string parsing that other issues (#15859, #32390, #22339) depend on. It also supersedes/overlaps two open PRs. The output-format choice — one [errors]: label per member rather than one for the group, and [cause]: rather than Node's braced style — is a design call a maintainer should confirm.

Other factors

All four of my earlier inline findings are resolved in 7cc8524 and 1f67f05: normalizeError now filters (\d+:\d+) frames so the minified-file snapshots don't pin debug-bundled builtin coordinates; the paren-less parseFrame branch strips async and sets isAsync; run() returns exitCode and callers assert it after content; get_index/get_length errors now clear_exception() unconditionally; and .errors printing is capped at 100. The comment-cop threads are resolved. Test coverage is solid (variant matrix across console.error/Bun.inspect/throw/reject/Promise.any, cause-chain-to-aggregate, reassigned V8 stack), and the gate evidence shows all 11 new/updated tests fail on main and pass on both debug-ASAN and release. Given the breadth of user-visible output change and the core-path location, this warrants a human sign-off rather than auto-approval.

[robobun]

Overlaps with #36726 (opened for #15859 before I saw this one). Same frames[i].remapped skip and the same V8StackTraceIterator no-paren frame handling. Closing #36726 in favor of this PR.

If useful, #36726's branch has a dedicated regression test at test/regression/issue/15859.test.ts that covers the double-remap, the dropped anonymous frame, and that printed frames match error.stack end-to-end (all three fail on canary).

[robobun]

Data point for fix 4 (the frames[i].remapped skip in remap_zig_exception): the same double remap also shows up without a rethrow, through Bun.inspect / console.log on an error whose .stack has already been read. Reproduces on 1.4.0 and current main, with a file on disk or with -e:

class Foo { constructor() { throw new Error("x"); } }
function g() {
  return new Foo();
}
try { g(); } catch (e) {
  const stack = e.stack;                 // materializes .stack, JSC drops the frame vector
  const printed = Bun.inspect(e);        // re-parses the string, every frame arrives remapped=true
  console.log(stack.split("\n")[2]);
  console.log(printed.split("\n").find(l => l.includes("at g ")));
}

    at g ([eval]:3:10)
      at g ([eval]:1:29)

The g frame (already an original position) is fed through the source map a second time. The top frame is fine because of the existing already_remapped check, and a fresh error (stack not yet read) prints 3:10.

The tests in this PR do not appear to exercise that hunk directly: the reassigned .stack test uses /fake-*.js paths that have no source map, so the non-top loop is a no-op there with or without the change. Something like the snippet above (assert that the g line in Bun.inspect(e) matches the one in e.stack) would fail on main and pass with this branch, if you want direct coverage for it.

[coderabbitai]

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/jsc/bindings/ZigException.cpp`:
- Around line 299-301: Update delimiter validation in the frame-location parsing
logic around hasParens so the location-only path is selected only when both
openingParentheses and closingParentheses are absent. Treat exactly one
delimiter as malformed and preserve the existing failure return instead of
parsing it as a location; add regressions covering unmatched opening and
unmatched closing delimiters.

In `@src/jsc/VirtualMachine.rs`:
- Around line 6412-6416: Update the visited-map lookups in the relevant
VirtualMachine code, including the expression assigning registered_self, to
handle ArrayHashMap::get_or_put allocation errors through
bun_core::handle_oom(...) or unwrap_or_oom() instead of expect("unreachable").
Apply the same replacement to both occurrences and preserve the existing
found_existing behavior.

In `@test/js/bun/util/inspect-error.test.js`:
- Around line 260-275: Add a regression test alongside the existing
reassigned-stack test that creates a source-mapped file, materializes the error
stack, and compares a non-top frame’s location from Bun.inspect(e) with the
corresponding frame in e.stack. Assert both report the same original source
location, covering the frames[i].remapped guard in VirtualMachine and the `#15859`
double-remap scenario.
- Around line 478-494: Strengthen both uncaught-throw and unhandled-rejection
parameterized tests by asserting that the rendered deep error output is bounded,
not merely that the outer header exists. Add an upper-bound assertion on the
number of “error: level” headers or on stderr.length, while preserving the
existing header and exitCode assertions.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 269bf82f-c4d9-4bb3-96c9-6ce9368bef78

📥 Commits

Reviewing files that changed from the base of the PR and between f426a8e and 7c9b285.

📒 Files selected for processing (5)

• src/jsc/VirtualMachine.rs
• src/jsc/bindings/ZigException.cpp
• src/runtime/jsc_hooks.rs
• test/js/bun/util/inspect-error.test.js
• test/regression/issue/jsx-template-string-crash.test.ts

[robobun]

Status: this PR is the consolidated fix for the AggregateError printing cluster. Closed in its favor: #35174, #36580 (@TheRodzz, same root cause), #35825, #31988, #34892, #35820.

Reproduced on main (f426a8e17, debug build) before the consolidation: bun -e 'console.log(new AggregateError([new Error("a")], "outer"))' prints only error: a; ae.errors.push(ae); console.log(ae) exits 139; delete ae.errors trips a null JSCell call; a 3000-deep chain exits 139; the #36963 two-test-file bun test repro segfaults on the released 1.4.0. With this branch all of them print the AggregateError: ... header and exit normally.

Self-review of the consolidated diff turned up three problems, all fixed in the follow-up commits: non-Error .errors members (a DOMException, a plain object) were printed as [Circular] because the loop pre-registered them in the visited set (74323e7, only Error members are tracked now; verified a DOMException member, a plain object member and an object with a reference back to the aggregate), and the development error page lost the members of a thrown AggregateError and the build log entries of a module with several build errors because the body was handed None for the exception list (d77ccd9, covered by the dev-error-page test in serve.test.ts, which fails on main). The stack-check headroom was also raised after the 128 KB default overflowed about one run in four under ASAN (0 crashes in 84 runs afterwards).

Remaining CI failures on the last build were retried-green or on unrelated lanes (inspect-error-leak timing out in the parallel ASAN batch, a Windows terminal test, cron on alpine, solc on Windows aarch64); the RSS delta of the leak test is unchanged by this diff.

Related, left open on purpose: #35816 (its VirtualMachine.rs hunk conflicts with this PR, the Event parts are independent), #35532 (Promise.any's empty message).

[claude]

Beyond the inline finding: the two formatter.map.get_or_put(...).expect("unreachable") calls (also flagged by CodeRabbit) were examined — they mirror the pre-PR sibling at the same site and the map is bounded by the 100-member cap plus the stack-check depth guard, so the OOM path is not reachable in practice. Commit 50d5699 added the source-mapped TypeScript fixture that covers the frames[i].remapped guard I flagged earlier; that thread is resolved from my side.

Extended reasoning...

The bug-hunting pass surfaced one new regression (non-Error object members of .errors now print as [Circular] because the outer loop pre-registers them in formatter.map before formatter.format runs its own cycle check on the same value). That is posted inline and warrants a fix before merge, so I am not approving.

Two concerns raised by other bots were re-examined and ruled out this run: the .expect("unreachable") on get_or_put matches the existing convention at the same call site pre-PR and cannot realistically fail (the map holds at most ~100 entries per level, bounded by MAX_AGGREGATE_ERRORS_PRINTED and the stack check); and the deep-chain uncaught-throw tests are crash guards, so asserting only the header plus a clean exit is the intended invariant — a truncation-count assertion would couple the test to the platform-dependent stack-check depth.

Since my last review the author added the #15859 source-mapped-TypeScript test in 50d5699, which directly exercises the || frames[i].remapped clause (positions in stderr must equal positions in error.stack). My earlier coverage nit on that clause is addressed. The undrained stdout: "pipe" at the two inline Bun.spawn sites remains as previously noted.

[claude]

I reviewed the latest revision and didn't find any new bugs — the earlier findings (plain-object .errors members printing as [Circular], missing #15859 coverage, undrained stdout pipes, unbalanced-paren frames) are all addressed. Given the scope — reworked print_errorlike_object recursion, a changed print_exception signature, an empirically-tuned 256 KB stack headroom bump, new clear_exception() calls in the .errors walk, and a user-visible output-format change — a human look is still worthwhile.

What was reviewed:

• Cycle-map registration now gated on JSType::ErrorInstance, so plain-object members reach formatter.format untracked and print correctly; the back-reference case still short-circuits via the error_instance self-registration.
• V8StackTraceIterator::parseFrame — unbalanced-paren frames now return false before the location-only path; async prefix is stripped for paren-less frames.
• errors_omitted accounting when get_index throws mid-walk (len - appended), and UnprotectAll drop guard now also derefs the label String.

Extended reasoning...

Overview

The PR reworks VirtualMachine::print_errorlike_object / print_error_instance_body in src/jsc/VirtualMachine.rs (~130 lines net), removing the C-ABI for_each special case for AggregateError and folding .errors iteration into the existing errors_to_append list with a label per entry. It changes the signature of print_exception (now takes value: JSValue, exception: Option<&Exception>) and seats StackCheck::init() there so the uncaught/unhandled sinks get the same depth guard as console.*/Bun.inspect. It bumps the non-Windows stack headroom from 0 to 256 KB. In src/jsc/bindings/ZigException.cpp it teaches V8StackTraceIterator::parseFrame to accept at /path:l:c frames with no parentheses and to reject frames with exactly one unmatched paren. src/runtime/jsc_hooks.rs collapses both branches of print_exception to route through the VM helper. ~430 lines of new tests in inspect-error.test.js plus two updated snapshots in jsx-template-string-crash.test.ts.

Security risks

None identified. The change is confined to error-formatting/diagnostic output. The .errors read uses getDirect (no getter invocation), the array walk is capped at 100, and element reads that throw are caught and clear the exception rather than propagating. No auth, crypto, or filesystem paths are touched.

Level of scrutiny

High. This is the native uncaught-exception / unhandled-rejection reporter — a crash here is a crash on every user program that throws. The PR changes recursion structure, adds a magic-number stack-headroom constant derived from gdb measurements under debug+ASAN, introduces two clear_exception() calls (REVIEW.md flags clearException() generally, though the error-printer path is a defensible exception since it already sits inside error handling), and changes user-visible output format ([cause]:/[errors]: labels now appear in every AggregateError render, including build-error output). The consolidated fix supersedes six other PRs, so a maintainer should confirm this is the shape they want to land.

Other factors

• All prior inline findings from this reviewer and CodeRabbit are marked resolved; the current diff reflects those fixes (Error-only cycle-map gating, handle_oom on get_or_put, unbalanced-paren rejection, source-mapped #15859 test, bounded-output assertions on the deep-chain tests, 3-way pipe drains).
• The comment-cop bot has ~10 unresolved "paragraph-long comment" flags on the Rust file; those are style-level and don't affect correctness.
• The last CI status shown (build #93949 for 50d5699) reported inspect-error.test.js failing on Windows x64 and aarch64; a follow-up commit (74323e7) landed after that, so the current Windows status is unknown to me.
• Test coverage is thorough (9 tampering shapes × 2 sinks, deep-chain × 3 sinks with depth branched on isDebug || isASAN, exact-layout snapshot for #21528, #36963 two-file repro, #15859 source-mapped fixture). The candidate concern about the #36963 test hardcoding "4 errors" was ruled out — that count is a stable property of the fixed lib.ts fixture, not a fragile parser-recovery detail.

[claude]

I reviewed the latest revision (d77ccd9) and the bug-hunting pass found nothing new — both prior findings (the exception_list not reaching nested members, and the ... 1 more errors plural) are addressed and covered by tests. Given the scope — ~200 lines in the native error printer on the uncaught-exception path, a user-visible output format change, an empirically-tuned stack-headroom constant, and consolidation of six other PRs — a maintainer sign-off is still warranted.

What was reviewed:

• .errors walk hardening (getDirect + array check, cycle guard, 100-member cap, throwing-index break) and the ErrorInstance-only visited-map registration — plain-object members verified to reach formatter.format uncached.
• exception_list reborrow in print_error_instance_js so nested cause/errors reach the dev-error page; new serve.test.ts cases cover both shapes.
• V8StackTraceIterator::parseFrame paren-less branch — unbalanced/mis-ordered parentheses still stop parsing; the || frames[i].remapped clause now has a source-mapped fixture that fails without it.
• print_exception signature change has no other callers; UnprotectAll drop guard derefs the new label String alongside unprotect().

Extended reasoning...

Overview

The PR reworks how the native error printer handles AggregateError: it removes the early-return special case in print_errorlike_object, folds .errors members into the existing errors_to_append list (alongside cause and error-valued own properties), labels each appended block [cause]: / [errors]: / [<prop>]:, and hardens the walk (getDirect + is_array guard, cycle detection via the formatter's visited map, 100-member cap with a counted trailer, break on throwing index reads, formatter.failed short-circuit). It also seats StackCheck::init() on the uncaught-exception/unhandled-rejection formatter and raises the non-Windows headroom to 256 KB, teaches V8StackTraceIterator::parseFrame to accept paren-less at /path:l:c frames, and skips already-remapped frames in remap_zig_exception. Six files touched; ~430 lines of new tests across inspect-error.test.js, serve.test.ts, and two updated snapshots.

Security risks

None identified. The change reads user-controlled error properties but does so through getDirect (own data slot, no getter invocation) for .errors and clears exceptions on the get_index/get_length paths. No new allocation sizes are derived from untrusted input beyond the existing errors_to_append Vec, which is capped at ≤100+few entries per level and freed by the UnprotectAll drop guard.

Level of scrutiny

High. This is the native error printer on the uncaught-exception / unhandled-rejection path — a bug here either crashes the process while it's already reporting a failure or silently drops diagnostics. It touches JSValue protect/unprotect pairing, the formatter's cycle map, bun_core::String refcounts, and an empirically-derived stack-headroom constant. It also changes user-visible output format (the [cause]:/[errors]: labels and the AggregateError header now appearing on bun -e build failures), which is a UX/API-surface decision a maintainer should confirm. The PR consolidates six other open PRs, so merging it is also a triage decision.

Other factors

The PR has been through several review rounds; every prior finding I raised (test coverage for the remapped clause, undrained stdout pipes, the plain-object [Circular] regression, the exception_list regression for the dev error page, the plural trailer) was addressed with a targeted fix and a test that fails without it. Test coverage is thorough (all four sinks × 9 tampering shapes × 2 sinks, deep-chain bounds on debug/ASAN and release, the #36963 and #15859 repros, the dev-error-page shapes). The robobun evidence gate shows 34+ tests fail on main and pass here across both ASAN and release. No outstanding unresolved threads. Still, the combination of native memory-safety surface, output-format change, and multi-PR consolidation puts this outside what I'd auto-approve.