fix(node:http): keep listener open in Server.closeAllConnections

[mxschmitt]

Summary

Per the Node docs, Server.prototype.closeAllConnections() is documented (since v18.2.0) to forcefully close every established HTTP(S) connection while leaving the listening socket open so subsequent connections can still be accepted. Bun's wrapper called server.stop(true), which goes through stop_listening and tears down the listener — so any caller that relied on the documented contract got ECONNREFUSED on the next request.

The most user-visible victim is Playwright's TestServer.reset(), which calls _server.closeAllConnections() between tests to drop straggling sockets and assumes the listener stays up. Under Bun every test after the first failed with ECONNREFUSED. With this fix the relevant Playwright tests/page/page-click.spec.ts chromium-page suite goes from 0/87 → 82/87 passing on a debug build (the remaining 3 are an unrelated Error.stack divergence).

Filed as #31301.

What changed

A dedicated closeAllConnections path was added through every layer parallel to the existing closeIdleConnections plumbing, then the JS wrapper was switched to delegate to it without touching serverSymbol, kConnectionsCheckingInterval, or listening (those belong to close()):

• packages/bun-uws/src/App.h — TemplatedApp::closeAllConnections(), walking httpContext->getSocketGroup()->head_sockets and us_socket_close()-ing each (mirror of closeIdle() minus the isIdle check; does not touch the listener).
• src/uws_sys/libuwsockets.cpp — uws_app_close_all_connections C shim (SSL + non-SSL).
• src/uws_sys/App.rs — App::close_all_connections Rust binding + extern decl.
• src/runtime/server/server_body.rs — close_all_connections host fn next to close_idle_connections.
• src/runtime/server/server.classes.ts — registration.
• src/js/node/_http_server.ts — Server.prototype.closeAllConnections now calls server.closeAllConnections?.() and nothing else.

Related

• Closes node:http: Server.closeAllConnections() shuts down the listening socket #31301.
• Adjacent issue: getTokenInteractive in @azure/msal-node will cause Bun to hang after the end of the script #30501 (@azure/msal-node hang). PR fix(http): preserve server reference across close() for closeAllConnections() #30505 targets that scenario but keeps the wrong server.stop(true) call in closeAllConnections. The two fixes are complementary — this PR addresses the listener-teardown bug; fix(http): preserve server reference across close() for closeAllConnections() #30505 addresses the msal close-then-closeAllConnections sequencing. Reviewers may want to converge them.

Test plan

• New regression test in test/js/node/http/node-http.test.ts (describe("Server.closeAllConnections")) with two cases: established keep-alive connection gets dropped and a follow-up fetch returns 200; no-op when no connections are established.
• Both new tests fail on system Bun (USE_SYSTEM_BUN=1 bun test ...) and pass on the debug build → load-bearing.
• test/js/node/test/parallel/test-http-server-close-all.js (Node parity script) exits 0.
• test/js/node/test/parallel/test-http-server-close-idle-wait-response.js (closeIdle parity) still exits 0.
• HTTP Server Security Tests - Advanced (14 tests, uses closeAllConnections in afterEach) all pass.
• CI green.

[claude]

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: ed7f55c5-d525-4282-87f1-0a325e7cc4b2

📥 Commits

Reviewing files that changed from the base of the PR and between d8ddb13 and d17bb1b.

📒 Files selected for processing (1)

• packages/bun-uws/src/App.h

---

Walkthrough

This PR implements Server.prototype.closeAllConnections end-to-end (C++ → FFI → Rust → JS) so it closes established HTTP(S) connections but preserves the listening socket, replacing the previous behavior that shut down the listener.

Changes

Server.closeAllConnections() feature

Layer / File(s)	Summary
C++ and C FFI closeAllConnections implementation packages/bun-uws/src/App.h, src/uws_sys/libuwsockets.cpp	uWS TemplatedApp gains closeAllConnections() that iterates all sockets in the HTTP socket group and closes each one with a clean shutdown code without touching the listener. C FFI wrapper uws_app_close_all_connections() dispatches to SSL or non-SSL app variants.
Rust FFI bindings and server host function src/uws_sys/App.rs, src/runtime/server/server_body.rs	Rust declares the C FFI and wraps it as App::close_all_connections(); close_all_connections host function checks whether self.app exists, calls the Rust wrapper when present, and returns undefined.
Server class proto registration src/runtime/server/server.classes.ts	New closeAllConnections entry added to server proto bindings, wiring the method name to the host function with arity 0.
JavaScript Server.prototype.closeAllConnections src/js/node/_http_server.ts	Method now delegates to server.closeAllConnections?.() to close established connections only instead of invoking server.stop(true), preserving the listening socket for new requests.
Node.js compatibility tests test/js/node/http/node-http.test.ts	Two tests verify that closeAllConnections() forcefully closes established keep-alive sockets while the listener continues accepting new requests, and that calling the method when no connections exist is a no-op.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'fix(node:http): keep listener open in Server.closeAllConnections' is specific and directly references the main bug fix: ensuring the listener remains open when closeAllConnections() is called, which matches the core issue.
Description check	✅ Passed	The PR description comprehensively covers the problem, solution, and test plan, exceeding the minimal template requirements with clear context, related issues, and detailed implementation across multiple layers.
Linked Issues check	✅ Passed	The code changes fully implement the requirement from #31301 to keep the listening socket open in closeAllConnections() by adding a dedicated native path that closes only established sockets without touching listener state.
Out of Scope Changes check	✅ Passed	All changes are tightly scoped to implementing the closeAllConnections functionality across uWS, Rust bindings, runtime layers, and the JS API, with supporting regression tests and no unrelated modifications.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[Jarred-Sumner]

@claude review

[mxschmitt]

@claude review

[robobun]

@mxschmitt apologies for the parallel PR — I opened #33394 for the same bug before finding this one, and a duplicate-detector bot pointed me here. Rather than quietly compete, here's what I found, because I think the two halves belong together.

Your native primitive catches sockets mine can't. A connection that's been accepted but hasn't sent a request head has no JS wrapper, so it isn't in kTrackedConnections and my JS-only loop skips it. Node tracks from TCP-accept, so it destroys those. Your App::closeAllConnections() walking head_sockets handles them.

But routing only through uWS leaves the JS socket object stale. closeIdleConnections() already takes that exact path today, so it's measurable without building your branch:

const serverSocket = /* captured from server.on("connection") */;
server.closeIdleConnections();           // native us_socket_close
// node: socket.destroyed === true,  'close' fired
// bun:  socket.destroyed === false, 'close' never fires

Node's closeAllConnections() calls socket.destroy() on each tracked socket, so the object ends up destroyed and emits 'close'. Closing the uSocket underneath it runs NodeHTTPServerSocket's onclose (which nulls the handle and untracks it) but never destroys the Duplex, so the socket object still claims to be alive.

That matters for the exact code this issue was filed about: Playwright's TestServer does server.on('connection', s => this._sockets.add(s)) and relies on the socket's 'close' to drop it again. With a native-only close, those sockets never leave the set and socket.destroyed lies.

So I think the complete fix is both: destroy the tracked sockets through socket.destroy() first (Node's own implementation, and what gives the JS object the right state), then your native sweep for anything JS never saw. There's also a third thing it buys — because closeAllConnections() then never consults this[serverSymbol], it keeps working after close(), which is the @azure/msal-node hang in #30501 that #30505 needs ~120 lines of JS + Rust for.

Happy to go either way:

• I hand you the JS half and you fold it into this PR (it's one commit, src/js/node/_http_server.ts only — take it from farm/b38f6cec/fix-http-close-all-connections, no attribution needed), or
• I pull your native primitive into http: closeAllConnections() must not stop the listener #33394 with credit and a Co-authored-by, and this one closes.

Your issue, your PR, your call. This branch also predates the Zig → Rust server port, so it'll need a rebase either way (server.zig is now src/runtime/server/server_body.rs).

[robobun]

Thanks @mxschmitt, both for the report and for the fix, and sorry this sat for so long. Your analysis was right: I re-checked against a fresh main build today and both of your tests still fail there (closeAllConnections() still goes through stop(true) and takes the listener down).

Since May the server side has moved under this branch: #32488 made node:http track every connection in JS from the moment it is accepted, and #35130 reworked the native stop path. That lets the fix live entirely in _http_server.ts as "destroy the tracked sockets" (the same thing Node does), which keeps the listener up, gives the socket objects Node's synchronous destroyed / 'close' behaviour, and also covers the close(); closeAllConnections() sequence from #30501 that the native sweep could not reach. With that in place the new uWS primitive is no longer needed, and this branch now conflicts with main in App.rs, server_body.rs and _http_server.ts, so I am consolidating the five open PRs in this area into #35839 rather than asking you to rework this one.

#35839 includes your "no connections established" test case with a Co-authored-by credit, and is set to close #31301. Closing this one; please shout on #35839 if the Playwright test server still misbehaves with it.