Skip to content

Print the AggregateError header before iterating its members - #35174

Closed
robobun wants to merge 4 commits into
mainfrom
farm/dcbf384a/aggregate-error-header
Closed

Print the AggregateError header before iterating its members#35174
robobun wants to merge 4 commits into
mainfrom
farm/dcbf384a/aggregate-error-header

Strengthen AggregateError test ordering assertions

88317f7
Select commit
Loading
Failed to load commit list.
Claude / Claude Code Review completed Jul 22, 2026 in 16m 30s

Code review found 2 potential issues

Found 3 candidates, confirmed 2. See review comments for details.

Details

Severity Count
🔴 Important 0
🟡 Nit 2
🟣 Pre-existing 0
Severity File:Line Issue
🟡 Nit src/jsc/VirtualMachine.rs:4776 AggregateError as .cause still omits its member errors
🟡 Nit src/jsc/VirtualMachine.rs:4812 Double blank line between BuildMessage members in AggregateError output

Annotations

Check warning on line 4776 in src/jsc/VirtualMachine.rs

See this annotation in the file changed.

@claude claude / Claude Code Review

AggregateError as .cause still omits its member errors

Related gap (pre-existing, not a regression): an `AggregateError` reached via `.cause` still omits its member errors. The `.errors` iteration lives only in `print_errorlike_object`, but the `.cause` recursion at ~line 6092 calls `print_error_instance_js` → `print_error_instance_body` directly, and `.errors` is `DontEnum` so the own-property loop skips it — so `throw new Error("outer", { cause: new AggregateError([new Error("inner")], "agg") })` prints `AggregateError: agg` but never `inner`. Wor

Check warning on line 4812 in src/jsc/VirtualMachine.rs

See this annotation in the file changed.

@claude claude / Claude Code Review

Double blank line between BuildMessage members in AggregateError output

The unconditional `writer.write_all(b"\n")` prepend gives one blank line between the header and the first member but *two* blank lines between consecutive `BuildMessage`/`ResolveMessage` members, because those already emit a trailing blank line of their own — the regenerated snapshot in `jsx-template-string-crash.test.ts` captures the double blank. Purely cosmetic (the pre-PR spacing between members was one blank line); could be tightened by tracking whether the previous member already ended in