fix(auth): do not remove session in case of network failure (#712)

* fix(auth): do not remove session in case of cancellation error

* add tests

* do not remove session

* fix integration tests

Author: Guilherme Souza

Sources/Auth/AuthClient.swift

@@ -109,7 +109,7 @@ public actor AuthClient {
       }
     )
 
-    Task { @MainActor in await observeAppLifecycleChanges() }
+    Task { @MainActor in observeAppLifecycleChanges() }
   }
 
   #if canImport(ObjectiveC) && canImport(Combine)

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,43 +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)")
-
-            // 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 {
-              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

@@ -0,0 +1,3 @@
+{
+  "recommendations": ["denoland.vscode-deno"]
+}

Tests/IntegrationTests/.vscode/extensions.json

@@ -0,0 +1,24 @@
+{
+  "deno.enablePaths": [
+    "supabase/functions"
+  ],
+  "deno.lint": true,
+  "deno.unstable": [
+    "bare-node-builtins",
+    "byonm",
+    "sloppy-imports",
+    "unsafe-proto",
+    "webgpu",
+    "broadcast-channel",
+    "worker-options",
+    "cron",
+    "kv",
+    "ffi",
+    "fs",
+    "http",
+    "net"
+  ],
+  "[typescript]": {
+    "editor.defaultFormatter": "denoland.vscode-deno"
+  }
+}

Tests/IntegrationTests/.vscode/settings.json

@@ -5,12 +5,14 @@
 //  Created by Guilherme Souza on 27/03/24.
 //
 
-@testable import Auth
 import ConcurrencyExtras
 import CustomDump
+import InlineSnapshotTesting
 import TestHelpers
 import XCTest
 
+@testable import Auth
+
 #if canImport(FoundationNetworking)
   import FoundationNetworking
 #endif
@@ -55,7 +57,7 @@ final class AuthClientIntegrationTests: XCTestCase {
       let password = mockPassword()
 
       let metadata: [String: AnyJSON] = [
-        "test": .integer(42),
+        "test": .integer(42)
       ]
 
       let response = try await authClient.signUp(
@@ -74,23 +76,23 @@ final class AuthClientIntegrationTests: XCTestCase {
     }
   }
 
-//  func testSignUpAndSignInWithPhone() async throws {
-//    try await XCTAssertAuthChangeEvents([.initialSession, .signedIn, .signedOut, .signedIn]) {
-//      let phone = mockPhoneNumber()
-//      let password = mockPassword()
-//      let metadata: [String: AnyJSON] = [
-//        "test": .integer(42),
-//      ]
-//      let response = try await authClient.signUp(phone: phone, password: password, data: metadata)
-//      XCTAssertNotNil(response.session)
-//      XCTAssertEqual(response.user.phone, phone)
-//      XCTAssertEqual(response.user.userMetadata["test"], 42)
-//
-//      try await authClient.signOut()
-//
-//      try await authClient.signIn(phone: phone, password: password)
-//    }
-//  }
+  //  func testSignUpAndSignInWithPhone() async throws {
+  //    try await XCTAssertAuthChangeEvents([.initialSession, .signedIn, .signedOut, .signedIn]) {
+  //      let phone = mockPhoneNumber()
+  //      let password = mockPassword()
+  //      let metadata: [String: AnyJSON] = [
+  //        "test": .integer(42),
+  //      ]
+  //      let response = try await authClient.signUp(phone: phone, password: password, data: metadata)
+  //      XCTAssertNotNil(response.session)
+  //      XCTAssertEqual(response.user.phone, phone)
+  //      XCTAssertEqual(response.user.userMetadata["test"], 42)
+  //
+  //      try await authClient.signOut()
+  //
+  //      try await authClient.signIn(phone: phone, password: password)
+  //    }
+  //  }
 
   func testSignInWithEmail_invalidEmail() async throws {
     let email = mockEmail()
@@ -108,12 +110,12 @@ final class AuthClientIntegrationTests: XCTestCase {
     }
   }
 
-//  func testSignInWithOTP_usingEmail() async throws {
-//    let email = mockEmail()
-//
-//    try await authClient.signInWithOTP(email: email)
-//    try await authClient.verifyOTP(email: email, token: "123456", type: .magiclink)
-//  }
+  //  func testSignInWithOTP_usingEmail() async throws {
+  //    let email = mockEmail()
+  //
+  //    try await authClient.signInWithOTP(email: email)
+  //    try await authClient.verifyOTP(email: email, token: "123456", type: .magiclink)
+  //  }
 
   func testSignOut_otherScope_shouldSignOutLocally() async throws {
     try await XCTAssertAuthChangeEvents([.initialSession, .signedIn]) {
@@ -170,7 +172,7 @@ final class AuthClientIntegrationTests: XCTestCase {
   func testUserIdentities() async throws {
     let session = try await signUpIfNeededOrSignIn(email: mockEmail(), password: mockPassword())
     let identities = try await authClient.userIdentities()
-    expectNoDifference(session.user.identities, identities)
+    expectNoDifference(session.user.identities?.map(\.identityId) ?? [], identities.map(\.identityId))
   }
 
   func testUnlinkIdentity_withOnlyOneIdentity() async throws {
@@ -181,15 +183,8 @@ final class AuthClientIntegrationTests: XCTestCase {
     do {
       try await authClient.unlinkIdentity(identity)
       XCTFail("Expect failure")
-    } catch {
-      if let error = error as? AuthError {
-        XCTAssertEqual(
-          error.localizedDescription,
-          "User must have at least 1 identity after unlinking"
-        )
-      } else {
-        XCTFail("Unexpected error: \(error)")
-      }
+    } catch let error as AuthError {
+      XCTAssertEqual(error.errorCode, .singleIdentityNotDeletable)
     }
   }
 
@@ -246,8 +241,8 @@ final class AuthClientIntegrationTests: XCTestCase {
   func testLinkIdentity() async throws {
     try await signUpIfNeededOrSignIn(email: mockEmail(), password: mockPassword())
 
-    try await authClient.linkIdentity(provider: .github) { url in
-      XCTAssertTrue(url.absoluteString.contains("github.com"))
+    try await authClient.linkIdentity(provider: .apple) { url in
+      XCTAssertTrue(url.absoluteString.contains("apple.com"))
     }
   }
 
@@ -291,10 +286,10 @@ final class AuthClientIntegrationTests: XCTestCase {
     }
   }
 
-  private func mockEmail(length: Int = Int.random(in: 5 ... 10)) -> String {
+  private func mockEmail(length: Int = Int.random(in: 5...10)) -> String {
     var username = ""
-    for _ in 0 ..< length {
-      let randomAscii = Int.random(in: 97 ... 122) // ASCII values for lowercase letters
+    for _ in 0..<length {
+      let randomAscii = Int.random(in: 97...122)  // ASCII values for lowercase letters
       let randomCharacter = Character(UnicodeScalar(randomAscii)!)
       username.append(randomCharacter)
     }
@@ -303,13 +298,13 @@ final class AuthClientIntegrationTests: XCTestCase {
 
   private func mockPhoneNumber() -> String {
     // Generate random country code (1 to 3 digits)
-    let countryCode = String(format: "%d", Int.random(in: 1 ... 999))
+    let countryCode = String(format: "%d", Int.random(in: 1...999))
 
     // Generate random area code (3 digits)
-    let areaCode = String(format: "%03d", Int.random(in: 100 ... 999))
+    let areaCode = String(format: "%03d", Int.random(in: 100...999))
 
     // Generate random subscriber number (7 digits)
-    let subscriberNumber = String(format: "%07d", Int.random(in: 1000000 ... 9999999))
+    let subscriberNumber = String(format: "%07d", Int.random(in: 1_000_000...9_999_999))
 
     // Format the phone number in E.164 format
     let phoneNumber = "\(countryCode)\(areaCode)\(subscriberNumber)"
@@ -322,12 +317,14 @@ final class AuthClientIntegrationTests: XCTestCase {
       "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789!@#$%^&*()-_=+"
     var password = ""
 
-    for _ in 0 ..< length {
-      let randomIndex = Int.random(in: 0 ..< allowedCharacters.count)
-      let character = allowedCharacters[allowedCharacters.index(
-        allowedCharacters.startIndex,
-        offsetBy: randomIndex
-      )]
+    for _ in 0..<length {
+      let randomIndex = Int.random(in: 0..<allowedCharacters.count)
+      let character = allowedCharacters[
+        allowedCharacters.index(
+          allowedCharacters.startIndex,
+          offsetBy: randomIndex
+        )
+      ]
       password.append(character)
     }
 

Tests/IntegrationTests/AuthClientIntegrationTests.swift

@@ -0,0 +1,8 @@
+# Supabase
+.branches
+.temp
+
+# dotenvx
+.env.keys
+.env.local
+.env.*.local

Tests/IntegrationTests/supabase/.gitignore

@@ -1 +1 @@
-v2.6.8
+v2.22.6