Skip to content

Non-retryable HTTP errors (4xx) are being retried unnecessarily #492

Description

@tsushanth

Summary

Task+Retrying.swift retries all non-2xx HTTP responses, including 4xx errors that are definitively not transient. This wastes bandwidth, adds latency, and delays surfacing permanent failures to callers.

Root cause

In Sources/SuperwallKit/Misc/Extensions/Task+Retrying.swift, after a successful URLSession data task, the generic Success overload checks the HTTP status code and throws a retryable error for anything outside 2xx:

do {
  let result = try await operation()
  if let (_, response) = result as? (Data, URLResponse),
    let httpResponse = response as? HTTPURLResponse,
    !(200...299).contains(httpResponse.statusCode) {
    throw URLError(.badServerResponse)   // retried unconditionally
  }
  return result
} catch {
  // …sleep, then continue → retries
}

A 401 Unauthorized or 404 Not Found will be thrown as URLError(.badServerResponse), fall into the catch block, sleep for the backoff interval, and be retried up to endpoint.retryCount times before the error is finally returned to CustomURLSession.getRequestId, which translates it to .notAuthenticated / .notFound.

CustomURLSession already knows which 4xx codes are terminal — it handles them explicitly in getRequestId. The retry loop should give them the same treatment.

Affected file

Sources/SuperwallKit/Misc/Extensions/Task+Retrying.swift

Suggested fix

Pass a predicate (or a set of terminal status codes) into Task.retrying so the caller can opt specific errors out of the retry cycle, or teach the status-code check to re-throw immediately instead of throwing URLError(.badServerResponse):

// Option A: rethrow terminal HTTP errors directly so they bypass the catch/retry path
let result = try await operation()
if let (_, response) = result as? (Data, URLResponse),
   let httpResponse = response as? HTTPURLResponse {
  if (400...499).contains(httpResponse.statusCode) {
    throw URLError(.badServerResponse)   // propagate immediately, don't catch
  }
  if !(200...299).contains(httpResponse.statusCode) {
    throw URLError(.badServerResponse)   // still retryable for 5xx
  }
}
return result

Because Swift's structured-concurrency withThrowingTaskGroup propagates the first uncaught throw out of the group, wrapping terminal errors in a typed sentinel (rather than URLError(.badServerResponse)) and re-throwing them outside the retry loop would be cleaner and avoid matching the generic catch.

Impact

Every config or network request that returns a 401/404 burns up to endpoint.retryCount extra round-trips (each with exponential-backoff sleep) before the SDK surfaces the error to the delegate. For a retryCount of 6 this can stall the SDK for tens of seconds on a permanent auth failure.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions