net(windows): surface the real WSA error from a failed connect()

[robobun]

On Windows, net.connect() reported ECONNREFUSED (or ENOENT for a path connect) for every failure, because the raw WSA code from Winsock was never mapped to the SystemErrno discriminant the connect-error whitelist compares against.

Repro

> bun test/js/node/test/parallel/test-net-client-bind-twice.js
AssertionError: actual 'ECONNREFUSED', expected 'EADDRINUSE'

> bun test/js/node/test/parallel/test-net-server-reset.js
AssertionError: actual 'ECONNREFUSED', expected 'ECONNRESET'

> bun test/js/node/test/parallel/test-net-pipe-connect-errors.js
AssertionError: received ENOENT instead of ENOTSOCK or ECONNREFUSED

All three pass on Linux/macOS.

Cause

Three places on the Windows connect path lose or mis-read the error:

• The async path (loop.c's SEMI_SOCKET getsockopt(SO_ERROR) read and context.c's recv probe) passes raw WSA codes (WSAECONNRESET = 10054, WSAEADDRINUSE = 10048) into handle_connect_error, whose whitelist is keyed on POSIX-numbered SystemErrno discriminants (104, 98). Nothing matched, so every async connect failure fell through to ECONNREFUSED.
• The synchronous path (connect_finish after do_connect() errs) read the CRT's thread-local _errno(), which Winsock never sets, so it always saw 0 and fell through to the default.
• bsd.c's AF_UNIX connect path discarded bsd_do_connect_raw's return value and let closesocket() overwrite WSAGetLastError.

Fix

• bsd.c: re-arm the WSA error with WSASetLastError after bsd_close_socket in both connect-failure tails, mirroring what the bind-failure path already does.
• Listener.rs connect_finish: read WSAGetLastError() (mapped via SystemErrno::init) on Windows instead of last_errno().
• socket_body.rs handle_connect_error: normalize any incoming value >= WSABASEERR through SystemErrno::init before the whitelist; remap every whitelisted code (not just ENOENT/ECONNREFUSED) to its UV_* value so err.errno matches Node.

Winsock AF_UNIX returns WSAECONNREFUSED for any path with no listening socket, existing or not, where Node (via libuv's uv_pipe_connect, which uses CreateFile) distinguishes ENOENT. connect_finish now refines ECONNREFUSED to ENOENT on the failure path when the target path does not exist, so a missing socket file still reports ENOENT while an existing non-socket file reports ECONNREFUSED (which the upstream test accepts).

Verification

Three Windows-only tests in test/js/node/net/node-net.test.ts cover the EADDRINUSE/ECONNRESET/ENOENT cases directly. The upstream parallel/test-net-client-bind-twice.js, parallel/test-net-server-reset.js, and parallel/test-net-pipe-connect-errors.js quarantines are removed.

parallel/test-net-pingpong.js is left quarantined: that failure is the separate named-pipe half-close gap (WindowsNamedPipe issues uv_close instead of uv_shutdown on end(), and on_read_error(EOF) tears down the writer), which is a larger pipe-writer change.

---

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/node/net/node-net.test.ts

[coderabbitai]

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 17 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 5d1fe76d-0786-4463-af23-42c02898155d

📥 Commits

Reviewing files that changed from the base of the PR and between 1b1efd0 and 2a6a33c.

📒 Files selected for processing (6)

• packages/bun-usockets/src/bsd.c
• packages/bun-usockets/src/loop.c
• src/runtime/socket/Listener.rs
• src/runtime/socket/socket_body.rs
• test/expectations.txt
• test/js/node/net/node-net.test.ts

---

_{Comment @coderabbitai help to get the list of available commands.}

[robobun]

Reproduced on windows-x64 with:

bun test/js/node/test/parallel/test-net-client-bind-twice.js     # ECONNREFUSED, expected EADDRINUSE
bun test/js/node/test/parallel/test-net-server-reset.js          # ECONNREFUSED, expected ECONNRESET
bun test/js/node/test/parallel/test-net-pipe-connect-errors.js   # ENOENT, expected ENOTSOCK|ECONNREFUSED

All three (plus the new node-net.test.ts describe block) pass with this change and still pass on Linux.

[claude]

No bugs found. This is a well-reasoned Windows-only fix, but it threads error-code semantics through three layers (usockets C, connect_finish, handle_connect_error) and un-quarantines three upstream Node tests, so it's worth a human look before landing.

What was reviewed:

• bsd.c: the two new WSASetLastError(rc) calls mirror the existing bind-failure path; rc is the WSA code returned by bsd_do_connect_raw, not a POSIX errno.
• Listener.rs: bun_sys::windows::WSAGetLastError() already routes through SystemErrno::init, so e is a discriminant that the existing whitelist below compares against; the ENOENT refinement only runs on the failure path.
• socket_body.rs: the >= 10000 gate is disjoint from both the CRT errno space and the SystemErrno discriminant space (max 137 + UV_ tail); every whitelisted code has a corresponding UV_* variant in windows_errno.rs.

Extended reasoning...

Overview

Fixes Windows net.connect() error reporting: raw WSA codes were reaching a whitelist keyed on POSIX-numbered SystemErrno discriminants, so every connect failure degraded to ECONNREFUSED/ENOENT. Touches packages/bun-usockets/src/bsd.c (re-arm WSA error after closesocket), src/runtime/socket/Listener.rs (connect_finish reads WSAGetLastError on Windows and refines ECONNREFUSED→ENOENT for missing AF_UNIX paths), and src/runtime/socket/socket_body.rs (handle_connect_error normalizes WSA codes ≥ WSABASEERR and remaps every whitelisted code to its UV_* value). Removes three Windows quarantines from expectations.txt and adds three Windows-only tests.

Security risks

None identified. This is error-code mapping on connect-failure paths; no auth, crypto, or untrusted-input parsing. The added bun_sys::exists() check on the AF_UNIX failure path is a benign TOCTOU (worst case: reports ECONNREFUSED instead of ENOENT if the file appears between the failed connect and the check).

Level of scrutiny

Moderate. Every code change is #ifdef _WIN32 / #[cfg(windows)]-gated, so POSIX behavior is untouched. But it changes error semantics across a C→Rust boundary in the socket connect path, un-quarantines three upstream Node tests, and adds a filesystem probe to an error path. Per the repo's cross-platform review guidance, platform-gated FFI changes warrant a human read.

Other factors

• I verified bun_sys::windows::WSAGetLastError() already maps via SystemErrno::init (windows/mod.rs:697-703), so the err as c_int cast in connect_finish yields the same discriminant space the existing whitelist compares against.
• I verified all seven UV_* variants used in the new match arm exist on the Windows SystemErrno (windows_errno.rs:36-53).
• The bsd.c change follows the exact pattern already used at the bind()-failure site in the same function.
• robobun confirmed all three previously-failing upstream tests plus the new describe block pass on windows-x64 and still pass on Linux.
• The PR description's root-cause analysis is precise and matches the code.

[claude]

No bugs found, but this touches Windows-specific error-code plumbing across the C↔Rust FFI boundary (bsd.c re-arming WSA error, cfg(windows) errno remapping, the exists() heuristic to split ECONNREFUSED→ENOENT), so it's worth a human look.

What was reviewed:

• Sign convention on the new e_discriminant_to_uv remap: -(-4078) → errno: -4078, matches the old ENOENT/ECONNREFUSED special-cases and extends to the full whitelist.
• WSASetLastError(rc) after bsd_close_socket mirrors the existing bind-failure path in the same file; rc is the WSA code returned by bsd_do_connect_raw.
• bun_sys::exists(path) refinement runs only after connect has already failed with ECONNREFUSED on a unix path — informational error-code selection, not check-then-act.
• Removed quarantines have direct new coverage in node-net.test.ts plus the un-quarantined upstream tests.

Extended reasoning...

Overview

Windows-only fix for net.connect() reporting the wrong error code on every failure. Three sites lost or misread the WSA error: bsd.c's connect-failure tail let closesocket() clobber WSAGetLastError; connect_finish read CRT _errno() (which Winsock never sets); and handle_connect_error's whitelist compared POSIX-numbered discriminants against raw WSA codes (10048 vs 98). The fix re-arms WSA error after close, reads WSAGetLastError() on Windows, and normalizes WSA codes ≥10000 through SystemErrno::init before the whitelist. The old two-case UV_* remap is generalized via e_discriminant_to_uv. Three upstream Node test quarantines are removed and three Windows-gated tests added.

Security risks

None. This is error-code presentation on an already-failed connect. The bun_sys::exists call refines ECONNREFUSED→ENOENT after failure for user-facing reporting only; it's not a security check-then-act.

Level of scrutiny

Medium-high. All runtime changes are #[cfg(windows)] / #ifdef _WIN32 gated, so POSIX behavior is unchanged. But it's platform-gated native code across the C↔Rust FFI boundary, and per the repo's cross-platform review guidance that warrants a maintainer look. Two spots a human might want to weigh in on: (1) the hardcoded >= 10000 for WSABASEERR rather than a named constant, and (2) whether the exists() refinement belongs in connect_finish or should live closer to the AF_UNIX connect in bsd.c.

Other factors

The PR is well-documented with a clear repro and mechanism trace. robobun confirmed all three upstream tests plus the new describe block pass on windows-x64 and still pass on Linux. The comment-cop bot flagged long comments on an earlier revision; those threads are marked resolved and the current diff's comments are concise. The loop.c change (WSAECONNRESET fallback) makes the SEMI_SOCKET fallback consistent with the WSA numbering the rest of the Windows path now expects.