From 05cc773202c05e9de347890f244c6397ff6b2fcc Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 14 Aug 2026 17:40:28 +0000 Subject: [PATCH] udp: report a failed connect() as the socket error, not a getaddrinfo error us_udp_socket_connect returns -1 when connect(2) fails and a getaddrinfo status when the name does not resolve. The connect option handled the -1 through a helper that returned nothing on Windows, so the value fell through to the getaddrinfo mapping and every connect(2) failure there was reported as ENOTFOUND. The connect() method node:dgram uses threw a bare "Failed to connect socket" on every platform and ignored getaddrinfo failures altogether. Both paths now go through one check: -1 reads errno (WSAGetLastError on Windows), anything else non-zero is a getaddrinfo code. bsd_connect_udp_socket captures the connect() error before freeaddrinfo() so that is what the caller reads. node:dgram decorates the error like Node's ExceptionWithHostPort ("connect EAFNOSUPPORT ::1:1", .address, .port), sharing the helper the send path already used. --- packages/bun-usockets/src/bsd.c | 23 ++++-- src/js/node/dgram.ts | 14 ++-- src/runtime/socket/udp_socket.rs | 112 ++++++++++------------------- test/js/bun/udp/dgram.test.ts | 48 +++++++++++++ test/js/bun/udp/udp_socket.test.ts | 49 +++++++++++-- 5 files changed, 155 insertions(+), 91 deletions(-) diff --git a/packages/bun-usockets/src/bsd.c b/packages/bun-usockets/src/bsd.c index 35c1ae1bf461..325460512a87 100644 --- a/packages/bun-usockets/src/bsd.c +++ b/packages/bun-usockets/src/bsd.c @@ -1828,8 +1828,11 @@ LIBUS_SOCKET_DESCRIPTOR bsd_create_udp_socket(const char *host, int port, int op return listenFd; } +/* Returns 0, a getaddrinfo() error code, or LIBUS_SOCKET_ERROR with the last + * connect() error in errno (WSAGetLastError() on Windows), captured before + * freeaddrinfo() can clobber it. An empty result list reports EAFNOSUPPORT. */ int bsd_connect_udp_socket(LIBUS_SOCKET_DESCRIPTOR fd, const char *host, int port) { - struct addrinfo hints, *result; + struct addrinfo hints, *result = NULL; memset(&hints, 0, sizeof(struct addrinfo)); hints.ai_family = AF_UNSPEC; @@ -1844,18 +1847,28 @@ int bsd_connect_udp_socket(LIBUS_SOCKET_DESCRIPTOR fd, const char *host, int por return gai_error; } - if (result == NULL) { - return -1; - } +#ifdef _WIN32 + int last_error = WSAEAFNOSUPPORT; +#else + int last_error = EAFNOSUPPORT; +#endif for (struct addrinfo *rp = result; rp != NULL; rp = rp->ai_next) { if (connect(fd, rp->ai_addr, rp->ai_addrlen) == 0) { freeaddrinfo(result); return 0; } + last_error = LIBUS_ERR; } - freeaddrinfo(result); + if (result != NULL) { + freeaddrinfo(result); + } +#ifdef _WIN32 + WSASetLastError(last_error); +#else + errno = last_error; +#endif return (int)LIBUS_SOCKET_ERROR; } diff --git a/src/js/node/dgram.ts b/src/js/node/dgram.ts index 2324ae3ef9bc..9f8cfaa11478 100644 --- a/src/js/node/dgram.ts +++ b/src/js/node/dgram.ts @@ -843,7 +843,7 @@ function doConnect(ex, self, ip, address, port, callback) { try { connectFn.$call(state.handle.socket, ip, port); } catch (e) { - ex = e; + ex = decorateHostPortError(e, "connect", address, port); } } @@ -1069,7 +1069,7 @@ function doSend(ex, self, ip, list, address, port, callback) { else err = state.handle.send(req, list, list.length, !!callback); if (typeof err !== "number") { - if (callback) process.nextTick(callback, decorateSendError(err, address, port)); + if (callback) process.nextTick(callback, decorateHostPortError(err, "send", address, port)); return; } @@ -1100,21 +1100,21 @@ function afterQueuedSend(err, sent) { } else if (typeof err === "number") { callback(new ExceptionWithHostPort(err, "send", this.address, this.port)); } else { - callback(decorateSendError(err, this.address, this.port)); + callback(decorateHostPortError(err, "send", this.address, this.port)); } } -// Native send failure: keep the original error (its code is already +// Native send/connect failure: keep the original error (its code is already // platform-correct) and decorate it like Node's ExceptionWithHostPort, which // omits the host segment when no address/port is known. -function decorateSendError(err, address, port) { - err.syscall = "send"; +function decorateHostPortError(err, syscall, address, port) { + err.syscall = syscall; err.address = address; if (port) err.port = port; let details = ""; if (port && port > 0) details = ` ${address}:${port}`; else if (address) details = ` ${address}`; - err.message = `send ${err.code}${details}`; + err.message = `${syscall} ${err.code}${details}`; return err; } diff --git a/src/runtime/socket/udp_socket.rs b/src/runtime/socket/udp_socket.rs index c5575d34bdf6..5effd1478ef2 100644 --- a/src/runtime/socket/udp_socket.rs +++ b/src/runtime/socket/udp_socket.rs @@ -30,46 +30,6 @@ use crate::socket::socket_address::inet::{self, INET6_ADDRSTRLEN, sockaddr_in, s bun_output::declare_scope!(UdpSocket, visible); -/// Local errno-classification shim — `bun_sys::Result` -/// is a plain `core::result::Result` alias in Rust and has no associated -/// `errno_sys` constructor. -/// -/// POSIX `getErrno(c_int)` semantics: only `rc == -1` is failure (any other -/// value — including positive packet counts from `us_udp_socket_send` and -/// negative EAI codes from `connect` — is "not a libc errno", so callers -/// handle it themselves). -/// -/// Windows semantics: for any non-NTSTATUS -/// integer `rc`, `if (rc != 0) return null` — i.e. *every* non-zero rc -/// (including `-1` / `SOCKET_ERROR`) falls through to the caller's own EAI / -/// WSA handling rather than synthesising an errno. This matters for the -/// `connect()` path (line ~575): `us_udp_socket_connect()` returns a Winsock -/// status, not a CRT errno, so reading `_errno()` here would be the wrong -/// source. -#[inline] -fn errno_sys(rc: c_int, tag: bun_sys::Tag) -> Option { - #[cfg(windows)] - { - if rc != 0 { - return None; - } - // rc == 0 → read the CRT `_errno()`; - // a zero errno must still yield `None`. - let errno_val = bun_sys::last_errno(); - if errno_val == 0 { - return None; - } - return Some(bun_sys::Error::from_code_int(errno_val, tag)); - } - #[cfg(not(windows))] - { - if rc != -1 { - return None; - } - Some(bun_sys::Error::from_code_int(bun_sys::last_errno(), tag)) - } -} - use bun_core::ip_address; use bun_sys::net::Address; @@ -762,22 +722,7 @@ impl UDPSocket { // `Socket` is an `opaque_ffi!` ZST — `opaque_mut` is the safe deref. let ret = uws::udp::Socket::opaque_mut(this.socket.get().unwrap()) .connect(address_z.as_ptr(), connect.port as u32); - if ret != 0 { - if let Some(sys_err) = errno_sys(ret, bun_sys::Tag::connect) { - return Err(global_this.throw_value(sys_err.to_js(global_this))); - } - - if let Some(eai_err) = c_ares::Error::init_eai(ret) { - return Err(global_this.throw_value( - crate::dns_jsc::cares_jsc::error_to_js_with_syscall_and_hostname( - eai_err, - global_this, - b"connect", - address_z.as_bytes(), - )?, - )); - } - } + check_connect(global_this, ret, address_z.as_bytes())?; this.connect_info .set(Some(ConnectInfo { port: connect.port })); } @@ -1916,9 +1861,8 @@ impl UDPSocket { return Err(global_this.throw(format_args!("Socket is closed"))); }; // `Socket` is an `opaque_ffi!` ZST — `opaque_mut` is the safe deref. - if uws::udp::Socket::opaque_mut(socket).connect(connect_host.as_ptr(), port as u32) == -1 { - return Err(global_this.throw(format_args!("Failed to connect socket"))); - } + let ret = uws::udp::Socket::opaque_mut(socket).connect(connect_host.as_ptr(), port as u32); + check_connect(global_this, ret, connect_host.as_bytes())?; this.connect_info.set(Some(ConnectInfo { port })); js::address_set_cached(call_frame.this(), global_this, JSValue::ZERO); @@ -2529,15 +2473,22 @@ struct Destination { address: JSValue, } +/// `Some` when a usockets call returned its failure status: `-1` on POSIX +/// (other values, such as `us_udp_socket_send`'s packet count, are results), +/// any negative value on Windows. fn get_us_error(res: c_int, tag: bun_sys::Tag) -> Option { #[cfg(windows)] - { - // setsockopt returns 0 on success, but errnoSys considers 0 to be failure on Windows. - // This applies to some other usockets functions too. - if res >= 0 { - return None; - } + let failed = res < 0; + #[cfg(not(windows))] + let failed = res == -1; + failed.then(|| last_socket_error::(tag)) +} +/// The error the socket call that just failed left behind: `WSAGetLastError()` +/// on Windows (the CRT errno when Winsock has nothing), `errno` elsewhere. +fn last_socket_error(tag: bun_sys::Tag) -> bun_sys::Error { + #[cfg(windows)] + { if USE_WSA { // The wrapper (src/sys/windows/mod.rs) already maps `SystemErrno` // → `E` for us, so `e` is `bun_sys::E` here. @@ -2547,17 +2498,32 @@ fn get_us_error(res: c_int, tag: bun_sys::Tag) -> Option JsResult<()> { + let error = match ret { + 0 => return Ok(()), + -1 => last_socket_error::(bun_sys::Tag::connect).to_js(global_this), + eai => crate::dns_jsc::cares_jsc::error_to_js_with_syscall_and_hostname( + // `init_eai` is only `None` for 0, matched above. + c_ares::Error::init_eai(eai).unwrap_or(c_ares::Error::ENOTFOUND), + global_this, + b"connect", + address, + )?, + }; + Err(global_this.throw_value(error)) } diff --git a/test/js/bun/udp/dgram.test.ts b/test/js/bun/udp/dgram.test.ts index 8ec119523004..db9c913c9205 100644 --- a/test/js/bun/udp/dgram.test.ts +++ b/test/js/bun/udp/dgram.test.ts @@ -467,6 +467,54 @@ test.skipIf(isWindows)("connected send() failure reports Node's error shape", as }); }); +// A udp4 socket cannot be connected to an IPv6 address: connect(2) itself +// fails, on every platform and without touching the network. Node reports +// that as an ExceptionWithHostPort; this used to be a bare "Failed to connect +// socket" with no code. Linux and Windows reject the address family, the BSDs +// may reject the sockaddr length first. +const connectFamilyMismatchCodes = ["EAFNOSUPPORT", "EINVAL"]; + +test("connect() failure reports Node's error shape to the callback", async () => { + const socket = createSocket("udp4"); + try { + const err: any = await new Promise(resolve => socket.connect(1, "::1", resolve)); + expect(err).toBeInstanceOf(Error); + expect(err.code).toBeOneOf(connectFamilyMismatchCodes); + expect(err.errno).toBeNegative(); + expect({ syscall: err.syscall, address: err.address, port: err.port, message: err.message }).toEqual({ + syscall: "connect", + address: "::1", + port: 1, + message: `connect ${err.code} ::1:1`, + }); + // Like Node, a failed connect() leaves the socket disconnected and usable. + await new Promise((resolve, reject) => socket.connect(1, "127.0.0.1", e => (e ? reject(e) : resolve()))); + expect(socket.remoteAddress()).toEqual({ address: "127.0.0.1", family: "IPv4", port: 1 }); + } finally { + socket.close(); + } +}); + +test("connect() failure without a callback is emitted as 'error' in Node's shape", async () => { + const socket = createSocket("udp4"); + try { + const { promise, resolve } = Promise.withResolvers(); + socket.on("error", resolve); + socket.on("connect", () => resolve(new Error("connected unexpectedly"))); + socket.connect(1, "::1"); + const err = await promise; + expect(err.code).toBeOneOf(connectFamilyMismatchCodes); + expect({ syscall: err.syscall, address: err.address, port: err.port, message: err.message }).toEqual({ + syscall: "connect", + address: "::1", + port: 1, + message: `connect ${err.code} ::1:1`, + }); + } finally { + socket.close(); + } +}); + // Every send callback must fire with (null, byteLength) — never (null, 0) or // EAGAIN — even if the kernel refuses some writes (they queue and drain like // libuv's uv_udp_try_send → uv_udp_send fallback). diff --git a/test/js/bun/udp/udp_socket.test.ts b/test/js/bun/udp/udp_socket.test.ts index 7ba051675ecb..19ffe8784b03 100644 --- a/test/js/bun/udp/udp_socket.test.ts +++ b/test/js/bun/udp/udp_socket.test.ts @@ -78,12 +78,49 @@ describe("udpSocket()", () => { expect(exitCode).toBe(0); }); - test("connect with invalid hostname rejects", async () => { - expect(async () => - udpSocket({ - connect: { hostname: "example!!!!!.com", port: 443 }, - }), - ).toThrow(); + // The native connect returns -1 when connect(2) fails and a getaddrinfo code + // when the name does not resolve. On Windows the -1 used to be run through + // the getaddrinfo mapping, which reported every connect(2) failure as a DNS + // ENOTFOUND for an address that had resolved fine. + describe("connect option failures", () => { + async function connectError(connect: { hostname: string; port: number }): Promise { + let socket; + try { + socket = await udpSocket({ connect }); + } catch (error) { + return error; + } + socket.close(); + throw new Error(`connecting to ${connect.hostname} unexpectedly succeeded`); + } + + test("connect with invalid hostname rejects with a DNS error", async () => { + const error = await connectError({ hostname: "example!!!!!.com", port: 443 }); + expect(error.name).toBe("DNSException"); + expect(error.syscall).toBe("connect"); + expect(error.message).toBe(`connect ${error.code} example!!!!!.com`); + }); + + test("the default (IPv4) socket cannot connect to an IPv6 address", async () => { + const error = await connectError({ hostname: "::1", port: 1 }); + expect(error.name).toBe("Error"); + // Linux and Windows reject the address family; the BSDs may reject the + // sockaddr length first. + expect(error.code).toBeOneOf(["EAFNOSUPPORT", "EINVAL"]); + expect(error.errno).toBeNegative(); + expect(error.syscall).toBe("connect"); + }); + + // Winsock refuses INADDR_ANY as a destination (POSIX kernels rewrite it to + // loopback), so this is a second, different connect(2) error on Windows: + // the reported code comes from the failed call, not from a fixed mapping. + test.if(isWindows)("connecting to 0.0.0.0 reports EADDRNOTAVAIL", async () => { + const error = await connectError({ hostname: "0.0.0.0", port: 1 }); + expect(error.name).toBe("Error"); + expect(error.code).toBe("EADDRNOTAVAIL"); + expect(error.errno).toBeNegative(); + expect(error.syscall).toBe("connect"); + }); }); // Out-of-range connect.port used to be silently rewritten to 0, so send()