Store a target's number of delivery attempts, at least one (closes #540)
check / check (push) Waiting to run

Target.MaxRetries (max_retries) becomes Target.MaxAttempts
(max_attempts), and the form field, parser and ceiling are renamed to
match. 1 is a single attempt with no circuit breaker; above 1 retries
behind the breaker up to that many attempts. Both target forms refuse
0 and less; a blank field on the add target form gives 1, and on the
edit form keeps the stored value. The target page shows the plain
number, and the README's two paragraphs on 0 against 1 become one
sentence.

Model: opus-5-5
This commit is contained in:
2026-10-15 11:34:18 +00:00
parent c0f7cc29fd
commit 14a38df093
37 changed files with 843 additions and 716 deletions
+110 -24
View File
@@ -142,7 +142,7 @@ func buildHTTPFixture(
dlv database.Delivery,
event database.Event,
targetID, name, config string,
maxRetries, attemptNum int,
maxAttempts, attemptNum int,
) httpDeliveryFixture {
task := &delivery.Task{
DeliveryID: dlv.ID,
@@ -152,7 +152,7 @@ func buildHTTPFixture(
TargetName: name,
TargetType: database.TargetTypeHTTP,
TargetConfig: config,
MaxRetries: maxRetries,
MaxAttempts: maxAttempts,
AttemptNum: attemptNum,
}
@@ -162,10 +162,10 @@ func buildHTTPFixture(
Status: dlv.Status,
Event: event,
Target: database.Target{
Name: name,
Type: database.TargetTypeHTTP,
Config: config,
MaxRetries: maxRetries,
Name: name,
Type: database.TargetTypeHTTP,
Config: config,
MaxAttempts: maxAttempts,
},
}
d.ID = dlv.ID
@@ -277,7 +277,7 @@ func TestDeliverHTTP_Success(t *testing.T) {
cfg := newHTTPTargetConfig(ts.URL)
fix := buildHTTPFixture(
dlv, event, targetID, "test-http", cfg, 0, 1,
dlv, event, targetID, "test-http", cfg, 1, 1,
)
e.ExportDeliverHTTP(
@@ -324,7 +324,7 @@ func TestDeliverHTTP_Failure(t *testing.T) {
cfg := newHTTPTargetConfig(ts.URL)
fix := buildHTTPFixture(
dlv, event, targetID,
"test-http-fail", cfg, 0, 1,
"test-http-fail", cfg, 1, 1,
)
e.ExportDeliverHTTP(
@@ -473,14 +473,23 @@ func TestDeliverHTTP_WithRetries_Success(t *testing.T) {
)
}
func TestDeliverHTTP_MaxRetriesExhausted(t *testing.T) {
// TestDeliverHTTP_MaxAttemptsExhausted proves a target with three
// delivery attempts makes at most three: the first two failures are
// retried, the third is final, and each attempt goes through the
// target's circuit breaker.
func TestDeliverHTTP_MaxAttemptsExhausted(t *testing.T) {
t.Parallel()
const maxAttempts = 3
db := testWebhookDB(t)
var requests atomic.Int32
ts := httptest.NewServer(
http.HandlerFunc(
func(w http.ResponseWriter, _ *http.Request) {
requests.Add(1)
w.WriteHeader(http.StatusBadGateway)
},
),
@@ -489,26 +498,103 @@ func TestDeliverHTTP_MaxRetriesExhausted(t *testing.T) {
e := testEngine(t, 1)
targetID := uuid.New().String()
// It opens on the last failure only if every attempt reached it.
cb := delivery.NewTestCircuitBreaker(maxAttempts, time.Minute)
e.ExportSetCircuitBreaker(targetID, cb)
event := seedEvent(t, db, `{"retry":"exhaust"}`)
dlv := seedDelivery(
t, db, event.ID, targetID,
database.DeliveryStatusRetrying,
database.DeliveryStatusPending,
)
cfg := newHTTPTargetConfig(ts.URL)
sched := &recordingScheduler{}
for attempt := 1; attempt <= maxAttempts; attempt++ {
fix := buildHTTPFixture(
dlv, event, targetID,
"test-http-exhaust", cfg, maxAttempts, attempt,
)
e.ExportDeliverHTTPWithScheduler(
context.TODO(), db, fix.Delivery, fix.Task, sched,
)
}
assert.Equal(t, int32(maxAttempts), requests.Load())
assert.Len(t, sched.delays, maxAttempts-1,
"every failure but the last should be retried",
)
assertDeliveryStatus(t, db, dlv.ID,
database.DeliveryStatusFailed,
)
assert.Equal(t, delivery.CircuitOpen, cb.State(),
"every attempt should be counted by the circuit breaker",
)
}
// TestDeliverHTTP_SingleAttemptSkipsCircuitBreaker proves a target
// with one delivery attempt makes exactly that attempt, even while
// its circuit breaker is open, and that a failure is final.
func TestDeliverHTTP_SingleAttemptSkipsCircuitBreaker(t *testing.T) {
t.Parallel()
db := testWebhookDB(t)
var requests atomic.Int32
ts := httptest.NewServer(
http.HandlerFunc(
func(w http.ResponseWriter, _ *http.Request) {
requests.Add(1)
w.WriteHeader(http.StatusBadGateway)
},
),
)
defer ts.Close()
e := testEngine(t, 1)
targetID := uuid.New().String()
cb := e.ExportGetCircuitBreaker(targetID)
for range delivery.ExportDefaultFailureThreshold {
cb.RecordFailure()
}
require.Equal(t, delivery.CircuitOpen, cb.State())
event := seedEvent(t, db, `{"retry":"none"}`)
dlv := seedDelivery(
t, db, event.ID, targetID,
database.DeliveryStatusPending,
)
cfg := newHTTPTargetConfig(ts.URL)
fix := buildHTTPFixture(
dlv, event, targetID,
"test-http-exhaust", cfg, 3, 3,
"test-http-single", cfg, 1, 1,
)
sched := &recordingScheduler{}
e.ExportDeliverHTTPWithScheduler(
context.TODO(), db, fix.Delivery, fix.Task, sched,
)
e.ExportDeliverHTTP(
context.TODO(), db, fix.Delivery, fix.Task,
assert.Equal(t, int32(1), requests.Load(),
"the open circuit breaker should not hold the attempt back",
)
assert.Empty(t, sched.delays, "no retry should be scheduled")
assertDeliveryStatus(t, db, dlv.ID,
database.DeliveryStatusFailed,
)
assertDeliveryResult(
t, db, dlv.ID, false, http.StatusBadGateway,
)
}
func TestDeliverHTTP_SchedulesRetryOnFailure(
@@ -741,7 +827,7 @@ func seedConcurrencyTasks(
TargetName: tasks[i].Target.Name,
TargetType: tasks[i].Target.Type,
TargetConfig: tasks[i].Target.Config,
MaxRetries: 0,
MaxAttempts: 1,
AttemptNum: 1,
}
}
@@ -2051,10 +2137,10 @@ func buildSlackRetryDelivery(
Status: database.DeliveryStatusPending,
Event: event,
Target: database.Target{
Name: "test-slack-retry",
Type: database.TargetTypeSlack,
Config: cfg,
MaxRetries: 5,
Name: "test-slack-retry",
Type: database.TargetTypeSlack,
Config: cfg,
MaxAttempts: 5,
},
}
d.ID = dlv.ID
@@ -2089,11 +2175,11 @@ func TestDeliverSlack_WithRetries_SchedulesRetry(
)
task := &delivery.Task{
DeliveryID: dlv.ID,
TargetID: targetID,
TargetType: database.TargetTypeSlack,
MaxRetries: 5,
AttemptNum: 1,
DeliveryID: dlv.ID,
TargetID: targetID,
TargetType: database.TargetTypeSlack,
MaxAttempts: 5,
AttemptNum: 1,
}
e.ExportProcessDelivery(context.TODO(), db, d, task)