diff --git a/Sources/AsyncHTTPClient/ConnectionPool.swift b/Sources/AsyncHTTPClient/ConnectionPool.swift index ce648469c..5e44da8a9 100644 --- a/Sources/AsyncHTTPClient/ConnectionPool.swift +++ b/Sources/AsyncHTTPClient/ConnectionPool.swift @@ -513,13 +513,13 @@ class HTTP1ConnectionProvider { } private func makeChannel(preference: HTTPClient.EventLoopPreference) -> EventLoopFuture { - let eventLoop = preference.bestEventLoop ?? self.eventLoop + let channelEventLoop = preference.bestEventLoop ?? self.eventLoop let requiresTLS = self.key.scheme.requiresTLS let bootstrap: NIOClientTCPBootstrap do { - bootstrap = try NIOClientTCPBootstrap.makeHTTPClientBootstrapBase(on: eventLoop, host: self.key.host, port: self.key.port, requiresTLS: requiresTLS, configuration: self.configuration) + bootstrap = try NIOClientTCPBootstrap.makeHTTPClientBootstrapBase(on: channelEventLoop, host: self.key.host, port: self.key.port, requiresTLS: requiresTLS, configuration: self.configuration) } catch { - return eventLoop.makeFailedFuture(error) + return channelEventLoop.makeFailedFuture(error) } let channel: EventLoopFuture @@ -564,7 +564,7 @@ class HTTP1ConnectionProvider { error = HTTPClient.NWErrorHandler.translateError(error) } #endif - return self.eventLoop.makeFailedFuture(error) + return channelEventLoop.makeFailedFuture(error) } } diff --git a/Sources/AsyncHTTPClient/HTTPClient.swift b/Sources/AsyncHTTPClient/HTTPClient.swift index 77647161e..ca2343045 100644 --- a/Sources/AsyncHTTPClient/HTTPClient.swift +++ b/Sources/AsyncHTTPClient/HTTPClient.swift @@ -545,6 +545,14 @@ public class HTTPClient { deadline: deadline, setupComplete: setupComplete.futureResult, logger: logger) + + let taskHandler = TaskHandler(task: task, + kind: request.kind, + delegate: delegate, + redirectHandler: redirectHandler, + ignoreUncleanSSLShutdown: self.configuration.ignoreUncleanSSLShutdown, + logger: logger) + connection.flatMap { connection -> EventLoopFuture in logger.debug("got connection for request", metadata: ["ahc-connection": "\(connection)", @@ -561,12 +569,6 @@ public class HTTPClient { } return future.flatMap { - let taskHandler = TaskHandler(task: task, - kind: request.kind, - delegate: delegate, - redirectHandler: redirectHandler, - ignoreUncleanSSLShutdown: self.configuration.ignoreUncleanSSLShutdown, - logger: logger) return channel.pipeline.addHandler(taskHandler) }.flatMap { task.setConnection(connection) @@ -590,7 +592,12 @@ public class HTTPClient { } }.always { _ in setupComplete.succeed(()) - }.cascadeFailure(to: task.promise) + }.whenFailure { error in + taskHandler.callOutToDelegateFireAndForget { task in + delegate.didReceiveError(task: task, error) + } + task.promise.fail(error) + } return task } diff --git a/Tests/AsyncHTTPClientTests/HTTPClientInternalTests+XCTest.swift b/Tests/AsyncHTTPClientTests/HTTPClientInternalTests+XCTest.swift index 8a89eb740..14c830e51 100644 --- a/Tests/AsyncHTTPClientTests/HTTPClientInternalTests+XCTest.swift +++ b/Tests/AsyncHTTPClientTests/HTTPClientInternalTests+XCTest.swift @@ -43,6 +43,7 @@ extension HTTPClientInternalTests { ("testUploadStreamingIsCalledOnTaskEL", testUploadStreamingIsCalledOnTaskEL), ("testWeCanActuallyExactlySetTheEventLoops", testWeCanActuallyExactlySetTheEventLoops), ("testTaskPromiseBoundToEL", testTaskPromiseBoundToEL), + ("testConnectErrorCalloutOnCorrectEL", testConnectErrorCalloutOnCorrectEL), ("testInternalRequestURI", testInternalRequestURI), ] } diff --git a/Tests/AsyncHTTPClientTests/HTTPClientInternalTests.swift b/Tests/AsyncHTTPClientTests/HTTPClientInternalTests.swift index 39d0c6f3a..34891c632 100644 --- a/Tests/AsyncHTTPClientTests/HTTPClientInternalTests.swift +++ b/Tests/AsyncHTTPClientTests/HTTPClientInternalTests.swift @@ -960,6 +960,45 @@ class HTTPClientInternalTests: XCTestCase { XCTAssertNoThrow(try task.wait()) } + func testConnectErrorCalloutOnCorrectEL() throws { + class TestDelegate: HTTPClientResponseDelegate { + typealias Response = Void + + let expectedEL: EventLoop + var receivedError: Bool = false + + init(expectedEL: EventLoop) { + self.expectedEL = expectedEL + } + + func didFinishRequest(task: HTTPClient.Task) throws {} + + func didReceiveError(task: HTTPClient.Task, _ error: Error) { + self.receivedError = true + XCTAssertTrue(self.expectedEL.inEventLoop) + } + } + + let elg = getDefaultEventLoopGroup(numberOfThreads: 2) + let el1 = elg.next() + let el2 = elg.next() + + let httpBin = HTTPBin(refusesConnections: true) + let client = HTTPClient(eventLoopGroupProvider: .shared(elg)) + + defer { + XCTAssertNoThrow(try client.syncShutdown()) + XCTAssertNoThrow(try elg.syncShutdownGracefully()) + } + + let request = try HTTPClient.Request(url: "http://localhost:\(httpBin.port)/get") + let delegate = TestDelegate(expectedEL: el1) + XCTAssertNoThrow(try httpBin.shutdown()) + let task = client.execute(request: request, delegate: delegate, eventLoop: .init(.testOnly_exact(channelOn: el2, delegateOn: el1))) + XCTAssertThrowsError(try task.wait()) + XCTAssertTrue(delegate.receivedError) + } + func testInternalRequestURI() throws { let request1 = try Request(url: "https://someserver.com:8888/some/path?foo=bar") XCTAssertEqual(request1.kind, .host) diff --git a/Tests/AsyncHTTPClientTests/HTTPClientTests+XCTest.swift b/Tests/AsyncHTTPClientTests/HTTPClientTests+XCTest.swift index dc82aba4b..0de4d0c07 100644 --- a/Tests/AsyncHTTPClientTests/HTTPClientTests+XCTest.swift +++ b/Tests/AsyncHTTPClientTests/HTTPClientTests+XCTest.swift @@ -114,6 +114,7 @@ extension HTTPClientTests { ("testNothingIsLoggedAtInfoOrHigher", testNothingIsLoggedAtInfoOrHigher), ("testAllMethodsLog", testAllMethodsLog), ("testClosingIdleConnectionsInPoolLogsInTheBackground", testClosingIdleConnectionsInPoolLogsInTheBackground), + ("testConnectErrorPropagatedToDelegate", testConnectErrorPropagatedToDelegate), ("testDelegateCallinsTolerateRandomEL", testDelegateCallinsTolerateRandomEL), ("testContentLengthTooLongFails", testContentLengthTooLongFails), ("testContentLengthTooShortFails", testContentLengthTooShortFails), diff --git a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift index 2de420716..eaf4c6564 100644 --- a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift +++ b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift @@ -2374,6 +2374,33 @@ class HTTPClientTests: XCTestCase { self.defaultClient = nil // so it doesn't get shut down again. } + func testConnectErrorPropagatedToDelegate() throws { + class TestDelegate: HTTPClientResponseDelegate { + typealias Response = Void + var error: Error? + func didFinishRequest(task: HTTPClient.Task) throws {} + func didReceiveError(task: HTTPClient.Task, _ error: Error) { + self.error = error + } + } + + let httpClient = HTTPClient(eventLoopGroupProvider: .shared(self.clientGroup), + configuration: .init(timeout: .init(connect: .milliseconds(10)))) + + defer { + XCTAssertNoThrow(try httpClient.syncShutdown()) + } + + // This must throw as 198.51.100.254 is reserved for documentation only + let request = try HTTPClient.Request(url: "http://198.51.100.254:65535/get") + let delegate = TestDelegate() + + XCTAssertThrowsError(try httpClient.execute(request: request, delegate: delegate).wait()) { error in + XCTAssertEqual(.connectTimeout(.milliseconds(10)), error as? ChannelError) + XCTAssertEqual(.connectTimeout(.milliseconds(10)), delegate.error as? ChannelError) + } + } + func testDelegateCallinsTolerateRandomEL() throws { class TestDelegate: HTTPClientResponseDelegate { typealias Response = Void