diff --git a/changelog.d/9511-error-name-ownership.md b/changelog.d/9511-error-name-ownership.md new file mode 100644 index 0000000000..6395b5db28 --- /dev/null +++ b/changelog.d/9511-error-name-ownership.md @@ -0,0 +1,9 @@ +### Fixed + +- **Error subclasses no longer expose their default `name` as an own, + enumerable property.** `name` now remains on the appropriate Error-family + prototype until user code explicitly assigns it, matching Node across + `JSON.stringify`, `Object.getOwnPropertyNames`, `Object.keys`, `for…in`, + object spread, property descriptors, and `util.inspect`. Error construction + also preserves Node's observable own-key order: `stack` precedes an optional + `message`. diff --git a/crates/perry-codegen/src/codegen/method.rs b/crates/perry-codegen/src/codegen/method.rs index 94c3c7f0d2..696690f59f 100644 --- a/crates/perry-codegen/src/codegen/method.rs +++ b/crates/perry-codegen/src/codegen/method.rs @@ -978,17 +978,10 @@ pub(super) fn compile_method( .cloned() .map(|slot| ctx.block().load(DOUBLE, &slot)) .unwrap_or_else(|| undef_lit.clone()); - let kind_idx = ctx.strings.intern(&pname_owned); - let kind_handle_global = - format!("@{}", ctx.strings.entry(kind_idx).handle_global); let blk = ctx.block(); - let kind_box = blk.load(DOUBLE, &kind_handle_global); - let kind_bits = blk.bitcast_double_to_i64(&kind_box); - let kind_raw = - blk.and(I64, &kind_bits, crate::nanbox::POINTER_MASK_I64); blk.call_void( "js_error_subclass_default_init", - &[(DOUBLE, &this_box), (DOUBLE, &msg_box), (I64, &kind_raw)], + &[(DOUBLE, &this_box), (DOUBLE, &msg_box)], ); } ("".to_string(), 0) diff --git a/crates/perry-codegen/src/expr/this_super_call.rs b/crates/perry-codegen/src/expr/this_super_call.rs index 724cd2a0ef..852a9d6ada 100644 --- a/crates/perry-codegen/src/expr/this_super_call.rs +++ b/crates/perry-codegen/src/expr/this_super_call.rs @@ -601,8 +601,8 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { // family) — HIR captures `extends_expr` for any unknown Ident, // INCLUDING the built-ins, so we'd otherwise eat the more-correct // Error-init path below. The built-in arms handle their own - // semantics (Error sets this.message + this.name; streams allocate - // a registry handle). Anything else with an extends_expr is a + // semantics (Error installs own message/stack slots; streams + // allocate a registry handle). Anything else with an extends_expr is a // real runtime-value parent and routes through this dispatch. // The classic node:stream / Web-Streams names are only the // genuine built-in parents when HIR did NOT capture an @@ -1169,8 +1169,8 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { } // Built-in parent (Error, TypeError, RangeError, etc.) // — user classes extending them need `super(message)` to - // assign `this.message = args[0]` and `this.name = parent_name` - // so downstream `err.message` / `err.name` access works. + // install the own non-enumerable `message`/`stack` slots; + // `name` resolves from the Error-family prototype. // `instanceof Error` walking the extends chain is handled // elsewhere; this just makes `err.message` non-undefined. if matches!( @@ -1274,6 +1274,17 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { let this_slot = ctx.this_stack.last().cloned(); if let Some(this_slot) = this_slot { let blk = ctx.block(); + // #9410/#9440: capture the own non-enumerable + // `stack` before installing `message`, matching + // V8's observable own-key order. Its lazy getter + // still reads `name`/`message` after `super()`. + let this_for_stack = blk.load(DOUBLE, &this_slot); + blk.call_void( + "js_error_subclass_capture_stack", + &[(DOUBLE, &this_for_stack)], + ); + // Capture can collect, so derive the raw receiver + // from a fresh load for the remaining stores. let this_box = blk.load(DOUBLE, &this_slot); let this_bits = blk.bitcast_double_to_i64(&this_box); let this_handle = blk.and(I64, &this_bits, POINTER_MASK_I64); @@ -1295,58 +1306,27 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { &[(I64, &this_handle), (I64, &key_raw), (DOUBLE, msg_val)], ); } - // this.name = as default (can be - // overridden by the subclass constructor body). - let name_idx = ctx.strings.intern("name"); - let name_handle_global = - format!("@{}", ctx.strings.entry(name_idx).handle_global); - let name_val_idx = ctx.strings.intern(&parent_name); - let name_val_global = - format!("@{}", ctx.strings.entry(name_val_idx).handle_global); - let blk = ctx.block(); - let name_key_box = blk.load(DOUBLE, &name_handle_global); - let name_key_bits = blk.bitcast_double_to_i64(&name_key_box); - let name_key_raw = blk.and(I64, &name_key_bits, POINTER_MASK_I64); - let name_val_box = blk.load(DOUBLE, &name_val_global); - blk.call_void( - "js_object_set_field_by_name", - &[ - (I64, &this_handle), - (I64, &name_key_raw), - (DOUBLE, &name_val_box), - ], - ); + // `name` is inherited from the Error-family + // prototype. Do not stamp it here: untouched Error + // subclasses must have no own `name`; a later + // `this.name = ...` remains an ordinary enumerable + // own assignment (#9440). // #5127: `super(message, options)` must forward the // ES2022 `cause` option. The instance is a generic // object, so install a non-enumerable `cause` // property from args[1] when present. if let Some(opts_val) = lowered_args.get(1) { let blk = ctx.block(); + // The message store above can collect. Reload + // the rooted receiver before applying `cause`. + let this_box = blk.load(DOUBLE, &this_slot); + let this_bits = blk.bitcast_double_to_i64(&this_box); + let this_handle = blk.and(I64, &this_bits, POINTER_MASK_I64); blk.call_void( "js_error_apply_cause_to_object", &[(I64, &this_handle), (DOUBLE, opts_val)], ); } - // #9410: `stack`. `super(message)` into a built-in - // Error stamps `message`/`name`/`cause` onto the - // already-allocated plain instance and stops there, - // so `new (class extends Error {})("x").stack` was - // `undefined` while `new Error("x").stack` is a - // string. The frame is captured HERE, at the - // construction site; the `name: message` head is - // formatted on read, because a subclass - // constructor assigns `this.name` after `super()` - // returns and Node reports the assigned name. - let blk = ctx.block(); - // Reload `this` from its slot: the stamps above - // can collect, and a DOUBLE held across a - // collecting call is the bare-pointer hazard - // #8770 is about. - let this_for_stack = blk.load(DOUBLE, &this_slot); - blk.call_void( - "js_error_subclass_capture_stack", - &[(DOUBLE, &this_for_stack)], - ); } } bind_derived_this_after_super(ctx); @@ -1591,11 +1571,11 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { } restore_inline_constructor_scope(ctx, saved_scope); - } else if let Some(error_kind) = { + } else if let Some(_error_kind) = { // Issue #573: walk the chain from `effective_parent_class` // upward; if it terminates at an Error-like built-in, - // emit the same Error init the no-parent-class branch - // does (sets this.message + this.name). Without this, + // emit the same Error init the no-parent-class branch does + // (own non-enumerable `stack`/`message`; inherited `name`). Without this, // `class C extends Error {}; class D extends C { ctor(m){ // super(m); } }` reaches here with `effective_parent_class // = C` (no own ctor) and a parent of "Error" (not in @@ -1633,6 +1613,14 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { let this_slot = ctx.this_stack.last().cloned(); if let Some(this_slot) = this_slot { let blk = ctx.block(); + // The indirect all-implicit chain is still an Error + // construction site. Capture `stack` first for the same + // own-key order as the direct built-in arm above. + let this_for_stack = blk.load(DOUBLE, &this_slot); + blk.call_void( + "js_error_subclass_capture_stack", + &[(DOUBLE, &this_for_stack)], + ); let this_box = blk.load(DOUBLE, &this_slot); let this_bits = blk.bitcast_double_to_i64(&this_box); let this_handle = blk.and(I64, &this_bits, POINTER_MASK_I64); @@ -1650,25 +1638,6 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { &[(I64, &this_handle), (I64, &key_raw), (DOUBLE, msg_val)], ); } - let name_idx = ctx.strings.intern("name"); - let name_handle_global = - format!("@{}", ctx.strings.entry(name_idx).handle_global); - let name_val_idx = ctx.strings.intern(&error_kind); - let name_val_global = - format!("@{}", ctx.strings.entry(name_val_idx).handle_global); - let blk = ctx.block(); - let name_key_box = blk.load(DOUBLE, &name_handle_global); - let name_key_bits = blk.bitcast_double_to_i64(&name_key_box); - let name_key_raw = blk.and(I64, &name_key_bits, POINTER_MASK_I64); - let name_val_box = blk.load(DOUBLE, &name_val_global); - blk.call_void( - "js_object_set_field_by_name", - &[ - (I64, &this_handle), - (I64, &name_key_raw), - (DOUBLE, &name_val_box), - ], - ); } } else if let Some(ctor) = ctx .imported_class_ctors diff --git a/crates/perry-codegen/src/lower_call/new_error_init.rs b/crates/perry-codegen/src/lower_call/new_error_init.rs index 86fd0d208b..7f529c8e3a 100644 --- a/crates/perry-codegen/src/lower_call/new_error_init.rs +++ b/crates/perry-codegen/src/lower_call/new_error_init.rs @@ -12,10 +12,15 @@ use crate::expr::FnCtx; use crate::nanbox::POINTER_MASK_I64; use crate::types::{DOUBLE, I64}; -/// Stamp `message`, `name` and `stack` onto the freshly allocated instance of -/// an Error-family subclass, mirroring the `SuperCall` Error-like arm in +/// Stamp `stack` and (when supplied) `message` onto the freshly allocated +/// instance of an Error-family subclass, mirroring the `SuperCall` Error-like arm in /// `expr/this_super_call.rs`. /// +/// `name` deliberately stays on the terminating Error-family prototype. A +/// plain assignment such as `error.name = "Custom"` then creates the ordinary +/// enumerable own property required by `[[Set]]`, while an untouched instance +/// keeps `name` out of every own-key consumer (#9440). +/// /// Returns `true` when the class's `extends` chain does terminate at an Error /// family base and the init was emitted — the caller then skips its /// imported-ctor fallback. Returns `false` (emitting nothing) otherwise. @@ -52,9 +57,19 @@ pub(super) fn emit_default_error_init( break; } } - if let Some(kind) = error_kind { + if error_kind.is_some() { let this_slot_for_err = ctx.this_stack.last().cloned().unwrap_or_default(); let blk = ctx.block(); + let this_for_stack = blk.load(DOUBLE, &this_slot_for_err); + // V8 creates `stack` before `message`; preserve that observable own-key + // order (`["stack", "message"]`) while the stack head itself remains + // lazy and therefore sees the later message/name values. + blk.call_void( + "js_error_subclass_capture_stack", + &[(DOUBLE, &this_for_stack)], + ); + // The capture allocates, so reload the rooted receiver before deriving + // the raw pointer consumed by the message store. let this_box = blk.load(DOUBLE, &this_slot_for_err); let this_bits = blk.bitcast_double_to_i64(&this_box); let this_handle = blk.and(I64, &this_bits, POINTER_MASK_I64); @@ -72,37 +87,6 @@ pub(super) fn emit_default_error_init( &[(I64, &this_handle), (I64, &key_raw), (DOUBLE, msg_val)], ); } - let name_idx = ctx.strings.intern("name"); - let name_handle_global = format!("@{}", ctx.strings.entry(name_idx).handle_global); - let name_val_idx = ctx.strings.intern(&kind); - let name_val_global = format!("@{}", ctx.strings.entry(name_val_idx).handle_global); - let blk = ctx.block(); - let name_key_box = blk.load(DOUBLE, &name_handle_global); - let name_key_bits = blk.bitcast_double_to_i64(&name_key_box); - let name_key_raw = blk.and(I64, &name_key_bits, POINTER_MASK_I64); - let name_val_box = blk.load(DOUBLE, &name_val_global); - blk.call_void( - "js_object_set_field_by_name", - &[ - (I64, &this_handle), - (I64, &name_key_raw), - (DOUBLE, &name_val_box), - ], - ); - // #9410: `stack`. This arm stamps `message` and `name` onto an - // ordinary class instance; nothing ever filled `stack`, so - // `new MyError("x").stack` was `undefined` where the base - // `new Error("x").stack` is a string. The runtime installs a - // lazily-formatted own accessor and captures the FRAME here, - // at the construction site. - let blk = ctx.block(); - // Reload `this`: the `message`/`name` stamps above can - // collect, so the earlier `this_box` may be stale (#8770). - let this_for_stack = blk.load(DOUBLE, &this_slot_for_err); - blk.call_void( - "js_error_subclass_capture_stack", - &[(DOUBLE, &this_for_stack)], - ); return true; } false diff --git a/crates/perry-codegen/src/runtime_decls/objects.rs b/crates/perry-codegen/src/runtime_decls/objects.rs index e60394b4c0..03a72c3386 100644 --- a/crates/perry-codegen/src/runtime_decls/objects.rs +++ b/crates/perry-codegen/src/runtime_decls/objects.rs @@ -120,13 +120,10 @@ pub fn declare_phase_b_objects(module: &mut LlModule) { VOID, &[I64, I64, DOUBLE], ); - // #6469: spec default Error-init for the synthesized standalone ctor of a - // no-own-ctor `class X extends Error {}` (this, message, name-string ptr). - module.declare_function( - "js_error_subclass_default_init", - VOID, - &[DOUBLE, DOUBLE, I64], - ); + // #6469/#9440: spec default Error-init for the synthesized standalone ctor + // of a no-own-ctor `class X extends Error {}` (this, message). `name` + // remains inherited from the terminating Error-family prototype. + module.declare_function("js_error_subclass_default_init", VOID, &[DOUBLE, DOUBLE]); module.declare_function( "js_object_set_field_by_name_nonconfigurable", VOID, diff --git a/crates/perry-runtime/src/builtins/formatting.rs b/crates/perry-runtime/src/builtins/formatting.rs index adfb719f8a..f5e4381626 100644 --- a/crates/perry-runtime/src/builtins/formatting.rs +++ b/crates/perry-runtime/src/builtins/formatting.rs @@ -12,6 +12,7 @@ use super::*; mod array_buffer; mod boxed_primitives; mod collection_equality; +mod errors; pub(crate) use boxed_primitives::{ boxed_primitive_json_value, boxed_primitive_payload, boxed_primitive_to_string_tag, prune_dead_boxed_primitive_payload_owners, @@ -679,109 +680,6 @@ impl Drop for InspectCompactGuard { } } -unsafe fn string_header_to_string(ptr: *mut StringHeader, fallback: &str) -> String { - if ptr.is_null() { - return fallback.to_string(); - } - let len = (*ptr).byte_len as usize; - let data = (ptr as *const u8).add(std::mem::size_of::()); - let bytes = std::slice::from_raw_parts(data, len); - std::str::from_utf8(bytes).unwrap_or(fallback).to_string() -} - -unsafe fn format_error_headline(error_ptr: *const crate::error::ErrorHeader) -> String { - let name_str = string_header_to_string((*error_ptr).name, "Error"); - let message_str = string_header_to_string((*error_ptr).message, ""); - if message_str.is_empty() { - name_str - } else { - format!("{}: {}", name_str, message_str) - } -} - -unsafe fn format_error_stack_frame(error_ptr: *const crate::error::ErrorHeader) -> Option { - let stack = string_header_to_string((*error_ptr).stack, ""); - stack - .lines() - .skip(1) - .find(|line| !line.trim().is_empty()) - .map(str::to_string) -} - -unsafe fn format_error_array(arr_ptr: *const crate::array::ArrayHeader, depth: usize) -> String { - if arr_ptr.is_null() { - return "[]".to_string(); - } - let length = (*arr_ptr).length as usize; - if length == 0 { - return "[]".to_string(); - } - let data_ptr = - (arr_ptr as *const u8).add(std::mem::size_of::()) as *const f64; - let mut out = String::from("["); - for i in 0..length { - out.push('\n'); - out.push_str(" "); - out.push_str(&format_jsvalue_for_json(*data_ptr.add(i), depth + 1)); - } - out.push('\n'); - out.push_str(" ]"); - out -} - -unsafe fn format_error_value(error_ptr: *const crate::error::ErrorHeader, depth: usize) -> String { - let headline = format_error_headline(error_ptr); - let mut entries: Vec<(String, String)> = - crate::node_submodules::error_user_props(error_ptr as usize) - .into_iter() - .filter(|(key, _)| key != "cause" && key != "errors") - .map(|(key, value)| (key, format_jsvalue_for_json(value, depth + 1))) - .collect(); - - let cause = (*error_ptr).cause; - if !crate::value::JSValue::from_bits(cause.to_bits()).is_undefined() { - entries.push(( - "[cause]".to_string(), - format_jsvalue_for_json(cause, depth + 1), - )); - } - - if !(*error_ptr).errors.is_null() { - entries.push(( - "[errors]".to_string(), - format_error_array((*error_ptr).errors, depth + 1), - )); - } - - if entries.is_empty() { - return headline; - } - - let mut out = headline; - if let Some(frame) = format_error_stack_frame(error_ptr) { - out.push('\n'); - out.push_str(&frame); - out.push_str(" {"); - } else { - out.push_str("\n{"); - } - - let last = entries.len().saturating_sub(1); - for (idx, (label, value)) in entries.into_iter().enumerate() { - out.push('\n'); - out.push_str(" "); - out.push_str(&label); - out.push_str(": "); - out.push_str(&value); - if idx != last { - out.push(','); - } - } - out.push('\n'); - out.push('}'); - out -} - /// #2089: a Date's `util.inspect` rendering — ISO string (unquoted) or "Invalid Date". DateCell pointer only (gated by callers). unsafe fn date_inspect_string(value: f64) -> String { let s_ptr = crate::date::js_date_to_iso_string(value); @@ -951,7 +849,7 @@ pub(crate) fn format_jsvalue(value: f64, depth: usize) -> String { if gc_type == crate::gc::GC_TYPE_ERROR { let error_ptr = ptr as *const crate::error::ErrorHeader; - format_error_value(error_ptr, depth) + errors::format_error_value(error_ptr, depth) } else if gc_type == crate::gc::GC_TYPE_ARRAY { // Array — format as [ elem1, elem2, ... ] matching Node.js util.inspect. // Cycle check FIRST so back-edges win over depth truncation @@ -1218,14 +1116,14 @@ fn format_weak_wrapper( /// crash safety net for cyclic structures; the Node-style `[Object]` truncation /// at depth > 2 is enforced by `format_jsvalue_for_json` on the way in. unsafe fn format_object_as_json( - obj_ptr: *const crate::object::ObjectHeader, + mut obj_ptr: *const crate::object::ObjectHeader, depth: usize, ) -> String { if depth > 10 { return "{...}".to_string(); } - let obj_addr = obj_ptr as usize; + let mut obj_addr = obj_ptr as usize; // `[util.inspect.custom]` hook: when the object carries a symbol-keyed // entry for `Symbol.for("nodejs.util.inspect.custom")` and the @@ -1336,6 +1234,18 @@ unsafe fn format_object_as_json( crate::object::class_name_for_id(class_id).filter(|name| !name.is_empty()) } }; + let error_headline = if crate::object::extends_builtin_error((*obj_ptr).class_id) { + let (headline, refreshed_obj_ptr) = errors::format_error_subclass_headline( + obj_ptr, + (*obj_ptr).class_id, + class_name.as_deref().unwrap_or("Error"), + ); + obj_ptr = refreshed_obj_ptr; + obj_addr = obj_ptr as usize; + Some(headline) + } else { + None + }; let has_class_name = class_name.is_some(); let class_name_ref = if deep_equal_skip_prototype_format_enabled() { None @@ -1355,14 +1265,21 @@ unsafe fn format_object_as_json( // null-proto plain object, otherwise nothing. (Distinct from // `class_name_ref`/`has_class_name`, which drive the private-field skip // and must reflect only a genuine class.) - let name_prefix: Option = match class_name_ref { - Some(name) => Some(name.to_string()), - None if boxed_base.is_none() && is_null_proto => { - Some("[Object: null prototype]".to_string()) + let name_prefix: Option = if let Some(headline) = error_headline.as_ref() { + Some(headline.clone()) + } else { + match class_name_ref { + Some(name) => Some(name.to_string()), + None if boxed_base.is_none() && is_null_proto => { + Some("[Object: null prototype]".to_string()) + } + None => None, } - None => None, }; let empty_object = || { + if let Some(headline) = error_headline.as_deref() { + return headline.to_string(); + } if let Some(base) = boxed_base.as_deref() { return base.to_string(); } @@ -1415,6 +1332,13 @@ unsafe fn format_object_as_json( continue; } + // Error inspection consumes an own `name` into the headline. Node + // does not print it again as an enumerable body property unless + // showHidden asks for the complete reflective surface. + if error_headline.is_some() && !show_hidden && key_str == "name" { + continue; + } + // Hide a boxed String's character index properties (`"0".."len-1"`): // they are rendered by the `[String: '…']` base, not the body. if let Some(char_count) = boxed_string_char_count { @@ -1671,7 +1595,7 @@ fn format_jsvalue_for_json(value: f64, depth: usize) -> String { if gc_type == crate::gc::GC_TYPE_ERROR { let error_ptr = ptr as *const crate::error::ErrorHeader; - format_error_value(error_ptr, depth) + errors::format_error_value(error_ptr, depth) } else if gc_type == crate::gc::GC_TYPE_ARRAY { // Cycle check FIRST so back-edges always print as // `[Circular *N]` regardless of depth (#1204). The diff --git a/crates/perry-runtime/src/builtins/formatting/errors.rs b/crates/perry-runtime/src/builtins/formatting/errors.rs new file mode 100644 index 0000000000..41cb56a80b --- /dev/null +++ b/crates/perry-runtime/src/builtins/formatting/errors.rs @@ -0,0 +1,223 @@ +//! `util.inspect` formatting for native Errors and ordinary-layout Error +//! subclasses. + +use super::*; + +unsafe fn string_header_to_string(ptr: *mut StringHeader, fallback: &str) -> String { + if ptr.is_null() { + return fallback.to_string(); + } + let len = (*ptr).byte_len as usize; + let data = (ptr as *const u8).add(std::mem::size_of::()); + let bytes = std::slice::from_raw_parts(data, len); + std::str::from_utf8(bytes).unwrap_or(fallback).to_string() +} + +unsafe fn format_error_headline(error_ptr: *const crate::error::ErrorHeader) -> String { + let scope = crate::gc::RuntimeHandleScope::new(); + let error_h = scope.root_raw_const_ptr(error_ptr); + let own_name_h = crate::node_submodules::error_user_prop( + error_h.get_raw_const_ptr::() as usize, + "name", + ) + .map(|value| scope.root_nanbox_f64(value)); + let own_message_h = crate::node_submodules::error_user_prop( + error_h.get_raw_const_ptr::() as usize, + "message", + ) + .map(|value| scope.root_nanbox_f64(value)); + let error_ptr = error_h.get_raw_const_ptr::(); + let display_part = |value: Option<&crate::gc::RuntimeHandle<'_>>, + header: *mut StringHeader, + fallback: &str| { + value + .and_then(|handle| jsvalue_string_content(handle.get_nanbox_f64())) + .unwrap_or_else(|| string_header_to_string(header, fallback)) + }; + // `ErrorHeader.name` is internal backing storage for the inherited + // Error-family prototype value. An explicit `error.name = ...` is an own + // expando and must drive inspection without being redundantly printed as + // a body property (#9440). + let name_str = display_part(own_name_h.as_ref(), (*error_ptr).name, "Error"); + let message_str = display_part(own_message_h.as_ref(), (*error_ptr).message, ""); + if message_str.is_empty() { + name_str + } else { + format!("{}: {}", name_str, message_str) + } +} + +unsafe fn format_error_stack_frame(error_ptr: *const crate::error::ErrorHeader) -> Option { + let stack = string_header_to_string((*error_ptr).stack, ""); + stack + .lines() + .skip(1) + .find(|line| !line.trim().is_empty()) + .map(str::to_string) +} + +unsafe fn format_error_array(arr_ptr: *const crate::array::ArrayHeader, depth: usize) -> String { + if arr_ptr.is_null() { + return "[]".to_string(); + } + let length = (*arr_ptr).length as usize; + if length == 0 { + return "[]".to_string(); + } + let data_ptr = + (arr_ptr as *const u8).add(std::mem::size_of::()) as *const f64; + let mut out = String::from("["); + for i in 0..length { + out.push('\n'); + out.push_str(" "); + out.push_str(&format_jsvalue_for_json(*data_ptr.add(i), depth + 1)); + } + out.push('\n'); + out.push_str(" ]"); + out +} + +pub(super) unsafe fn format_error_value( + error_ptr: *const crate::error::ErrorHeader, + depth: usize, +) -> String { + // Headline lookup consults the ordinary expando bag and may allocate. + // Keep the native Error live and re-read its address for every later slot. + let scope = crate::gc::RuntimeHandleScope::new(); + let error_h = scope.root_raw_const_ptr(error_ptr); + let headline = format_error_headline(error_h.get_raw_const_ptr()); + let mut entries: Vec<(String, String)> = crate::node_submodules::error_user_props( + error_h.get_raw_const_ptr::() as usize, + ) + .into_iter() + .filter(|(key, _)| key != "cause" && key != "errors" && key != "name") + .map(|(key, value)| (key, format_jsvalue_for_json(value, depth + 1))) + .collect(); + + let error_ptr = error_h.get_raw_const_ptr::(); + let cause = (*error_ptr).cause; + if !crate::value::JSValue::from_bits(cause.to_bits()).is_undefined() { + entries.push(( + "[cause]".to_string(), + format_jsvalue_for_json(cause, depth + 1), + )); + } + + let errors = (*error_h.get_raw_const_ptr::()).errors; + if !errors.is_null() { + entries.push(( + "[errors]".to_string(), + format_error_array(errors, depth + 1), + )); + } + + if entries.is_empty() { + return headline; + } + + let mut out = headline; + if let Some(frame) = format_error_stack_frame(error_h.get_raw_const_ptr()) { + out.push('\n'); + out.push_str(&frame); + out.push_str(" {"); + } else { + out.push_str("\n{"); + } + + let last = entries.len().saturating_sub(1); + for (idx, (label, value)) in entries.into_iter().enumerate() { + out.push('\n'); + out.push_str(" "); + out.push_str(&label); + out.push_str(": "); + out.push_str(&value); + if idx != last { + out.push(','); + } + } + out.push('\n'); + out.push('}'); + out +} + +/// Build Node's Error headline for a user class whose instances use the +/// ordinary object layout. The class registry supplies the Error-family +/// prototype name; own `message` and explicitly assigned `name` values come +/// from the instance slots (#9440). +pub(super) unsafe fn format_error_subclass_headline( + obj_ptr: *const crate::object::ObjectHeader, + class_id: u32, + class_name: &str, +) -> (String, *const crate::object::ObjectHeader) { + // String coercion below can collect. Keep both the receiver and the two + // values which can participate in the headline live across either + // conversion, and return the receiver's refreshed address to the caller. + let scope = crate::gc::RuntimeHandleScope::new(); + let obj_h = scope.root_raw_const_ptr(obj_ptr); + let keys = crate::object::object_keys_array(obj_h.get_raw_const_ptr()); + let mut own_name: Option = None; + let mut own_message: Option = None; + if !keys.is_null() { + let len = crate::array::js_array_length(keys); + for index in 0..len { + let key = crate::array::js_array_get(keys, index); + if !key.is_string() { + continue; + } + let key_ptr = key.as_string_ptr(); + if key_ptr.is_null() { + continue; + } + let key_len = (*key_ptr).byte_len as usize; + let key_data = (key_ptr as *const u8).add(std::mem::size_of::()); + let key_bytes = std::slice::from_raw_parts(key_data, key_len); + if key_bytes == b"name" { + own_name = Some(crate::object::js_object_get_field_f64( + obj_h.get_raw_const_ptr(), + index, + )); + } else if key_bytes == b"message" { + own_message = Some(crate::object::js_object_get_field_f64( + obj_h.get_raw_const_ptr(), + index, + )); + } + } + } + + let own_name_h = own_name.map(|value| scope.root_nanbox_f64(value)); + let own_message_h = own_message.map(|value| scope.root_nanbox_f64(value)); + let value_string = |value: &crate::gc::RuntimeHandle<'_>, fallback: &str| { + let value = value.get_nanbox_f64(); + let js = JSValue::from_bits(value.to_bits()); + if js.is_undefined() { + return fallback.to_string(); + } + jsvalue_string_content(value).unwrap_or_else(|| { + let string = crate::value::js_jsvalue_to_string(value); + string_header_to_string(string, fallback) + }) + }; + let prototype_name = crate::object::builtin_error_prototype_name(class_id); + let name = own_name_h + .as_ref() + .map(|value| value_string(value, prototype_name)) + .unwrap_or_else(|| prototype_name.to_string()); + let message = own_message_h + .as_ref() + .map(|value| value_string(value, "")) + .unwrap_or_default(); + let display_name = if own_name_h.is_none() && class_name != name { + format!("{class_name} [{name}]") + } else { + name + }; + let headline = if display_name.is_empty() { + message + } else if message.is_empty() { + display_name + } else { + format!("{display_name}: {message}") + }; + (headline, obj_h.get_raw_const_ptr()) +} diff --git a/crates/perry-runtime/src/object/class_constructors.rs b/crates/perry-runtime/src/object/class_constructors.rs index 37b2b33ea2..cc679828ea 100644 --- a/crates/perry-runtime/src/object/class_constructors.rs +++ b/crates/perry-runtime/src/object/class_constructors.rs @@ -1054,15 +1054,24 @@ unsafe fn default_error_init_for_implicit_chain( if !crate::object::extends_builtin_error(class_cid) { return; } + let scope = crate::gc::RuntimeHandleScope::new(); + let this_h = scope.root_nanbox_f64(crate::value::js_nanbox_pointer(inst as i64)); + // Read and root the forwarded value before stack capture can collect; the + // caller-owned argument slice itself is not a runtime handle. + let msg_h = if args_ptr.is_null() || args_len == 0 { + None + } else { + Some(scope.root_nanbox_f64(*args_ptr)) + }; // #9410: the dynamic replay path is a construction site like any other, // so the instance gets its own lazily-formatted `stack` here — before the // message guard below, which returns early for `new X()` with no argument // and would otherwise leave exactly those instances trace-less. - crate::error::js_error_subclass_capture_stack(crate::value::js_nanbox_pointer(inst as i64)); - if args_ptr.is_null() || args_len == 0 { + crate::error::js_error_subclass_capture_stack(this_h.get_nanbox_f64()); + let Some(msg_h) = msg_h else { return; - } - let msg = *args_ptr; + }; + let msg = msg_h.get_nanbox_f64(); if msg.to_bits() == crate::value::TAG_UNDEFINED { return; } @@ -1070,9 +1079,16 @@ unsafe fn default_error_init_for_implicit_chain( if msg_str.is_null() { return; } + let msg_str_h = scope.root_string_ptr(msg_str); + let key_h = scope.root_string_ptr(crate::string::js_string_from_bytes( + b"message".as_ptr(), + b"message".len() as u32, + )); + let inst = crate::value::js_nanbox_get_pointer(this_h.get_nanbox_f64()) as *mut ObjectHeader; + let msg_str = msg_str_h.get_raw_const_ptr::(); let boxed = f64::from_bits(crate::value::STRING_TAG | (msg_str as u64 & crate::value::POINTER_MASK)); - let key = crate::string::js_string_from_bytes(b"message".as_ptr(), b"message".len() as u32); + let key = key_h.get_raw_const_ptr::(); crate::object::js_object_set_field_by_name(inst, key, boxed); } @@ -1089,53 +1105,48 @@ unsafe fn default_error_init_for_implicit_chain( /// standalone ctor's forwarding params are padded with undefined for missing /// call args, and setting an OWN undefined `message` would shadow /// `Error.prototype.message` (""). Set non-enumerable, matching the built-in -/// (test262 NativeError/*-message). `name` mirrors the static arm: the -/// terminating Error-family kind as an own property. +/// (test262 NativeError/*-message). `name` remains inherited from the +/// terminating Error-family prototype (#9440). #[no_mangle] -pub unsafe extern "C" fn js_error_subclass_default_init( - this_val: f64, - msg: f64, - name_ptr: *const crate::StringHeader, -) { +pub unsafe extern "C" fn js_error_subclass_default_init(this_val: f64, msg: f64) { let bits = this_val.to_bits(); let raw = (bits & crate::value::POINTER_MASK) as usize; if raw < 0x10000 { return; } - let inst = raw as *mut ObjectHeader; + let scope = crate::gc::RuntimeHandleScope::new(); + let this_h = scope.root_nanbox_f64(this_val); + let msg_h = scope.root_nanbox_f64(msg); + // Capture first: V8 exposes `stack` before `message` from + // `getOwnPropertyNames`, and the lazy getter still observes the later + // message or an explicit user-assigned `name`. + crate::error::js_error_subclass_capture_stack(this_h.get_nanbox_f64()); + let msg = msg_h.get_nanbox_f64(); if msg.to_bits() != crate::value::TAG_UNDEFINED { let msg_str = crate::value::js_jsvalue_to_string(msg); if !msg_str.is_null() { + let msg_str_h = scope.root_string_ptr(msg_str); + let key_h = scope.root_string_ptr(crate::string::js_string_from_bytes( + b"message".as_ptr(), + b"message".len() as u32, + )); + let inst = + crate::value::js_nanbox_get_pointer(this_h.get_nanbox_f64()) as *mut ObjectHeader; + let msg_str = msg_str_h.get_raw_const_ptr::(); let boxed = f64::from_bits( crate::value::STRING_TAG | (msg_str as u64 & crate::value::POINTER_MASK), ); - let key = - crate::string::js_string_from_bytes(b"message".as_ptr(), b"message".len() as u32); + let key = key_h.get_raw_const_ptr::(); crate::object::js_object_set_field_by_name_nonenum(inst, key, boxed); } } - if !name_ptr.is_null() { - let name_boxed = f64::from_bits( - crate::value::STRING_TAG | (name_ptr as u64 & crate::value::POINTER_MASK), - ); - let key = crate::string::js_string_from_bytes(b"name".as_ptr(), b"name".len() as u32); - crate::object::js_object_set_field_by_name(inst, key, name_boxed); - } - // #9410: `stack`. The synthesized standalone ctor stamps `message` and - // `name` but installed nothing for `stack`, so `new X("m").stack` was - // `undefined` for every `class X extends Error {}` with no own - // constructor. Last, so the getter's head sees the `name` just written. - crate::error::js_error_subclass_capture_stack(this_val); } /// Keepalive: generated code is the only caller (#6469). #[cfg(feature = "keepalive-anchors")] #[used] -static KEEP_JS_ERROR_SUBCLASS_DEFAULT_INIT: unsafe extern "C" fn( - f64, - f64, - *const crate::StringHeader, -) = js_error_subclass_default_init; +static KEEP_JS_ERROR_SUBCLASS_DEFAULT_INIT: unsafe extern "C" fn(f64, f64) = + js_error_subclass_default_init; /// Find the per-evaluation class object that owns `target_cid` while walking a /// fresh derived class's pinned parent chain. The template class-id registry diff --git a/crates/perry-runtime/src/object/class_meta_registry.rs b/crates/perry-runtime/src/object/class_meta_registry.rs index bd8580677f..83c2693d91 100644 --- a/crates/perry-runtime/src/object/class_meta_registry.rs +++ b/crates/perry-runtime/src/object/class_meta_registry.rs @@ -329,6 +329,31 @@ fn extends_builtin_error_slow(class_id: u32) -> bool { false } +/// Resolve the Error-family prototype at the bottom of a registered class +/// chain. Callers first establish [`extends_builtin_error`]; returning +/// `"Error"` on an incomplete/cyclic chain is the same fallback used by +/// ordinary prototype-property lookup. +pub(crate) fn builtin_error_prototype_name(class_id: u32) -> &'static str { + let mut current = class_id; + for _ in 0..32 { + match current { + crate::error::CLASS_ID_TYPE_ERROR => return "TypeError", + crate::error::CLASS_ID_RANGE_ERROR => return "RangeError", + crate::error::CLASS_ID_REFERENCE_ERROR => return "ReferenceError", + crate::error::CLASS_ID_SYNTAX_ERROR => return "SyntaxError", + crate::error::CLASS_ID_EVAL_ERROR => return "EvalError", + crate::error::CLASS_ID_URI_ERROR => return "URIError", + crate::error::CLASS_ID_AGGREGATE_ERROR => return "AggregateError", + crate::error::CLASS_ID_ERROR => return "Error", + _ => match get_parent_class_id(current) { + Some(parent) if parent != 0 && parent != current => current = parent, + _ => break, + }, + } + } + "Error" +} + #[cfg(test)] mod dense_parent_tests { use super::*; diff --git a/crates/perry-runtime/src/object/descriptors.rs b/crates/perry-runtime/src/object/descriptors.rs index ac983a3733..f3bb2f62cc 100644 --- a/crates/perry-runtime/src/object/descriptors.rs +++ b/crates/perry-runtime/src/object/descriptors.rs @@ -1351,7 +1351,18 @@ fn js_object_get_own_property_names_shape(obj_value: f64) -> f64 { use super::exotic_expando::ExoticKind; let mut names = match kind { ExoticKind::RegExp => vec!["lastIndex".to_string()], - ExoticKind::Error => vec!["message".to_string(), "stack".to_string()], + ExoticKind::Error => { + // V8 creates the lazy own `stack` before Error's optional + // `message`. The header keeps both payload slots, but only + // `message` values supplied to the constructor are own + // properties; `name` is always inherited until assigned. + let mut builtin = vec!["stack".to_string()]; + let error = addr as *mut crate::error::ErrorHeader; + if crate::error::js_error_has_own_property(error, "message") { + builtin.push("message".to_string()); + } + builtin + } ExoticKind::Date | ExoticKind::Temporal | ExoticKind::Promise diff --git a/crates/perry-runtime/src/object/field_get_set/accessors.rs b/crates/perry-runtime/src/object/field_get_set/accessors.rs index 589bae723f..6e8d91ed66 100644 --- a/crates/perry-runtime/src/object/field_get_set/accessors.rs +++ b/crates/perry-runtime/src/object/field_get_set/accessors.rs @@ -316,45 +316,7 @@ pub(crate) unsafe fn ordinary_object_prototype_property_value( scope.root_nanbox_f64(crate::value::js_nanbox_pointer(obj as usize as i64)); let key_h = scope.root_nanbox_f64(crate::value::nanbox_string_key(key)); let _guard = object_prototype_lookup_guard()?; - let mut current = class_id; - let mut prototype_name = "Error"; - for _ in 0..32 { - match current { - crate::error::CLASS_ID_TYPE_ERROR => { - prototype_name = "TypeError"; - break; - } - crate::error::CLASS_ID_RANGE_ERROR => { - prototype_name = "RangeError"; - break; - } - crate::error::CLASS_ID_REFERENCE_ERROR => { - prototype_name = "ReferenceError"; - break; - } - crate::error::CLASS_ID_SYNTAX_ERROR => { - prototype_name = "SyntaxError"; - break; - } - crate::error::CLASS_ID_EVAL_ERROR => { - prototype_name = "EvalError"; - break; - } - crate::error::CLASS_ID_URI_ERROR => { - prototype_name = "URIError"; - break; - } - crate::error::CLASS_ID_AGGREGATE_ERROR => { - prototype_name = "AggregateError"; - break; - } - crate::error::CLASS_ID_ERROR => break, - _ => match super::super::get_parent_class_id(current) { - Some(parent) if parent != 0 && parent != current => current = parent, - _ => break, - }, - } - } + let prototype_name = super::super::builtin_error_prototype_name(class_id); let prototype = super::super::builtin_prototype_value(prototype_name); let prototype_value = JSValue::from_bits(prototype.to_bits()); if prototype_value.is_pointer() { diff --git a/crates/perry-runtime/src/object/global_this/fetch_globals.rs b/crates/perry-runtime/src/object/global_this/fetch_globals.rs index 8252c3c739..5d0e3f6f62 100644 --- a/crates/perry-runtime/src/object/global_this/fetch_globals.rs +++ b/crates/perry-runtime/src/object/global_this/fetch_globals.rs @@ -720,7 +720,7 @@ pub unsafe extern "C" fn js_fetch_or_value_super( // value routes through the dynamic-parent registry): the parent value is // the global Error-family constructor. The ordinary value-super dispatch // below invokes it as a plain call, which builds a FRESH error cell and - // drops it — `this` never receives `message`/`name`, so every subclass + // drops it — `this` never receives `message`, so every subclass // instance constructed through this path printed "An error has occurred". // Apply the spec default Error-init directly on `this`, mirroring the // static-`new` arm (#573, `lower_call/new.rs`) and the standalone-ctor arm @@ -736,28 +736,28 @@ pub unsafe extern "C" fn js_fetch_or_value_super( let stash = crate::object::class_registry::js_get_dynamic_parent_value(cid); super::super::class_registry::identify_global_builtin_constructor(stash) }); - if let Some(kind) = err_parent.filter(|k| { - matches!( - *k, - "Error" - | "TypeError" - | "RangeError" - | "ReferenceError" - | "SyntaxError" - | "URIError" - | "EvalError" - | "AggregateError" - ) - }) { + if err_parent + .filter(|k| { + matches!( + *k, + "Error" + | "TypeError" + | "RangeError" + | "ReferenceError" + | "SyntaxError" + | "URIError" + | "EvalError" + | "AggregateError" + ) + }) + .is_some() + { let msg = if !args_ptr.is_null() && args_len >= 1 { *args_ptr } else { undef }; - let name_str = crate::string::js_string_from_bytes(kind.as_ptr(), kind.len() as u32); - crate::object::class_constructors::js_error_subclass_default_init( - this_box, msg, name_str, - ); + crate::object::class_constructors::js_error_subclass_default_init(this_box, msg); return undef; } } diff --git a/crates/perry-runtime/src/object/mod.rs b/crates/perry-runtime/src/object/mod.rs index 302b3fe0fa..4fd9536efc 100644 --- a/crates/perry-runtime/src/object/mod.rs +++ b/crates/perry-runtime/src/object/mod.rs @@ -236,8 +236,9 @@ pub use with_env::*; // named re-exports keep existing `crate::object::X` / bare-name call sites in // the object submodules resolving unchanged. pub(crate) use class_meta_registry::{ - class_generic_origin, extends_builtin_error, fetch_parent_kind, lookup_has_instance_hook, - lookup_to_string_tag_hook, register_fetch_parent_kind, CLASS_REGISTRY, + builtin_error_prototype_name, class_generic_origin, extends_builtin_error, fetch_parent_kind, + lookup_has_instance_hook, lookup_to_string_tag_hook, register_fetch_parent_kind, + CLASS_REGISTRY, }; pub use class_meta_registry::{ js_register_class_extends_error, js_register_class_generic_origin, diff --git a/test-files/test_gap_9410_error_subclass_stack.ts b/test-files/test_gap_9410_error_subclass_stack.ts index 57b8ea9477..bbc2939c1a 100644 --- a/test-files/test_gap_9410_error_subclass_stack.ts +++ b/test-files/test_gap_9410_error_subclass_stack.ts @@ -76,12 +76,9 @@ describe("factory-subclass", make("factory-msg"), "Error", "factory-msg"); // `stack` is not (node installs `stack` as a non-enumerable own property). const withField = new WithField("field-msg"); console.log("field value: " + withField.code); -// The subclass's own field enumerates; `stack` must not. NOT asserted here: -// the full `Object.keys` list, because perry additionally stamps an own -// ENUMERABLE `name` onto an Error-subclass instance where node leaves `name` -// on `Error.prototype` — a separate, pre-existing divergence (perry -// `["code","name"]` vs node `["code"]`) with its own fix, and asserting the -// whole list here would tie this fixture to that one. +// The subclass's own field enumerates; inherited `name` and own `stack` do +// not. The complete reflection/serialization contract is covered by #9440's +// dedicated fixture. console.log("field key enumerates: " + Object.keys(withField).includes("code")); console.log("stack key enumerates: " + Object.keys(withField).includes("stack")); console.log( diff --git a/test-files/test_gap_9440_error_name_ownership.ts b/test-files/test_gap_9440_error_name_ownership.ts new file mode 100644 index 0000000000..e3b608f332 --- /dev/null +++ b/test-files/test_gap_9440_error_name_ownership.ts @@ -0,0 +1,91 @@ +// #9440 — an Error subclass inherited `.name` from the Error-family +// prototype in Node, but Perry stamped it onto every instance as an enumerable +// own property. That leaked `name` through every own-key consumer. +// +// Keep all output portable and byte-comparable with +// `node --experimental-strip-types`: stack frames contain host paths, so the +// util.inspect check removes only `at ...` lines while retaining the Error +// headline and any rendered properties. + +import { inspect } from "node:util"; + +class Implicit extends Error {} + +class Explicit extends Error { + constructor(message: string) { + super(message); + } +} + +class Deep extends Implicit { + constructor(message: string) { + super(message); + } +} + +class TypeSubclass extends TypeError {} + +function forInKeys(value: object): string[] { + const keys: string[] = []; + for (const key in value) { + keys.push(key); + } + return keys; +} + +function stableInspect(value: unknown): string { + return inspect(value, { breakLength: Infinity }).replace( + /\n\s+at [^\n]*/g, + "", + ); +} + +function describe(label: string, error: Error): void { + console.log( + label + + " reflection: " + + JSON.stringify({ + name: error.name, + ownName: Object.prototype.hasOwnProperty.call(error, "name"), + descriptor: Object.getOwnPropertyDescriptor(error, "name"), + json: JSON.stringify(error), + ownNames: Object.getOwnPropertyNames(error), + keys: Object.keys(error), + forIn: forInKeys(error), + spread: { ...error }, + }), + ); + console.log(label + " inspect: " + JSON.stringify(stableInspect(error))); +} + +describe("base", new Error("base")); +describe("base-empty", new Error()); +describe("implicit", new Implicit("implicit")); +describe("implicit-empty", new Implicit()); +describe("explicit", new Explicit("explicit")); +describe("deep", new Deep("deep")); +describe("type-subclass", new TypeSubclass("typed")); + +// Exercise dynamic construction, which runs the synthesized standalone +// constructor rather than the direct-new initialization path. +const Dynamic: typeof Implicit = Implicit; +describe("dynamic", new Dynamic("dynamic")); + +function makeEscapedSubclass(): typeof Error { + return class Escaped extends Error {}; +} + +// Force the runtime constructor-replay path: the concrete subclass is created +// inside a function and only reaches this construction site as a value. +const Escaped = makeEscapedSubclass(); +describe("escaped-dynamic", new Escaped("escaped-dynamic")); + +// An explicit assignment must still create an ordinary own enumerable +// property, just as it does for any inherited writable data property. +const custom = new Implicit("custom"); +custom.name = "Custom"; +describe("assigned", custom); + +const customBase = new Error("custom-base"); +customBase.name = "CustomBase"; +describe("assigned-base", customBase);