Skip to content

fix(gateway): single-newline SSE keepalive - #2334

Merged
steebchen merged 1 commit into
mainfrom
jackson-v1
May 19, 2026
Merged

steebchen merged 1 commit into
mainfrom
jackson-v1

Conversation

@steebchen

@steebchen steebchen commented May 19, 2026 •

Copy link
Copy Markdown
Member

Summary

The chat-streaming keepalive emits : ping\n\n every 15s. The trailing \n\n is a comment followed by a blank line — and the openai-python SSE decoder dispatches an event on any blank line as long as last_event_id was set by an earlier real event (which it always is, since the gateway sets id: on every chunk). The dispatched event has event=None, data='', and sse.json() then raises JSONDecodeError.

This explains the report at openai/openai-python#2722 and matches the observed timing: Claude generations slow enough to still be streaming when the first 15s keepalive fires crash ~16-17s in; faster generations (DeepSeek) finish before any keepalive and never trip it.

Fix: drop the trailing newline. : ping\n is still a valid SSE comment line and still flushes bytes for proxy keepalive, but no blank line follows, so no buggy parser dispatches an empty-data event. Spec-compliant parsers behave identically.

Test plan

  • Run a slow Anthropic stream (sonnet/haiku via direct or Bedrock) through the gateway with openai-python ≤ 2.37.0; confirm no JSONDecodeError after 15s
  • Verify GCP LB / Cloudflare connections still stay open across long idle gaps (keepalive bytes still flush)
  • pnpm test:unit clean

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Improved streaming reliability in chat responses by fixing SSE keepalive message format to prevent unintended empty events from being dispatched by some SSE parsers.

Review Change Stack

The `: ping\n\n` keepalive emitted a blank line that openai-python's
SSEDecoder treats as an event dispatch once last_event_id is set,
producing an empty-data event that crashes sse.json()
(openai/openai-python#2722). Drop the trailing newline so the comment
is no longer followed by a blank line.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings May 19, 2026 14:49
@coderabbitai

coderabbitai Bot commented May 19, 2026 •

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 670cca16-be7c-422f-b6e6-4eadaa2f793f

📥 Commits

Reviewing files that changed from the base of the PR and between 00221c8 and 80e40af.

📒 Files selected for processing (1)
  • apps/gateway/src/chat/chat.ts

Walkthrough

The chat streaming SSE keepalive mechanism is modified to write a single-line comment (: ping\n) instead of the previous format (: ping\n\n), with inline documentation explaining that this avoids triggering empty-data event dispatch in certain SSE parsers when last_event_id is already set.

Changes

SSE Keepalive Ping Format

Layer / File(s) Summary
SSE keepalive format and documentation
apps/gateway/src/chat/chat.ts
Keepalive payload changed from : ping\n\n to : ping\n with added comments explaining the rationale for avoiding \n\n sequences that can trigger unwanted empty-data events in buggy SSE parsers.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5 minutes

Possibly related PRs

  • theopenco/llmgateway#2024: Both PRs modify the SSE keepalive ping formatting used during chat streaming—this PR changes the ping comment from : ping\n\n to : ping\n, while the related PR introduces periodic keepalive pings using the : ping\n\n format.
  • theopenco/llmgateway#2089: Both PRs adjust how streaming keepalive ping events are handled—this PR changes the SSE keepalive payload format in apps/gateway/src/chat/chat.ts, while the related PR suppresses/ignores Anthropic ping chunks in apps/gateway/src/chat/tools/transform-streaming-to-openai.ts.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix(gateway): single-newline SSE keepalive' directly and clearly describes the main change: modifying the SSE keepalive mechanism from double-newline to single-newline format to fix a downstream parsing issue.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch jackson-v1

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

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

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes an SSE keepalive bug where : ping\n\n triggered buggy SSE parsers (openai-python ≤ 2.37.0) to dispatch an empty-data event and raise JSONDecodeError. Changing to a single \n keeps the comment valid and still flushes bytes for proxy keepalive without producing a blank line.

Changes:

  • Replace ": ping\n\n" with ": ping\n" in the chat-streaming keepalive interval
  • Update the accompanying comment to explain the rationale

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@steebchen
steebchen merged commit 2de5f6c into main May 19, 2026
18 of 19 checks passed
@steebchen
steebchen deleted the jackson-v1 branch May 19, 2026 21:52
@fanaticalfishing

Copy link
Copy Markdown

Thank you so much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants