Don’t call didReceiveError twice if deadline is exceeded and request is canceled aftewards (#609)

Author: dnadoba

Sources/AsyncHTTPClient/RequestBag+StateMachine.swift

@@ -607,13 +607,18 @@ extension RequestBag.StateMachine {
             // An error occurred after the request has finished. Ignore...
             return .none
         case .deadlineExceededWhileQueued:
-            // if we just get a `HTTPClientError.cancelled` we can use the original cancellation reason
-            // to give a more descriptive error to the user.
-            if (error as? HTTPClientError) == .cancelled {
-                return .failTask(HTTPClientError.deadlineExceeded, nil, nil)
-            }
-            // otherwise we already had an intermediate connection error which we should present to the user instead
-            return .failTask(error, nil, nil)
+            let realError: Error = {
+                if (error as? HTTPClientError) == .cancelled {
+                    /// if we just get a `HTTPClientError.cancelled` we can use the original cancellation reason
+                    /// to give a more descriptive error to the user.
+                    return HTTPClientError.deadlineExceeded
+                } else {
+                    /// otherwise we already had an intermediate connection error which we should present to the user instead
+                    return error
+                }
+            }()
+            self.state = .finished(error: realError)
+            return .failTask(realError, nil, nil)
         case .finished(.some(_)):
             // this might happen, if the stream consumer has failed... let's just drop the data
             return .none

Tests/AsyncHTTPClientTests/RequestBagTests+XCTest.swift

@@ -29,6 +29,7 @@ extension RequestBagTests {
             ("testTaskIsFailedIfWritingFails", testTaskIsFailedIfWritingFails),
             ("testCancelFailsTaskBeforeRequestIsSent", testCancelFailsTaskBeforeRequestIsSent),
             ("testDeadlineExceededFailsTaskEvenIfRaceBetweenCancelingSchedulerAndRequestStart", testDeadlineExceededFailsTaskEvenIfRaceBetweenCancelingSchedulerAndRequestStart),
+            ("testCancelHasNoEffectAfterDeadlineExceededFailsTask", testCancelHasNoEffectAfterDeadlineExceededFailsTask),
             ("testCancelFailsTaskAfterRequestIsSent", testCancelFailsTaskAfterRequestIsSent),
             ("testCancelFailsTaskWhenTaskIsQueued", testCancelFailsTaskWhenTaskIsQueued),
             ("testFailsTaskWhenTaskIsWaitingForMoreFromServer", testFailsTaskWhenTaskIsWaitingForMoreFromServer),

Tests/AsyncHTTPClientTests/RequestBagTests.swift

@@ -266,6 +266,48 @@ final class RequestBagTests: XCTestCase {
         }
     }
 
+    func testCancelHasNoEffectAfterDeadlineExceededFailsTask() {
+        struct MyError: Error, Equatable {}
+        let embeddedEventLoop = EmbeddedEventLoop()
+        defer { XCTAssertNoThrow(try embeddedEventLoop.syncShutdownGracefully()) }
+        let logger = Logger(label: "test")
+
+        var maybeRequest: HTTPClient.Request?
+        XCTAssertNoThrow(maybeRequest = try HTTPClient.Request(url: "https://swift.org"))
+        guard let request = maybeRequest else { return XCTFail("Expected to have a request") }
+
+        let delegate = UploadCountingDelegate(eventLoop: embeddedEventLoop)
+        var maybeRequestBag: RequestBag<UploadCountingDelegate>?
+        XCTAssertNoThrow(maybeRequestBag = try RequestBag(
+            request: request,
+            eventLoopPreference: .delegate(on: embeddedEventLoop),
+            task: .init(eventLoop: embeddedEventLoop, logger: logger),
+            redirectHandler: nil,
+            connectionDeadline: .now() + .seconds(30),
+            requestOptions: .forTests(),
+            delegate: delegate
+        ))
+        guard let bag = maybeRequestBag else { return XCTFail("Expected to be able to create a request bag.") }
+        XCTAssert(bag.eventLoop === embeddedEventLoop)
+
+        let queuer = MockTaskQueuer()
+        bag.requestWasQueued(queuer)
+
+        XCTAssertEqual(queuer.hitCancelCount, 0)
+        bag.deadlineExceeded()
+        XCTAssertEqual(queuer.hitCancelCount, 1)
+        XCTAssertEqual(delegate.hitDidReceiveError, 0)
+        bag.fail(MyError())
+        XCTAssertEqual(delegate.hitDidReceiveError, 1)
+
+        bag.cancel()
+        XCTAssertEqual(delegate.hitDidReceiveError, 1)
+
+        XCTAssertThrowsError(try bag.task.futureResult.wait()) {
+            XCTAssertEqualTypeAndValue($0, MyError())
+        }
+    }
+
     func testCancelFailsTaskAfterRequestIsSent() {
         let embeddedEventLoop = EmbeddedEventLoop()
         defer { XCTAssertNoThrow(try embeddedEventLoop.syncShutdownGracefully()) }