Skip to content

node:http: make closeAllConnections()/closeIdleConnections() leave the listener alone and work after close() - #35839

Open
robobun wants to merge 11 commits into
mainfrom
farm/6ccde698/http-close-connections-after-close
Open

node:http: make closeAllConnections()/closeIdleConnections() leave the listener alone and work after close()#35839
robobun wants to merge 11 commits into
mainfrom
farm/6ccde698/http-close-connections-after-close

node:http: make closeIdleConnections/closeAllConnections work after c…

ba8e513
Select commit
Loading
Failed to load commit list.
Claude / Claude Code Review completed Jul 26, 2026 in 24m 25s

Code review found 1 important issue

Found 2 candidates, confirmed 1. See review comments for details.

Details

Severity Count
🔴 Important 1
🟡 Nit 0
🟣 Pre-existing 0
Severity File:Line Issue
🔴 Important src/js/node/_http_server.ts:508-513 closeIdleConnections() idle check destroys non-idle sockets (upgraded WS/CONNECT, and partial-head)

Annotations

Check failure on line 513 in src/js/node/_http_server.ts

See this annotation in the file changed.

@claude claude / Claude Code Review

closeIdleConnections() idle check destroys non-idle sockets (upgraded WS/CONNECT, and partial-head)

The new `closeIdleConnections()` idle check (`!socket._httpMessage && !socket[kPipelinedResponses]?.length`) destroys sockets Node treats as non-idle: **upgraded sockets** (WebSocket via `'upgrade'`, CONNECT tunnels) are in `kTrackedConnections` but never get `_httpMessage`, so a graceful-drain `server.closeIdleConnections()` now tears down every live `ws` connection — the exact pattern this PR targets. It also destroys sockets with a **partial request head buffered** (they are wrapped in `onSer