Skip to content

Commit 5a0adba

Browse files
authored
Merge pull request #2 from hex-ci/devin/1780644362-eof-statusless-gate
Limit immediate unexpected-EOF retry to statusless errors
2 parents 7a863eb + 84b40f0 commit 5a0adba

2 files changed

Lines changed: 42 additions & 5 deletions

File tree

sdk/cliproxy/auth/conductor.go

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2319,11 +2319,16 @@ func (m *Manager) shouldRetryAfterError(err error, attempt int, providers []stri
23192319
if isRequestInvalidError(err) {
23202320
return 0, false
23212321
}
2322-
// Treat transient upstream stream interruptions (e.g. "unexpected EOF")
2323-
// like a normal retryable API error instead of surfacing them to the
2324-
// client. These carry no HTTP status, so they would otherwise fall through
2325-
// the 429-only retry path below and abort the request.
2326-
if isUnexpectedEOFError(err) {
2322+
// Treat transient, statusless upstream stream interruptions (e.g.
2323+
// "unexpected EOF") like a normal retryable API error instead of surfacing
2324+
// them to the client. These carry no HTTP status, so they would otherwise
2325+
// fall through the 429-only retry path below and abort the request. The
2326+
// status==0 guard is important: a status-bearing error (e.g. a 5xx whose
2327+
// body merely contains "unexpected EOF") has already had a cooldown applied
2328+
// by MarkResult, so it must fall through to the cooldown-aware path below
2329+
// rather than force an immediate retry that selection would reject with a
2330+
// model-cooldown error.
2331+
if status == 0 && isUnexpectedEOFError(err) {
23272332
if !m.retryAllowed(attempt, providers) {
23282333
return 0, false
23292334
}

sdk/cliproxy/auth/conductor_overrides_test.go

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,38 @@ func TestManager_ShouldRetryAfterError_RetriesUnexpectedEOF(t *testing.T) {
144144
}
145145
}
146146

147+
func TestManager_ShouldRetryAfterError_StatusBearingEOFUsesCooldownPath(t *testing.T) {
148+
m := NewManager(nil, nil, nil)
149+
m.SetRetryConfig(3, 30*time.Second, 0)
150+
151+
model := "test-model"
152+
// MarkResult applies a one-minute cooldown for transient 5xx responses.
153+
next := time.Now().Add(time.Minute)
154+
auth := &Auth{
155+
ID: "auth-1",
156+
Provider: "claude",
157+
ModelStates: map[string]*ModelState{
158+
model: {
159+
Unavailable: true,
160+
Status: StatusError,
161+
NextRetryAfter: next,
162+
},
163+
},
164+
}
165+
if _, errRegister := m.Register(context.Background(), auth); errRegister != nil {
166+
t.Fatalf("register auth: %v", errRegister)
167+
}
168+
169+
_, _, maxWait := m.retrySettings()
170+
// A 5xx whose body merely contains "unexpected EOF" must NOT take the
171+
// immediate fast path; with a single cooled-down credential and a cooldown
172+
// exceeding max-retry-interval, the cooldown-aware path declines the retry.
173+
statusErr := &Error{HTTPStatus: 500, Message: "internal error: unexpected EOF"}
174+
if wait, shouldRetry := m.shouldRetryAfterError(statusErr, 0, []string{"claude"}, model, maxWait); shouldRetry {
175+
t.Fatalf("expected shouldRetry=false for status-bearing EOF beyond cooldown, got true (wait=%v)", wait)
176+
}
177+
}
178+
147179
func TestManager_ShouldRetryAfterError_UnexpectedEOFRespectsRetryDisabled(t *testing.T) {
148180
m := NewManager(nil, nil, nil)
149181
m.SetRetryConfig(0, 30*time.Second, 0)

0 commit comments

Comments
 (0)