Preserve connection errors for user

fabianfett · fabianfett · commit 701c1d489a7e · 2021-09-13T18:13:54.000+02:00
diff --git a/Sources/AsyncHTTPClient/ConnectionPool/State Machine/HTTPConnectionPool+HTTP1Connections.swift b/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 --git a/Sources/AsyncHTTPClient/ConnectionPool/State Machine/HTTPConnectionPool+HTTP1StateMachine.swift b/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 --git a/Tests/AsyncHTTPClientTests/HTTPConnectionPool+HTTP1StateTests+XCTest.swift b/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 --git a/Tests/AsyncHTTPClientTests/HTTPConnectionPool+HTTP1StateTests.swift b/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 --git a/Tests/AsyncHTTPClientTests/HTTPConnectionPoolTests.swift b/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)

-Original file line number
+Diff line change
 return connecting
+}
 + /// Is there at least one connection that is able to run requests
 +varhasActiveConnections:Bool{
 +self.connections.contains(where:{ $0.isIdle || $0.isLeased })
 +}
++
 func startingEventLoopConnections(on eventLoop:EventLoop)->Int{
 returnself.connections[self.overflowIndex..<self.connections.endIndex].reduce(into:0){ count, connection in
 guard connection.eventLoop === eventLoop else{return}
-Original file line number
+Diff line change
 privatevarconnections:HTTP1Connections
 privatevarfailedConsecutiveConnectionAttempts:Int=0
 + /// the error from the last connection creation
 +privatevarlastConnectFailure:Error?
 privatevarrequests:RequestQueue
 privatevarstate:State=.running
 mutatingfunc newHTTP1ConnectionEstablished(_ connection:Connection)->Action{
 self.failedConsecutiveConnectionAttempts =0
 +self.lastConnectFailure =nil
 let(index, context)=self.connections.newHTTP1ConnectionEstablished(connection)
 returnself.nextActionForIdleConnection(at: index, context: context)
+}
 mutatingfunc failedToCreateNewConnection(_ error:Error, connectionID:Connection.ID)->Action{
 self.failedConsecutiveConnectionAttempts +=1
 +self.lastConnectFailure = error
 switchself.state {
 case.running:
 mutatingfunc timeoutRequest(_ requestID:Request.ID)->Action{
  // 1. check requests in queue
 iflet request =self.requests.remove(requestID){
 +varerror:Error=HTTPClientError.getConnectionFromPoolTimeout
 +iflet lastError =self.lastConnectFailure {
 + error = lastError
 +}elseif !self.connections.hasActiveConnections {
 + error =HTTPClientError.connectTimeout
 +}
 return.init(
 - request:.failRequest(request,HTTPClientError.getConnectionFromPoolTimeout, cancelTimeout:false),
 + request:.failRequest(request,error, cancelTimeout:false),
  connection:.none
+)
+}
Original file line number	Diff line number	Diff line change
`@@ -38,6 +38,9 @@ extension HTTPConnectionPool_HTTP1StateMachineTests{`
`38`	`38`	`("testConnectionPoolFullOfParkedConnectionsIsShutdownImmediately", testConnectionPoolFullOfParkedConnectionsIsShutdownImmediately),`
`39`	`39`	`("testParkedConnectionTimesOutButIsAlsoClosedByRemote", testParkedConnectionTimesOutButIsAlsoClosedByRemote),`
`40`	`40`	`("testConnectionBackoffVsShutdownRace", testConnectionBackoffVsShutdownRace),`
	`41`	`+("testRequestThatTimesOutIsFailedWithLastConnectionCreationError", testRequestThatTimesOutIsFailedWithLastConnectionCreationError),`
	`42`	`+("testRequestThatTimesOutBeforeAConnectionIsEstablishedIsFailedWithConnectTimeoutError", testRequestThatTimesOutBeforeAConnectionIsEstablishedIsFailedWithConnectTimeoutError),`
	`43`	`+("testRequestThatTimesOutAfterAConnectionWasEstablishedSuccessfullyTimesOutWithGenericError", testRequestThatTimesOutAfterAConnectionWasEstablishedSuccessfullyTimesOutWithGenericError),`
`41`	`44`	`]`
`42`	`45`	`}`
`43`	`46`	`}`
-Original file line number
+Diff line change
 returnXCTFail("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.
 XCTAssertEqual(state.connectionCreationBackoffDone(connectionID),.none)
+}
++
 +func testRequestThatTimesOutIsFailedWithLastConnectionCreationError(){
 +letelg=EmbeddedEventLoopGroup(loops:1)
 +defer{XCTAssertNoThrow(try elg.syncShutdownGracefully())}
++
 +varstate=HTTPConnectionPool.StateMachine(
 + eventLoopGroup: elg,
 + idGenerator:.init(),
 + maximumConcurrentHTTP1Connections:6
 +)
++
 +letmockRequest=MockHTTPRequest(eventLoop: elg.next(), requiresEventLoopForChannel:false)
 +letrequest=HTTPConnectionPool.Request(mockRequest)
++
 +letexecuteAction= state.executeRequest(request)
 +guard case .createConnection(let connectionID, on:let connEL)= executeAction.connection else{
 +returnXCTFail("Expected to create a connection")
 +}
++
 +XCTAssertEqual(executeAction.request,.scheduleRequestTimeout(for: request, on: mockRequest.eventLoop))
++
 +letfailAction= state.failedToCreateNewConnection(HTTPClientError.httpProxyHandshakeTimeout, connectionID: connectionID)
 +guard case .scheduleBackoffTimer(connectionID, backoff: _, on:let timerEL)= failAction.connection else{
 +returnXCTFail("Expected to create a backoff timer")
 +}
 +XCTAssert(timerEL === connEL)
 +XCTAssertEqual(failAction.request,.none)
++
 +lettimeoutAction= state.timeoutRequest(request.id)
 +XCTAssertEqual(timeoutAction.request,.failRequest(request,HTTPClientError.httpProxyHandshakeTimeout, cancelTimeout:false))
 +XCTAssertEqual(timeoutAction.connection,.none)
 +}
++
 +func testRequestThatTimesOutBeforeAConnectionIsEstablishedIsFailedWithConnectTimeoutError(){
 +leteventLoop=EmbeddedEventLoop()
 +defer{XCTAssertNoThrow(try eventLoop.syncShutdownGracefully())}
++
 +varstate=HTTPConnectionPool.StateMachine(
 + eventLoopGroup: eventLoop,
 + idGenerator:.init(),
 + maximumConcurrentHTTP1Connections:6
 +)
++
 +letmockRequest=MockHTTPRequest(eventLoop: eventLoop.next(), requiresEventLoopForChannel:false)
 +letrequest=HTTPConnectionPool.Request(mockRequest)
++
 +letexecuteAction= state.executeRequest(request)
 +guard case .createConnection(_, on: _)= executeAction.connection else{
 +returnXCTFail("Expected to create a connection")
 +}
 +XCTAssertEqual(executeAction.request,.scheduleRequestTimeout(for: request, on: mockRequest.eventLoop))
++
 +lettimeoutAction= state.timeoutRequest(request.id)
 +XCTAssertEqual(timeoutAction.request,.failRequest(request,HTTPClientError.connectTimeout, cancelTimeout:false))
 +XCTAssertEqual(timeoutAction.connection,.none)
 +}
++
 +func testRequestThatTimesOutAfterAConnectionWasEstablishedSuccessfullyTimesOutWithGenericError(){
 +letelg=EmbeddedEventLoopGroup(loops:1)
 +defer{XCTAssertNoThrow(try elg.syncShutdownGracefully())}
++
 +varstate=HTTPConnectionPool.StateMachine(
 + eventLoopGroup: elg,
 + idGenerator:.init(),
 + maximumConcurrentHTTP1Connections:6
 +)
++
 +letmockRequest1=MockHTTPRequest(eventLoop: elg.next(), requiresEventLoopForChannel:false)
 +letrequest1=HTTPConnectionPool.Request(mockRequest1)
++
 +letexecuteAction1= state.executeRequest(request1)
 +guard case .createConnection(let connectionID1, on:let connEL1)= executeAction1.connection else{
 +returnXCTFail("Expected to create a connection")
 +}
 +XCTAssert(mockRequest1.eventLoop === connEL1)
++
 +XCTAssertEqual(executeAction1.request,.scheduleRequestTimeout(for: request1, on: mockRequest1.eventLoop))
++
 +letmockRequest2=MockHTTPRequest(eventLoop: elg.next(), requiresEventLoopForChannel:false)
 +letrequest2=HTTPConnectionPool.Request(mockRequest2)
++
 +letexecuteAction2= state.executeRequest(request2)
 +guard case .createConnection(let connectionID2, on:let connEL2)= executeAction2.connection else{
 +returnXCTFail("Expected to create a connection")
 +}
 +XCTAssert(mockRequest2.eventLoop === connEL2)
++
 +XCTAssertEqual(executeAction2.request,.scheduleRequestTimeout(for: request2, on: connEL1))
++
 +letfailAction= state.failedToCreateNewConnection(HTTPClientError.httpProxyHandshakeTimeout, connectionID: connectionID1)
 +guard case .scheduleBackoffTimer(connectionID1, backoff: _, on:let timerEL)= failAction.connection else{
 +returnXCTFail("Expected to create a backoff timer")
 +}
 +XCTAssert(timerEL === connEL2)
 +XCTAssertEqual(failAction.request,.none)
++
 +letconn2=HTTPConnectionPool.Connection.__testOnly_connection(id: connectionID2, eventLoop: connEL2)
 +letcreatedAction= state.newHTTP1ConnectionCreated(conn2)
++
 +XCTAssertEqual(createdAction.request,.executeRequest(request1, conn2, cancelTimeout:true))
 +XCTAssertEqual(createdAction.connection,.none)
++
 +lettimeoutAction= state.timeoutRequest(request2.id)
 +XCTAssertEqual(timeoutAction.request,.failRequest(request2,HTTPClientError.getConnectionFromPoolTimeout, cancelTimeout:false))
 +XCTAssertEqual(timeoutAction.connection,.none)
 +}
+}
Original file line number	Diff line number	Diff line change
`@@ -229,7 +229,7 @@ class HTTPConnectionPoolTests: XCTestCase{`
`229`	`229`
`230`	`230`	`pool.executeRequest(requestBag)`
`231`	`231`	`XCTAssertThrowsError(try requestBag.task.futureResult.wait()){`
`232`		`-XCTAssertEqual($0 as?HTTPClientError,.getConnectionFromPoolTimeout)`
	`232`	`+XCTAssertEqual($0 as?HTTPClientError,.proxyAuthenticationRequired)`
`233`	`233`	`}`
`234`	`234`	`XCTAssertGreaterThanOrEqual(httpBin.createdConnections,8)`
`235`	`235`	`XCTAssertEqual(httpBin.activeConnections,0)`