#3006: Less Fatal SessionManager State Checking (#3010)

* Make failures less fatal when the stateProvider isn’t found.

* Attempt to make tests faster / more reliable.

* Attempt to make upload test more reliable.

* Further attempt test stabalization.

* More test reliability.

* Increase image size again to make download test more reliable.

* Increase image size again.

* Make test more reliable.

* Formatting.

* Put cancellation on the root queue to see if its more reliable.

Source/SessionDelegate.swift

@@ -38,6 +38,24 @@ open class SessionDelegate: NSObject {
     public init(fileManager: FileManager = .default) {
         self.fileManager = fileManager
     }
+
+    /// Internal method to find and cast requests while maintaining some integrity checking.
+    ///
+    /// - Parameters:
+    ///   - task: The `URLSessionTask` for which to find the associated `Request`.
+    ///   - type: The `Request` subclass type to cast any `Request` associate with `task`.
+    func request<R: Request>(for task: URLSessionTask, as type: R.Type) -> R? {
+        guard let provider = stateProvider else {
+            assertionFailure("StateProvider is nil.")
+            return nil
+        }
+
+        guard let request = provider.request(for: task) as? R else {
+            fatalError("Returned Request is not of expected type: \(R.self).")
+        }
+
+        return request
+    }
 }
 
 /// Type which provides various `Session` state values.
@@ -79,7 +97,8 @@ extension SessionDelegate: URLSessionTaskDelegate {
         switch challenge.protectionSpace.authenticationMethod {
         case NSURLAuthenticationMethodServerTrust:
             evaluation = attemptServerTrustAuthentication(with: challenge)
-        case NSURLAuthenticationMethodHTTPBasic, NSURLAuthenticationMethodHTTPDigest, NSURLAuthenticationMethodNTLM, NSURLAuthenticationMethodNegotiate, NSURLAuthenticationMethodClientCertificate:
+        case NSURLAuthenticationMethodHTTPBasic, NSURLAuthenticationMethodHTTPDigest, NSURLAuthenticationMethodNTLM,
+             NSURLAuthenticationMethodNegotiate, NSURLAuthenticationMethodClientCertificate:
             evaluation = attemptCredentialAuthentication(for: challenge, belongingTo: task)
         default:
             evaluation = (.performDefaultHandling, nil, nil)
@@ -159,8 +178,10 @@ extension SessionDelegate: URLSessionTaskDelegate {
                          needNewBodyStream completionHandler: @escaping (InputStream?) -> Void) {
         eventMonitor?.urlSession(session, taskNeedsNewBodyStream: task)
 
-        guard let request = stateProvider?.request(for: task) as? UploadRequest else {
-            fatalError("needNewBodyStream for request that isn't UploadRequest.")
+        guard let request = request(for: task, as: UploadRequest.self) else {
+            assertionFailure("needNewBodyStream did not find UploadRequest.")
+            completionHandler(nil)
+            return
         }
 
         completionHandler(request.inputStream())
@@ -208,8 +229,9 @@ extension SessionDelegate: URLSessionDataDelegate {
     open func urlSession(_ session: URLSession, dataTask: URLSessionDataTask, didReceive data: Data) {
         eventMonitor?.urlSession(session, dataTask: dataTask, didReceive: data)
 
-        guard let request = stateProvider?.request(for: dataTask) as? DataRequest else {
-            fatalError("dataTask received data for incorrect Request subclass: \(String(describing: stateProvider?.request(for: dataTask)))")
+        guard let request = request(for: dataTask, as: DataRequest.self) else {
+            assertionFailure("dataTask did not find DataRequest.")
+            return
         }
 
         request.didReceive(data: data)
@@ -240,9 +262,9 @@ extension SessionDelegate: URLSessionDownloadDelegate {
                                  downloadTask: downloadTask,
                                  didResumeAtOffset: fileOffset,
                                  expectedTotalBytes: expectedTotalBytes)
-
-        guard let downloadRequest = stateProvider?.request(for: downloadTask) as? DownloadRequest else {
-            fatalError("No DownloadRequest found for downloadTask: \(downloadTask)")
+        guard let downloadRequest = request(for: downloadTask, as: DownloadRequest.self) else {
+            assertionFailure("downloadTask did not find DownloadRequest.")
+            return
         }
 
         downloadRequest.updateDownloadProgress(bytesWritten: fileOffset,
@@ -259,9 +281,9 @@ extension SessionDelegate: URLSessionDownloadDelegate {
                                  didWriteData: bytesWritten,
                                  totalBytesWritten: totalBytesWritten,
                                  totalBytesExpectedToWrite: totalBytesExpectedToWrite)
-
-        guard let downloadRequest = stateProvider?.request(for: downloadTask) as? DownloadRequest else {
-            fatalError("No DownloadRequest found for downloadTask: \(downloadTask)")
+        guard let downloadRequest = request(for: downloadTask, as: DownloadRequest.self) else {
+            assertionFailure("downloadTask did not find DownloadRequest.")
+            return
         }
 
         downloadRequest.updateDownloadProgress(bytesWritten: bytesWritten,
@@ -271,8 +293,9 @@ extension SessionDelegate: URLSessionDownloadDelegate {
     open func urlSession(_ session: URLSession, downloadTask: URLSessionDownloadTask, didFinishDownloadingTo location: URL) {
         eventMonitor?.urlSession(session, downloadTask: downloadTask, didFinishDownloadingTo: location)
 
-        guard let request = stateProvider?.request(for: downloadTask) as? DownloadRequest else {
-            fatalError("Download finished but either no request found or request wasn't DownloadRequest")
+        guard let request = request(for: downloadTask, as: DownloadRequest.self) else {
+            assertionFailure("downloadTask did not find DownloadRequest.")
+            return
         }
 
         guard let response = request.response else {

Tests/DownloadTests.swift

@@ -141,7 +141,7 @@ class DownloadResponseTestCase: BaseTestCase {
 
     func testDownloadRequestWithProgress() {
         // Given
-        let randomBytes = 1 * 1024 * 1024
+        let randomBytes = 1 * 25 * 1024
         let urlString = "https://httpbin.org/bytes/\(randomBytes)"
 
         let expectation = self.expectation(description: "Bytes download progress should be reported: \(urlString)")
@@ -474,7 +474,7 @@ final class DownloadRequestEventsTestCase: BaseTestCase {
 // MARK: -
 
 final class DownloadResumeDataTestCase: BaseTestCase {
-    let urlString = "https://upload.wikimedia.org/wikipedia/commons/6/69/NASA-HS201427a-HubbleUltraDeepField2014-20140603.jpg"
+    let urlString = "https://upload.wikimedia.org/wikipedia/commons/thumb/5/5f/HubbleDeepField.800px.jpg/2048px-HubbleDeepField.800px.jpg"
 
     func testThatCancelledDownloadRequestDoesNotProduceResumeData() {
         // Given
@@ -595,7 +595,7 @@ final class DownloadResumeDataTestCase: BaseTestCase {
         download.downloadProgress { progress in
             guard !cancelled else { return }
 
-            if progress.fractionCompleted > 0.4 {
+            if progress.fractionCompleted > 0.1 {
                 download.cancel(producingResumeData: true)
                 cancelled = true
             }
@@ -640,7 +640,7 @@ final class DownloadResumeDataTestCase: BaseTestCase {
         XCTAssertEqual(response2?.result.isSuccess, true)
         XCTAssertNil(response2?.result.failure)
 
-        progressValues.forEach { XCTAssertGreaterThanOrEqual($0, 0.4) }
+        progressValues.forEach { XCTAssertGreaterThanOrEqual($0, 0.1) }
     }
 
     func testThatCancelledDownloadProducesMatchingResumeData() {

Tests/RequestTests.swift

@@ -51,7 +51,7 @@ final class RequestResponseTestCase: BaseTestCase {
 
     func testRequestResponseWithProgress() {
         // Given
-        let randomBytes = 1 * 1024 * 1024
+        let randomBytes = 1 * 25 * 1024
         let urlString = "https://httpbin.org/bytes/\(randomBytes)"
 
         let expectation = self.expectation(description: "Bytes download progress should be reported: \(urlString)")
@@ -591,8 +591,9 @@ final class RequestResponseTestCase: BaseTestCase {
 
     func testThatCancelledRequestTriggersAllAppropriateLifetimeEvents() {
         // Given
-        let eventMonitor = ClosureEventMonitor()
-        let session = Session(startRequestsImmediately: false, eventMonitors: [eventMonitor])
+        let queue = DispatchQueue(label: "org.alamofire.Session.TestRootQueue")
+        let eventMonitor = ClosureEventMonitor(queue: queue)
+        let session = Session(rootQueue: queue, startRequestsImmediately: false, eventMonitors: [eventMonitor])
 
         let taskDidFinishCollecting = expectation(description: "taskDidFinishCollecting should fire")
         let didCreateURLRequest = expectation(description: "didCreateInitialURLRequest should fire")

Tests/UploadTests.swift

@@ -200,7 +200,7 @@ class UploadDataTestCase: BaseTestCase {
     func testUploadDataRequestWithProgress() {
         // Given
         let urlString = "https://httpbin.org/post"
-        let string = String(repeating: "Lorem ipsum dolor sit amet, consectetur adipiscing elit. ", count: 100)
+        let string = String(repeating: "Lorem ipsum dolor sit amet, consectetur adipiscing elit. ", count: 300)
         let data = Data(string.utf8)
 
         let expectation = self.expectation(description: "Bytes upload progress should be reported: \(urlString)")