diff --git a/docs/release-control/v6/internal/subsystems/notifications.md b/docs/release-control/v6/internal/subsystems/notifications.md index a2c57128e..c971e68c6 100644 --- a/docs/release-control/v6/internal/subsystems/notifications.md +++ b/docs/release-control/v6/internal/subsystems/notifications.md @@ -877,3 +877,18 @@ proves a truncated 403 stops after one attempt with terminal history, while a truncated 503 can retry to 204 with its event identity intact. No queue or storage workers are started by these tests; this is not installed delivery acceptance or a change to retry budgets. + +### HTTP retry classification agrees across delivery layers + +HTTP 421 retains a connectivity class; 423 and 425 retain a server-error class +for temporary receiver conditions. These are already retryable exceptions in +the webhook transport, and must not become terminal rejections when a wrapped +transport error reaches the queue. Authentication, configuration and other +permanent HTTP rejections still stop early; attempt limits remain unchanged. + +`TestWebhookHTTPRetryPolicyMatchesQueueClassification` exercises every status +400–599 through a synthetic HTTP transport with misleading diagnostic text, +wraps its returned error and checks both transport retry policy and the shared +class predicate used by the queue. `TestClassFromHTTPStatus` pins the reason +classes. This proves the classification boundary without starting queue/storage +workers; it does not establish installed receipt or queue scheduling execution. diff --git a/internal/notifications/failure_class.go b/internal/notifications/failure_class.go index 9995a9d21..db2c339bd 100644 --- a/internal/notifications/failure_class.go +++ b/internal/notifications/failure_class.go @@ -71,8 +71,13 @@ func ClassFromHTTPStatus(status int) NotificationFailureClass { return NotificationFailureAuthentication case 402: return NotificationFailureConfiguration - case 408: + case 408, 421: return NotificationFailureConnectivity + case 423, 425: + // The webhook transport already retries locked/too-early responses. + // These temporary receiver conditions must retain the queue's retry + // budget too, rather than becoming terminal request rejections. + return NotificationFailureServerError case 429: return NotificationFailureRateLimited } diff --git a/internal/notifications/failure_class_test.go b/internal/notifications/failure_class_test.go index a2ba78da2..740e206a4 100644 --- a/internal/notifications/failure_class_test.go +++ b/internal/notifications/failure_class_test.go @@ -18,6 +18,9 @@ func TestClassFromHTTPStatus(t *testing.T) { 407: NotificationFailureAuthentication, 402: NotificationFailureConfiguration, 408: NotificationFailureConnectivity, + 421: NotificationFailureConnectivity, + 423: NotificationFailureServerError, + 425: NotificationFailureServerError, 429: NotificationFailureRateLimited, 400: NotificationFailureRejected, 404: NotificationFailureRejected, diff --git a/internal/notifications/webhook_retry_test.go b/internal/notifications/webhook_retry_test.go index e2adcece6..7c7351238 100644 --- a/internal/notifications/webhook_retry_test.go +++ b/internal/notifications/webhook_retry_test.go @@ -5,6 +5,7 @@ import ( "fmt" "io" "net/http" + "strings" "sync/atomic" "testing" ) @@ -495,3 +496,34 @@ func TestWebhookRetryRateLimitThenTerminalRejection(t *testing.T) { t.Errorf("history lost final rejection or retry accounting: %+v", history[0]) } } + +// Queue retry policy consumes the classified, wrapped transport error, not the +// HTTP response. Keep both retry layers consistent for every rejection status. +func TestWebhookHTTPRetryPolicyMatchesQueueClassification(t *testing.T) { + for code := 400; code <= 599; code++ { + t.Run(fmt.Sprint(code), func(t *testing.T) { + nm := &NotificationManager{webhookClient: &http.Client{ + Transport: confidentialityTransport(func(*http.Request) (*http.Response, error) { + return &http.Response{ + StatusCode: code, Header: make(http.Header), + Body: io.NopCloser(strings.NewReader("unauthorized timeout rate limit")), + }, nil + }), + }} + _, err := nm.executeWebhookRequest(WebhookConfig{URL: "https://example.test/hook"}, + []byte("{}"), webhookRequestOptions{}) + if err == nil { + t.Fatal("expected HTTP rejection") + } + err = fmt.Errorf("webhook delivery exhausted: %w", err) + wantRetry := code >= 500 || code == 408 || code == 421 || code == 423 || code == 425 || code == 429 + if got := isRetryableWebhookError(err); got != wantRetry { + t.Errorf("transport retryable = %v, want %v", got, wantRetry) + } + class := ClassifyNotificationFailureError(err) + if got := class.Retryable(); got != wantRetry { + t.Errorf("queue class %s retryable = %v, want %v", class, got, wantRetry) + } + }) + } +}