Skip to content

Commit

Permalink
Merge pull request #1489 from apollographql/add/invalidation-error
Browse files Browse the repository at this point in the history
Check that `URLSessionClient` hasn't been invalidated before sending
  • Loading branch information
designatednerd authored Nov 2, 2020
2 parents a0e0e27 + 9df9f04 commit a91fccd
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 1 deletion.
6 changes: 5 additions & 1 deletion Sources/Apollo/NetworkFetchInterceptor.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
15 changes: 15 additions & 0 deletions Sources/Apollo/URLSessionClient.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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!"
}
}
}
Expand All @@ -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:
Expand All @@ -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()
Expand Down Expand Up @@ -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)
Expand Down
27 changes: 27 additions & 0 deletions Tests/ApolloTests/URLSessionClientTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

0 comments on commit a91fccd

Please sign in to comment.