Merge pull request #1489 from apollographql/add/invalidation-error

Check that `URLSessionClient` hasn't been invalidated before sending

Author: designatednerd

Sources/Apollo/NetworkFetchInterceptor.swift

@@ -29,7 +29,11 @@ public class NetworkFetchInterceptor: ApolloInterceptor, Cancellable {
       return
     }
 
-    self.currentTask = self.client.sendRequest(urlRequest) { result in
+    self.currentTask = self.client.sendRequest(urlRequest) { [weak self] result in
+      guard let self = self else {
+        return
+      }
+      
       defer {
         self.currentTask = nil
       }

Sources/Apollo/URLSessionClient.swift

@@ -18,6 +18,7 @@ open class URLSessionClient: NSObject, URLSessionDelegate, URLSessionTaskDelegat
     case sessionBecameInvalidWithoutUnderlyingError
     case dataForRequestNotFound(request: URLRequest?)
     case networkError(data: Data, response: HTTPURLResponse?, underlying: Error)
+    case sessionInvalidated
 
     public var errorDescription: String? {
       switch self {
@@ -29,6 +30,8 @@ open class URLSessionClient: NSObject, URLSessionDelegate, URLSessionTaskDelegat
         return "URLSessionClient was not able to locate the stored data for request \(String(describing: request))"
       case .networkError(_, _, let underlyingError):
         return "A network error occurred: \(underlyingError.localizedDescription)"
+      case .sessionInvalidated:
+        return "Attempting to create a new request after the session has been invalidated!"
       }
     }
   }
@@ -44,6 +47,12 @@ open class URLSessionClient: NSObject, URLSessionDelegate, URLSessionTaskDelegat
   /// The raw URLSession being used for this client
   open private(set) var session: URLSession!
 
+  private var hasBeenInvalidated = Atomic<Bool>(false)
+  
+  private var hasNotBeenInvalidated: Bool {
+    !self.hasBeenInvalidated.value
+  }
+  
   /// Designated initializer.
   ///
   /// - Parameters:
@@ -61,6 +70,7 @@ open class URLSessionClient: NSObject, URLSessionDelegate, URLSessionTaskDelegat
   ///
   /// NOTE: This must be called from the `deinit` of anything holding onto this client in order to break a retain cycle with the delegate.
   public func invalidate() {
+    self.hasBeenInvalidated.value = true
     func cleanup() {
       self.session = nil
       self.clearAllTasks()
@@ -107,6 +117,11 @@ open class URLSessionClient: NSObject, URLSessionDelegate, URLSessionTaskDelegat
   open func sendRequest(_ request: URLRequest,
                         rawTaskCompletionHandler: RawCompletion? = nil,
                         completion: @escaping Completion) -> URLSessionTask {
+    guard self.hasNotBeenInvalidated else {
+      completion(.failure(URLSessionClientError.sessionInvalidated))
+      return URLSessionTask()
+    }
+    
     let task = self.session.dataTask(with: request)
     let taskData = TaskData(rawCompletion: rawTaskCompletionHandler,
                             completionBlock: completion)

Tests/ApolloTests/URLSessionClientTests.swift

@@ -214,4 +214,31 @@ class URLSessionClientLiveTests: XCTestCase {
     let set = Set(taskIDs.value)
     XCTAssertEqual(set.count, iterations)
   }
+  
+  func testInvalidatingClientAndThenTryingToSendARequestReturnsAppropriateError() {
+    let client = URLSessionClient()
+    client.invalidate()
+    
+    let expectation = self.expectation(description: "Basic GET request completed")
+    client.sendRequest(self.request(for: .get)) { result in
+      defer {
+        expectation.fulfill()
+      }
+      
+      switch result {
+      case .failure(let error):
+        switch error {
+        case URLSessionClient.URLSessionClientError.sessionInvalidated:
+          // This is what we want
+          break
+        default:
+          XCTFail("Unexpected error: \(error)")
+        }
+      case .success:
+        XCTFail("This should not have succeeded")
+      }
+    }
+    
+    self.wait(for: [expectation], timeout: 10)
+  }
 }