Skip to content

Commit 9025183

Browse files
fix(streaming): normalize non-SSE upstream chat completions (#412)
* fix(streaming): normalize non-SSE upstream chat completions Some OpenAI-compatible upstreams ignore stream:true and reply with a single buffered application/json completion (no data: framing, no [DONE]). The gateway forwarded that body verbatim under a text/event-stream content type, so SSE clients waited forever for an end-of-stream marker that never arrived — the connection appeared to hang after the model had clearly finished (issue #411). Add EnsureChatCompletionSSE, applied in the shared CompatibleProvider.StreamChatCompletion so every OpenAI-compatible provider benefits. Genuine SSE streams pass through untouched with no buffering; a buffered JSON completion is re-emitted as one SSE chunk (object -> chat.completion.chunk, message -> delta) followed by a terminal data: [DONE]. This is a long-standing latent defect, not a regression between the versions named in the issue; the fix hardens the gateway against any upstream that silently drops streaming. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(streaming): widen non-SSE normalization and avoid passthrough stalls Address PR review feedback on the chat-completions SSE normalizer: - Classify the stream from its first non-whitespace byte via incremental Peek instead of Peek(512), so a genuine SSE upstream that emits a small first token then pauses is no longer held back waiting for a full buffer. - Preserve partially-read bodies: when the upstream looked like buffered JSON but the read failed mid-body, forward what arrived before [DONE] instead of replacing it with a bare done marker. - Apply EnsureChatCompletionSSE to the deepseek and xai providers, which stream via raw DoStream and previously bypassed the normalizer. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(streaming): avoid tainted size arithmetic in SSE buffer build CodeQL flagged the make() capacity computation (len(payload)+constants) as a possible allocation-size overflow. Build the normalized SSE chunk with a bytes.Buffer instead, which removes the explicit size arithmetic; behavior is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(streaming): guard nil xai request and drop dead empty-body branch Address follow-up PR review: - xai StreamChatCompletion now guards a nil request before dereferencing it, matching the deepseek provider's contract in this PR. - Remove the unreachable len(body)==0 branch in EnsureChatCompletionSSE: the '{' that classifies the body is already buffered, so io.ReadAll always returns at least that byte. bufferedCompletionToSSE already forwards partial bytes and appends [DONE], so behavior is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 68cd937 commit 9025183

5 files changed

Lines changed: 242 additions & 3 deletions

File tree

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
package providers
2+
3+
import (
4+
"bufio"
5+
"bytes"
6+
"encoding/json"
7+
"io"
8+
)
9+
10+
// chatDonePayload terminates a chat completions SSE stream.
11+
var chatDonePayload = []byte("data: [DONE]\n\n")
12+
13+
// peekForNonSSE inspects up to this many leading bytes to classify the upstream
14+
// response. SSE payloads begin with a field name (data:, event:, id:, retry:) or
15+
// a ':' comment; a buffered JSON completion begins with '{'. 512 bytes comfortably
16+
// clears any leading whitespace or comment lines without buffering real streams.
17+
const peekForNonSSE = 512
18+
19+
// EnsureChatCompletionSSE normalizes a chat completions stream so the client
20+
// always receives well-formed Server-Sent Events terminated by data: [DONE].
21+
//
22+
// Some OpenAI-compatible upstreams ignore stream:true and reply with a single
23+
// buffered application/json completion (no data: framing, no [DONE]). Forwarding
24+
// that verbatim under a text/event-stream content type leaves SSE clients waiting
25+
// forever for an end-of-stream marker that never arrives. When the upstream body
26+
// is detected as a buffered JSON object it is re-emitted as one SSE chunk plus a
27+
// terminal [DONE]; genuine SSE streams pass through untouched with no buffering.
28+
func EnsureChatCompletionSSE(stream io.ReadCloser) io.ReadCloser {
29+
if stream == nil {
30+
return nil
31+
}
32+
33+
reader := bufio.NewReaderSize(stream, peekForNonSSE)
34+
if firstNonSpaceByte(reader, peekForNonSSE) != '{' {
35+
// Genuine SSE (or empty): stream through unchanged, no buffering.
36+
return &bufferedReadCloser{Reader: reader, closer: stream}
37+
}
38+
39+
// The '{' that classified this body is already buffered, so io.ReadAll
40+
// always returns at least that byte; a mid-read failure still yields the
41+
// partial bytes. Either way bufferedCompletionToSSE forwards what arrived
42+
// (raw when the JSON is truncated) and appends [DONE], so generated content
43+
// is never dropped and the client always receives a terminator.
44+
body, _ := io.ReadAll(reader)
45+
_ = stream.Close() //nolint:errcheck
46+
return io.NopCloser(bytes.NewReader(bufferedCompletionToSSE(body)))
47+
}
48+
49+
// firstNonSpaceByte reports the first non-whitespace byte buffered by reader,
50+
// peeking one byte further at a time so a genuine SSE stream is classified from
51+
// its first token without blocking until a full buffer fills. It never consumes
52+
// input, so a passed-through stream is forwarded byte-for-byte. Returns 0 when
53+
// the stream ends, errors, or yields only whitespace within max bytes.
54+
func firstNonSpaceByte(r *bufio.Reader, max int) byte {
55+
for i := 1; i <= max; i++ {
56+
prefix, err := r.Peek(i)
57+
if len(prefix) < i {
58+
_ = err // EOF or error before any non-space byte was found
59+
return 0
60+
}
61+
switch b := prefix[i-1]; b {
62+
case ' ', '\t', '\r', '\n':
63+
continue
64+
default:
65+
return b
66+
}
67+
}
68+
return 0
69+
}
70+
71+
// bufferedCompletionToSSE wraps a buffered chat completion JSON object as a
72+
// single SSE chunk followed by the terminal [DONE] marker. The object field is
73+
// rewritten to chat.completion.chunk and each choice's message is moved to delta
74+
// so OpenAI SSE clients parse it as a streaming chunk. If the body does not parse
75+
// as a JSON object it is forwarded as-is so no data is lost, still followed by
76+
// [DONE] so the client stops waiting.
77+
func bufferedCompletionToSSE(body []byte) []byte {
78+
payload := body
79+
var obj map[string]any
80+
if err := json.Unmarshal(body, &obj); err == nil {
81+
obj["object"] = "chat.completion.chunk"
82+
if choices, ok := obj["choices"].([]any); ok {
83+
for _, c := range choices {
84+
choice, ok := c.(map[string]any)
85+
if !ok {
86+
continue
87+
}
88+
if msg, ok := choice["message"]; ok {
89+
choice["delta"] = msg
90+
delete(choice, "message")
91+
}
92+
}
93+
}
94+
if encoded, err := json.Marshal(obj); err == nil {
95+
payload = encoded
96+
}
97+
}
98+
99+
var out bytes.Buffer
100+
out.WriteString("data: ")
101+
out.Write(payload)
102+
out.WriteString("\n\n")
103+
out.Write(chatDonePayload)
104+
return out.Bytes()
105+
}
106+
107+
// bufferedReadCloser pairs a buffered reader with the underlying stream's Close.
108+
type bufferedReadCloser struct {
109+
*bufio.Reader
110+
closer io.Closer
111+
}
112+
113+
func (b *bufferedReadCloser) Close() error { return b.closer.Close() }
Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
package providers
2+
3+
import (
4+
"io"
5+
"strings"
6+
"testing"
7+
)
8+
9+
// errAfterReadCloser yields its data once, then fails — simulating a connection
10+
// that drops mid-body after some bytes have arrived.
11+
type errAfterReadCloser struct {
12+
data []byte
13+
err error
14+
done bool
15+
}
16+
17+
func (r *errAfterReadCloser) Read(p []byte) (int, error) {
18+
if r.done {
19+
return 0, r.err
20+
}
21+
n := copy(p, r.data)
22+
r.data = r.data[n:]
23+
if len(r.data) == 0 {
24+
r.done = true
25+
}
26+
return n, nil
27+
}
28+
29+
func (r *errAfterReadCloser) Close() error { return nil }
30+
31+
func TestEnsureChatCompletionSSE_ConvertsBufferedJSON(t *testing.T) {
32+
// Upstream ignored stream:true and returned a buffered, non-SSE completion.
33+
body := `{"id":"x","object":"chat.completion","choices":[{"finish_reason":"stop","message":{"role":"assistant","content":"Hi there"}}]}`
34+
stream := io.NopCloser(strings.NewReader(body))
35+
36+
got, err := io.ReadAll(EnsureChatCompletionSSE(stream))
37+
if err != nil {
38+
t.Fatalf("read stream: %v", err)
39+
}
40+
41+
out := string(got)
42+
if !strings.HasPrefix(out, "data: {") {
43+
t.Fatalf("expected SSE data framing, got %q", out)
44+
}
45+
if !strings.HasSuffix(out, "data: [DONE]\n\n") {
46+
t.Fatalf("expected terminal done marker, got %q", out)
47+
}
48+
if !strings.Contains(out, `"object":"chat.completion.chunk"`) {
49+
t.Fatalf("expected object rewritten to chunk, got %q", out)
50+
}
51+
if !strings.Contains(out, `"delta":`) || strings.Contains(out, `"message":`) {
52+
t.Fatalf("expected message rewritten to delta, got %q", out)
53+
}
54+
}
55+
56+
func TestEnsureChatCompletionSSE_PassesThroughRealSSE(t *testing.T) {
57+
chunks := [][]byte{
58+
[]byte("data: {\"choices\":[{\"delta\":{\"content\":\"Hi\"}}]}\n\n"),
59+
[]byte("data: {\"choices\":[{\"delta\":{\"content\":\" there\"}}]}\n\n"),
60+
[]byte("data: [DONE]\n\n"),
61+
}
62+
original := strings.Join([]string{string(chunks[0]), string(chunks[1]), string(chunks[2])}, "")
63+
stream := &chunkedReadCloser{chunks: chunks}
64+
65+
got, err := io.ReadAll(EnsureChatCompletionSSE(stream))
66+
if err != nil {
67+
t.Fatalf("read stream: %v", err)
68+
}
69+
if string(got) != original {
70+
t.Fatalf("expected genuine SSE passed through unchanged.\n got: %q\nwant: %q", string(got), original)
71+
}
72+
}
73+
74+
func TestEnsureChatCompletionSSE_PassesThroughSSEWithLeadingComment(t *testing.T) {
75+
// Providers like OpenRouter emit a leading ": ... PROCESSING" comment line.
76+
body := ": OPENROUTER PROCESSING\n\ndata: {\"choices\":[{\"delta\":{\"content\":\"Hi\"}}]}\n\ndata: [DONE]\n\n"
77+
stream := io.NopCloser(strings.NewReader(body))
78+
79+
got, err := io.ReadAll(EnsureChatCompletionSSE(stream))
80+
if err != nil {
81+
t.Fatalf("read stream: %v", err)
82+
}
83+
if string(got) != body {
84+
t.Fatalf("expected comment-prefixed SSE unchanged, got %q", string(got))
85+
}
86+
}
87+
88+
func TestEnsureChatCompletionSSE_PreservesPartialBodyOnReadError(t *testing.T) {
89+
// Upstream began a buffered JSON body, then the connection dropped mid-read.
90+
// The partial content must still reach the client, followed by [DONE].
91+
partial := `{"id":"x","choices":[{"message":{"content":"Hel`
92+
stream := &errAfterReadCloser{data: []byte(partial), err: io.ErrUnexpectedEOF}
93+
94+
got, err := io.ReadAll(EnsureChatCompletionSSE(stream))
95+
if err != nil {
96+
t.Fatalf("read stream: %v", err)
97+
}
98+
out := string(got)
99+
if !strings.Contains(out, "Hel") {
100+
t.Fatalf("expected partial content preserved, got %q", out)
101+
}
102+
if !strings.HasSuffix(out, "data: [DONE]\n\n") {
103+
t.Fatalf("expected terminal done marker, got %q", out)
104+
}
105+
}
106+
107+
func TestEnsureChatCompletionSSE_NilStream(t *testing.T) {
108+
if EnsureChatCompletionSSE(nil) != nil {
109+
t.Fatal("expected nil for nil stream")
110+
}
111+
}

internal/providers/deepseek/deepseek.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,11 +149,15 @@ func (p *Provider) StreamChatCompletion(ctx context.Context, req *core.ChatReque
149149
if err != nil {
150150
return nil, err
151151
}
152-
return p.client.DoStream(ctx, llmclient.Request{
152+
stream, err := p.client.DoStream(ctx, llmclient.Request{
153153
Method: http.MethodPost,
154154
Endpoint: "/chat/completions",
155155
Body: body,
156156
})
157+
if err != nil {
158+
return nil, err
159+
}
160+
return providers.EnsureChatCompletionSSE(stream), nil
157161
}
158162

159163
// ListModels retrieves the list of available models from DeepSeek.

internal/providers/openai/compatible_provider.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,11 +125,15 @@ func (p *CompatibleProvider) StreamChatCompletion(ctx context.Context, req *core
125125
if err != nil {
126126
return nil, err
127127
}
128-
return p.client.DoStream(ctx, p.prepareRequest(llmclient.Request{
128+
stream, err := p.client.DoStream(ctx, p.prepareRequest(llmclient.Request{
129129
Method: http.MethodPost,
130130
Endpoint: "/chat/completions",
131131
Body: body,
132132
}))
133+
if err != nil {
134+
return nil, err
135+
}
136+
return providers.EnsureChatCompletionSSE(stream), nil
133137
}
134138

135139
func (p *CompatibleProvider) ListModels(ctx context.Context) (*core.ModelsResponse, error) {

internal/providers/xai/xai.go

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -184,12 +184,19 @@ func (p *Provider) ChatCompletion(ctx context.Context, req *core.ChatRequest) (*
184184

185185
// StreamChatCompletion returns a raw response body for streaming (caller must close)
186186
func (p *Provider) StreamChatCompletion(ctx context.Context, req *core.ChatRequest) (io.ReadCloser, error) {
187-
return p.client.DoStream(ctx, llmclient.Request{
187+
if req == nil {
188+
return nil, core.NewInvalidRequestError("chat request is required", nil)
189+
}
190+
stream, err := p.client.DoStream(ctx, llmclient.Request{
188191
Method: http.MethodPost,
189192
Endpoint: "/chat/completions",
190193
Body: req.WithStreaming(),
191194
Headers: xGrokConversationHeaders(ctx, req),
192195
})
196+
if err != nil {
197+
return nil, err
198+
}
199+
return providers.EnsureChatCompletionSSE(stream), nil
193200
}
194201

195202
// ListModels retrieves the list of available models from xAI

0 commit comments

Comments
 (0)