diff --git a/docs/release-control/v6/internal/subsystems/notifications.md b/docs/release-control/v6/internal/subsystems/notifications.md index 6760b98d8..22ebb76e1 100644 --- a/docs/release-control/v6/internal/subsystems/notifications.md +++ b/docs/release-control/v6/internal/subsystems/notifications.md @@ -154,6 +154,26 @@ verifies three requests and the two-second exponential wait before the final request. This is transport timing proof, not durable queue retry, installed provider acceptance or recipient receipt. +### Webhook transport retry exhaustion + +The enhanced retry sender makes one initial HTTP attempt plus `RetryCount` +retries; non-positive counts select `WebhookDefaultRetries`. Repeated valid +zero-delay 429 hints do not reset that budget. When all attempts fail, the +sender returns the total attempt count and records one failed webhook history +entry with the final status and the number of retries, not one entry per HTTP +attempt. Every attempt retains the event ID and payload. A queue delivery that +uses this sender has a transport ceiling of effective retry count plus one; +layering up to `MaxAttempts` queue deliveries multiplies that ceiling, but does +not guarantee destination receipt. Permanent errors may stop retries earlier. + +`TestSendWebhookWithRetry_RateLimitExhaustion` in +`internal/notifications/webhook_enhanced_test.go` verifies configured counts +1 and 2 and default selection for 0 and -1 against an owned HTTP fixture that +always returns 429 with `Retry-After: 0`. It checks request identity and body, +attempt exhaustion, and the single failed history record's status, retry +count, payload size and error text. This is transport-budget and local history +proof only, not queue persistence, provider receipt or installed qualification. + Notification-management HTTP production and its unit/contract proof now live together under `internal/api/alerting/`. Router-level scope and integration tests remain in `internal/api`, while the compatibility aliases there keep the diff --git a/internal/notifications/webhook_enhanced.go b/internal/notifications/webhook_enhanced.go index 0917468ed..3c8d79a3c 100644 --- a/internal/notifications/webhook_enhanced.go +++ b/internal/notifications/webhook_enhanced.go @@ -302,10 +302,11 @@ func (n *NotificationManager) shouldSendWebhook(webhook EnhancedWebhookConfig, a // sendWebhookWithRetry implements exponential backoff retry with enhanced error tracking // Note: When used with the persistent queue, retry behavior is layered: -// - Transport retries (this function): up to RetryCount attempts with exponential backoff -// - Queue retries: up to MaxAttempts (default 3) with exponential backoff -// Total attempts = RetryCount * MaxAttempts (e.g., 3 * 3 = 9 HTTP calls for a single notification) -// This ensures delivery even during transient failures at either layer. +// - Transport: one initial attempt plus RetryCount retries (non-positive counts use WebhookDefaultRetries) +// - Queue: up to MaxAttempts delivery attempts with exponential backoff +// The HTTP attempt ceiling is (effective RetryCount + 1) * MaxAttempts when +// each queue delivery uses this transport. Permanent errors can stop earlier; +// exhausting either retry budget does not guarantee delivery. func (n *NotificationManager) sendWebhookWithRetry(webhook EnhancedWebhookConfig, payload []byte, eventID string) error { maxRetries := webhook.RetryCount if maxRetries <= 0 { diff --git a/internal/notifications/webhook_enhanced_test.go b/internal/notifications/webhook_enhanced_test.go index 68a2ce0c9..48f2396f0 100644 --- a/internal/notifications/webhook_enhanced_test.go +++ b/internal/notifications/webhook_enhanced_test.go @@ -5,6 +5,7 @@ import ( "fmt" "io" "net/http" + "sync/atomic" "testing" "time" @@ -292,6 +293,49 @@ func TestParseRetryAfterBackoff(t *testing.T) { } } +// Even repeated valid zero-delay hints must not reset the transport retry budget. +func TestSendWebhookWithRetry_RateLimitExhaustion(t *testing.T) { + for _, retries := range []int{1, 2, 0, -1} { + t.Run(fmt.Sprint(retries), func(t *testing.T) { + nm := NewNotificationManager("http://pulse.local") + t.Cleanup(nm.Stop) + require.NoError(t, nm.UpdateAllowedPrivateCIDRs("127.0.0.1")) + var attempts atomic.Int32 + payload := []byte(`{"test":true}`) + server := newIPv4HTTPServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + attempts.Add(1) + assert.Equal(t, "limited:alert", r.Header.Get("X-Pulse-Event-ID")) + body, err := io.ReadAll(r.Body) + assert.NoError(t, err) + assert.Equal(t, payload, body) + w.Header().Set("Retry-After", "0") + w.WriteHeader(http.StatusTooManyRequests) + })) + defer server.Close() + webhook := EnhancedWebhookConfig{ + WebhookConfig: WebhookConfig{Name: "Limited webhook", URL: server.URL}, + RetryEnabled: true, + RetryCount: retries, + } + wantRetries := retries + if wantRetries <= 0 { + wantRetries = WebhookDefaultRetries + } + err := nm.sendWebhookWithRetry(webhook, payload, "limited:alert") + require.Error(t, err) + assert.Contains(t, err.Error(), fmt.Sprintf("after %d attempts", wantRetries+1)) + assert.Equal(t, int32(wantRetries+1), attempts.Load()) + history := nm.GetWebhookHistory() + require.Len(t, history, 1, "one failed delivery, not one history entry per HTTP attempt") + assert.False(t, history[0].Success) + assert.Equal(t, http.StatusTooManyRequests, history[0].StatusCode) + assert.Equal(t, wantRetries, history[0].RetryAttempts) + assert.Equal(t, len(payload), history[0].PayloadSize) + assert.Contains(t, history[0].ErrorMessage, "HTTP 429") + }) + } +} + func TestSendWebhookWithRetry_429RetryAfter(t *testing.T) { nm := NewNotificationManager("http://pulse.local") _ = nm.UpdateAllowedPrivateCIDRs("127.0.0.1")