Skip to content

fix(jsc): print AggregateError message and stack before its children #36580

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
TheRodzz wants to merge 1 commit into from
+37 −36
Closed

fix(jsc): print AggregateError message and stack before its children #36580

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 37 additions & 36 deletions src/jsc/VirtualMachine.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4754,6 +4754,42 @@ impl VirtualMachine {
// tail instead of via a drop guard (the body has no early-`?` returns
// once the AggregateError branch is taken).
let global_ref = self.global();
let exception_list_ptr = exception_list
.as_deref_mut()
.map(|l| l as *mut ExceptionList)
.unwrap_or(core::ptr::null_mut());

// Note: reborrow so the add-to-error-list tail can still see it after
// `print_error_from_maybe_private_data`.
let mut exception_list = exception_list;
let was_internal = self.print_error_from_maybe_private_data(
value,
exception_list.as_deref_mut(),
formatter,
writer,
allow_ansi_color,
allow_side_effects,
);

if was_internal {
if let Some(exception_) = exception {
let mut holder = crate::zig_exception::Holder::init();
// Note: `holder.deinit(self)` runs at the tail (for borrowck)
// — semantics unchanged because
// `need_to_clear_parser_arena_on_deinit` is false here.
let zig_exception: &mut ZigException = holder.zig_exception();
exception_.get_stack_trace(global_ref, &mut zig_exception.stack);
if zig_exception.stack.frames_len > 0 {
let _ = Self::print_stack_trace(writer, &zig_exception.stack, allow_ansi_color);
}
if let Some(list) = exception_list {
let top_level_dir = self.top_level_dir();
let _ =
zig_exception.add_to_error_list(list, top_level_dir, Some(&self.origin));
}
holder.deinit(self);
}
}
Comment on lines +4757 to +4792

@coderabbitai coderabbitai Bot Aug 5, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add regression coverage for AggregateError output order.

Add a rendering test with a custom parent name and message plus multiple child errors. Assert that the parent name, message, and stack trace appear before the child errors. This protects the behavior fixed by issue #21528.

Also applies to: 4843-4851

🤖 Prompt for 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.

In `@src/jsc/VirtualMachine.rs` around lines 4757 - 4792, Add regression coverage
for AggregateError rendering near the existing error-output tests, using a
custom parent name and message with multiple child errors. Assert the rendered
output places the parent name, message, and stack trace before all child error
text, covering the behavior in the internal-error handling around
print_error_from_maybe_private_data and add_to_error_list.


if value.is_aggregate_error(global_ref) {
// Note: `JSValue::for_each` takes a C-ABI fn
Expand Down Expand Up @@ -4804,9 +4840,7 @@ impl VirtualMachine {
let mut ctx = AggCtx {
formatter: std::ptr::from_mut(formatter),
writer: std::ptr::from_mut(writer),
exception_list: exception_list
.map(std::ptr::from_mut::<ExceptionList>)
.unwrap_or(core::ptr::null_mut()),
exception_list: exception_list_ptr,
allow_ansi_color,
allow_side_effects,
};
Expand All @@ -4815,39 +4849,6 @@ impl VirtualMachine {
// which case the error is swallowed.
let errors = value.get_errors_property(global_ref);
let _ = errors.for_each(global_ref, (&raw mut ctx).cast(), agg_iter);
return;
}

// Note: reborrow so the add-to-error-list tail can still see it after
// `print_error_from_maybe_private_data`.
let mut exception_list = exception_list;
let was_internal = self.print_error_from_maybe_private_data(
value,
exception_list.as_deref_mut(),
formatter,
writer,
allow_ansi_color,
allow_side_effects,
);

if was_internal {
if let Some(exception_) = exception {
let mut holder = crate::zig_exception::Holder::init();
// Note: `holder.deinit(self)` runs at the tail (for borrowck)
// — semantics unchanged because
// `need_to_clear_parser_arena_on_deinit` is false here.
let zig_exception: &mut ZigException = holder.zig_exception();
exception_.get_stack_trace(global_ref, &mut zig_exception.stack);
if zig_exception.stack.frames_len > 0 {
let _ = Self::print_stack_trace(writer, &zig_exception.stack, allow_ansi_color);
}
if let Some(list) = exception_list {
let top_level_dir = self.top_level_dir();
let _ =
zig_exception.add_to_error_list(list, top_level_dir, Some(&self.origin));
}
holder.deinit(self);
}
}
}

Expand Down