feature: WebSocket ping/pong heartbeat — Netty server (PR 3)#586
Conversation
Add the heartbeat to the Netty WebSocket server using Netty's built-in IdleStateHandler (the idiomatic mechanism), driving the shared WebSocketHeartbeat. - NettyRequestHandler.doWebSocketHandshake adds an IdleStateHandler (reader-idle = pingIntervalMillis) to the pipeline before the WS handler, when the interval > 0. - NettyWebSocketHandler: on the IdleStateEvent, drive WebSocketHeartbeat.onTick (writeAndFlush a PingWebSocketFrame on SendPing; close 1011 on an unanswered ping); reset the heartbeat on any inbound frame in channelRead0. - NettyServerConfig.webSocketPingIntervalMillis (0 = off) + builder, threaded through NettyHttpServer -> NettyRequestHandler. Test: a Netty integration test — server pings every 150ms, the JDK client auto-pongs, the live-but-idle connection is not reaped. netty/test: 38 passed. 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 to keep idle connections alive or reap unresponsive peers. It adds a webSocketPingIntervalMillis configuration, integrates Net's IdleStateHandler into the pipeline, and implements heartbeat logic in NettyWebSocketHandler. Feedback on the changes suggests two robustness improvements: explicitly verifying that the IdleStateEvent is a READER_IDLE event, and closing the channel directly via ctx.close() on ping timeout to prevent potential hangs with unresponsive peers.
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 detect and close unresponsive or idle connections. It adds a webSocketPingIntervalMillis configuration parameter, integrates Netty's IdleStateHandler into the pipeline, and implements heartbeat logic in NettyWebSocketHandler. A test case is also added to verify the functionality. The review feedback suggests replacing the magic number 1011 with WebSocketCloseStatus.INTERNAL_SERVER_ERROR.code() for better code maintainability.
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.
- Explicitly match IdleState.READER_IDLE (defensive: the IdleStateHandler is configured reader-idle only, but guard against future config changes). - On a ping timeout, force-close the channel (ctx.close()) instead of a graceful WS close — the peer is likely half-open, and a graceful close-frame write could linger on a full/dead send buffer. This also drops the magic 1011. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
## Why PR 4 (final) of the WebSocket heartbeat series (after #584 shared core + Native, #585 Node server, #586 Netty server). Wires the heartbeat into the **JVM `java.net.http` client** — the last backend that can do it. ## What - **`JavaWebSocketListener`** drives the shared `WebSocketHeartbeat` from a shared daemon `ScheduledExecutorService` (one for all client connections): `sendPing` on an idle tick, `close(1011, "ping timeout")` on an unanswered ping. Resets on any inbound frame (`onText`/`onBinary`/`onPong`); cancels the scheduled future in `notifyClose`. - **`JavaWebSocketContext.sendPing`** enqueues `WebSocket.sendPing` through the existing serialized send chain (the JDK forbids overlapping sends). - This consumes the `pingIntervalMillis` param added to `WebSocketClient.connect` in #584 (previously accept-and-ignored on the JVM client). The **JS client** stays the only backend without a heartbeat — the browser/Node global `WebSocket` can't send protocol pings from JS — documented on the `connect` API. ## Testing A Netty `WebSocketClientTest` case: the client pings every 150 ms, the server auto-pongs, and the live-but-idle connection is **not** reaped. **`uniJVM/test`: 1640, `netty/test`: 38 pass.** ## Heartbeat series complete | Backend | PR | |---------|-----| | Shared `WebSocketHeartbeat` + Native server & client | #584 | | Node server | #585 | | Netty server | #586 | | JVM client | this PR | | JS client | N/A — can't send protocol pings | 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 participant
Why
PR 3 of the WebSocket heartbeat series (after #584 shared core + Native, #585 Node). Adds the heartbeat to the Netty WebSocket server to reap half-open client connections.
What
NettyRequestHandler.doWebSocketHandshakeadds Netty's built-inIdleStateHandler(reader-idle =pingIntervalMillis) to the pipeline before the WS handler, when the interval > 0 — the idiomatic Netty mechanism.NettyWebSocketHandler— on theIdleStateEvent, drives the sharedWebSocketHeartbeat.onTick(writeAndFlush(PingWebSocketFrame())onSendPing;close(1011, "ping timeout")on an unanswered ping); resets the heartbeat on any inbound frame inchannelRead0.NettyServerConfig.webSocketPingIntervalMillis(0 = off) +withWebSocketPingIntervalMillis, threaded throughNettyHttpServer→NettyRequestHandler.Testing
A Netty integration test: the server pings every 150 ms, the JDK client auto-pongs, and the live-but-idle connection is not reaped.
netty/test: 38 passed.Up next
PR 4 — JVM client (
ScheduledExecutorService+sendPing/onPong), the last backend (JS client can't send protocol pings).🤖 Generated with Claude Code