improve HTTPNetworkTransportRetryDelegate to enable returning custom error

Sources/Apollo/HTTPNetworkTransport.swift

@@ -59,11 +59,52 @@ public protocol HTTPNetworkTransportRetryDelegate: HTTPNetworkTransportDelegate
   ///   - request: The URLRequest which generated the error
   ///   - response: [Optional] Any response received when the error was generated
   ///   - retryHandler: A closure indicating whether the operation should be retried. Asyncrhonous to allow for re-authentication or other async operations to complete.
+  @available(*, deprecated, message: "Use networkTransport(_:receivedError:for:,response:continueHandler:) instead")
   func networkTransport(_ networkTransport: HTTPNetworkTransport,
                         receivedError error: Error,
                         for request: URLRequest,
                         response: URLResponse?,
                         retryHandler: @escaping (_ shouldRetry: Bool) -> Void)
+
+  /// Called when an error has been received after a request has been sent to the server to see if an operation should be retried or not.
+  /// NOTE: Don't just call the `continueHandler` with `.retry` all the time, or you can potentially wind up in an infinite loop of errors
+  ///
+  /// - Parameters:
+  ///   - networkTransport: The network transport which received the error
+  ///   - error: The received error
+  ///   - request: The URLRequest which generated the error
+  ///   - response: [Optional] Any response received when the error was generated
+  ///   - continueHandler: A closure indicating whether the operation should be retried. Asyncrhonous to allow for re-authentication or other async operations to complete.
+  func networkTransport(_ networkTransport: HTTPNetworkTransport,
+                        receivedError error: Error,
+                        for request: URLRequest,
+                        response: URLResponse?,
+                        continueHandler: @escaping (_ action: HTTPNetworkTransport.ContinueAction) -> Void)
+}
+
+public extension HTTPNetworkTransportRetryDelegate {
+
+  func networkTransport(_ networkTransport: HTTPNetworkTransport,
+                        receivedError error: Error,
+                        for request: URLRequest,
+                        response: URLResponse?,
+                        retryHandler: @escaping (_ shouldRetry: Bool) -> Void) {
+    retryHandler(false)
+  }
+
+  func networkTransport(_ networkTransport: HTTPNetworkTransport,
+                        receivedError error: Error,
+                        for request: URLRequest,
+                        response: URLResponse?,
+                        continueHandler: @escaping (_ action: HTTPNetworkTransport.ContinueAction) -> Void) {
+    self.networkTransport(networkTransport, receivedError: error, for: request, response: response) { (_ shouldRetry: Bool) in
+      if shouldRetry {
+        continueHandler(.retry)
+      } else {
+        continueHandler(.fail(error))
+      }
+    }
+  }
 }
 
 // MARK: -
@@ -93,6 +134,12 @@ public protocol HTTPNetworkTransportGraphQLErrorDelegate: HTTPNetworkTransportDe
 
 /// A network transport that uses HTTP POST requests to send GraphQL operations to a server, and that uses `URLSession` as the networking implementation.
 public class HTTPNetworkTransport {
+
+  public enum ContinueAction {
+    case retry
+    case fail(_ error: Error)
+  }
+
   let url: URL
   let session: URLSession
   let serializationFormat = JSONSerializationFormat.self
@@ -301,21 +348,21 @@ public class HTTPNetworkTransport {
       receivedError: error,
       for: request,
       response: response,
-      retryHandler: { [weak self] shouldRetry in
+      continueHandler: { [weak self] (action: HTTPNetworkTransport.ContinueAction) in
         guard let self = self else {
           // None of the rest of this really matters
           return
         }
 
-        guard shouldRetry else {
+        switch action {
+        case .retry:
+          _ = self.send(operation: operation,
+                        isPersistedQueryRetry: self.enableAutoPersistedQueries,
+                        files: files,
+                        completionHandler: completionHandler)
+        case .fail(let error):
           completionHandler(.failure(error))
-          return
         }
-
-        _ = self.send(operation: operation,
-                      isPersistedQueryRetry: self.enableAutoPersistedQueries,
-                      files: files,
-                      completionHandler: completionHandler)
     })
   }
 

Tests/ApolloCacheDependentTests/StarWarsServerTests.swift

@@ -22,9 +22,10 @@ class APQsConfig: TestConfig {
 }
 
 class APQsWithGetMethodConfig: TestConfig, HTTPNetworkTransportRetryDelegate{
+
   var alreadyRetried = false
-  func networkTransport(_ networkTransport: HTTPNetworkTransport, receivedError error: Error, for request: URLRequest, response: URLResponse?, retryHandler: @escaping (Bool) -> Void) {
-    retryHandler(!alreadyRetried)
+  func networkTransport(_ networkTransport: HTTPNetworkTransport, receivedError error: Error, for request: URLRequest, response: URLResponse?, continueHandler: @escaping (HTTPNetworkTransport.ContinueAction) -> Void) {
+    continueHandler(!alreadyRetried ? .retry : .fail(error))
     alreadyRetried = true
   }
 

Tests/ApolloTests/HTTPTransportTests.swift

@@ -190,6 +190,74 @@ class HTTPTransportTests: XCTestCase {
     self.wait(for: [expectation], timeout: 10)
   }
 
+  func testRetryDelegateReturnsApolloError() throws {
+    class MockRetryDelegate: HTTPNetworkTransportRetryDelegate {
+      func networkTransport(_ networkTransport: HTTPNetworkTransport,
+                            receivedError error: Error,
+                            for request: URLRequest,
+                            response: URLResponse?,
+                            continueHandler: @escaping (HTTPNetworkTransport.ContinueAction) -> Void) {
+        continueHandler(.fail(error))
+      }
+    }
+
+    let mockRetryDelegate = MockRetryDelegate()
+
+    let transport = HTTPNetworkTransport(url: URL(string: "http://localhost:8080/graphql_non_existant")!)
+    transport.delegate = mockRetryDelegate
+
+    let expectationErrorResponse = self.expectation(description: "Send operation completed")
+
+    let _ = transport.send(operation: HeroNameQuery()) { result in
+      switch result {
+      case .success:
+        XCTFail()
+        expectationErrorResponse.fulfill()
+      case .failure(let error):
+        XCTAssertTrue(error is GraphQLHTTPResponseError)
+        expectationErrorResponse.fulfill()
+      }
+    }
+
+    wait(for: [expectationErrorResponse], timeout: 1)
+  }
+
+  func testRetryDelegateReturnsCustomError() throws {
+    enum MockError: Error, Equatable {
+      case customError
+    }
+
+    class MockRetryDelegate: HTTPNetworkTransportRetryDelegate {
+      func networkTransport(_ networkTransport: HTTPNetworkTransport,
+                            receivedError error: Error,
+                            for request: URLRequest,
+                            response: URLResponse?,
+                            continueHandler: @escaping (HTTPNetworkTransport.ContinueAction) -> Void) {
+        continueHandler(.fail(MockError.customError))
+      }
+    }
+
+    let mockRetryDelegate = MockRetryDelegate()
+
+    let transport = HTTPNetworkTransport(url: URL(string: "http://localhost:8080/graphql_non_existant")!)
+    transport.delegate = mockRetryDelegate
+
+    let expectationErrorResponse = self.expectation(description: "Send operation completed")
+
+    let _ = transport.send(operation: HeroNameQuery()) { result in
+      switch result {
+      case .success:
+        XCTFail()
+        expectationErrorResponse.fulfill()
+      case .failure(let error):
+        XCTAssertTrue(error is MockError)
+        expectationErrorResponse.fulfill()
+      }
+    }
+
+    wait(for: [expectationErrorResponse], timeout: 1)
+  }
+
   func testEquality() {
     let identicalTransport = HTTPNetworkTransport(url: self.url,
                                                   useGETForQueries: true)
@@ -340,9 +408,9 @@ extension HTTPTransportTests: HTTPNetworkTransportRetryDelegate {
                         receivedError error: Error,
                         for request: URLRequest,
                         response: URLResponse?,
-                        retryHandler: @escaping (Bool) -> Void) {
+                        continueHandler: @escaping (HTTPNetworkTransport.ContinueAction) -> Void) {
     guard let graphQLError = error as? GraphQLHTTPResponseError else {
-      retryHandler(false)
+      continueHandler(.fail(error))
       return
     }
 
@@ -352,12 +420,12 @@ extension HTTPTransportTests: HTTPNetworkTransportRetryDelegate {
       if retryCount > 1 {
         self.shouldModifyURLInWillSend = false
       }
-      retryHandler(true)
+      continueHandler(.retry)
     case .invalidResponse:
-      retryHandler(false)
+      continueHandler(.fail(error))
     case .persistedQueryNotFound,
          .persistedQueryNotSupported:
-      retryHandler(false)
+      continueHandler(.fail(error))
     }
   }
 }

docs/source/api/Apollo/protocols/HTTPNetworkTransportRetryDelegate.md

@@ -9,14 +9,14 @@ public protocol HTTPNetworkTransportRetryDelegate: HTTPNetworkTransportDelegate
 > Methods which will be called if an error is receieved at the network level.
 
 ## Methods
-### `networkTransport(_:receivedError:for:response:retryHandler:)`
+### `networkTransport(_:receivedError:for:response:continueHandler:)`
 
 ```swift
 func networkTransport(_ networkTransport: HTTPNetworkTransport,
                       receivedError error: Error,
                       for request: URLRequest,
                       response: URLResponse?,
-                      retryHandler: @escaping (_ shouldRetry: Bool) -> Void)
+                      continueHandler: @escaping (_ action: HTTPNetworkTransport.ContinueAction) -> Void)
 ```
 
 > Called when an error has been received after a request has been sent to the server to see if an operation should be retried or not.
@@ -27,7 +27,7 @@ func networkTransport(_ networkTransport: HTTPNetworkTransport,
 >   - error: The received error
 >   - request: The URLRequest which generated the error
 >   - response: [Optional] Any response received when the error was generated
->   - retryHandler: A closure indicating whether the operation should be retried. Asyncrhonous to allow for re-authentication or other async operations to complete.
+>   - continueHandler: A closure indicating whether the operation should be retried. Asyncrhonous to allow for re-authentication or other async operations to complete.
 
 #### Parameters
 
@@ -37,4 +37,4 @@ func networkTransport(_ networkTransport: HTTPNetworkTransport,
 | error | The received error |
 | request | The URLRequest which generated the error |
 | response | [Optional] Any response received when the error was generated |
-| retryHandler | A closure indicating whether the operation should be retried. Asyncrhonous to allow for re-authentication or other async operations to complete. |
+| retryHandler | A closure indicating whether the operation should be retried. Asyncrhonous to allow for re-authentication or other async operations to complete. |

docs/source/initialization.md

@@ -161,18 +161,24 @@ extension Network: HTTPNetworkTransportRetryDelegate {
                         receivedError error: Error,
                         for request: URLRequest,
                         response: URLResponse?,
-                        retryHandler: @escaping (_ shouldRetry: Bool) -> Void) {
+                        continueHandler: @escaping (_ action: HTTPNetworkTransport.ContinueAction) -> Void) {
     // Check if the error and/or response you've received are something that requires authentication
     guard UserManager.shared.requiresReAuthentication(basedOn: error, response: response) else {
       // This is not something this application can handle, do not retry.
-      retryHandler(false)
+      continueHandler(.fail(error))
       return
     }
 
     // Attempt to re-authenticate asynchronously
-    UserManager.shared.reAuthenticate { success in 
+    UserManager.shared.reAuthenticate { (reAuthenticateError: Error?) in 
       // If re-authentication succeeded, try again. If it didn't, don't.
-      retryHandler(success)
+      if let reAuthenticateError = reAuthenticateError {
+        continueHandler(.fail(reAuthenticateError)) // Will return re authenticate error to query callback 
+        // or (depending what error you want to get to callback)
+        continueHandler(.fail(error)) // Will return original error
+      } else {
+        continueHandler(.retry)
+      }
     }
   }
 }