Don't retry terminal HTTP errors, and surface them properly

[yusuftor]

Changes in this pull request

Fixes #492, plus a related error-reporting problem found while working on it.

1. Terminal HTTP errors are no longer retried (cd31438d3)

Task.retrying threw URLError(.badServerResponse) for any non-2xx response, which its own catch immediately swallowed and retried. A permanent failure such as 401 or 404 therefore burned the full backoff schedule before surfacing:

attempt	sleep		attempt	sleep
0	5.0s		3	11.2s
1	6.5s		4	14.6s
2	8.5s		5	19.1s

That's ~65s and 7 round-trips at the default retryCount of 6, for a request that could only ever fail the same way.

Terminal statuses now return the response immediately instead of throwing. This matches what already happened once retries were exhausted — the loop's final attempt returns the response unchecked — so callers see the same error, just without the wasted requests and delay.

Per @anglinb's review note on the issue, client errors that may succeed on a retry are explicitly excluded and keep their existing behaviour: 408 Request Timeout, 425 Too Early, 429 Too Many Requests, 499 Client Closed Request. All 5xx are unaffected.

2. HTTP errors are no longer reported as decoding failures (d9fbbf7f2)

getRequestId only threw for 401 and 404. Every other non-2xx response returned normally, so the error body was then decoded as if it were a success payload. That failed, and the caller received NetworkError.decoding — describing the wrong failure — while the SDK tracked a network_decoding_fail event for what was really an HTTP error.

Adds NetworkError.http(statusCode:) and classifies every non-2xx response through a new NetworkError.make(fromStatusCode:). 401 and 404 keep their dedicated cases and log messages, so nothing matching on those changes behaviour. The three near-identical logging blocks collapse into one that also records the status code.

Note: NetworkError needed an explicit Equatable conformance, because adding an associated value drops the conformance simple enums get implicitly — and PaywallLogic.handlePaywallError relies on it for error == .notFound.

3. Redirects classify consistently across both paths (671d6a2b7)

The two changes above initially disagreed about 3xx. NetworkError.make(fromStatusCode:) treats any non-2xx response as an error, but TaskRetryLogic.isTerminal only considered 4xx terminal — so a redirect would have burned the full retry schedule before surfacing as .http(statusCode:). The tests asserted both opinions, so the suite documented the contradiction rather than catching it.

A redirect only reaches the caller when URLSession couldn't follow it — a 3xx with no Location header, for instance — and it comes back the same way however many times it's sent. isTerminal is now framed as "not 2xx, and retrying can't help", so both paths agree for every status code.

The commits are kept separate: the first is a pure latency fix with no API change, the second alters the SDK's internal error surface, the third reconciles them. NetworkError is not public, so integrators are unaffected either way.

Verification

888 tests in 93 suites pass. New coverage asserts actual invocation counts through the real Task.retrying (1 call for a 401, 4 for a 429), not just the predicate:

• 401/404 keep their existing errors — regression guard for the compatibility claim above
• terminal client errors are not retried: 400, 401, 403, 404, 410, 422
• redirects are not retried and surface as errors: 301, 302, 303, 307, 308
• retryable client errors still retry: 408, 425, 429, 499
• server errors still retry: 500, 502, 503
• status-code classification, error description and log messages

Checklist

• All unit tests pass.
• All UI tests pass. — not run
• Demo project builds and runs on iOS. — not run
• Demo project builds and runs on Mac Catalyst. — not run
• Demo project builds and runs on visionOS. — not run
• I added/updated tests or detailed why my change isn't tested.
• I added an entry to the CHANGELOG.md for any breaking changes, enhancements, or bug fixes.
• I have run swiftlint in the main directory and fixed any issues.
• I have updated the SDK documentation as well as the online docs. — no public API changed, so no docs update needed
• I have reviewed the contributing guide

🤖 Generated with Claude Code

Greptile Summary

This PR improves HTTP failure handling.

• Stops retrying terminal 4xx responses while preserving retries for 408, 425, 429, 499, and 5xx responses.
• Adds status-aware HTTP errors so non-2xx responses no longer surface as decoding failures.
• Adds retry-count, status classification, error-description, and logging-message tests.

Confidence Score: 5/5

The PR appears safe to merge, with terminal and retryable HTTP statuses handled consistently through the request pipeline.

The changed retry classification matches the added invocation-count tests, and existing callers either generically propagate or handle the new status-aware errors without relying on the former decoding behavior.

Important Files Changed

Filename	Overview
Sources/SuperwallKit/Misc/Extensions/Task+Retrying.swift	Returns terminal 4xx responses immediately while retaining the existing retry path for transient statuses.
Sources/SuperwallKit/Network/Custom URL Session/TaskRetryLogic.swift	Centralizes terminal-client-error classification with explicit exceptions for retryable 4xx statuses.
Sources/SuperwallKit/Network/Custom URL Session/CustomURLSession.swift	Introduces status-bearing HTTP errors and consistently classifies every non-2xx response before decoding.
Tests/SuperwallKitTests/Misc/Extensions/TaskRetryingTests.swift	Verifies actual invocation counts for successful, terminal, retryable-client, and server responses.
Tests/SuperwallKitTests/Network/NetworkErrorTests.swift	Covers HTTP status mapping, localized descriptions, and preserved dedicated-case log messages.

Sequence Diagram

sequenceDiagram
  participant Caller
  participant Retry as Task.retrying
  participant Backend
  participant Session as CustomURLSession
  Caller->>Retry: Start request
  Retry->>Backend: Send HTTP request
  Backend-->>Retry: HTTP response
  alt Terminal 4xx
    Retry-->>Session: Return response immediately
  else Retryable 4xx or 5xx
    Retry->>Backend: Retry with backoff
    Backend-->>Retry: Final response
    Retry-->>Session: Return response
  else Successful 2xx
    Retry-->>Session: Return response
  end
  alt Non-2xx
    Session-->>Caller: Throw classified NetworkError
  else 2xx
    Session-->>Caller: Decode response
  end

Loading

_{Reviews (1): Last reviewed commit: "Surface HTTP errors instead of reporting..." | Re-trigger Greptile}

Context used:

• Knowledge Base — Networking Layer

[pullfrog]

✅ No new issues found.

Reviewed changes — a two-commit fix that stops retrying permanently-failing HTTP responses and classifies non-2xx responses as HTTP errors instead of decoding failures.

• Skip retries for terminal client errors — Task+Retrying.swift now returns the response early when TaskRetryLogic.isTerminal(statusCode:) is true, avoiding the full ~65s backoff on requests that can only fail the same way (e.g. 401, 404). Retryable client codes (408, 425, 429, 499) and all 5xx keep retrying.
• Surface HTTP errors properly — getRequestId in CustomURLSession.swift classifies every non-2xx response through new NetworkError.make(fromStatusCode:), adding NetworkError.http(statusCode:). Responses other than 401/404 no longer decode-fail into NetworkError.decoding or emit a spurious network_decoding_fail event; the three near-identical logging blocks collapse into one that records status_code.
• Equatable conformance — added explicitly to NetworkError because the new associated value drops the implicit conformance that PaywallLogic.handlePaywallError's error == .notFound check relies on.
• Tests — new coverage asserts real Task.retrying invocation counts (1 for terminal errors, 4 for retryable/5xx), isTerminal classification, and error/description/log-message mapping. CHANGELOG.md updated under unreleased 4.16.2.

I verified the two classification paths (isTerminal for retry decisions and make for the surfaced error) stay consistent across all status ranges, that no exhaustive switch over NetworkError exists that the new .http case would break, and that the Equatable conformance is genuinely required. NetworkError is internal, so integrators are unaffected.

ℹ️ 3xx responses classify inconsistently between the two new code paths

NetworkError.make(fromStatusCode:) maps a 3xx (e.g. 302) to .http(statusCode:), and a test asserts that. But TaskRetryLogic.isTerminal returns false for 3xx (it only treats 400...499 as terminal), so a redirect reaching the retry-loop check would throw URLError(.badServerResponse) and burn the retry schedule before surfacing. In practice this is essentially unreachable — URLSession follows redirects by default, so a raw 3xx rarely reaches either path — so it's a cosmetic inconsistency, not a bug. Flagging only for awareness; no change needed.

  ^{｜ View workflow run ｜ Using Claude Opus ｜ 𝕏}

[pullfrog]

✅ No new issues found.

Reviewed changes — the one new commit since the prior review resolves the 3xx classification inconsistency flagged last time by aligning the retry loop's terminal check with the surfaced-error mapping.

• isTerminal now treats 3xx as terminal — TaskRetryLogic.isTerminal(statusCode:) was rewritten from an early 400...499-only guard to explicit 2xx → false, 5xx → false, else !retryableClientErrorCodes.contains. A redirect that reaches the loop (because URLSession couldn't follow it) now short-circuits instead of burning the backoff schedule, matching NetworkError.make(fromStatusCode:) which already maps 3xx to .http.
• Test coverage for redirects — added redirectsAreNotRetried (301/302/307/308 → 1 invocation through real Task.retrying) and isTerminal_redirects (301/302/303/307/308 → terminal). isTerminal_successfulResponses dropped 301/304 from its success set, correctly reflecting that 3xx is no longer non-terminal.

I confirmed the two classification paths (isTerminal for retry decisions, make for the surfaced error) now agree across every status range: both treat 5xx and 408/425/429/499 as retryable and everything else non-2xx as terminal. The prior review's sole note is addressed; no new concerns.

  ^{｜ View workflow run ｜ Using Claude Opus ｜ 𝕏}