Skip to content

Commit 48b9fab

Browse files
authored
fix: deadline and timeout with retry (#5)
1 parent c08e525 commit 48b9fab

2 files changed

Lines changed: 60 additions & 0 deletions

File tree

http_retrier.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,16 @@ func (r *retryTransport) RoundTrip(req *http.Request) (*http.Response, error) {
126126
}
127127
}
128128

129+
// Do not retry if the error is due to context cancellation or deadline exceeded.
130+
// When http.Client.Timeout fires, it cancels the request context. Since this
131+
// context is shared across all retry attempts, subsequent retries would fail
132+
// immediately. Return the original error to avoid misleading "retry cancelled" messages.
133+
if err != nil {
134+
if ctx := req.Context(); ctx != nil && ctx.Err() != nil {
135+
return nil, err
136+
}
137+
}
138+
129139
// Check if we should retry
130140
if attempt < r.MaxRetries {
131141
delay := retryStrategy(attempt)

http_retrier_test.go

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1047,6 +1047,56 @@ func TestRetryTransport_ContextCancellation(t *testing.T) {
10471047
}
10481048
}
10491049

1050+
func TestRetryTransport_NoRetryOnContextDeadlineExceeded(t *testing.T) {
1051+
var attempts int32 = 0
1052+
1053+
mockRT := &mockRoundTripper{
1054+
roundTripFunc: func(req *http.Request) (*http.Response, error) {
1055+
atomic.AddInt32(&attempts, 1)
1056+
// Simulate the error that http.Client produces when Client.Timeout fires
1057+
return nil, fmt.Errorf("Post \"http://localhost:11434/api/generate\": context deadline exceeded (Client.Timeout exceeded while awaiting headers)")
1058+
},
1059+
}
1060+
1061+
retryRT := &retryTransport{
1062+
Transport: mockRT,
1063+
MaxRetries: 5,
1064+
RetryStrategy: FixedDelay(1 * time.Millisecond),
1065+
}
1066+
1067+
// Create a request with an already-expired context to simulate Client.Timeout behavior
1068+
ctx, cancel := context.WithCancel(context.Background())
1069+
cancel() // Cancel immediately to simulate expired deadline
1070+
1071+
req, err := http.NewRequestWithContext(ctx, "POST", "http://localhost:11434/api/generate", nil)
1072+
if err != nil {
1073+
t.Fatalf("Failed to create request: %v", err)
1074+
}
1075+
1076+
start := time.Now()
1077+
_, err = retryRT.RoundTrip(req)
1078+
elapsed := time.Since(start)
1079+
1080+
if err == nil {
1081+
t.Fatal("Expected an error, got nil")
1082+
}
1083+
1084+
// Should have made exactly 1 attempt — no retries when context is done
1085+
if atomic.LoadInt32(&attempts) != 1 {
1086+
t.Errorf("Expected exactly 1 attempt (no retries on context deadline), got %d", atomic.LoadInt32(&attempts))
1087+
}
1088+
1089+
// Should return quickly without waiting for retry delays
1090+
if elapsed > 500*time.Millisecond {
1091+
t.Errorf("Expected immediate return, but took %v", elapsed)
1092+
}
1093+
1094+
// The error should be the original transport error, not wrapped as "retry cancelled"
1095+
if strings.Contains(err.Error(), "retry cancelled") {
1096+
t.Errorf("Error should not contain 'retry cancelled', got: %v", err)
1097+
}
1098+
}
1099+
10501100
func TestRetryTransport_ContextCancelledDuringRetryDelay(t *testing.T) {
10511101
var attempts int32 = 0
10521102

0 commit comments

Comments
 (0)