Conversation
bsd_create_udp_socket() wrote the negated getaddrinfo() return code into
the errno out-parameter when the host failed to resolve, so
Bun.udpSocket({ hostname }) reported whichever errno shares that number:
"bind ENOENT <host>" for EAI_NONAME on glibc, "bind ESRCH" for EAI_AGAIN,
"bind ENOEXEC" on macOS.
Give bsd_create_udp_socket() / us_create_udp_socket() a separate
dns_error out-parameter carrying the raw getaddrinfo() code, the same
channel the TCP connect and listen paths use, and have udp_socket.rs map
it with c_ares::Error::init_eai into the resolver error Bun.connect and
fetch() already produce: "getaddrinfo ENOTFOUND <host>" with code,
syscall and hostname set. Genuine socket/bind errno failures are
reported as before.
|
Warning Review limit reached
Next review available in: 1 minute Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (9)
Comment |
|
Updated 6:47 PM PT - Aug 11th, 2026
✅ @robobun, your commit 780fddc85c75d58a3191ac5ea740b34bc4df6dac passed in 🧪 To try this PR locally: bunx bun-pr 37707That installs a local version of the PR into your bun-37707 --bun |
|
Status Reproduced on the released build (1.4.0) and on main: Test: CI build 92716 is green on every lane (the only retries were unrelated install/napi tests). Review turned up nothing that needs a code change. Ready for a maintainer. |
There was a problem hiding this comment.
I reviewed this PR and didn't find any bugs. Because it threads an FFI signature change across the C/Rust boundary (bsd.c → udp.c → uws_sys → two Rust callers) and changes user-visible error shapes, a human look would still be worthwhile.
What was reviewed:
- Verified every
bsd_create_udp_socket/us_create_udp_socket/uws::udp::Socket::createcall site is updated for the newdns_errorout-param (quic.c passes NULL, endpoint.rs and udp_socket.rs pass a local). - Checked
init_eai(0)returnsNoneon all platforms and the Windows arm's_ => ENOTFOUNDcatch-all handles raw WSA getaddrinfo codes; the.filter(|_| dns_error != 0)is redundant but harmless. - Confirmed the
drop(hostname_z)removal is required (now read in the new error branch) andZBox::as_bytes()excludes the NUL. - The new tests use a 64-byte DNS label (no network round-trip) and bind without reuse flags, so both should be hermetic.
Extended reasoning...
Overview
This PR fixes UDP bind error reporting when getaddrinfo() fails: previously the raw EAI_* code was written to the errno out-param and misreported as bind ENOENT/bind ESRCH/bind ENOEXEC depending on platform. The fix adds a separate int *dns_error out-parameter to bsd_create_udp_socket / us_create_udp_socket, threads it through the Rust FFI wrapper, and has UDPSocket::udp_socket map it via c_ares::Error::init_eai + system_error_with_syscall_and_hostname — the exact pattern already used by Bun.connect, fetch(), and (per #37690) Bun.listen. Nine files touched: 5 in bun-usockets (C), 3 Rust files, plus tests.
Security risks
None identified. This is error-message shaping; no new input parsing, no changes to what gets bound or resolved. The unresolvable-hostname test uses an RFC-1035-illegal label so resolution fails locally without touching the network.
Level of scrutiny
Medium. The change is mechanically straightforward and follows a precedent set twice before in this codebase, but it is an FFI ABI change coordinated across C headers, C implementations, and Rust extern declarations — a mismatch here would be a silent miscompile. I grepped every caller of both functions and the Rust wrapper and confirmed all are updated with matching arity/position. The quic.c callers correctly pass NULL (they bind IP literals or ignore the error), and endpoint.rs sensibly falls back to dns_error for the close status so a resolver-level failure there doesn't report status 0.
Other factors
- The removed
drop(hostname_z)is intentional: the new DNS-error branch readshostname_z.as_bytes()to populate thehostnamefield, so it must survive pastSocket::create.ZBox::as_bytes()excludes the trailing NUL, so the reported hostname is clean. init_eai(0)returnsNoneon both the Windows and POSIX arms, so the.filter(|_| dns_error != 0)is belt-and-suspenders; the fd-adoption path (which never touchesdns_error) can't accidentally take the DNS branch.- Tests cover both the new resolver-error shape and pin the unchanged
bind EADDRINUSEerrno shape. The EADDRINUSE test binds two sockets to the same 127.0.0.1:port with no reuse flags, which should fail deterministically; if it turns out flaky on a particular CI lane that would be worth a second look, but it looks sound. - Given the cross-language ABI coordination and the user-visible behavior change, deferring to a human reviewer rather than auto-approving.
|
Heads-up from #40986, which adds a If #40986 lands first, the 64-byte label this test uses is answered in-process with The two changes are independent otherwise: #40986 does not touch |
#40986) ### Problem - `Bun.serve`, `Bun.listen` and `Bun.udpSocket` (bind and connect) resolve their hostname with a synchronous `getaddrinfo` in uSockets (`packages/bun-usockets/src/bsd.c:1340`, `:1733`, `:1818`), even for a name that can never be a hostname. Some resolvers never answer such a query, and the call blocks the JS thread (#40970 saw this on the connect side). - Its error is also wrong: `Bun.serve({ hostname: "this is not a hostname" })` throws `ENOENT: no such file or directory, listen`, `Bun.listen` a bare `Failed to listen at <host>`, `Bun.udpSocket` `bind ENOENT <host>`, and `dgram`'s `connect()` reports success. ### Fix - Each path checks the hostname with `bun_dns::is_valid_hostname` before it calls into uSockets. A rejected name throws `getaddrinfo ENOTFOUND <host>` with `code`, `syscall` and `hostname`: what `Bun.connect` reports for it and what Node emits from `server.listen()`. - `Bun.serve` now strips brackets only around an IPv6 literal: `"[::1]"` still binds, `"[foo]"` no longer resolves as `foo`. - A well-formed name that does not resolve (`nxdomain.invalid`) still gets the wrong errno. #37690 (serve/listen) and #37707 (udp) fix that. This PR does not touch `bsd.c` or fix #25765. - Verified: 18 new cases in `test/js/bun/http/serve-listen.test.ts`, `test/js/bun/net/socket-dns-error.test.ts`, `test/js/bun/udp/udp_socket.test.ts` and `test/js/bun/udp/dgram.test.ts` fail on 1.4.1 and pass here. Also ran the serve, socket, net and dgram suites. ### Background - `is_valid_hostname` (`src/dns/lib.rs`) accepts an IP literal or an RFC 1035 name (labels of 1 to 63 bytes from the c-ares set, 253 total). - Node resolves with `dns.lookup` before it binds, so no name reaches a blocking call. - Self-reviewed: 2 concerns, both addressed (an unchecked udp `connect` path; the framing is now the resolver skip). <details><summary>Notes</summary> What is and is not shown here: - The blocking resolver is not observed in this PR. It is the same libc `getaddrinfo` call, on the same names, that #40970 measured on the connect side (`dns.lookup` waiting out the macOS resolver timeout for a label with a space). Linux glibc rejects these names locally, so on Linux the visible change is only the error. - No user report exists for a malformed bind hostname. #25765 (`something.localhost` on macOS) is a well-formed name and is #37690's case. Repro on 1.4.1 (Linux, no resolver needed for the first four names): ``` serve "this is not a hostname" => ENOENT listen "ENOENT: no such file or directory, listen" listen "this is not a hostname" => (no code) "Failed to listen at this is not a hostname" udpSocket "this is not a hostname" => ENOENT bind "bind ENOENT this is not a hostname" udpSocket connect { hostname: "this is not a hostname" } => DNSException "connect ENOTFOUND this is not a hostname" dgram socket.connect(1234, "this is not a hostname") via a custom lookup => no error, socket marked connected serve "aaaa...(64).com" => EMSGSIZE listen serve "nxdomain.invalid" => EAGAIN listen (left to #37690) udpSocket "nxdomain.invalid" => ESRCH bind (left to #37707) ``` After this change the first six report `getaddrinfo ENOTFOUND <host>` (`name: "Error"`, `code: "ENOTFOUND"`, `syscall: "getaddrinfo"`, `hostname`). `node:net`, `node:http` and `node:dgram` (custom `lookup`) servers emit the same error object from `listen()` / `bind()`. Where the check sits: - `Bun.serve`: at the top of `NewServer::listen` (`src/runtime/server/mod.rs`), before the uws app is created. The existing `deinit` path frees the server and `Bun.serve` throws synchronously, as it does for `EADDRINUSE`. The bracket strip now lives in one function, `strip_ipv6_brackets`, used by the check and by the existing `Addr` code. It used to drop the first and last byte of any hostname that starts with `[`, so `[foo]` resolved as `foo` (`[localhost]`, `[127.0.0.1]` and `[example.com` were already rejected by the base URL parse). It now strips only when the inside is an IPv6 literal (`to_ip_address`, so a `%zone` still passes); anything else keeps its brackets and fails `is_valid_hostname`. - `Bun.listen` (`src/runtime/socket/Listener.rs`): right after `SocketConfig::from_js`, only when a port is set (a unix path or an adopted fd has no hostname). `SocketConfig::from_js` itself is shared with `Bun.connect`, whose error is delivered asynchronously, so the check is not placed there. - `Bun.udpSocket` (`src/runtime/socket/udp_socket.rs`): bind and `connect.hostname` are both checked right after the config parse, before the socket is created. Bind failures there throw synchronously today, and so does this. The `connect` option used to report these names as `connect ENOTFOUND <host>` (`DNSException`) from the `EAI_*` code; a well-formed unresolvable `connect.hostname` still does, that block is #38622's. - The internal `UDPSocket.jsConnect` that `node:dgram`'s `socket.connect()` calls gets the same check. `dgram` resolves with `dns.lookup` first, so only a custom `lookup` can hand it a raw name. glibc rejected such a name with a code `jsConnect` never read (it checks for `-1` only), so the socket was marked connected with no error. #38622 reworks that return-code handling; the check here sits before the call and does not overlap it. The error is built by `not_a_hostname_error` in `src/runtime/dns_jsc/cares_jsc.rs` from `system_error_with_syscall_and_hostname`, the helper `Bun.connect` uses. For #37690 and #37707: their tests use a 64-byte label, which `is_valid_hostname` rejects, so once this lands that input is answered in-process and no longer exercises the `dns_error` plumbing. glibc rejects `a*b.com`, `a/b.com`, `-a.com` and `ünicode.com` locally (`EAI_NONAME`, no query sent) while `is_valid_hostname` accepts all four. Other platforms need checking in CI. `Bun.listen({ hostname: "[::1]" })` and `Bun.udpSocket({ hostname: "[::1]" })` never stripped brackets and already fail on 1.4.1 (`Failed to listen at [::1]`, `bind ENOENT [::1]`). They now fail with `getaddrinfo ENOTFOUND [::1]`. Suites run on the debug build: `serve-listen.test.ts` (37 pass), `socket-dns-error.test.ts` (11 pass), `udp_socket.test.ts` (214 pass), `dgram.test.ts` + `udp_socket_recv_flags.test.ts` (64 pass), `serve.test.ts` + `serve-http3.test.ts` + `server-url-invalid.test.ts` (349 pass, 2 environment failures: root can bind port 1003, egress proxy), `node-net-server.test.ts`, `serve-epoll-add-fail.test.ts`, `test/internal/source-lints/` (171 pass), node `test-dgram-bind*`, `test-dgram-connect*`, `test-dgram-custom-lookup`, `test-dgram-error-message-address`, `test-net-listen-error`, `test-http-listening`. `socket.test.ts` and `node-net.test.ts` have the same `localhost` dual-stack failures on the released bun in this container. </details> <!-- robobun:evidence:begin --> --- **no test proof** · iteration 0 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/bun/udp/udp_socket.test.ts, test/js/bun/udp/dgram.test.ts, test/js/bun/http/serve-listen.test.ts <!-- robobun:evidence:end -->
Problem
Bun.udpSocket({ hostname })with a hostname that does not resolve rejects with an errno-shaped error:bind ENOENT <host>on glibc,bind ESRCH <host>with no resolver reachable,bind ENOEXEC <host>on macOS. Node reportsgetaddrinfo ENOTFOUND <host>.EAI_*value getaddrinfo returned and on the platform's sign for it, so one failure is reported differently per OS and per resolver state.Fix
dns_errorout-parameter and the errno parameter stays 0 on that path, so a resolver code can no longer be read as an errno.Bun.udpSocketmaps it the wayBun.connectandfetch()already map a failed lookup (Report DNS lookup failures from fetch() and Bun.connect as ENOTFOUND #32990; Report an unresolvable listen hostname as a getaddrinfo error #37690 does the same forBun.listen/Bun.serve) and throwsgetaddrinfo ENOTFOUND <host>withsyscallandhostnameset, as Node does. Real bind failures (bind EADDRINUSE <host>with.address) and{ fd }adoption are unchanged.node:quic, which usesdns_erroras its close status when the errno is 0.node:dgramresolves throughdns.lookupfirst, so it only reaches this branch via a customlookupreturning a non-IP string.bind ENOENTshape and passes with this change; a second pins theEADDRINUSEshape. CI is green on glibc, musl, macOS and Windows. The numericerrnostill differs from Node and is deliberately not asserted.Background
getaddrinfo(3)returns its ownEAI_*codes, not errno values; they are negative on glibc (EAI_NONAME= -2) and positive on macOS (EAI_NONAME= 8), so the same integer in the errno channel names a different errno on each platform.packages/bun-usockets) is the C layer under Bun's sockets. It reports failures to Rust throughint *out-parameters, and Rust turns an errno into the JScodestring viaSystemErrno.c_ares::Error::init_eaiis Bun's shared mapping from a raw getaddrinfo code to the lookup error it reports elsewhere (ENOTFOUNDand friends; on Windows it folds WSA codes intoENOTFOUND). A separatedns_errorchannel fed into it is howBun.connectandfetch()already report lookup failures.Original description
Repro
Before (Linux, 1.4.0 and main):
The code depends on which
EAI_*value getaddrinfo returned:EAI_NONAME(-2 on glibc) comes out asENOENT,EAI_AGAIN(-3, e.g. no resolver reachable) asbind ESRCH <host>. On macOS the constants are positive (EAI_NONAME= 8), so the negated value is abs()ed back intoENOEXEC.After:
Cause
bsd_create_udp_socket()inpackages/bun-usockets/src/bsd.cdid*err = -gai_result;whengetaddrinfo()failed. Every other write toerrin that function is an errno, and the consumer (UDPSocket::udp_socketinsrc/runtime/socket/udp_socket.rs) maps it withSystemErrno::initand reportssyscall: "bind", so a resolver return code was being named as whatever errno shares its number.Fix
bsd_create_udp_socket()/us_create_udp_socket()gain a separateint *dns_errorout-parameter that receives the rawgetaddrinfo()return code (errstays 0 on that path), threaded throughuws::udp::Socket::create.udp_socket.rsmaps it withc_ares::Error::init_eaiand builds the error withsystem_error_with_syscall_and_hostname(.., "getaddrinfo", hostname). The errno path (bind EADDRINUSE <host>with.address,open <code>for{ fd }adoption) is unchanged.Why this shape: it is the one Bun already uses for a failed lookup everywhere else.
Bun.connectandfetch()carry the raw code on a separatedns_errorchannel and map it withinit_eai(#32990), #37690 does the same forBun.listen/Bun.serve, and this PR is the UDP bind counterpart. It is also what Node reports for the same input:dgram.createSocket("udp4").bind({ address: host })on Node 26.3 emitsgetaddrinfo ENOTFOUND <host>withcode: "ENOTFOUND",syscall: "getaddrinfo",hostnameset and noaddress, both for the 64-byte label and for name-shaped strings such as"256.256.256.256"(getaddrinfo treats those as names too, so they take the same new branch). The remaining difference from Node is the numericerrno(Node uses the libuv code, Bun the same c-ares valueBun.connect/fetch()report), which the tests deliberately do not pin. Only the errno path is platform-specific on purpose: on Windowsinit_eaifolds the raw WSA codes intoENOTFOUND, exactly as it already does forBun.connect/fetch().node:dgramis not affected in practice: with the defaultlookupit resolves throughdns.lookupand handsBun.udpSocketan IP, so this branch is only reachable through a customlookupthat yields a non-IP string, and that case now gets the resolver error instead of the mis-named errno.Other
us_create_udp_socketcallers: the three inquic.cnever read the error and passNULL;node:quic'sensure_bound()always binds an IP literal, and now falls back todns_errorfor the close status so a (memory/system) resolver failure there still reports a nonzero status instead of 0.Not changed here: the UDP
connectpath already reports a resolver error (connect ENOTFOUND <host>, nameDNSException); the existing synchronous throw vs. rejection behaviour is #35217's subject.Verification
New tests in
test/js/bun/udp/udp_socket.test.ts(bind failure errors): the unresolvable-hostname case fails on the released build with thebind ENOENTshape above and passes with this change; the second test pins thebind EADDRINUSE 127.0.0.1/.addressshape of a real bind failure.CI is green on every lane, so the
ENOTFOUNDexpectation holds for the libcgetaddrinfoon macOS, Windows and musl as well as glibc.