Skip to content

Commit e55a958

Browse files
xmtp-coder-agentclaude
andcommitted
Rename MaxRetries to MaxAttempts and clamp to minimum 1
Renamed to MaxAttempts (CLI: --http-max-attempts, env: HTTP_MAX_ATTEMPTS) so the semantics are clearer: maxAttempts=1 means one try (no retries), maxAttempts=2 means one try plus one retry. Values below 1 are clamped to 1 in the constructor to prevent misconfiguration where nothing would be delivered. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 5c90362 commit e55a958

3 files changed

Lines changed: 36 additions & 21 deletions

File tree

pkg/delivery/http.go

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,16 +17,21 @@ type HttpDelivery struct {
1717
address string
1818
authHeader string
1919
logger *zap.Logger
20-
maxRetries int
20+
maxAttempts int
2121
initialRetryDelay time.Duration
2222
}
2323

2424
func NewHttpDelivery(logger *zap.Logger, opts options.HttpDeliveryOptions) *HttpDelivery {
25+
maxAttempts := opts.MaxAttempts
26+
if maxAttempts < 1 {
27+
maxAttempts = 1
28+
}
29+
2530
return &HttpDelivery{
2631
logger: logger,
2732
address: opts.Address,
2833
authHeader: opts.AuthHeader,
29-
maxRetries: opts.MaxRetries,
34+
maxAttempts: maxAttempts,
3035
initialRetryDelay: time.Duration(opts.InitialRetryDelayMs) * time.Millisecond,
3136
}
3237
}
@@ -43,17 +48,17 @@ func (h HttpDelivery) Send(ctx context.Context, req interfaces.SendRequest) erro
4348
}
4449

4550
var lastErr error
46-
for attempt := 0; attempt <= h.maxRetries; attempt++ {
51+
for attempt := 0; attempt < h.maxAttempts; attempt++ {
4752
lastErr = h.doSend(ctx, jsonData)
4853
if lastErr == nil {
4954
return nil
5055
}
5156

52-
if attempt < h.maxRetries {
57+
if attempt < h.maxAttempts-1 {
5358
delay := h.initialRetryDelay * (1 << uint(attempt))
5459
h.logger.Warn("HTTP delivery failed, retrying",
5560
zap.Int("attempt", attempt+1),
56-
zap.Int("max_retries", h.maxRetries),
61+
zap.Int("max_attempts", h.maxAttempts),
5762
zap.Duration("next_delay", delay),
5863
zap.Error(lastErr),
5964
)

pkg/delivery/http_test.go

Lines changed: 25 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,12 @@ func newTestRequest() interfaces.SendRequest {
2323

2424
// testServerAndDelivery creates an httptest server with the given handler and
2525
// an HttpDelivery pointed at it. The caller should defer server.Close().
26-
func testServerAndDelivery(t *testing.T, handler http.HandlerFunc, maxRetries int, initialDelayMs int) (*httptest.Server, *HttpDelivery) {
26+
func testServerAndDelivery(t *testing.T, handler http.HandlerFunc, maxAttempts int, initialDelayMs int) (*httptest.Server, *HttpDelivery) {
2727
t.Helper()
2828
server := httptest.NewServer(handler)
2929
d := NewHttpDelivery(zaptest.NewLogger(t), options.HttpDeliveryOptions{
3030
Address: server.URL,
31-
MaxRetries: maxRetries,
31+
MaxAttempts: maxAttempts,
3232
InitialRetryDelayMs: initialDelayMs,
3333
})
3434
return server, d
@@ -70,17 +70,16 @@ func TestHttpDelivery_RetryOnFailureThenSuccess(t *testing.T) {
7070
assert.Equal(t, int32(2), atomic.LoadInt32(&requestCount))
7171
}
7272

73-
func TestHttpDelivery_ExhaustsRetries(t *testing.T) {
73+
func TestHttpDelivery_ExhaustsAttempts(t *testing.T) {
7474
var requestCount int32
75-
maxRetries := 2
76-
server, d := testServerAndDelivery(t, countingHandler(&requestCount, http.StatusInternalServerError), maxRetries, 10)
75+
maxAttempts := 3
76+
server, d := testServerAndDelivery(t, countingHandler(&requestCount, http.StatusInternalServerError), maxAttempts, 10)
7777
defer server.Close()
7878

7979
err := d.Send(context.Background(), newTestRequest())
8080
require.Error(t, err)
8181
assert.Equal(t, "HTTP request failed", err.Error())
82-
// Initial attempt + maxRetries retries
83-
assert.Equal(t, int32(maxRetries+1), atomic.LoadInt32(&requestCount))
82+
assert.Equal(t, int32(maxAttempts), atomic.LoadInt32(&requestCount))
8483
}
8584

8685
func TestHttpDelivery_ContextCancellation(t *testing.T) {
@@ -109,11 +108,11 @@ func TestHttpDelivery_ContextCancellation(t *testing.T) {
109108
func TestHttpDelivery_DefaultConfig(t *testing.T) {
110109
d := NewHttpDelivery(zaptest.NewLogger(t), options.HttpDeliveryOptions{
111110
Address: "http://localhost:9999",
112-
MaxRetries: 1,
111+
MaxAttempts: 1,
113112
InitialRetryDelayMs: 250,
114113
})
115114

116-
assert.Equal(t, 1, d.maxRetries)
115+
assert.Equal(t, 1, d.maxAttempts)
117116
assert.Equal(t, 250*time.Millisecond, d.initialRetryDelay)
118117
}
119118

@@ -122,12 +121,12 @@ func TestHttpDelivery_ExponentialBackoff(t *testing.T) {
122121
server, d := testServerAndDelivery(t, func(w http.ResponseWriter, r *http.Request) {
123122
timestamps = append(timestamps, time.Now())
124123
w.WriteHeader(http.StatusInternalServerError)
125-
}, 3, 50)
124+
}, 4, 50)
126125
defer server.Close()
127126

128127
_ = d.Send(context.Background(), newTestRequest())
129128

130-
// Should have 4 requests total (1 initial + 3 retries)
129+
// Should have 4 requests total (maxAttempts=4)
131130
require.Len(t, timestamps, 4)
132131

133132
// Verify delays roughly double each time
@@ -141,17 +140,28 @@ func TestHttpDelivery_ExponentialBackoff(t *testing.T) {
141140
}
142141
}
143142

144-
func TestHttpDelivery_ZeroRetries(t *testing.T) {
143+
func TestHttpDelivery_SingleAttempt(t *testing.T) {
145144
var requestCount int32
146-
server, d := testServerAndDelivery(t, countingHandler(&requestCount, http.StatusInternalServerError), 0, 10)
145+
server, d := testServerAndDelivery(t, countingHandler(&requestCount, http.StatusInternalServerError), 1, 10)
147146
defer server.Close()
148147

149148
err := d.Send(context.Background(), newTestRequest())
150149
require.Error(t, err)
151-
// With maxRetries=0, only one attempt is made
150+
// With maxAttempts=1, only one attempt is made (no retries)
152151
assert.Equal(t, int32(1), atomic.LoadInt32(&requestCount))
153152
}
154153

154+
func TestHttpDelivery_MaxAttemptsClampsToMinimumOne(t *testing.T) {
155+
d := NewHttpDelivery(zaptest.NewLogger(t), options.HttpDeliveryOptions{
156+
Address: "http://localhost:9999",
157+
MaxAttempts: 0,
158+
InitialRetryDelayMs: 10,
159+
})
160+
161+
// Value of 0 should be clamped to 1
162+
assert.Equal(t, 1, d.maxAttempts)
163+
}
164+
155165
func TestHttpDelivery_CanDeliver(t *testing.T) {
156166
d := NewHttpDelivery(zaptest.NewLogger(t), options.HttpDeliveryOptions{})
157167
assert.True(t, d.CanDeliver(newTestRequest()))
@@ -168,7 +178,7 @@ func TestHttpDelivery_AuthHeader(t *testing.T) {
168178
d := NewHttpDelivery(zaptest.NewLogger(t), options.HttpDeliveryOptions{
169179
Address: server.URL,
170180
AuthHeader: "Bearer test-token",
171-
MaxRetries: 0,
181+
MaxAttempts: 1,
172182
InitialRetryDelayMs: 10,
173183
})
174184

pkg/options/options.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ type HttpDeliveryOptions struct {
3232
Enabled bool `long:"http-delivery"`
3333
Address string `long:"http-delivery-address"`
3434
AuthHeader string `long:"http-auth-header"`
35-
MaxRetries int `long:"http-max-retries" env:"HTTP_MAX_RETRIES" default:"1" description:"Maximum number of retry attempts for failed HTTP deliveries"`
35+
MaxAttempts int `long:"http-max-attempts" env:"HTTP_MAX_ATTEMPTS" default:"1" description:"Maximum number of delivery attempts (minimum 1, includes initial attempt)"`
3636
InitialRetryDelayMs int `long:"http-initial-retry-delay-ms" env:"HTTP_INITIAL_RETRY_DELAY_MS" default:"250" description:"Initial retry delay in milliseconds (doubles with each retry)"`
3737
}
3838

0 commit comments

Comments
 (0)