Skip to content

Commit 9a18e19

Browse files
authored
refactor(model): centralize early stream termination classification (#184)
Move the empty-vs-truncated classification of streams that close before message stop into model.NewStreamEndedEarlyError, deleting the duplicated streamEndedEarlyError helpers in the Bedrock and Anthropic adapters. The model package owns the stream integrity error contract; adapters now only report the provider, operation, and whether a message had started.
1 parent 3fa8597 commit 9a18e19

6 files changed

Lines changed: 91 additions & 93 deletions

File tree

DESIGN.md

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -278,11 +278,16 @@ shapes are classified instead of surfaced as opaque protocol errors:
278278
`errors.Is(err, model.ErrEmptyStream)` and may retry the request a bounded
279279
number of times before surfacing the failure.
280280
- **Truncated stream** — the stream closes cleanly after a message started but
281-
before `messageStop`. Adapters classify it as a retryable `unavailable`
281+
before `messageStop`. The classification is a retryable `unavailable`
282282
ProviderError (code `truncated_stream`) without the empty-stream sentinel:
283283
output was partially produced, so blind pre-output retry policies must not
284284
match it.
285285

286+
`model.NewStreamEndedEarlyError(provider, operation, started)` is the single
287+
classifier for streams that close before message stop; adapters pass whether a
288+
message had started and the model package owns which of the two shapes
289+
applies.
290+
286291
Adapters never retry internally; retry policy belongs to the integrating
287292
application (for example, a guard that retries only before the first visible
288293
output chunk).

features/model/anthropic/stream.go

Lines changed: 5 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,11 @@ func (s *anthropicStreamer) run() {
113113
} else if err := s.ctx.Err(); err != nil {
114114
s.setErr(err)
115115
} else if !processor.complete {
116-
s.setErr(streamEndedEarlyError(processor.started))
116+
s.setErr(model.NewStreamEndedEarlyError(
117+
anthropicProviderName,
118+
"stream_recv",
119+
processor.started,
120+
))
117121
} else {
118122
translated, err := translateResponse(&response, s.toolNameMap)
119123
if err != nil {
@@ -500,30 +504,3 @@ func decodeToolPayload(raw string) (rawjson.Message, error) {
500504
}
501505
return rawjson.Message(data), nil
502506
}
503-
504-
// streamEndedEarlyError classifies an Anthropic event stream that closed
505-
// cleanly before message stop. When no message ever started, the provider
506-
// produced an empty completion and callers may retry (model.ErrEmptyStream).
507-
// When a message was underway, the stream was truncated mid-generation: a
508-
// fresh request regenerates the full response, so the failure is a retryable
509-
// provider fault but not an empty stream.
510-
func streamEndedEarlyError(started bool) error {
511-
if !started {
512-
return model.NewEmptyStreamError(
513-
anthropicProviderName,
514-
"stream_recv",
515-
"stream ended before message start",
516-
)
517-
}
518-
return model.NewProviderError(
519-
anthropicProviderName,
520-
"stream_recv",
521-
0,
522-
model.ProviderErrorKindUnavailable,
523-
"truncated_stream",
524-
"stream ended before message stop",
525-
"",
526-
true,
527-
nil,
528-
)
529-
}

features/model/bedrock/stream.go

Lines changed: 5 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,11 @@ func (s *bedrockStreamer) run() {
127127
} else if err := s.ctx.Err(); err != nil {
128128
s.setErr(err)
129129
} else if !processor.complete {
130-
s.setErr(streamEndedEarlyError(processor.started))
130+
s.setErr(model.NewStreamEndedEarlyError(
131+
bedrockProviderName,
132+
"converse_stream",
133+
processor.started,
134+
))
131135
} else if err := processor.finishStream(); err != nil {
132136
s.setErr(err)
133137
} else {
@@ -589,33 +593,6 @@ func (p *chunkProcessor) finishStream() error {
589593
return p.emit(model.StopChunk{Reason: p.canonical.StopReason})
590594
}
591595

592-
// streamEndedEarlyError classifies a Bedrock event stream that closed cleanly
593-
// before message stop. When no message ever started, the provider produced an
594-
// empty completion and callers may retry (model.ErrEmptyStream). When a
595-
// message was underway, the stream was truncated mid-generation: a fresh
596-
// request regenerates the full response, so the failure is a retryable
597-
// provider fault but not an empty stream.
598-
func streamEndedEarlyError(started bool) error {
599-
if !started {
600-
return model.NewEmptyStreamError(
601-
bedrockProviderName,
602-
"converse_stream",
603-
"stream ended before message start",
604-
)
605-
}
606-
return model.NewProviderError(
607-
bedrockProviderName,
608-
"converse_stream",
609-
0,
610-
model.ProviderErrorKindUnavailable,
611-
"truncated_stream",
612-
"stream ended before message stop",
613-
"",
614-
true,
615-
nil,
616-
)
617-
}
618-
619596
type toolBuffer struct {
620597
name string
621598
id string

features/model/bedrock/stream_usage_test.go

Lines changed: 0 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -369,35 +369,3 @@ func TestChunkProcessorRejectsDuplicateMessageStop(t *testing.T) {
369369
require.EqualError(t, err, "bedrock stream: duplicate message stop")
370370
require.NotErrorIs(t, err, model.ErrEmptyStream)
371371
}
372-
373-
// TestStreamEndedEarlyErrorClassification verifies the two terminal shapes of
374-
// a Bedrock event stream that closes before messageStop: never-started
375-
// streams are retryable empty streams, mid-message closes are retryable
376-
// truncations that must not carry the empty-stream sentinel.
377-
func TestStreamEndedEarlyErrorClassification(t *testing.T) {
378-
tests := []struct {
379-
name string
380-
started bool
381-
wantEmpty bool
382-
wantCode string
383-
}{
384-
{name: "never started", started: false, wantEmpty: true, wantCode: "empty_stream"},
385-
{name: "truncated mid message", started: true, wantEmpty: false, wantCode: "truncated_stream"},
386-
}
387-
for _, tt := range tests {
388-
t.Run(tt.name, func(t *testing.T) {
389-
err := streamEndedEarlyError(tt.started)
390-
391-
if tt.wantEmpty {
392-
require.ErrorIs(t, err, model.ErrEmptyStream)
393-
} else {
394-
require.NotErrorIs(t, err, model.ErrEmptyStream)
395-
}
396-
pe, ok := model.AsProviderError(err)
397-
require.True(t, ok)
398-
require.Equal(t, model.ProviderErrorKindUnavailable, pe.Kind())
399-
require.Equal(t, tt.wantCode, pe.Code())
400-
require.True(t, pe.Retryable())
401-
})
402-
}
403-
}

runtime/agent/model/provider_error.go

Lines changed: 41 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,17 @@ const (
2929
ProviderErrorKindUnknown ProviderErrorKind = "unknown"
3030
)
3131

32-
// providerErrorCodeEmptyStream is the stable code carried by empty-stream
33-
// provider errors. Detection goes through errors.Is(err, ErrEmptyStream);
34-
// the code exists for observability (span/error attributes), not matching.
35-
const providerErrorCodeEmptyStream = "empty_stream"
32+
const (
33+
// providerErrorCodeEmptyStream is the stable code carried by empty-stream
34+
// provider errors. Detection goes through errors.Is(err, ErrEmptyStream);
35+
// the code exists for observability (span/error attributes), not matching.
36+
providerErrorCodeEmptyStream = "empty_stream"
37+
38+
// providerErrorCodeTruncatedStream is the stable code carried by provider
39+
// errors for streams that closed mid-message. Like the empty-stream code,
40+
// it exists for observability, not matching.
41+
providerErrorCodeTruncatedStream = "truncated_stream"
42+
)
3643

3744
// ProviderError describes a failure returned by a model provider (e.g. Bedrock).
3845
// It is intended to cross package boundaries so runtimes can surface stable,
@@ -154,6 +161,36 @@ func NewEmptyStreamError(provider, operation, message string) error {
154161
return errors.Join(ErrEmptyStream, pe)
155162
}
156163

164+
// NewStreamEndedEarlyError classifies an event stream that terminated cleanly
165+
// before the provider's message-stop boundary. Adapters call it from their
166+
// stream event loops when the provider closes the connection without error
167+
// but the message protocol is unfinished. started reports whether an
168+
// assistant message had begun:
169+
//
170+
// - started=false means the provider produced an empty completion; the
171+
// result is an empty-stream error (see NewEmptyStreamError) that callers
172+
// may retry via errors.Is(err, ErrEmptyStream).
173+
// - started=true means the stream was truncated mid-generation; the result
174+
// is a retryable unavailable ProviderError (code truncated_stream)
175+
// without the empty-stream sentinel, so pre-output retry policies never
176+
// match partially delivered responses.
177+
func NewStreamEndedEarlyError(provider, operation string, started bool) error {
178+
if !started {
179+
return NewEmptyStreamError(provider, operation, "stream ended before message start")
180+
}
181+
return NewProviderError(
182+
provider,
183+
operation,
184+
0,
185+
ProviderErrorKindUnavailable,
186+
providerErrorCodeTruncatedStream,
187+
"stream ended before message stop",
188+
"",
189+
true,
190+
nil,
191+
)
192+
}
193+
157194
// ClassifyHTTPStatus maps an HTTP status code returned by a model provider to
158195
// the goa-ai provider error contract. It is the single status-to-kind table
159196
// for adapters that need only status-based classification (Vertex Gemini,

runtime/agent/model/provider_error_test.go

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,40 @@ func TestNewEmptyStreamError(t *testing.T) {
6565
assert.Equal(t, "message stop received without an active message", pe.Message())
6666
}
6767

68+
// TestNewStreamEndedEarlyError verifies the two terminal shapes of an event
69+
// stream that closes before message stop: never-started streams are retryable
70+
// empty streams, mid-message closes are retryable truncations that must not
71+
// carry the empty-stream sentinel.
72+
func TestNewStreamEndedEarlyError(t *testing.T) {
73+
tests := []struct {
74+
name string
75+
started bool
76+
wantEmpty bool
77+
wantCode string
78+
}{
79+
{name: "never started", started: false, wantEmpty: true, wantCode: "empty_stream"},
80+
{name: "truncated mid message", started: true, wantEmpty: false, wantCode: "truncated_stream"},
81+
}
82+
for _, tt := range tests {
83+
t.Run(tt.name, func(t *testing.T) {
84+
err := NewStreamEndedEarlyError("test-provider", "converse_stream", tt.started)
85+
86+
if tt.wantEmpty {
87+
require.ErrorIs(t, err, ErrEmptyStream)
88+
} else {
89+
require.NotErrorIs(t, err, ErrEmptyStream)
90+
}
91+
pe, ok := AsProviderError(err)
92+
require.True(t, ok)
93+
assert.Equal(t, ProviderErrorKindUnavailable, pe.Kind())
94+
assert.Equal(t, tt.wantCode, pe.Code())
95+
assert.True(t, pe.Retryable())
96+
assert.Equal(t, "test-provider", pe.Provider())
97+
assert.Equal(t, "converse_stream", pe.Operation())
98+
})
99+
}
100+
}
101+
68102
func TestClassifyHTTPStatusPreservesRateLimitedCause(t *testing.T) {
69103
// A pre-classified sentinel (status 0) must still satisfy errors.Is via
70104
// the Unwrap chain even though the status alone does not select the

0 commit comments

Comments
 (0)