do not remove session

Author: grdsdev

Sources/Auth/AuthError.swift

@@ -298,13 +298,3 @@ public enum AuthError: LocalizedError, Equatable {
     return lhs == rhs
   }
 }
-
-extension AuthError: RetryableError {
-  package var shouldRetry: Bool {
-    switch self {
-    case .api(_, _, _, let response):
-      defaultRetryableHTTPStatusCodes.contains(response.statusCode)
-    default: false
-    }
-  }
-}

Sources/Auth/Internal/SessionManager.swift

@@ -78,52 +78,24 @@ private actor LiveSessionManager {
             logger?.debug("Refresh task ended")
           }
 
-          do {
-            let session = try await api.execute(
-              HTTPRequest(
-                url: configuration.url.appendingPathComponent("token"),
-                method: .post,
-                query: [
-                  URLQueryItem(name: "grant_type", value: "refresh_token")
-                ],
-                body: configuration.encoder.encode(
-                  UserCredentials(refreshToken: refreshToken)
-                )
+          let session = try await api.execute(
+            HTTPRequest(
+              url: configuration.url.appendingPathComponent("token"),
+              method: .post,
+              query: [
+                URLQueryItem(name: "grant_type", value: "refresh_token")
+              ],
+              body: configuration.encoder.encode(
+                UserCredentials(refreshToken: refreshToken)
               )
             )
-            .decoded(as: Session.self, decoder: configuration.decoder)
-
-            update(session)
-            eventEmitter.emit(.tokenRefreshed, session: session)
-
-            return session
-          } catch {
-            logger?.debug("Refresh token failed with error: \(error)")
-
-            if let error = error as? URLError, error.code == .cancelled {
-              throw error
-            }
-
-            if error is CancellationError {
-              throw error
-            }
-
-            // DO NOT remove session in case it is an error that should be retried.
-            // i.e. server instability, connection issues, ...
-            //
-            // Need to do this check here, because not all RetryableError's should be retried.
-            // URLError conforms to RetryableError, but only a subset of URLError should be retried,
-            // the same is true for AuthError.
-            if let error = error as? URLError, error.shouldRetry {
-              throw error
-            } else if let error = error as? any RetryableError, error.shouldRetry {
-              throw error
-            } else {
-              logger?.debug("No retryable error, removing session")
-              remove()
-              throw error
-            }
-          }
+          )
+          .decoded(as: Session.self, decoder: configuration.decoder)
+
+          update(session)
+          eventEmitter.emit(.tokenRefreshed, session: session)
+
+          return session
         }
 
         return try await inFlightRefreshTask!.value

Sources/Helpers/HTTP/RetryRequestInterceptor.swift

@@ -30,6 +30,22 @@ package actor RetryRequestInterceptor: HTTPClientInterceptor {
     .delete, .get, .head, .options, .put, .trace,
   ]
 
+  /// The default set of retryable URL error codes.
+  package static let defaultRetryableURLErrorCodes: Set<URLError.Code> = [
+    .backgroundSessionInUseByAnotherProcess, .backgroundSessionWasDisconnected,
+    .badServerResponse, .callIsActive, .cannotConnectToHost, .cannotFindHost,
+    .cannotLoadFromNetwork, .dataNotAllowed, .dnsLookupFailed,
+    .downloadDecodingFailedMidStream, .downloadDecodingFailedToComplete,
+    .internationalRoamingOff, .networkConnectionLost, .notConnectedToInternet,
+    .secureConnectionFailed, .serverCertificateHasBadDate,
+    .serverCertificateNotYetValid, .timedOut,
+  ]
+
+  /// The default set of retryable HTTP status codes.
+  package static let defaultRetryableHTTPStatusCodes: Set<Int> = [
+    408, 500, 502, 503, 504,
+  ]
+
   /// The maximum number of retries.
   package let retryLimit: Int
   /// The base value for exponential backoff.
@@ -56,12 +72,14 @@ package actor RetryRequestInterceptor: HTTPClientInterceptor {
     retryLimit: Int = RetryRequestInterceptor.defaultRetryLimit,
     exponentialBackoffBase: UInt = RetryRequestInterceptor.defaultExponentialBackoffBase,
     exponentialBackoffScale: Double = RetryRequestInterceptor.defaultExponentialBackoffScale,
-    retryableHTTPMethods: Set<HTTPTypes.HTTPRequest.Method> = RetryRequestInterceptor.defaultRetryableHTTPMethods,
-    retryableHTTPStatusCodes: Set<Int> = defaultRetryableHTTPStatusCodes,
-    retryableErrorCodes: Set<URLError.Code> = defaultRetryableURLErrorCodes
+    retryableHTTPMethods: Set<HTTPTypes.HTTPRequest.Method> = RetryRequestInterceptor
+      .defaultRetryableHTTPMethods,
+    retryableHTTPStatusCodes: Set<Int> = RetryRequestInterceptor.defaultRetryableHTTPStatusCodes,
+    retryableErrorCodes: Set<URLError.Code> = RetryRequestInterceptor.defaultRetryableURLErrorCodes
   ) {
     precondition(
-      exponentialBackoffBase >= 2, "The `exponentialBackoffBase` must be a minimum of 2."
+      exponentialBackoffBase >= 2,
+      "The `exponentialBackoffBase` must be a minimum of 2."
     )
 
     self.retryLimit = retryLimit
@@ -114,10 +132,11 @@ package actor RetryRequestInterceptor: HTTPClientInterceptor {
     }
 
     if retryCount < retryLimit, shouldRetry(request: request, result: result) {
-      let retryDelay = pow(
-        Double(exponentialBackoffBase),
-        Double(retryCount)
-      ) * exponentialBackoffScale
+      let retryDelay =
+        pow(
+          Double(exponentialBackoffBase),
+          Double(retryCount)
+        ) * exponentialBackoffScale
 
       let nanoseconds = UInt64(retryDelay)
       try? await Task.sleep(nanoseconds: NSEC_PER_SEC * nanoseconds)

Sources/Helpers/RetryableError.swift

@@ -118,46 +118,4 @@ final class SessionManagerTests: XCTestCase {
       (0..<10).map { _ in validSession.accessToken }
     )
   }
-
-  func testRefreshSession_shouldNotRemoveSession_whenErrorIsURLErrorCancelled() async throws {
-    let currentSession = Session.validSession
-    Dependencies[clientID].sessionStorage.store(currentSession)
-
-    await http.when(
-      { $0.url.path.contains("/token") },
-      return: { _ in
-        throw URLError(.cancelled)
-      }
-    )
-
-    do {
-      _ = try await sut.refreshSession(currentSession.refreshToken)
-      XCTFail("Expected a \(URLError.cancelled) failure")
-    } catch let error as URLError {
-      XCTAssertEqual(error.code, .cancelled)
-    }
-
-    XCTAssertNotNil(Dependencies[clientID].sessionStorage.get())
-  }
-
-  func testRefreshSession_shouldNotRemoveSession_whenErrorIsCancellationError() async throws {
-    let currentSession = Session.validSession
-    Dependencies[clientID].sessionStorage.store(currentSession)
-
-    await http.when(
-      { $0.url.path.contains("/token") },
-      return: { _ in
-        throw CancellationError()
-      }
-    )
-
-    do {
-      _ = try await sut.refreshSession(currentSession.refreshToken)
-      XCTFail("Expected a \(CancellationError.self) failure")
-    } catch is CancellationError {
-      // no-op
-    }
-
-    XCTAssertNotNil(Dependencies[clientID].sessionStorage.get())
-  }
 }