From ebf2b3e4873a59ab83f10374fa910e9ae8cb0b06 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 13 Jun 2026 11:23:41 +0200 Subject: [PATCH] =?UTF-8?q?fix(runtime):=20node:http=20client=20transport?= =?UTF-8?q?=20errors=20=E2=86=92=20real=20coded=20Error=20(code/syscall/er?= =?UTF-8?q?rno)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A failed `http.request`/`http.get` handed `request.on('error')` listeners a bare string instead of Node's real system `Error`, so every library that branches on `err.code === 'ECONNREFUSED'` (which is most of them) saw `undefined`. Two root causes: * `reqwest::Error::to_string()` carries only the top-level "error sending request for url (...)" text — the OS reason ("Connection refused") lives in the `source()` chain, so `client_events::error_event_arg`'s substring match never fired and fell through to the plain-string branch. * even the recognized path only set `.code`; Node also sets `.syscall`, `.errno` and a `${syscall} ${CODE} ${detail}` message. Fix: * new runtime FFI `js_node_system_error_value` builds an `Error` with `.message` + `.code` (message→code side table) and `.syscall`/`.errno` own properties (Error expando store), exposed via `perry_ffi::system_error_value`. * `perry-ext-http::transport_error` classifies a `reqwest::Error` (+ request URL) into `(message, code, syscall, errno)` by walking the `source()` chain for the underlying `io::Error` (`raw_os_error()` → exact errno, `ErrorKind` → code) and a text trail for DNS failures (`getaddrinfo ENOTFOUND`). Recognized failures push a new `PendingHttpEvent::TransportError` that the drain turns into the coded Error; anything unrecognized keeps the legacy string path (so the existing `socket hang up`/ECONNRESET surface is untouched). Now byte-identical to node v26: connect ECONNREFUSED 127.0.0.1:1 | ECONNREFUSED | connect | -61 getaddrinfo ENOTFOUND host | ENOTFOUND | getaddrinfo | -3008 node-suite http: 18/18 → 19/19 (adds client-request/transport-error.ts). https 5/5 unaffected (the change only alters the request-send *failure* branch); runtime/ffi changes are purely additive. --- crates/perry-ext-http/src/client_dispatch.rs | 13 ++ crates/perry-ext-http/src/client_events.rs | 29 +++ crates/perry-ext-http/src/lib.rs | 31 +++ crates/perry-ext-http/src/transport_error.rs | 213 ++++++++++++++++++ crates/perry-ffi/src/error.rs | 34 +++ crates/perry-ffi/src/lib.rs | 4 +- crates/perry-runtime/src/error.rs | 51 +++++ .../http/client-request/transport-error.ts | 40 ++++ 8 files changed, 414 insertions(+), 1 deletion(-) create mode 100644 crates/perry-ext-http/src/transport_error.rs create mode 100644 test-parity/node-suite/http/client-request/transport-error.ts diff --git a/crates/perry-ext-http/src/client_dispatch.rs b/crates/perry-ext-http/src/client_dispatch.rs index 6c726aab3d..afb60f8809 100644 --- a/crates/perry-ext-http/src/client_dispatch.rs +++ b/crates/perry-ext-http/src/client_dispatch.rs @@ -170,6 +170,19 @@ pub(crate) fn dispatch_request( // event instead of a generic error. if e.is_timeout() { push_event(PendingHttpEvent::Timeout { request_handle }); + } else if let Some((message, code, syscall, errno)) = + crate::transport_error::classify_reqwest(&e, &url) + { + // A recognized transport failure (connect refused, DNS + // lookup failure, …) — hand listeners the real coded + // Node Error instead of a bare string. + push_event(PendingHttpEvent::TransportError { + request_handle, + message, + code, + syscall, + errno, + }); } else { push_event(PendingHttpEvent::Error { request_handle, diff --git a/crates/perry-ext-http/src/client_events.rs b/crates/perry-ext-http/src/client_events.rs index aa4df821ef..fd372ddf80 100644 --- a/crates/perry-ext-http/src/client_events.rs +++ b/crates/perry-ext-http/src/client_events.rs @@ -384,6 +384,35 @@ pub(crate) unsafe fn handle_error_event(request_handle: Handle, error_message: & fire_request_close_once(request_handle); } +/// Drain handler for `PendingHttpEvent::TransportError`: fire `'error'` +/// listeners with a real Node-coded `Error` (`.code` / `.syscall` / `.errno`) +/// then `'close'`. Suppressed once the request already completed (same race +/// guard as [`handle_error_event`]). +/// +/// # Safety +/// +/// Same listener-liveness contract as [`fire_request_event_listeners`]. +pub(crate) unsafe fn handle_transport_error_event( + request_handle: Handle, + message: &str, + code: &str, + syscall: &str, + errno: i64, +) { + let already_done = with_handle_mut::(request_handle, |req| { + let was = req.completed; + req.completed = true; + was + }) + .unwrap_or(false); + if already_done { + return; + } + let err = perry_ffi::system_error_value(message, code, syscall, errno); + fire_request_error_listeners(request_handle, f64::from_bits(err.bits())); + fire_request_close_once(request_handle); +} + /// #4905 / #4909 — drain handler for `PendingHttpEvent::Timeout`. /// /// `'timeout'` fires at most once per request and never after the diff --git a/crates/perry-ext-http/src/lib.rs b/crates/perry-ext-http/src/lib.rs index ea208dd5eb..fee6f316f4 100644 --- a/crates/perry-ext-http/src/lib.rs +++ b/crates/perry-ext-http/src/lib.rs @@ -83,6 +83,10 @@ mod client_outgoing; mod validation; use validation::{validate_client_options, validate_client_url_string}; +// Classifies transport-layer client failures (connect refused, DNS lookup +// failure, …) into the Node `Error` shape (`.code`/`.syscall`/`.errno`). +mod transport_error; + use lazy_static::lazy_static; use perry_ffi::{ alloc_string, gc_register_mutable_root_scanner_named, get_handle_mut, iter_handles_of_mut, @@ -139,6 +143,18 @@ pub(crate) enum PendingHttpEvent { request_handle: Handle, error_message: String, }, + /// A classified transport failure (connect refused, DNS lookup failure, + /// connection reset, …). Unlike [`PendingHttpEvent::Error`] — which hands + /// listeners a bare string — this carries the Node error shape so the + /// drain builds a real coded `Error` with `.code`/`.syscall`/`.errno`, + /// matching what Node passes to `request.on('error')`. + TransportError { + request_handle: Handle, + message: String, + code: String, + syscall: String, + errno: i64, + }, /// #4905 — the transport deadline from `req.setTimeout(ms)` / /// `options.timeout` fired. Drains to the request's `'timeout'` /// listeners when any exist; falls back to the Error surface @@ -1577,6 +1593,21 @@ pub unsafe extern "C" fn js_http_process_pending() -> i32 { } => { client_events::handle_error_event(request_handle, &error_message); } + PendingHttpEvent::TransportError { + request_handle, + message, + code, + syscall, + errno, + } => { + client_events::handle_transport_error_event( + request_handle, + &message, + &code, + &syscall, + errno, + ); + } PendingHttpEvent::Timeout { request_handle } => { client_events::handle_timeout_event(request_handle); } diff --git a/crates/perry-ext-http/src/transport_error.rs b/crates/perry-ext-http/src/transport_error.rs new file mode 100644 index 0000000000..a5cbac6afb --- /dev/null +++ b/crates/perry-ext-http/src/transport_error.rs @@ -0,0 +1,213 @@ +//! Classify a client transport failure into the Node `Error` shape. +//! +//! Node hands `request.on('error')` a real `Error` carrying `.code` +//! (`ECONNREFUSED`, `ENOTFOUND`, …), `.syscall` (`connect` / `getaddrinfo`) +//! and `.errno` (the libuv-negative number), with a message like +//! `connect ECONNREFUSED 127.0.0.1:1` or `getaddrinfo ENOTFOUND host`. +//! Perry previously passed the bare `reqwest::Error::to_string()` text, which +//! (a) doesn't even contain the OS reason — that lives in the error's +//! `source()` chain, not its `Display` — and (b) reaches listeners as a plain +//! string, so `err.code === 'ECONNREFUSED'` checks (every HTTP client library +//! does them) saw `undefined`. +//! +//! Only the failure modes Perry can name with confidence are classified; +//! anything else returns `None` so the caller keeps the legacy string path +//! (which still maps `ECONNRESET` / `socket hang up` via `error_event_arg`). + +use std::error::Error as StdError; +use std::io::ErrorKind; + +/// `(message, code, syscall, errno)` describing a Node-shaped transport error. +pub(crate) type Classified = (String, String, String, i64); + +/// Host (and explicit-or-default port) parsed from an http(s) URL, for the +/// `connect :` message form. +fn host_port(url: &str) -> (String, Option) { + match reqwest::Url::parse(url) { + Ok(u) => ( + u.host_str().unwrap_or_default().to_string(), + u.port_or_known_default(), + ), + Err(_) => (String::new(), None), + } +} + +/// Platform errno (libuv-negative) for a code, used only when no concrete +/// `std::io::Error` surfaced in the source chain to read `raw_os_error()`. +fn fallback_errno(code: &str) -> i64 { + match code { + "ECONNREFUSED" => { + if cfg!(target_os = "macos") { + -61 + } else { + -111 + } + } + "ETIMEDOUT" => { + if cfg!(target_os = "macos") { + -60 + } else { + -110 + } + } + "ECONNABORTED" => { + if cfg!(target_os = "macos") { + -53 + } else { + -103 + } + } + "EADDRNOTAVAIL" => { + if cfg!(target_os = "macos") { + -49 + } else { + -99 + } + } + "EHOSTUNREACH" => { + if cfg!(target_os = "macos") { + -65 + } else { + -113 + } + } + "ENETUNREACH" => { + if cfg!(target_os = "macos") { + -51 + } else { + -101 + } + } + _ => 0, + } +} + +/// Map a `std::io::ErrorKind` to a `(code, syscall)` for the connect path. +fn kind_to_code(kind: ErrorKind) -> Option<(&'static str, &'static str)> { + match kind { + ErrorKind::ConnectionRefused => Some(("ECONNREFUSED", "connect")), + ErrorKind::TimedOut => Some(("ETIMEDOUT", "connect")), + ErrorKind::ConnectionAborted => Some(("ECONNABORTED", "connect")), + ErrorKind::AddrNotAvailable => Some(("EADDRNOTAVAIL", "connect")), + ErrorKind::HostUnreachable => Some(("EHOSTUNREACH", "connect")), + ErrorKind::NetworkUnreachable => Some(("ENETUNREACH", "connect")), + _ => None, + } +} + +fn connect_message(code: &str, host: &str, port: Option) -> String { + match port { + Some(p) => format!("connect {code} {host}:{p}"), + None => format!("connect {code} {host}"), + } +} + +/// Classify a `reqwest::Error` raised by `request.send()`. Walks the error's +/// `source()` chain for the underlying `std::io::Error` (whose `raw_os_error` +/// gives the exact errno) and a lowercased text trail (for DNS detection, +/// since resolver failures carry no OS errno). +pub(crate) fn classify_reqwest(e: &reqwest::Error, url: &str) -> Option { + let (host, port) = host_port(url); + + let mut io_errno: Option = None; + let mut io_kind: Option = None; + let mut chain = String::new(); + let mut cur: Option<&(dyn StdError + 'static)> = Some(e); + while let Some(s) = cur { + chain.push_str(&s.to_string().to_lowercase()); + chain.push(' '); + if io_kind.is_none() { + if let Some(io) = s.downcast_ref::() { + io_errno = io.raw_os_error(); + io_kind = Some(io.kind()); + } + } + cur = s.source(); + } + + // Concrete OS connect error (the common case): exact code + errno. + if let Some((code, syscall)) = io_kind.and_then(kind_to_code) { + let errno = io_errno + .map(|n| -(n as i64)) + .filter(|n| *n != 0) + .unwrap_or_else(|| fallback_errno(code)); + return Some(( + connect_message(code, &host, port), + code.to_string(), + syscall.to_string(), + errno, + )); + } + + // DNS resolution failure → getaddrinfo ENOTFOUND (no OS errno; libuv -3008). + let is_dns = chain.contains("dns error") + || chain.contains("failed to lookup address") + || chain.contains("failed to lookup") + || chain.contains("name or service not known") + || chain.contains("nodename nor servname") + || chain.contains("no such host") + || chain.contains("name resolution") + || chain.contains("name not resolved"); + if is_dns { + return Some(( + format!("getaddrinfo ENOTFOUND {host}"), + "ENOTFOUND".to_string(), + "getaddrinfo".to_string(), + -3008, + )); + } + + // Text fallback when no `io::Error` surfaced but the reason is recognizable. + if chain.contains("connection refused") { + return Some(( + connect_message("ECONNREFUSED", &host, port), + "ECONNREFUSED".to_string(), + "connect".to_string(), + fallback_errno("ECONNREFUSED"), + )); + } + + None +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn host_port_parses_explicit_port() { + assert_eq!( + host_port("http://127.0.0.1:1/p"), + ("127.0.0.1".to_string(), Some(1)) + ); + } + + #[test] + fn host_port_defaults_known_scheme() { + assert_eq!( + host_port("http://example.com/"), + ("example.com".to_string(), Some(80)) + ); + } + + #[test] + fn connect_message_shapes() { + assert_eq!( + connect_message("ECONNREFUSED", "127.0.0.1", Some(1)), + "connect ECONNREFUSED 127.0.0.1:1" + ); + assert_eq!( + connect_message("ECONNREFUSED", "127.0.0.1", None), + "connect ECONNREFUSED 127.0.0.1" + ); + } + + #[test] + fn kind_mapping() { + assert_eq!( + kind_to_code(ErrorKind::ConnectionRefused), + Some(("ECONNREFUSED", "connect")) + ); + assert_eq!(kind_to_code(ErrorKind::NotFound), None); + } +} diff --git a/crates/perry-ffi/src/error.rs b/crates/perry-ffi/src/error.rs index 33903446cc..f33ac37d42 100644 --- a/crates/perry-ffi/src/error.rs +++ b/crates/perry-ffi/src/error.rs @@ -39,6 +39,18 @@ extern "C" { /// Runtime entry: pointer to a Buffer/TypedArray value's bytes (with /// length via `out_len`), or null for any other value. fn js_value_buffer_or_typedarray_data(bits: f64, out_len: *mut u32) -> *const u8; + + /// Runtime entry: build a Node-style system Error with `.message`, + /// `.code`, `.syscall` and `.errno`. + fn js_node_system_error_value( + msg_ptr: *const u8, + msg_len: usize, + code_ptr: *const u8, + code_len: usize, + syscall_ptr: *const u8, + syscall_len: usize, + errno: f64, + ) -> f64; } /// Which JS Error subclass [`throw_with_code`] raises. @@ -91,6 +103,28 @@ pub fn error_value_with_code(msg: &str, code: &str, kind: ErrorKind) -> JsValue JsValue::from_bits(value.to_bits()) } +/// Build a Node-style system `Error` value carrying `.message`, `.code` +/// (a Node `E*` string), `.syscall` (the failing syscall, e.g. `"connect"` +/// or `"getaddrinfo"`) and `.errno` (the libuv-negative number). This is the +/// shape Node hands to `socket`/`request` `'error'` listeners for transport +/// failures, so consumers branching on `err.code === 'ECONNREFUSED'` work. +pub fn system_error_value(msg: &str, code: &str, syscall: &str, errno: i64) -> JsValue { + // SAFETY: all three slices are valid for their lengths; the runtime copies + // the bytes into arena-owned storage before returning the error value. + let value = unsafe { + js_node_system_error_value( + msg.as_ptr(), + msg.len(), + code.as_ptr(), + code.len(), + syscall.as_ptr(), + syscall.len(), + errno as f64, + ) + }; + JsValue::from_bits(value.to_bits()) +} + /// Borrow the raw bytes of a `Buffer` or `TypedArray` value. Returns /// `None` for any value that is neither (the caller should raise a /// `TypeError` in that case). The borrow is valid for the duration of the diff --git a/crates/perry-ffi/src/lib.rs b/crates/perry-ffi/src/lib.rs index 7bd7d755ed..8d1a81ab5f 100644 --- a/crates/perry-ffi/src/lib.rs +++ b/crates/perry-ffi/src/lib.rs @@ -88,7 +88,9 @@ mod json; pub use json::json_stringify; mod error; -pub use error::{error_value_with_code, throw_with_code, value_byte_slice, ErrorKind}; +pub use error::{ + error_value_with_code, system_error_value, throw_with_code, value_byte_slice, ErrorKind, +}; mod event_pump; pub use event_pump::notify_main_thread; diff --git a/crates/perry-runtime/src/error.rs b/crates/perry-runtime/src/error.rs index 03a72fd995..04df1d385e 100644 --- a/crates/perry-runtime/src/error.rs +++ b/crates/perry-runtime/src/error.rs @@ -318,10 +318,61 @@ pub unsafe extern "C" fn js_throw_error_with_code( )) } +/// Build a Node-style system `Error`: `.message` + `.code` (the message→code +/// side table the `.code` getter reads) plus `.syscall` (string) and `.errno` +/// (number) own properties. `perry-ext-http` calls this to surface client +/// transport failures (`ECONNREFUSED`, `ENOTFOUND`, `ECONNRESET`, …) as the +/// real coded `Error` objects Node hands to `request.on('error')`, instead of +/// the bare message string the legacy path passed. +/// +/// The `.syscall`/`.errno` properties land in the `Error` expando side table +/// (`ERROR_USER_PROPS`) via [`crate::object::js_object_set_field_by_name`], +/// which recognizes `Error` cells by their NaN-box tag — `ErrorHeader` is not +/// an `ObjectHeader`, so a raw field write would corrupt it. +/// +/// # Safety +/// `msg_ptr`/`code_ptr`/`syscall_ptr` must each point to their stated number of +/// valid bytes, or be null with the matching length `0`. +#[no_mangle] +pub unsafe extern "C" fn js_node_system_error_value( + msg_ptr: *const u8, + msg_len: usize, + code_ptr: *const u8, + code_len: usize, + syscall_ptr: *const u8, + syscall_len: usize, + errno: f64, +) -> f64 { + let err_val = js_error_value_with_code(msg_ptr, msg_len, code_ptr, code_len, 0); + let obj = err_val.to_bits() as *mut crate::object::ObjectHeader; + if !syscall_ptr.is_null() && syscall_len > 0 { + let key = js_string_from_bytes(b"syscall".as_ptr(), 7) as *const StringHeader; + let sval_str = js_string_from_bytes(syscall_ptr, syscall_len as u32); + let sval = crate::value::js_nanbox_string(sval_str as i64); + crate::object::js_object_set_field_by_name(obj, key, sval); + } + { + let key = js_string_from_bytes(b"errno".as_ptr(), 5) as *const StringHeader; + crate::object::js_object_set_field_by_name(obj, key, errno); + } + err_val +} + // These FFI entries are referenced only from extension archives (linked after // the runtime's bitcode is optimized), so the auto-optimize LTO pass would // otherwise dead-strip them (see project_auto_optimize_keepalive_3320). The // `#[used]` anchors pin them. +#[used] +static KEEP_JS_NODE_SYSTEM_ERROR_VALUE: unsafe extern "C" fn( + *const u8, + usize, + *const u8, + usize, + *const u8, + usize, + f64, +) -> f64 = js_node_system_error_value; + #[used] static KEEP_JS_ERROR_VALUE_WITH_CODE: unsafe extern "C" fn( *const u8, diff --git a/test-parity/node-suite/http/client-request/transport-error.ts b/test-parity/node-suite/http/client-request/transport-error.ts new file mode 100644 index 0000000000..497f0a3840 --- /dev/null +++ b/test-parity/node-suite/http/client-request/transport-error.ts @@ -0,0 +1,40 @@ +// A failed `http.request` / `http.get` hands `request.on('error')` a real +// Node system `Error`, not a bare string: `.code` (`ECONNREFUSED` / +// `ENOTFOUND`), `.syscall` (`connect` / `getaddrinfo`), `.errno` (the +// libuv-negative number) and a `${syscall} ${CODE} ${detail}` message. +// Libraries branch on `err.code === 'ECONNREFUSED'`, so the prior bare-string +// surface broke every one of them. errno is read from the same OS error Node +// sees (this suite diffs Perry vs node on the one host), so it matches. +import http from "node:http"; + +function describe(label: string, e: any) { + console.log( + label, + [ + typeof e, + e && e.name, + e && e.message, + e && e.code, + e && e.syscall, + e && e.errno, + e instanceof Error, + ].join("|"), + ); +} + +// Port 1 is reserved/unused — a connect there is refused immediately. +const refused = http.get({ host: "127.0.0.1", port: 1, path: "/" }, () => {}); +refused.on("error", (e: any) => { + describe("refused:", e); + + // A DNS lookup failure: `.invalid` is reserved (RFC 6761) and never resolves. + const notfound = http.get( + { host: "does-not-exist.invalid", port: 80, path: "/" }, + () => {}, + ); + notfound.on("error", (e2: any) => { + describe("notfound:", e2); + }); +}); + +setTimeout(() => {}, 2000);