fix(notifications): align with Worker push relay (#374)

* fix(notifications): align with Worker push relay

* fix(notifications): disable invalid APNs tokens

internal/api/handlers/admin_apple_push.go

@@ -3,8 +3,10 @@ package handlers
 import (
 	"encoding/json"
 	"errors"
+	"math"
 	"net/http"
 	"os"
+	"strconv"
 	"time"
 
 	"github.com/Silo-Server/silo-server/internal/notifications"
@@ -146,6 +148,9 @@ func (h *AdminApplePushHandler) HandleRegisterRelay(w http.ResponseWriter, r *ht
 			return
 		}
 		status, code, message := mapRelayRegistrationError(err)
+		if errors.As(err, &relayErr) && relayErr.RetryAfter > 0 {
+			w.Header().Set("Retry-After", strconv.Itoa(max(1, int(math.Ceil(relayErr.RetryAfter.Seconds())))))
+		}
 		writeError(w, status, code, message)
 		return
 	}

internal/api/handlers/admin_apple_push_test.go

@@ -103,6 +103,7 @@ func TestAdminApplePushHandlerRegistersRelayAndStoresKey(t *testing.T) {
 func TestAdminApplePushHandlerMapsRelayRateLimit(t *testing.T) {
 	settings := &fakeServerSettingsStore{values: map[string]string{}}
 	relay := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		w.Header().Set("Retry-After", "60")
 		writeJSON(w, http.StatusTooManyRequests, map[string]any{
 			"error": map[string]string{"code": "rate_limited", "message": "too many deployment registrations from this network"},
 		})
@@ -120,6 +121,9 @@ func TestAdminApplePushHandlerMapsRelayRateLimit(t *testing.T) {
 	if rec.Code != http.StatusTooManyRequests {
 		t.Fatalf("status = %d (%s), want 429", rec.Code, rec.Body.String())
 	}
+	if got := rec.Header().Get("Retry-After"); got != "60" {
+		t.Fatalf("Retry-After = %q, want 60", got)
+	}
 	if settings.values[notifications.SettingPushRelayAPIKey] != "" {
 		t.Fatal("api key was stored after relay rejected registration")
 	}

internal/notifications/push_sender.go

@@ -25,10 +25,12 @@ var pushRetrySchedule = []time.Duration{
 }
 
 const (
-	pushMaxAttempts     = 5
-	pushDispatchQueue   = 512
-	pushRetryClaimLimit = 100
-	relayAppleSendPath  = "/v1/apple/send"
+	pushMaxAttempts         = 5
+	pushDispatchQueue       = 512
+	pushRetryClaimLimit     = 100
+	pushRelayRequestTimeout = 15 * time.Second
+	pushRelayMaxRetryAfter  = 23 * time.Hour
+	relayAppleSendPath      = "/v1/apple/send"
 )
 
 func pushRetryDelay(completedAttempt int) (time.Duration, bool) {
@@ -38,6 +40,30 @@ func pushRetryDelay(completedAttempt int) (time.Duration, bool) {
 	return pushRetrySchedule[completedAttempt] - pushRetrySchedule[completedAttempt-1], true
 }
 
+func pushRetryDelayWithHint(completedAttempt int, retryAfter time.Duration) (time.Duration, bool) {
+	delay, more := pushRetryDelay(completedAttempt)
+	if retryAfter > 0 {
+		// The relay retains idempotency state for 24 hours. Stay safely inside
+		// that window even if APNs returns an unusually large Retry-After value.
+		delay = min(retryAfter, pushRelayMaxRetryAfter)
+	}
+	return delay, more
+}
+
+func terminalAPNsDeviceRejection(status int, code, message string) bool {
+	if status != http.StatusUnprocessableEntity || code != "apns_rejected" {
+		return false
+	}
+	const prefix = "APNs rejected the notification:"
+	reason := strings.TrimSpace(strings.TrimPrefix(message, prefix))
+	switch reason {
+	case "BadDeviceToken", "InvalidToken", "DeviceTokenNotForTopic", "Unregistered":
+		return true
+	default:
+		return false
+	}
+}
+
 type pushRelayAppleRequest struct {
 	Token          string  `json:"token"`
 	Environment    string  `json:"environment"`
@@ -85,12 +111,16 @@ type pushSender struct {
 }
 
 func newPushSender(devices *PushDeviceRepository, deliveries *DeliveryRepository, cipher *secret.Cipher, settings *Settings) *pushSender {
+	// The Worker allows APNs up to 10 seconds. Leave enough room for edge
+	// routing and response processing so Silo receives the relay's classified
+	// outcome instead of manufacturing an ambiguous client-side timeout.
+	client := newNotificationHTTPClient(nil, pushRelayRequestTimeout)
 	return &pushSender{
 		devices:             devices,
 		deliveries:          deliveries,
 		cipher:              cipher,
 		settings:            settings,
-		client:              newWebhookHTTPClient(nil),
+		client:              client,
 		logger:              slog.Default().With("component", "notifications.apple_push"),
 		developmentRelayURL: os.Getenv("SILO_PUSH_RELAY_DEVELOPMENT_URL"),
 		now:                 time.Now,
@@ -156,10 +186,7 @@ func (s *pushSender) processAttempt(ctx context.Context, attempt PushDeliveryAtt
 	}
 	_ = s.devices.RecordPushFailure(ctx, device.ID, code, result.TerminalDevice)
 
-	delay, more := pushRetryDelay(attemptNumber)
-	if result.RetryAfter > 0 {
-		delay = result.RetryAfter
-	}
+	delay, more := pushRetryDelayWithHint(attemptNumber, result.RetryAfter)
 	if more && !result.TerminalDevice && retryableHTTPStatus(result.HTTPStatus) {
 		nextRetry := time.Now().Add(delay)
 		return s.finalize(ctx, attempt, PushOutcomeRetrying, result.UpstreamReason, result.Message, statusPtr, result.RelayRequestID, &nextRetry)
@@ -304,7 +331,7 @@ func (s *pushSender) sendWithCapability(ctx context.Context, attempt PushDeliver
 		RelayRequestID: parsed.Error.RequestID,
 		UpstreamReason: code,
 		Message:        strings.TrimSpace(message),
-		TerminalDevice: resp.StatusCode == http.StatusUnprocessableEntity && code == "apns_rejected",
+		TerminalDevice: terminalAPNsDeviceRejection(resp.StatusCode, code, message),
 	}
 }
 

internal/notifications/push_sender_test.go

@@ -7,6 +7,7 @@ import (
 	"net/http/httptest"
 	"strings"
 	"testing"
+	"time"
 )
 
 type mapSettingStore map[string]string
@@ -152,6 +153,65 @@ func TestPushSenderSendMapsRelayTerminalAPNsRejection(t *testing.T) {
 	}
 }
 
+func TestTerminalAPNsDeviceRejectionReasons(t *testing.T) {
+	for _, reason := range []string{
+		"BadDeviceToken",
+		"InvalidToken",
+		"DeviceTokenNotForTopic",
+		"Unregistered",
+	} {
+		t.Run(reason, func(t *testing.T) {
+			message := "APNs rejected the notification: " + reason
+			if !terminalAPNsDeviceRejection(http.StatusUnprocessableEntity, "apns_rejected", message) {
+				t.Fatalf("reason %q was not terminal for the device", reason)
+			}
+		})
+	}
+
+	if terminalAPNsDeviceRejection(
+		http.StatusUnprocessableEntity,
+		"apns_rejected",
+		"APNs rejected the notification: PayloadTooLarge",
+	) {
+		t.Fatal("request-level APNs rejection was terminal for the device")
+	}
+}
+
+func TestPushSenderDoesNotDisableDeviceForRequestLevelAPNsRejection(t *testing.T) {
+	server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		w.WriteHeader(http.StatusUnprocessableEntity)
+		_ = json.NewEncoder(w).Encode(pushRelayErrorResponse{
+			Error: struct {
+				Code      string `json:"code"`
+				Message   string `json:"message"`
+				RequestID string `json:"request_id"`
+			}{
+				Code:      "apns_rejected",
+				Message:   "APNs rejected the notification: PayloadTooLarge",
+				RequestID: "relay-request-request-rejection",
+			},
+		})
+	}))
+	defer server.Close()
+
+	sender := newPushSender(nil, nil, nil, NewSettings(mapSettingReader{
+		SettingPushRelayURL:    server.URL,
+		SettingPushRelayAPIKey: "relay-key",
+	}))
+	sender.client = server.Client()
+	sender.developmentRelayURL = server.URL
+
+	result := sender.send(context.Background(), PushDeliveryAttempt{ID: "attempt-1"}, &PushDevice{
+		APNsEnvironment: APNsEnvironmentSandbox,
+		APNsTopic:       ApplePushTopicSilo,
+		ServerDeviceID:  "server-device-1",
+	}, strings.Repeat("a", 64))
+
+	if result.OK || result.TerminalDevice || result.HTTPStatus != http.StatusUnprocessableEntity {
+		t.Fatalf("request-level rejection result = %+v", result)
+	}
+}
+
 func TestPushSenderSendMapsRelayRetryAfter(t *testing.T) {
 	server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.Header().Set("Retry-After", "30")
@@ -189,6 +249,20 @@ func TestPushSenderSendMapsRelayRetryAfter(t *testing.T) {
 	}
 }
 
+func TestPushSenderUsesRelayAwareTimeoutAndRetryHorizon(t *testing.T) {
+	sender := newPushSender(nil, nil, nil, NewSettings(mapSettingStore{}))
+	if sender.client.Timeout != pushRelayRequestTimeout {
+		t.Fatalf("relay client timeout = %s, want %s", sender.client.Timeout, pushRelayRequestTimeout)
+	}
+
+	if delay, more := pushRetryDelayWithHint(1, 10*time.Second); !more || delay != 10*time.Second {
+		t.Fatalf("short Retry-After delay = %s, more = %v", delay, more)
+	}
+	if delay, more := pushRetryDelayWithHint(1, 24*time.Hour); !more || delay != pushRelayMaxRetryAfter {
+		t.Fatalf("capped Retry-After delay = %s, more = %v", delay, more)
+	}
+}
+
 func TestPushSenderRenewsExpiredCapabilityAndRetriesStableDelivery(t *testing.T) {
 	token := strings.Repeat("a", 64)
 	var sendKeys []string

internal/notifications/relay_credentials.go

@@ -51,9 +51,10 @@ type relayNestedError struct {
 }
 
 type RelayCredentialError struct {
-	Status  int
-	Code    string
-	Message string
+	Status     int
+	Code       string
+	Message    string
+	RetryAfter time.Duration
 }
 
 func (e RelayCredentialError) Error() string { return e.Code }
@@ -143,7 +144,12 @@ func RequestRelayCredential(ctx context.Context, client RelayHTTPDoer, relayURL,
 		if message == "" {
 			message = http.StatusText(resp.StatusCode)
 		}
-		return RelayCredentialResult{}, RelayCredentialError{Status: resp.StatusCode, Code: code, Message: message}
+		return RelayCredentialResult{}, RelayCredentialError{
+			Status:     resp.StatusCode,
+			Code:       code,
+			Message:    message,
+			RetryAfter: parseRetryAfter(resp.Header.Get("Retry-After"), time.Now()),
+		}
 	}
 	var parsed relayCredentialResponse
 	if err := json.Unmarshal(data, &parsed); err != nil {

internal/notifications/webhook_http.go

@@ -32,12 +32,19 @@ type webhookSendResult struct {
 	Message string
 }
 
-// newWebhookHTTPClient builds the delivery client: 10s total timeout,
-// non-overridable TLS verification, bounded redirects, and a dialer Control
-// hook that re-validates every resolved address at connect time (DNS
-// rebinding mitigation — the guard runs on the address actually being
-// connected to, each redirect hop included).
+// newWebhookHTTPClient builds the webhook delivery client with its standard
+// request timeout.
 func newWebhookHTTPClient(allowPrivate func() bool) *http.Client {
+	return newNotificationHTTPClient(allowPrivate, webhookRequestTimeout)
+}
+
+// newNotificationHTTPClient builds a delivery client with non-overridable TLS
+// verification, bounded redirects, and a dialer Control hook that re-validates
+// every resolved address at connect time (DNS rebinding mitigation — the guard
+// runs on the address actually being connected to, each redirect hop included).
+// Callers may use a longer total timeout when an upstream service has its own
+// request deadline that must expire before Silo gives up waiting for a response.
+func newNotificationHTTPClient(allowPrivate func() bool, requestTimeout time.Duration) *http.Client {
 	dialer := &net.Dialer{
 		Timeout: 5 * time.Second,
 		Control: func(network, address string, _ syscall.RawConn) error {
@@ -61,10 +68,10 @@ func newWebhookHTTPClient(allowPrivate func() bool) *http.Client {
 		MaxIdleConns:          16,
 		IdleConnTimeout:       60 * time.Second,
 		TLSHandshakeTimeout:   5 * time.Second,
-		ResponseHeaderTimeout: webhookRequestTimeout,
+		ResponseHeaderTimeout: requestTimeout,
 	}
 	return &http.Client{
-		Timeout:   webhookRequestTimeout,
+		Timeout:   requestTimeout,
 		Transport: transport,
 		CheckRedirect: func(req *http.Request, via []*http.Request) error {
 			if len(via) >= webhookMaxRedirects {