fix(api): add bounded retry to critical external calls (#243)

[thomasluizon]

What & why

Five critical outbound integrations issued a single HTTP request with no retry, so any transient blip (network error, timeout, 429, or 5xx) failed the whole operation. This adds bounded retry at source, following the PushNotificationService pattern from #327 (≤2 retries, exponential backoff, transient-only).

Shared policy — HttpRetryPolicy (Orbit.Infrastructure/Common)

SendWithRetryAsync retries an outbound call up to 2 times with ~300ms exponential backoff (300ms, 600ms). Retries only transient outcomes: statuses 408 / 429 / 5xx and HttpRequestException / timeout. Permanent responses (2xx, other 4xx) and user cancellation are returned/propagated without a retry. The send delegate rebuilds request content each attempt (an HttpContent is single-use); transient responses are disposed before the next attempt.

Wired into:

• ResendEmailService — transactional emails (welcome, verification code, account-deletion code, waitlist, support). Stays best-effort: after the retry budget it still logs and returns.
• GoogleTokenService — OAuth token refresh. A revoked refresh token (400 invalid_grant / unauthorized_client) is permanent → returned unretried; only transient failures retry.
• ResendContactsService — waitlist contact add. A duplicate 409 Conflict is benign success → not retried.
• SupabaseObjectStorageService — signed-URL generation. Non-transient 4xx throws immediately; transient 5xx/timeout retries first.

Stripe — SDK-native retry

StripeConfiguration.MaxNetworkRetries = 2 at startup. This is Stripe's own network-retry with exponential backoff, and it auto-attaches an idempotency key to retried writes, so non-idempotent operations (customer / checkout-session create) stay safe. A hand-rolled app loop was deliberately not used here — it could not make those writes idempotent the way the SDK does.

Tests

New behavior + failure coverage for each path (SequencedHttpMessageHandler plays back a scripted status/exception sequence and counts sends):

• transient-then-success — one 5xx (or a transport exception) then 2xx retries once and succeeds;
• persistent 5xx — gives up after the retry budget (3 total sends);
• permanent not retried — 400 (email/contacts/supabase), revoked invalid_grant (Google), duplicate 409 (contacts) each send exactly once.

Stripe's retry is delegated to the SDK's own (well-tested) network-retry, so it isn't re-tested here.

Full solution builds with 0 errors; all four test suites green locally (4867 tests, 0 failed).

No API contract, endpoint, or DTO change — purely internal resilience hardening, so no consumer-side mirror is required.

Refs thomasluizon/orbit-ui-mobile#243

[sonarqubecloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[claude]

PR Review: fix(api): add bounded retry to critical external calls (#243)

Recommendation: APPROVE

Summary

Clean, well-scoped resilience change. A single shared HttpRetryPolicy.SendWithRetryAsync (src/Orbit.Infrastructure/Common/HttpRetryPolicy.cs) adds bounded retry (<=2 attempts, 300/600ms exponential backoff, transient-only: 408/429/5xx + HttpRequestException/timeout) to four outbound integrations (GoogleTokenService, ResendEmailService, ResendContactsService, SupabaseObjectStorageService), plus Stripe's own SDK-native MaxNetworkRetries = 2. No Controllers, DTOs, or endpoints touched — purely internal hardening, consistent with the PR's own "no consumer-side mirror required" claim.

Findings

None at Critical/High. No concretely-actionable Medium findings survived verification.

Dimensions checked

• Correctness — traced the retry loop's exception-filter ordering (user-cancellation short-circuits before the transient-retry filter; internal timeouts distinct from user cancellation correctly retry), the exponential-backoff math (300 << attempt → 300ms/600ms, matches PR description), and the single-use-HttpContent rebuild-per-attempt in every call site (GoogleTokenService.cs:44, ResendContactsService.cs:20, ResendEmailService.cs:270). All four call sites that can throw after retry exhaustion (GoogleTokenService, ResendEmailService) wrap the call in try/catch; ResendContactsService.AddContactAsync and SupabaseObjectStorageService.CreateSignedUploadAsync don't wrap it, but that's unchanged from pre-PR behavior (a single failed attempt already propagated the same way) — retry only adds attempts before the same propagation path, not a new failure mode.
• Batch/background-job impact — CalendarAutoSyncService sequentially processes up to 50 users/tick and calls GoogleTokenService.TryRefreshAsync per user; during a genuine Google outage this adds up to ~900ms/user (~45s/tick) versus failing fast pre-PR. Bounded, only triggers on real outages, and the job already tolerates per-user latency (15-min interval, per-user try/catch) — not flagged as a finding.
• Retry-attempt logging — verified against the PushNotificationService pattern this PR explicitly follows (PushNotificationService.cs:196-233): that precedent also logs only on final failure, never per-retry-attempt. This PR is consistent with established convention, not a deviation.
• Dead code — none introduced.
• Comment policy — only /// XML-doc blocks on HttpRetryPolicy; no narration comments.
• No-workaround / root-cause — this is the root-cause fix (previously zero retry on transient failures); Stripe correctly uses SDK-native retry instead of a hand-rolled loop specifically because it needs idempotency-key handling that a manual loop can't replicate.
• Type safety — no null!, no unsafe casts.
• Security — dedicated security-reviewer subagent pass (credential handling, duplicate-refresh side effects, error-message exposure, amplification/cost, surface area) returned PASS with no issues; independently confirmed the Supabase raw-body InvalidOperationException never reaches the client (caught by UnhandledExceptionHandler, generic message only).
• Backend hard rules — no timezone, authorization, or transaction-teardown code touched; N/A.
• Contract drift / parity / i18n / DESIGN.md — N/A, no orbit-ui-mobile surface touched (confirmed via PR body and diff: no DTO/endpoint changes).
• Tests — new SequencedHttpMessageHandler scripts multi-step transport outcomes; coverage includes transient-then-success, persistent-failure (attempt budget verified via call count), and non-retried-permanent-failure for every wired service. Assertions match the policy's documented semantics (3 total sends on persistent failure, 1 send on permanent failure).

SonarCloud quality gate passed (1 new issue, not gate-blocking) — treated as the separate required CI check per instructions, not re-litigated here.