From 3ba207c60e84230d0bfeef509f5891b628eefe0d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 13 Jun 2026 11:23:49 +0200 Subject: [PATCH] =?UTF-8?q?fix(runtime):=20node:dns=20=E2=80=94=20correct?= =?UTF-8?q?=20resolve/reverse=20error=20shape=20(syscall=20+=20hostname)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit node:dns node-suite: 5/6 -> 6/6 locally (zero regressions in assert/net/dgram + dns_resolver unit tests + error-shape probes). Root causes: 1. resolve*/reverse rejections were missing Node's `.syscall` and `.hostname` own properties. A caught `dns.resolve4` error exposed only `.code` ("ESERVFAIL"); Node's c-ares errors also carry `e.syscall === "queryA"` and `e.hostname === "localhost"` (with `errno` left undefined). `dns.reverse` errors likewise carry `syscall === "getHostByAddr"`. - dns.rs: new `dns_query_error_value` builds the error and registers code + syscall + hostname on the message StringHeader (mirrors the existing `.code` side-table path); resolve_error_value / reverse_error_value route through it. - node_submodules/diagnostics.rs: new ERROR_MESSAGE_HOSTNAMES side table + register_error_hostname / error_hostname_for_message, paralleling the syscall/path/dest tables. - object/field_get_set.rs: new `.hostname` Error getter arm (mirrors `.path`/`.syscall`). The user-assigned-prop path is consulted first, so a user-set `err.hostname` still wins — verified against Node. 2. test imports/default-export.ts asserted that `resolver.resolve4("localhost")` returns an array. resolve4 queries the configured nameserver directly (it does NOT consult /etc/hosts), so "localhost" yields an A record on some resolvers and ESERVFAIL/ENOTFOUND on others — on a SERVFAIL machine Node itself aborts on the unhandled rejection, making the assertion unportable. Rewrote it to print array-or-error-code, the same environment-robust record-or-error approach #5040 already applied to the sibling dns/resolve/localhost.ts test. (No product behavior was papered over: Perry already threw the correct ESERVFAIL there — the fix above just completes that error's shape.) --- crates/perry-runtime/src/dns.rs | 29 +++++++++++++++++-- .../src/node_submodules/diagnostics.rs | 25 ++++++++++++++++ .../perry-runtime/src/object/field_get_set.rs | 15 ++++++++++ .../node-suite/dns/imports/default-export.ts | 16 ++++++++-- 4 files changed, 81 insertions(+), 4 deletions(-) diff --git a/crates/perry-runtime/src/dns.rs b/crates/perry-runtime/src/dns.rs index 850bcb1f64..9005462246 100644 --- a/crates/perry-runtime/src/dns.rs +++ b/crates/perry-runtime/src/dns.rs @@ -825,14 +825,39 @@ fn dns_error_code(err: DnsError) -> &'static str { } } +/// Build a c-ares-style DNS error carrying Node's `.code`, `.syscall`, and +/// `.hostname` own properties. Node's `dns.resolve*`/`dns.reverse` rejections +/// always set all three (with `errno` left `undefined`); registering them on +/// the message StringHeader mirrors the `.code` path so caught errors expose +/// the full shape, not just `.code`. +fn dns_query_error_value( + message: &str, + code: &'static str, + syscall: &'static str, + hostname: &str, +) -> f64 { + let msg = crate::string::js_string_from_bytes(message.as_ptr(), message.len() as u32); + crate::node_submodules::register_error_code_pub(msg, code); + crate::node_submodules::register_error_syscall(msg, syscall); + crate::node_submodules::register_error_hostname(msg, hostname.to_string()); + let err = crate::error::js_error_new_with_message(msg); + boxed_pointer(err as *const u8) +} + fn resolve_error_value(kind: RecordKind, host: &str, err: DnsError) -> f64 { let code = dns_error_code(err); - plain_error_value(&format!("{} {code} {host}", resolve_syscall(kind)), code) + let syscall = resolve_syscall(kind); + dns_query_error_value(&format!("{syscall} {code} {host}"), code, syscall, host) } fn reverse_error_value(host: &str, err: DnsError) -> f64 { let code = dns_error_code(err); - plain_error_value(&format!("getHostByAddr {code} {host}"), code) + dns_query_error_value( + &format!("getHostByAddr {code} {host}"), + code, + "getHostByAddr", + host, + ) } /// Deterministic-mode (`PERRY_DETERMINISTIC_NET=1`) loopback answers — the diff --git a/crates/perry-runtime/src/node_submodules/diagnostics.rs b/crates/perry-runtime/src/node_submodules/diagnostics.rs index 91020176a4..008d6ec6f8 100644 --- a/crates/perry-runtime/src/node_submodules/diagnostics.rs +++ b/crates/perry-runtime/src/node_submodules/diagnostics.rs @@ -477,6 +477,31 @@ pub fn error_path_for_message(message_ptr: *const StringHeader) -> Option> = + RefCell::new(HashMap::new()); +} + +/// Attach a Node-style `hostname` string to an Error keyed by its message +/// StringHeader. Node's c-ares `dns.resolve*`/`dns.reverse` failures carry the +/// queried hostname (or address) on `err.hostname`. Read back from the +/// `.hostname` getter in `field_get_set` (mirrors `.path`). +pub fn register_error_hostname(message_ptr: *const StringHeader, hostname: String) { + if message_ptr.is_null() { + return; + } + ERROR_MESSAGE_HOSTNAMES.with(|m| { + m.borrow_mut().insert(message_ptr as usize, hostname); + }); +} + +pub fn error_hostname_for_message(message_ptr: *const StringHeader) -> Option { + if message_ptr.is_null() { + return None; + } + ERROR_MESSAGE_HOSTNAMES.with(|m| m.borrow().get(&(message_ptr as usize)).cloned()) +} + /// Attach a Node-style `dest` string to an Error keyed by its message /// StringHeader. Node sets `dest` on two-path fs errors (rename/copyFile/ /// link/symlink) alongside `path`. Read back from the `.dest` getter. diff --git a/crates/perry-runtime/src/object/field_get_set.rs b/crates/perry-runtime/src/object/field_get_set.rs index 29660dc9cd..b1920e2272 100644 --- a/crates/perry-runtime/src/object/field_get_set.rs +++ b/crates/perry-runtime/src/object/field_get_set.rs @@ -4205,6 +4205,21 @@ pub extern "C" fn js_object_get_field_by_name( } return JSValue::undefined(); } + b"hostname" => { + // Node attaches `hostname` to c-ares dns errors + // (`dns.resolve*`/`dns.reverse`). Mirrors `.path`. + let msg = crate::error::js_error_get_message(err_ptr); + if let Some(hostname) = + crate::node_submodules::error_hostname_for_message(msg) + { + let s = crate::string::js_string_from_bytes( + hostname.as_ptr(), + hostname.len() as u32, + ); + return JSValue::from_bits(crate::js_nanbox_string(s as i64).to_bits()); + } + return JSValue::undefined(); + } b"dest" => { // Node attaches `dest` to two-path fs errors // (rename/copyFile/link/symlink). Mirrors `.path`. diff --git a/test-parity/node-suite/dns/imports/default-export.ts b/test-parity/node-suite/dns/imports/default-export.ts index df8919a689..baf5fe9c54 100644 --- a/test-parity/node-suite/dns/imports/default-export.ts +++ b/test-parity/node-suite/dns/imports/default-export.ts @@ -28,5 +28,17 @@ console.log( ); const resolver = new dnsPromisesDefault.Resolver(); -const resolver4 = await resolver.resolve4("localhost"); -console.log("promises default resolve4:", Array.isArray(resolver4), JSON.stringify(resolver4)); +// resolve4 queries the configured nameserver directly (it does not read +// /etc/hosts), so "localhost" yields an A record on some resolvers and +// ESERVFAIL/ENOTFOUND on others — node itself rejects (and aborts on the +// unhandled rejection) when the resolver has no answer for it. Print +// array-or-error-code so node and Perry agree regardless of the machine's +// resolver, the same record-or-error approach the dns/resolve suite uses. +let resolver4Summary: string; +try { + const resolver4 = await resolver.resolve4("localhost"); + resolver4Summary = Array.isArray(resolver4) ? "array" : typeof resolver4; +} catch (e: any) { + resolver4Summary = "err:" + e.code; +} +console.log("promises default resolve4:", resolver4Summary);