Repository navigation
feat(streaming): add SSE keepalive and error handling - #2860
Conversation
Native Anthropic streaming had no keepalive, so idle gaps (slow first token, long reasoning, slow tool-arg generation) let proxies and clients time out the socket, surfacing as "The socket connection was closed unexpectedly" in coding clients. Add a 15s SSE ping, mirroring the chat-completions path. Also stop throwing HTTPException from the streaming catch block: headers are already sent, so it tore down the socket instead of producing an HTTP error. Emit a well-formed Anthropic error event + message_stop so the client ends cleanly, and treat client aborts as info-level. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018uuGUkp6NaPEVSRw553RYJ
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThe Anthropic and ChangesSSE streaming updates
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/gateway/src/anthropic/anthropic.ts (1)
796-801: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider terminating the keepalive comment with a blank line (
\n\n).A lone
: ping\nis a valid comment but is not a self-terminating SSE frame; the line stays in the parser's pending buffer until the next event boundary. Emitting: ping\n\nmakes the keepalive a complete, immediately-dispatched frame, which is the conventional form and avoids any dependency on a subsequent event to flush it. Bytes still reach the proxy either way, so this is non-blocking.♻️ Proposed tweak
- const keepaliveInterval = setInterval(() => { - stream.write(": ping\n").catch(() => { + const keepaliveInterval = setInterval(() => { + stream.write(": ping\n\n").catch(() => { // Stream likely closed; cleanup happens in finally. }); }, KEEPALIVE_INTERVAL_MS);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/gateway/src/anthropic/anthropic.ts` around lines 796 - 801, The keepalive in `anthropic.ts` is sending a comment frame with only `: ping\n`, which may stay buffered instead of being dispatched immediately. Update the `setInterval` keepalive write so the SSE comment is terminated as a complete frame, and keep the change localized to the keepalive logic in the streaming handler around `KEEPALIVE_INTERVAL_MS` and `stream.write`.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@apps/gateway/src/anthropic/anthropic.ts`:
- Around line 796-801: The keepalive in `anthropic.ts` is sending a comment
frame with only `: ping\n`, which may stay buffered instead of being dispatched
immediately. Update the `setInterval` keepalive write so the SSE comment is
terminated as a complete frame, and keep the change localized to the keepalive
logic in the streaming handler around `KEEPALIVE_INTERVAL_MS` and
`stream.write`.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 9eede167-a9d8-49b2-bd78-89e25028f0c9
📒 Files selected for processing (1)
apps/gateway/src/anthropic/anthropic.ts
The Responses API streaming translator had no keepalive, so idle gaps (slow first token, long reasoning, slow tool-arg generation) let proxies and clients time out the socket, surfacing as "The socket connection was closed unexpectedly" in coding clients. Add a 15s SSE ping, matching the chat-completions and messages paths. Also harden the streaming catch: skip the terminal write on client aborts (the socket is gone) and guard the response.failed write so a secondary failure can't escape the handler. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018uuGUkp6NaPEVSRw553RYJ
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bd5cc0b902
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const keepaliveInterval = setInterval(() => { | ||
| stream.write(": ping\n").catch(() => { | ||
| // Stream likely closed; cleanup happens in finally. | ||
| }); | ||
| }, KEEPALIVE_INTERVAL_MS); |
There was a problem hiding this comment.
Wrap the first SSE write in the cleanup path
If the client disconnects before or during the initial response.created write just below this block, that await stream.writeSSE(...) rejects before execution enters the try/finally that clears this interval. The new keepalive timer will then keep firing and swallowing failed writes for the lifetime of the process, so early-aborted /v1/responses streams can leak timers; start the interval inside the guarded section or include the initial write in the same try/finally.
Useful? React with 👍 / 👎.
Summary
Improves reliability of Anthropic streaming responses by adding SSE keepalive pings to prevent proxy/load balancer timeouts during quiet gaps, and implements proper error handling for mid-stream failures that gracefully terminates the SSE stream instead of abruptly closing the connection.
Key Changes
SSE Keepalive: Added 15-second keepalive interval that emits SSE ping comments (
: ping\n) to keep the connection alive during slow time-to-first-token, long reasoning phases, or slow tool argument generation. These pings are consumed by the translator and never reach the client.Improved Streaming Error Handling: Replaced immediate HTTP exception throwing (which cannot work after headers are sent) with proper SSE error event emission:
Cleanup: Added
clearInterval(keepaliveInterval)in the finally block to ensure the keepalive timer is properly cleaned up when the stream ends.Implementation Details
The keepalive mechanism uses SSE spec-compliant comment syntax (
: ping\n) which is ignored by the Anthropic SDK but serves to keep the connection active. This prevents "The socket connection was closed unexpectedly" errors that clients would otherwise see when the connection is terminated by proxies or load balancers during quiet periods.The error handling distinguishes between client-initiated aborts (which are expected and logged at info level) and actual streaming errors (which are logged at error level and communicated to the client via SSE events).
https://claude.ai/code/session_018uuGUkp6NaPEVSRw553RYJ
Summary by CodeRabbit
: pingkeepalive messages during streaming responses to reduce idle timeout/disconnect issues.