From 701c1d489a7e7dfc4531b731671f893d0869e53a Mon Sep 17 00:00:00 2001
From: Fabian Fett <fabianfett@apple.com>
Date: Fri, 10 Sep 2021 14:46:12 +0200
Subject: [PATCH] Preserve connection errors for user
---
.../HTTPConnectionPool+HTTP1Connections.swift | 5 +
...HTTPConnectionPool+HTTP1StateMachine.swift | 12 +-
...onnectionPool+HTTP1StateTests+XCTest.swift | 3 +
.../HTTPConnectionPool+HTTP1StateTests.swift | 108 +++++++++++++++++-
.../HTTPConnectionPoolTests.swift | 2 +-
5 files changed, 127 insertions(+), 3 deletions(-)
diff --git a/Sources/AsyncHTTPClient/ConnectionPool/State Machine/HTTPConnectionPool+HTTP1Connections.swift b/Sources/AsyncHTTPClient/ConnectionPool/State Machine/HTTPConnectionPool+HTTP1Connections.swift
index 13f8149f7..c38479778 100644
--- 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
index 441457253..3695b3c90 100644
--- 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
index e892f957f..16377d07f 100644
--- 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
index 28059063a..864e895e7 100644
--- 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
index 019e1d414..943b1e489 100644
--- 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)