Preserve connection errors for user

Author: fabianfett

Diff for: Sources/AsyncHTTPClient/ConnectionPool/State Machine/HTTPConnectionPool+HTTP1Connections.swift

@@ -257,6 +257,11 @@ extension HTTPConnectionPool {
             return connecting
         }
 
+        /// Is there at least one connection that is able to run requests
+        var hasActiveConnections: Bool {
+            self.connections.contains(where: { $0.isIdle || $0.isLeased })
+        }
+
         func startingEventLoopConnections(on eventLoop: EventLoop) -> Int {
             return self.connections[self.overflowIndex..<self.connections.endIndex].reduce(into: 0) { count, connection in
                 guard connection.eventLoop === eventLoop else { return }

Diff for: Sources/AsyncHTTPClient/ConnectionPool/State Machine/HTTPConnectionPool+HTTP1StateMachine.swift

@@ -26,6 +26,8 @@ extension HTTPConnectionPool {
 
         private var connections: HTTP1Connections
         private var failedConsecutiveConnectionAttempts: Int = 0
+        /// the error from the last connection creation
+        private var lastConnectFailure: Error?
 
         private var requests: RequestQueue
         private var state: State = .running
@@ -136,12 +138,14 @@ extension HTTPConnectionPool {
 
         mutating func newHTTP1ConnectionEstablished(_ connection: Connection) -> Action {
             self.failedConsecutiveConnectionAttempts = 0
+            self.lastConnectFailure = nil
             let (index, context) = self.connections.newHTTP1ConnectionEstablished(connection)
             return self.nextActionForIdleConnection(at: index, context: context)
         }
 
         mutating func failedToCreateNewConnection(_ error: Error, connectionID: Connection.ID) -> Action {
             self.failedConsecutiveConnectionAttempts += 1
+            self.lastConnectFailure = error
 
             switch self.state {
             case .running:
@@ -223,8 +227,14 @@ extension HTTPConnectionPool {
         mutating func timeoutRequest(_ requestID: Request.ID) -> Action {
             // 1. check requests in queue
             if let request = self.requests.remove(requestID) {
+                var error: Error = HTTPClientError.getConnectionFromPoolTimeout
+                if let lastError = self.lastConnectFailure {
+                    error = lastError
+                } else if !self.connections.hasActiveConnections {
+                    error = HTTPClientError.connectTimeout
+                }
                 return .init(
-                    request: .failRequest(request, HTTPClientError.getConnectionFromPoolTimeout, cancelTimeout: false),
+                    request: .failRequest(request, error, cancelTimeout: false),
                     connection: .none
                 )
             }

Diff for: Tests/AsyncHTTPClientTests/HTTPConnectionPool+HTTP1StateTests+XCTest.swift

@@ -38,6 +38,9 @@ extension HTTPConnectionPool_HTTP1StateMachineTests {
             ("testConnectionPoolFullOfParkedConnectionsIsShutdownImmediately", testConnectionPoolFullOfParkedConnectionsIsShutdownImmediately),
             ("testParkedConnectionTimesOutButIsAlsoClosedByRemote", testParkedConnectionTimesOutButIsAlsoClosedByRemote),
             ("testConnectionBackoffVsShutdownRace", testConnectionBackoffVsShutdownRace),
+            ("testRequestThatTimesOutIsFailedWithLastConnectionCreationError", testRequestThatTimesOutIsFailedWithLastConnectionCreationError),
+            ("testRequestThatTimesOutBeforeAConnectionIsEstablishedIsFailedWithConnectTimeoutError", testRequestThatTimesOutBeforeAConnectionIsEstablishedIsFailedWithConnectTimeoutError),
+            ("testRequestThatTimesOutAfterAConnectionWasEstablishedSuccessfullyTimesOutWithGenericError", testRequestThatTimesOutAfterAConnectionWasEstablishedSuccessfullyTimesOutWithGenericError),
         ]
     }
 }

Diff for: Tests/AsyncHTTPClientTests/HTTPConnectionPool+HTTP1StateTests.swift

@@ -153,7 +153,7 @@ class HTTPConnectionPool_HTTP1StateMachineTests: XCTestCase {
             return XCTFail("Unexpected request action: \(action.request)")
         }
         XCTAssert(requestToFail.__testOnly_wrapped_request() === mockRequest) // XCTAssertIdentical not available on Linux
-        XCTAssertEqual(requestError as? HTTPClientError, .getConnectionFromPoolTimeout)
+        XCTAssertEqual(requestError as? HTTPClientError, .connectTimeout)
         XCTAssertEqual(failRequest.connection, .none)
 
         // 4. retry connection, but no more queued requests.
@@ -626,4 +626,110 @@ class HTTPConnectionPool_HTTP1StateMachineTests: XCTestCase {
 
         XCTAssertEqual(state.connectionCreationBackoffDone(connectionID), .none)
     }
+
+    func testRequestThatTimesOutIsFailedWithLastConnectionCreationError() {
+        let elg = EmbeddedEventLoopGroup(loops: 1)
+        defer { XCTAssertNoThrow(try elg.syncShutdownGracefully()) }
+
+        var state = HTTPConnectionPool.StateMachine(
+            eventLoopGroup: elg,
+            idGenerator: .init(),
+            maximumConcurrentHTTP1Connections: 6
+        )
+
+        let mockRequest = MockHTTPRequest(eventLoop: elg.next(), requiresEventLoopForChannel: false)
+        let request = HTTPConnectionPool.Request(mockRequest)
+
+        let executeAction = state.executeRequest(request)
+        guard case .createConnection(let connectionID, on: let connEL) = executeAction.connection else {
+            return XCTFail("Expected to create a connection")
+        }
+
+        XCTAssertEqual(executeAction.request, .scheduleRequestTimeout(for: request, on: mockRequest.eventLoop))
+
+        let failAction = state.failedToCreateNewConnection(HTTPClientError.httpProxyHandshakeTimeout, connectionID: connectionID)
+        guard case .scheduleBackoffTimer(connectionID, backoff: _, on: let timerEL) = failAction.connection else {
+            return XCTFail("Expected to create a backoff timer")
+        }
+        XCTAssert(timerEL === connEL)
+        XCTAssertEqual(failAction.request, .none)
+
+        let timeoutAction = state.timeoutRequest(request.id)
+        XCTAssertEqual(timeoutAction.request, .failRequest(request, HTTPClientError.httpProxyHandshakeTimeout, cancelTimeout: false))
+        XCTAssertEqual(timeoutAction.connection, .none)
+    }
+
+    func testRequestThatTimesOutBeforeAConnectionIsEstablishedIsFailedWithConnectTimeoutError() {
+        let eventLoop = EmbeddedEventLoop()
+        defer { XCTAssertNoThrow(try eventLoop.syncShutdownGracefully()) }
+
+        var state = HTTPConnectionPool.StateMachine(
+            eventLoopGroup: eventLoop,
+            idGenerator: .init(),
+            maximumConcurrentHTTP1Connections: 6
+        )
+
+        let mockRequest = MockHTTPRequest(eventLoop: eventLoop.next(), requiresEventLoopForChannel: false)
+        let request = HTTPConnectionPool.Request(mockRequest)
+
+        let executeAction = state.executeRequest(request)
+        guard case .createConnection(_, on: _) = executeAction.connection else {
+            return XCTFail("Expected to create a connection")
+        }
+        XCTAssertEqual(executeAction.request, .scheduleRequestTimeout(for: request, on: mockRequest.eventLoop))
+
+        let timeoutAction = state.timeoutRequest(request.id)
+        XCTAssertEqual(timeoutAction.request, .failRequest(request, HTTPClientError.connectTimeout, cancelTimeout: false))
+        XCTAssertEqual(timeoutAction.connection, .none)
+    }
+
+    func testRequestThatTimesOutAfterAConnectionWasEstablishedSuccessfullyTimesOutWithGenericError() {
+        let elg = EmbeddedEventLoopGroup(loops: 1)
+        defer { XCTAssertNoThrow(try elg.syncShutdownGracefully()) }
+
+        var state = HTTPConnectionPool.StateMachine(
+            eventLoopGroup: elg,
+            idGenerator: .init(),
+            maximumConcurrentHTTP1Connections: 6
+        )
+
+        let mockRequest1 = MockHTTPRequest(eventLoop: elg.next(), requiresEventLoopForChannel: false)
+        let request1 = HTTPConnectionPool.Request(mockRequest1)
+
+        let executeAction1 = state.executeRequest(request1)
+        guard case .createConnection(let connectionID1, on: let connEL1) = executeAction1.connection else {
+            return XCTFail("Expected to create a connection")
+        }
+        XCTAssert(mockRequest1.eventLoop === connEL1)
+
+        XCTAssertEqual(executeAction1.request, .scheduleRequestTimeout(for: request1, on: mockRequest1.eventLoop))
+
+        let mockRequest2 = MockHTTPRequest(eventLoop: elg.next(), requiresEventLoopForChannel: false)
+        let request2 = HTTPConnectionPool.Request(mockRequest2)
+
+        let executeAction2 = state.executeRequest(request2)
+        guard case .createConnection(let connectionID2, on: let connEL2) = executeAction2.connection else {
+            return XCTFail("Expected to create a connection")
+        }
+        XCTAssert(mockRequest2.eventLoop === connEL2)
+
+        XCTAssertEqual(executeAction2.request, .scheduleRequestTimeout(for: request2, on: connEL1))
+
+        let failAction = state.failedToCreateNewConnection(HTTPClientError.httpProxyHandshakeTimeout, connectionID: connectionID1)
+        guard case .scheduleBackoffTimer(connectionID1, backoff: _, on: let timerEL) = failAction.connection else {
+            return XCTFail("Expected to create a backoff timer")
+        }
+        XCTAssert(timerEL === connEL2)
+        XCTAssertEqual(failAction.request, .none)
+
+        let conn2 = HTTPConnectionPool.Connection.__testOnly_connection(id: connectionID2, eventLoop: connEL2)
+        let createdAction = state.newHTTP1ConnectionCreated(conn2)
+
+        XCTAssertEqual(createdAction.request, .executeRequest(request1, conn2, cancelTimeout: true))
+        XCTAssertEqual(createdAction.connection, .none)
+
+        let timeoutAction = state.timeoutRequest(request2.id)
+        XCTAssertEqual(timeoutAction.request, .failRequest(request2, HTTPClientError.getConnectionFromPoolTimeout, cancelTimeout: false))
+        XCTAssertEqual(timeoutAction.connection, .none)
+    }
 }

Diff for: Tests/AsyncHTTPClientTests/HTTPConnectionPoolTests.swift

@@ -229,7 +229,7 @@ class HTTPConnectionPoolTests: XCTestCase {
 
         pool.executeRequest(requestBag)
         XCTAssertThrowsError(try requestBag.task.futureResult.wait()) {
-            XCTAssertEqual($0 as? HTTPClientError, .getConnectionFromPoolTimeout)
+            XCTAssertEqual($0 as? HTTPClientError, .proxyAuthenticationRequired)
         }
         XCTAssertGreaterThanOrEqual(httpBin.createdConnections, 8)
         XCTAssertEqual(httpBin.activeConnections, 0)