Application password experiment: Fall back to Jetpack proxy after app password generation fails (#16079)

Author: itsmeichigo

Modules/Sources/NetworkingCore/ApplicationPassword/RequestProcessor.swift

@@ -1,13 +1,8 @@
 import Alamofire
 import Foundation
 
-enum AppPasswordFailureReason {
-    case notSupported
-    case unknown
-}
-
 protocol RequestProcessorDelegate: AnyObject {
-    func didFailToAuthenticateRequestWithAppPassword(siteID: Int64, reason: AppPasswordFailureReason)
+    func didFailToAuthenticateRequestWithAppPassword(siteID: Int64)
 }
 
 /// Authenticates and retries requests
@@ -92,7 +87,7 @@ private extension RequestProcessor {
                     isAuthenticating = false
                     completeRequests(false)
                     if let currentSiteID {
-                        notifyFailure(error, for: currentSiteID)
+                        notifyFailureIfNeeded(error, for: currentSiteID)
                     }
                 }
             }
@@ -120,22 +115,24 @@ private extension RequestProcessor {
         }
     }
 
-    func notifyFailure(_ error: Error, for siteID: Int64) {
-        let reason: AppPasswordFailureReason = {
+    func notifyFailureIfNeeded(_ error: Error, for siteID: Int64) {
+        let appPasswordNotSupported: Bool = {
             switch error {
             case NetworkError.notFound:
-                return .notSupported
+                return true
             case let networkError as NetworkError:
                 if let code = networkError.errorCode,
                    AppPasswordConstants.disabledCodes.contains(code) {
-                    return .notSupported
+                    return true
                 }
-                return .unknown
+                return false
             default:
-                return .unknown
+                return false
             }
         }()
-        delegate?.didFailToAuthenticateRequestWithAppPassword(siteID: siteID, reason: reason)
+        if appPasswordNotSupported {
+            delegate?.didFailToAuthenticateRequestWithAppPassword(siteID: siteID)
+        }
     }
 
     func shouldRetry(_ error: Error) -> Bool {

Modules/Sources/NetworkingCore/Network/AlamofireNetwork.swift

@@ -49,11 +49,8 @@ public class AlamofireNetwork: Network {
 
     private var subscription: AnyCancellable?
 
-    /// Keeps track of failure counts for each site when switching to direct requests
-    private var appPasswordFailures: [Int64: Int] = [:]
-
-    /// Keeps track of retried requests when direct requests fail
-    private var retriedJetpackRequests: [RetriedJetpackRequest] = []
+    /// Thread-safe error handler for failure tracking and retry logic
+    private let errorHandler: AlamofireNetworkErrorHandler
 
     /// Public Initializer
     ///
@@ -71,6 +68,7 @@ public class AlamofireNetwork: Network {
         self.credentials = credentials
         self.selectedSite = selectedSite
         self.userDefaults = userDefaults
+        self.errorHandler = AlamofireNetworkErrorHandler(credentials: credentials, userDefaults: userDefaults)
         self.requestConverter = {
             let siteAddress: String? = {
                 switch credentials {
@@ -123,7 +121,7 @@ public class AlamofireNetwork: Network {
         alamofireSession.request(convertedRequest)
             .validateIfRestRequest(for: convertedRequest)
             .responseData { [weak self] response in
-                self?.handleFailureForDirectRequestIfNeeded(
+                self?.errorHandler.handleFailureForDirectRequestIfNeeded(
                     originalRequest: request,
                     convertedRequest: convertedRequest,
                     failure: response.networkingError,
@@ -151,7 +149,7 @@ public class AlamofireNetwork: Network {
         alamofireSession.request(convertedRequest)
             .validateIfRestRequest(for: convertedRequest)
             .responseData { [weak self] response in
-                self?.handleFailureForDirectRequestIfNeeded(
+                self?.errorHandler.handleFailureForDirectRequestIfNeeded(
                     originalRequest: request,
                     convertedRequest: convertedRequest,
                     failure: response.networkingError,
@@ -176,15 +174,15 @@ public class AlamofireNetwork: Network {
         let response = await sessionRequest.serializingData().response
         let failure = response.networkingError
 
-        if shouldRetryJetpackRequest(
+        if errorHandler.shouldRetryJetpackRequest(
             originalRequest: request,
             convertedRequest: convertedRequest,
             failure: failure
         ) {
             return try await responseDataAndHeaders(for: request)
         }
 
-        flagSiteAsUnsupportedForAppPasswordIfNeeded(originalRequest: request, failure: failure)
+        errorHandler.flagSiteAsUnsupportedForAppPasswordIfNeeded(originalRequest: request, failure: failure)
 
         if let error = response.networkingError {
             throw error
@@ -212,7 +210,7 @@ public class AlamofireNetwork: Network {
                 .request(convertedRequest)
                 .validateIfRestRequest(for: convertedRequest)
                 .responseData { [weak self] response in
-                    self?.handleFailureForDirectRequestIfNeeded(
+                    self?.errorHandler.handleFailureForDirectRequestIfNeeded(
                         originalRequest: request,
                         convertedRequest: convertedRequest,
                         failure: response.networkingError,
@@ -240,7 +238,7 @@ public class AlamofireNetwork: Network {
         alamofireSession
             .upload(multipartFormData: multipartFormData, with: convertedRequest)
             .responseData { [weak self] response in
-                self?.handleFailureForDirectRequestIfNeeded(
+                self?.errorHandler.handleFailureForDirectRequestIfNeeded(
                     originalRequest: request,
                     convertedRequest: convertedRequest,
                     failure: response.networkingError,
@@ -284,7 +282,7 @@ private extension AlamofireNetwork {
                     network: self
                 ))
                 requestAuthenticator.delegate = self
-                appPasswordFailures.removeValue(forKey: site.siteID) // reset failure count
+                errorHandler.resetFailureCount(for: site.siteID) // reset failure count
             }
     }
 }
@@ -293,127 +291,22 @@ private extension AlamofireNetwork {
 //
 private extension AlamofireNetwork {
     func convertRequestIfNeeded(_ request: URLRequestConvertible) -> URLRequestConvertible {
-        let isRetried = retriedJetpackRequests.contains { retriedRequest in
-            let urlRequest = try? request.asURLRequest()
-            let currentItem = try? retriedRequest.request.asURLRequest()
-            return currentItem == urlRequest
-        }
-        if isRetried {
+        if errorHandler.isRequestRetried(request) {
             return request // do not convert
         }
         return requestConverter.convert(request)
     }
 
-    /// Checks if the specified request and error are eligible for retrying as Jetpack request.
-    /// If yes, enqueue the original request to the retried list before returning.
-    ///
-    func shouldRetryJetpackRequest(originalRequest: URLRequestConvertible,
-                                   convertedRequest: URLRequestConvertible,
-                                   failure: Error?) -> Bool {
-        if let request = originalRequest as? JetpackRequest,
-           convertedRequest is RESTRequest,
-           case .some(.wpcom) = credentials,
-           let failure = failure as? NetworkError {
-            let retriedRequest = RetriedJetpackRequest(request: request, error: failure)
-            retriedJetpackRequests.append(retriedRequest)
-            return true
-        }
-        return false
-    }
-
-    /// Determines if the site has issue with application password based on the original request.
-    ///
-    func flagSiteAsUnsupportedForAppPasswordIfNeeded(originalRequest: URLRequestConvertible,
-                                                     failure: Error?) {
-        let retriedRequestIndex = retriedJetpackRequests.firstIndex { retriedRequest in
-            let urlRequest = try? originalRequest.asURLRequest()
-            let retriedRequest = try? retriedRequest.request.asURLRequest()
-            return urlRequest == retriedRequest
-        }
-        guard let index = retriedRequestIndex else { return }
-
-        if failure == nil {
-            let siteID = retriedJetpackRequests[index].request.siteID
-            let originalFailure = retriedJetpackRequests[index].error
-            switch originalFailure {
-            case .unacceptableStatusCode(statusCode: 401, _),
-                .unacceptableStatusCode(statusCode: 403, _),
-                .unacceptableStatusCode(statusCode: 429, _):
-                flagSiteAsUnsupported(for: siteID)
-            default:
-                if let code = originalFailure.errorCode, AppPasswordConstants.disabledCodes.contains(code) {
-                    flagSiteAsUnsupported(for: siteID)
-                } else {
-                    incrementFailureCount(for: siteID)
-                }
-            }
-        }
-
-        // remove retried request from list
-        retriedJetpackRequests.remove(at: index)
-    }
-
-    func handleFailureForDirectRequestIfNeeded(originalRequest: URLRequestConvertible,
-                                               convertedRequest: URLRequestConvertible,
-                                               failure: Error?,
-                                               onRetry: @escaping () -> Void,
-                                               onCompletion: @escaping () -> Void) {
-        if shouldRetryJetpackRequest(originalRequest: originalRequest,
-                                     convertedRequest: convertedRequest,
-                                     failure: failure) {
-            onRetry()
-        } else {
-            flagSiteAsUnsupportedForAppPasswordIfNeeded(originalRequest: originalRequest, failure: failure)
-            onCompletion()
-        }
-    }
-
-    /// Helper type to keep track of retried requests with accompanied error
-    ///
-    struct RetriedJetpackRequest {
-        let request: JetpackRequest
-        let error: NetworkError
-    }
 }
 
 // MARK: `RequestProcessorDelegate` conformance
 //
 extension AlamofireNetwork: RequestProcessorDelegate {
-    func didFailToAuthenticateRequestWithAppPassword(siteID: Int64, reason: AppPasswordFailureReason) {
-        switch reason {
-        case .notSupported:
-            flagSiteAsUnsupported(for: siteID)
-        case .unknown:
-            incrementFailureCount(for: siteID)
-        }
-    }
-
-    func flagSiteAsUnsupported(for siteID: Int64) {
-        let currentList = userDefaults.applicationPasswordUnsupportedList
-        userDefaults.applicationPasswordUnsupportedList = currentList + [siteID]
-    }
-
-    func incrementFailureCount(for siteID: Int64) {
-        let currentFailureCount = appPasswordFailures[siteID] ?? 0
-        let updatedCount = currentFailureCount + 1
-        if updatedCount == AppPasswordConstants.requestFailureThreshold {
-            let currentList = userDefaults.applicationPasswordUnsupportedList
-            userDefaults.applicationPasswordUnsupportedList = currentList + [siteID]
-        }
-        appPasswordFailures[siteID] = updatedCount
+    func didFailToAuthenticateRequestWithAppPassword(siteID: Int64) {
+        errorHandler.flagSiteAsUnsupported(for: siteID)
     }
 }
 
-// MARK: - Constants for direct request error handling
-enum AppPasswordConstants {
-    // flag site as disabled after threshold is reached
-    static let requestFailureThreshold = 10
-    static let disabledCodes = [
-        "application_passwords_disabled",
-        "application_passwords_disabled_for_user",
-        "incorrect_password"
-    ]
-}
 
 private extension DataRequest {
     /// Validates only for `RESTRequest`
@@ -450,6 +343,10 @@ extension Alamofire.DataResponse {
             return error
         }
 
+        if case .some(AFError.requestAdaptationFailed) = error?.asAFError {
+            return error
+        }
+
         return response.flatMap { response in
             NetworkError(responseData: data,
                          statusCode: response.statusCode)