feature: WebSocket ping/pong heartbeat — Node server (PR 2)#585
Conversation
Add the heartbeat to the Node WebSocket server, building on the shared WebSocketHeartbeat + dispatch onActivity hook (#584). - NodeWebSocket.accept: when pingIntervalMillis > 0, start a setInterval that drives the heartbeat (sendPing on idle / ctx.close(1011) on unanswered); reset on any inbound frame via the dispatch onActivity hook; clearInterval in notifyClose. sendPing added to NodeWebSocketContext. - NodeServerConfig.webSocketPingIntervalMillis (0 = off) + builder, threaded through handleUpgrade/gateAndAccept/accept. Test: a Node integration test — server pings every 150ms, the global WebSocket runtime auto-pongs, and the live-but-idle connection is not reaped. uniJS/test: 1406 tests, 0 failed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a WebSocket ping/pong heartbeat mechanism for the Node.js HTTP server backend, allowing idle connections to be kept alive or closed on timeout. A potential memory leak was identified in NodeWebSocket.scala where the heartbeat interval could be scheduled after the connection has already been closed during initialization, preventing the interval from being cleared. It is recommended to check if the connection is closed before starting the interval.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
Code Review
This pull request introduces WebSocket ping/pong heartbeat support to the Node.js HTTP server implementation, allowing idle connections to be kept alive or reaped if the peer stops responding. A review comment points out a potential memory leak where a permanent timer could be scheduled if the connection is closed synchronously during initial head processing. It is recommended to add a check to ensure the connection is not already closed before setting up the heartbeat interval.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
If onOpen/drive(head) already triggered notifyClose before the interval was scheduled, the interval would leak (notifyClose ran before intervalHandle was set, so it couldn't clear it). Guard the setInterval on `!closed`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 participant
Why
PR 2 of the WebSocket heartbeat series (after #584's shared core + Native backends). Adds the outbound ping/pong heartbeat to the Node.js WebSocket server so it reaps half-open client connections.
What
NodeWebSocket.accept— whenpingIntervalMillis > 0, start asetIntervalthat drives the sharedWebSocketHeartbeat(send a ping on an idle tick viactx.sendPing;ctx.close(1011, "ping timeout")when a ping goes unanswered). The heartbeat is reset on any inbound frame via theonActivityhook onWebSocketDispatcher.dispatch(from feature: WebSocket ping/pong heartbeat — shared core + Native server & client #584), and the interval is cleared innotifyClose(every close path).sendPingadded toNodeWebSocketContext.NodeServerConfig.webSocketPingIntervalMillis(0 = off) +withWebSocketPingIntervalMillis, threaded throughhandleUpgrade→gateAndAccept→accept.Testing
A Node integration test: the server pings every 150 ms, the Node runtime auto-pongs (the global
WebSocketanswers protocol pings automatically), and the live-but-idle connection is not reaped.uniJS/test: 1406 tests, 0 failed. JS-only change (the shared core is already onmain).Up next
PR 3 — Netty server (
IdleStateHandler). PR 4 — JVM client (ScheduledExecutorService+sendPing/onPong).🤖 Generated with Claude Code