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.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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