Skip to content

Commit 571675d

Browse files
committed
fix(providers): stop persisting a bucketed 500 and bound error cause
A streamed error arrives on a 200 response, so the local 500 default used for classification is not an observed status; report StatusCode only when the payload code maps to a known status. Cap the persisted cause at 8 KiB (rune-safe, with a truncation marker) since provider bodies read up to 64 KiB.
1 parent ba92df9 commit 571675d

4 files changed

Lines changed: 76 additions & 6 deletions

File tree

‎internal/providers/openai/provider.go‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -298,27 +298,31 @@ func (provider *Provider) emitPayload(ctx context.Context, data string, state *t
298298
if chunk.Error != nil {
299299
state.flushContent(ctx, events)
300300
state.closeOpen(ctx, events)
301+
// statusCode buckets the error for classification only. The HTTP response
302+
// was 200, so observedStatus stays 0 unless the payload's code maps to a
303+
// known status; a bucket default must not be persisted as a real status.
301304
statusCode := http.StatusInternalServerError
305+
observedStatus := 0
302306
if chunk.Error.Code != nil {
303307
switch c := chunk.Error.Code.(type) {
304308
case string:
305309
if code, ok := openAIStreamErrorStatusByCode[c]; ok {
306-
statusCode = code
310+
statusCode, observedStatus = code, code
307311
}
308312
case float64:
309313
if code, ok := openAIStreamErrorStatusByCode[strconv.Itoa(int(c))]; ok {
310-
statusCode = code
314+
statusCode, observedStatus = code, code
311315
}
312316
case int:
313317
if code, ok := openAIStreamErrorStatusByCode[strconv.Itoa(c)]; ok {
314-
statusCode = code
318+
statusCode, observedStatus = code, code
315319
}
316320
}
317321
}
318322
sendEvent(ctx, events, zeroruntime.StreamEvent{
319323
Type: zeroruntime.StreamEventError,
320324
Error: provider.classifiedError(statusCode, chunk.Error.Message),
321-
StatusCode: statusCode,
325+
StatusCode: observedStatus,
322326
Cause: provider.redact(chunk.Error.Message),
323327
})
324328
state.done = true

‎internal/providers/openai/provider_test.go‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -647,6 +647,25 @@ func TestStreamCompletionStreamedErrorCarriesStatusCodeAndCause(t *testing.T) {
647647
}
648648
}
649649

650+
// An error inside a 200 OK SSE payload whose code is not a known status was
651+
// never an observed HTTP status, so none must be reported (#674 review).
652+
func TestStreamCompletionStreamedErrorWithUnknownCodeReportsNoStatus(t *testing.T) {
653+
provider := newTestProvider(t, func(w http.ResponseWriter, r *http.Request) {
654+
writeSSE(w, `{"error":{"message":"model overloaded","code":"server_error"}}`)
655+
})
656+
657+
events := collectProviderEvents(t, provider)
658+
if len(events) != 1 || events[0].Type != zeroruntime.StreamEventError {
659+
t.Fatalf("events = %#v, want one error", events)
660+
}
661+
if events[0].StatusCode != 0 {
662+
t.Fatalf("StatusCode = %d, want 0 (no HTTP status was observed)", events[0].StatusCode)
663+
}
664+
if !strings.Contains(events[0].Cause, "model overloaded") {
665+
t.Fatalf("Cause = %q, want the upstream detail", events[0].Cause)
666+
}
667+
}
668+
650669
func TestStreamCompletionEmitsErrorForMalformedJSON(t *testing.T) {
651670
provider := newTestProvider(t, func(w http.ResponseWriter, r *http.Request) {
652671
writeSSE(w, `{"choices":`)

‎internal/sessions/error_payload.go‎

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,13 @@ import (
66
"github.com/Gitlawb/zero/internal/zeroruntime"
77
)
88

9+
// maxErrorCauseBytes bounds the persisted cause. Provider error bodies are read
10+
// up to 64 KiB, which is far more than a diagnostic needs and would otherwise be
11+
// copied whole into every error event in events.jsonl.
12+
const maxErrorCauseBytes = 8 << 10
13+
14+
const errorCauseTruncationMarker = "… [truncated]"
15+
916
// ErrorEventPayload builds the payload for an EventError session event. It
1017
// always includes the flattened message (unchanged, for backward
1118
// compatibility with existing consumers of events.jsonl), and additionally
@@ -15,7 +22,8 @@ import (
1522
// call failed from the CLI or its logs — only the generic top-level message
1623
// was ever recorded (#674). Cause has already been redacted for secrets by
1724
// the provider before reaching here (see zeroruntime.StreamEvent.Cause) — it
18-
// is never re-scrubbed or stored raw at this layer.
25+
// is never re-scrubbed or stored raw at this layer. Cause is cut to
26+
// maxErrorCauseBytes (on a rune boundary) with a truncation marker appended.
1927
func ErrorEventPayload(err error) map[string]any {
2028
payload := map[string]any{"message": err.Error()}
2129
var streamErr *zeroruntime.StreamError
@@ -24,8 +32,15 @@ func ErrorEventPayload(err error) map[string]any {
2432
payload["statusCode"] = streamErr.StatusCode
2533
}
2634
if streamErr.Cause != "" {
27-
payload["cause"] = streamErr.Cause
35+
payload["cause"] = boundErrorCause(streamErr.Cause)
2836
}
2937
}
3038
return payload
3139
}
40+
41+
func boundErrorCause(cause string) string {
42+
if len(cause) <= maxErrorCauseBytes {
43+
return cause
44+
}
45+
return truncateUTF8(cause, maxErrorCauseBytes) + errorCauseTruncationMarker
46+
}

‎internal/sessions/error_payload_test.go‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,9 @@ import (
44
"encoding/json"
55
"errors"
66
"fmt"
7+
"strings"
78
"testing"
9+
"unicode/utf8"
810

911
"github.com/Gitlawb/zero/internal/zeroruntime"
1012
)
@@ -125,3 +127,33 @@ func TestStoreAppendEventPersistsErrorEventStatusCodeAndCause(t *testing.T) {
125127
t.Fatalf("persisted cause = %v, want %v", got, want)
126128
}
127129
}
130+
131+
func TestErrorEventPayloadBoundsCause(t *testing.T) {
132+
// "é" is two bytes; an odd cap boundary must not split it.
133+
cause := strings.Repeat("é", maxErrorCauseBytes)
134+
payload := ErrorEventPayload(&zeroruntime.StreamError{Message: "m", StatusCode: 500, Cause: cause})
135+
136+
got, ok := payload["cause"].(string)
137+
if !ok {
138+
t.Fatalf("cause = %#v, want string", payload["cause"])
139+
}
140+
if !strings.HasSuffix(got, errorCauseTruncationMarker) {
141+
t.Fatalf("cause missing truncation marker: ...%q", got[len(got)-20:])
142+
}
143+
body := strings.TrimSuffix(got, errorCauseTruncationMarker)
144+
if len(body) > maxErrorCauseBytes {
145+
t.Fatalf("bounded cause body is %d bytes, want <= %d", len(body), maxErrorCauseBytes)
146+
}
147+
if !utf8.ValidString(got) {
148+
t.Fatal("bounded cause is not valid UTF-8")
149+
}
150+
}
151+
152+
func TestErrorEventPayloadKeepsCauseAtCap(t *testing.T) {
153+
cause := strings.Repeat("a", maxErrorCauseBytes)
154+
payload := ErrorEventPayload(&zeroruntime.StreamError{Message: "m", Cause: cause})
155+
156+
if got := payload["cause"]; got != cause {
157+
t.Fatalf("cause at exactly the cap was altered (len %d)", len(got.(string)))
158+
}
159+
}

0 commit comments

Comments
 (0)