Skip to content

fix(httpclient): drop comment-only SSE events that crash openai-go#2662

Merged
dgageot merged 1 commit intodocker:mainfrom
dgageot:board/cb49b20cc621f479
May 6, 2026
Merged

fix(httpclient): drop comment-only SSE events that crash openai-go#2662
dgageot merged 1 commit intodocker:mainfrom
dgageot:board/cb49b20cc621f479

Conversation

@dgageot
Copy link
Copy Markdown
Member

@dgageot dgageot commented May 6, 2026

Some upstreams (notably OpenRouter) inject SSE comment frames as keep-alives:

: OPENROUTER PROCESSING

data: {"id":"...", ...}

The OpenAI Go SDK's ssestream.Stream correctly ignores comment lines but still treats the trailing blank line as an event boundary, then unconditionally JSON-unmarshals the resulting empty payload. That fails with unexpected end of JSON input and tears down the whole completion.

Fix

Wrap the HTTP transport with a streaming pre-filter that, on responses whose Content-Type starts with text/event-stream (case-insensitive, charset parameters allowed), drops events containing no data: lines. Well-formed events pass through verbatim and non-SSE responses are untouched.

The filter:

  • handles arbitrary data: payload sizes (32 MB scanner buffer, matching openai-go's own decoder);
  • normalises CRLF / mixed line endings to LF;
  • propagates underlying-reader errors instead of swallowing them as EOF;
  • silently drops half-built events at EOF (a downstream parser would not have dispatched them anyway);
  • is safe for concurrent use (verified under go test -race).

Tests

pkg/httpclient/sse_filter_test.go covers, among other things:

  • the OpenRouter comment-only keep-alive scenario;
  • pass-through of well-formed streams;
  • events bearing only event: / id: headers (also dropped);
  • preservation of event: / id: when accompanied by data:;
  • Content-Type matching with charset parameter, mixed case, and uppercase (the case-insensitive matching is verified to actually catch a regression);
  • 256 KB events, partial reads with tiny caller buffers, empty input, comment-only streams;
  • scanner error propagation, Close-without-Read, and concurrent requests.

Validation

  • go build ./...
  • go test ./pkg/httpclient/... -race -count=1
  • mise lint ✅ (golangci-lint and the project's custom linter)

Assisted-By: docker-agent

Some upstreams (notably OpenRouter) inject SSE comment frames as keep-alives. The OpenAI Go SDK's stream parser treats the trailing blank line as an event boundary and then JSON-unmarshals the empty payload, which fails with 'unexpected end of JSON input' and tears down the whole completion.

Wrap the HTTP transport with a streaming pre-filter that, on responses whose Content-Type starts with text/event-stream (case-insensitive), drops events containing no 'data:' lines. Well-formed events pass through verbatim and non-SSE responses are untouched.

Assisted-By: docker-agent
@dgageot dgageot requested a review from a team as a code owner May 6, 2026 12:30
Copy link
Copy Markdown

@docker-agent docker-agent Bot left a comment

Choose a reason for hiding this comment

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

Assessment: 🟢 APPROVE

The SSE comment-only event filter is well-implemented. The logic correctly identifies and drops events with no data: lines, handles CRLF normalization, propagates underlying reader errors, and the transport wrapping in client.go is clean. No bugs found in the changed code.

@dgageot dgageot merged commit 95abfce into docker:main May 6, 2026
7 checks passed
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.

2 participants