diff --git a/docs/release-control/v6/internal/subsystems/notifications.md b/docs/release-control/v6/internal/subsystems/notifications.md index 290aa0369..a8ac05bad 100644 --- a/docs/release-control/v6/internal/subsystems/notifications.md +++ b/docs/release-control/v6/internal/subsystems/notifications.md @@ -139,6 +139,21 @@ negative overflow cases alongside ordinary seconds, dates, whitespace and invalid inputs. This is pure parser proof: it does not establish elapsed HTTP retry timing, queue persistence, provider acceptance or recipient receipt. +For HTTP 429 transport retries, a valid `Retry-After` overrides only the wait +following that response. It must not replace the independent exponential +backoff schedule. In particular, a zero-delay response cannot cause later +headerless 429 or 503 failures to exhaust their remaining retries immediately. +The schedule continues doubling up to `WebhookMaxBackoff`; retry budgets, +classification, parsing and the existing header-delay cap remain unchanged. +This does not extend header handling to 503 responses. + +`TestSendWebhookWithRetry_ZeroRetryAfterPreservesLaterBackoff` in +`internal/notifications/webhook_enhanced_test.go` sends a zero-delay 429, +then a headerless 429 or 503, then success through a local HTTP fixture. It +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. + 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 48d346eab..0917468ed 100644 --- a/internal/notifications/webhook_enhanced.go +++ b/internal/notifications/webhook_enhanced.go @@ -330,7 +330,8 @@ func (n *NotificationManager) sendWebhookWithRetry(webhook EnhancedWebhookConfig Dur("retryAfter", customBackoff). Msg("using Retry-After header for backoff") time.Sleep(customBackoff) - backoff = customBackoff // Use this for next iteration + // The header overrides this wait only. Preserve the exponential + // schedule so a zero delay cannot erase later failure backoff. usedBackoff = true } else { log.Debug(). diff --git a/internal/notifications/webhook_enhanced_test.go b/internal/notifications/webhook_enhanced_test.go index d47f4f482..68a2ce0c9 100644 --- a/internal/notifications/webhook_enhanced_test.go +++ b/internal/notifications/webhook_enhanced_test.go @@ -844,3 +844,41 @@ func TestIsRetryableWebhookErrorEnhanced(t *testing.T) { }) } } + +// A response-specific zero delay must not erase backoff for later failures. +func TestSendWebhookWithRetry_ZeroRetryAfterPreservesLaterBackoff(t *testing.T) { + for _, status := range []int{http.StatusServiceUnavailable, http.StatusTooManyRequests} { + t.Run(fmt.Sprint(status), func(t *testing.T) { + nm := NewNotificationManager("http://pulse.local") + t.Cleanup(nm.Stop) + require.NoError(t, nm.UpdateAllowedPrivateCIDRs("127.0.0.1")) + attemptTimes := make(chan time.Time, 3) + attempts := 0 + server := newIPv4HTTPServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + attempts++ + attemptTimes <- time.Now() + switch attempts { + case 1: + w.Header().Set("Retry-After", "0") + w.WriteHeader(http.StatusTooManyRequests) + case 2: + w.WriteHeader(status) + default: + w.WriteHeader(http.StatusOK) + } + })) + defer server.Close() + webhook := EnhancedWebhookConfig{ + WebhookConfig: WebhookConfig{Name: "Backoff regression", URL: server.URL}, + RetryEnabled: true, + RetryCount: 2, + } + require.NoError(t, nm.sendWebhookWithRetry(webhook, []byte("{}"), "backoff:alert")) + require.Len(t, attemptTimes, 3) + <-attemptTimes + second, third := <-attemptTimes, <-attemptTimes + assert.GreaterOrEqual(t, third.Sub(second), 2*WebhookInitialBackoff, + "later failure must retain exponential backoff after a zero Retry-After") + }) + } +}