From 9e26b5ecfbb0459c5164bdfdbf9b984fced00926 Mon Sep 17 00:00:00 2001 From: cnkwocha Date: Tue, 21 Jan 2025 09:22:50 +0000 Subject: [PATCH 1/8] Add "debug initializer" hook for channels Motivation: As requested in #596, it can be handy to have a lower-level access to channels (HTTP/1 connection, HTTP/2 connection, or HTTP/2 stream) to enable a more fine-grained interaction for, say, observability, testing, etc. Modifications: - Add 3 new properties (`http1_1ConnectionDebugInitializer`, `http2ConnectionDebugInitializer` and `http2StreamChannelDebugInitializer`) to `HTTPClient.Configuration` with access to the respective channels. These properties are of `Optional` type `@Sendable (Channel) -> EventLoopFuture` and are called when creating a connection/stream. Result: Provides APIs for a lower-level access to channels. --- .../HTTP2/HTTP2Connection.swift | 30 +++++-- .../HTTPConnectionPool+Factory.swift | 40 ++++++++- .../ConnectionPool/HTTPConnectionPool.swift | 24 +++++- Sources/AsyncHTTPClient/HTTPClient.swift | 75 ++++++++++++++-- .../HTTPClientTests.swift | 85 +++++++++++++++++++ 5 files changed, 233 insertions(+), 21 deletions(-) diff --git a/Sources/AsyncHTTPClient/ConnectionPool/HTTP2/HTTP2Connection.swift b/Sources/AsyncHTTPClient/ConnectionPool/HTTP2/HTTP2Connection.swift index 5e4ae6e01..d6047901b 100644 --- a/Sources/AsyncHTTPClient/ConnectionPool/HTTP2/HTTP2Connection.swift +++ b/Sources/AsyncHTTPClient/ConnectionPool/HTTP2/HTTP2Connection.swift @@ -141,12 +141,21 @@ final class HTTP2Connection { return connection._start0().map { maxStreams in (connection, maxStreams) } } - func executeRequest(_ request: HTTPExecutableRequest) { + func executeRequest( + _ request: HTTPExecutableRequest, + streamChannelDebugInitializer: (@Sendable (Channel) -> EventLoopFuture)? = nil + ) { if self.channel.eventLoop.inEventLoop { - self.executeRequest0(request) + self.executeRequest0( + request, + streamChannelDebugInitializer: streamChannelDebugInitializer + ) } else { self.channel.eventLoop.execute { - self.executeRequest0(request) + self.executeRequest0( + request, + streamChannelDebugInitializer: streamChannelDebugInitializer + ) } } } @@ -218,7 +227,10 @@ final class HTTP2Connection { return readyToAcceptConnectionsPromise.futureResult } - private func executeRequest0(_ request: HTTPExecutableRequest) { + private func executeRequest0( + _ request: HTTPExecutableRequest, + streamChannelDebugInitializer: (@Sendable (Channel) -> EventLoopFuture)? + ) { self.channel.eventLoop.assertInEventLoop() switch self.state { @@ -259,8 +271,14 @@ final class HTTP2Connection { self.openStreams.remove(box) } - channel.write(request, promise: nil) - return channel.eventLoop.makeSucceededVoidFuture() + if let streamChannelDebugInitializer { + return streamChannelDebugInitializer(channel).map { _ in + channel.write(request, promise: nil) + } + } else { + channel.write(request, promise: nil) + return channel.eventLoop.makeSucceededVoidFuture() + } } catch { return channel.eventLoop.makeFailedFuture(error) } diff --git a/Sources/AsyncHTTPClient/ConnectionPool/HTTPConnectionPool+Factory.swift b/Sources/AsyncHTTPClient/ConnectionPool/HTTPConnectionPool+Factory.swift index 32af23830..db09e6ef7 100644 --- a/Sources/AsyncHTTPClient/ConnectionPool/HTTPConnectionPool+Factory.swift +++ b/Sources/AsyncHTTPClient/ConnectionPool/HTTPConnectionPool+Factory.swift @@ -84,7 +84,21 @@ extension HTTPConnectionPool.ConnectionFactory { decompression: self.clientConfiguration.decompression, logger: logger ) - requester.http1ConnectionCreated(connection) + + if let debugInitializer + = self.clientConfiguration.http1_1ConnectionDebugInitializer + { + debugInitializer(channel).whenComplete { debugInitializerResult in + switch debugInitializerResult { + case .success: + requester.http1ConnectionCreated(connection) + case .failure(let error): + requester.failedToCreateHTTPConnection(connectionID, error: error) + } + } + } else { + requester.http1ConnectionCreated(connection) + } } catch { requester.failedToCreateHTTPConnection(connectionID, error: error) } @@ -99,7 +113,29 @@ extension HTTPConnectionPool.ConnectionFactory { ).whenComplete { result in switch result { case .success((let connection, let maximumStreams)): - requester.http2ConnectionCreated(connection, maximumStreams: maximumStreams) + if let debugInitializer + = self.clientConfiguration.http2ConnectionDebugInitializer + { + debugInitializer(channel).whenComplete { debugInitializerResult in + switch debugInitializerResult { + case .success: + requester.http2ConnectionCreated( + connection, + maximumStreams: maximumStreams + ) + case .failure(let error): + requester.failedToCreateHTTPConnection( + connectionID, + error: error + ) + } + } + } else { + requester.http2ConnectionCreated( + connection, + maximumStreams: maximumStreams + ) + } case .failure(let error): requester.failedToCreateHTTPConnection(connectionID, error: error) } diff --git a/Sources/AsyncHTTPClient/ConnectionPool/HTTPConnectionPool.swift b/Sources/AsyncHTTPClient/ConnectionPool/HTTPConnectionPool.swift index eebe4d029..46b766781 100644 --- a/Sources/AsyncHTTPClient/ConnectionPool/HTTPConnectionPool.swift +++ b/Sources/AsyncHTTPClient/ConnectionPool/HTTPConnectionPool.swift @@ -321,10 +321,20 @@ final class HTTPConnectionPool: private func runUnlockedRequestAction(_ action: Actions.RequestAction.Unlocked) { switch action { case .executeRequest(let request, let connection): - connection.executeRequest(request.req) + connection.executeRequest( + request.req, + http2StreamChannelDebugInitializer: + self.clientConfiguration.http2StreamChannelDebugInitializer + ) case .executeRequests(let requests, let connection): - for request in requests { connection.executeRequest(request.req) } + for request in requests { + connection.executeRequest( + request.req, + http2StreamChannelDebugInitializer: + self.clientConfiguration.http2StreamChannelDebugInitializer + ) + } case .failRequest(let request, let error): request.req.fail(error) @@ -651,12 +661,18 @@ extension HTTPConnectionPool { } } - fileprivate func executeRequest(_ request: HTTPExecutableRequest) { + fileprivate func executeRequest( + _ request: HTTPExecutableRequest, + http2StreamChannelDebugInitializer: (@Sendable (Channel) -> EventLoopFuture)? + ) { switch self._ref { case .http1_1(let connection): return connection.executeRequest(request) case .http2(let connection): - return connection.executeRequest(request) + return connection.executeRequest( + request, + streamChannelDebugInitializer: http2StreamChannelDebugInitializer + ) case .__testOnly_connection: break } diff --git a/Sources/AsyncHTTPClient/HTTPClient.swift b/Sources/AsyncHTTPClient/HTTPClient.swift index f1655c7c5..009455420 100644 --- a/Sources/AsyncHTTPClient/HTTPClient.swift +++ b/Sources/AsyncHTTPClient/HTTPClient.swift @@ -847,6 +847,18 @@ public class HTTPClient { /// By default, don't use it public var enableMultipath: Bool + /// A method with access to the HTTP/1 connection channel that is called when creating the connection. + public var http1_1ConnectionDebugInitializer: + (@Sendable (Channel) -> EventLoopFuture)? + + /// A method with access to the HTTP/2 connection channel that is called when creating the connection. + public var http2ConnectionDebugInitializer: + (@Sendable (Channel) -> EventLoopFuture)? + + /// A method with access to the HTTP/2 stream channel that is called when creating the stream. + public var http2StreamChannelDebugInitializer: + (@Sendable (Channel) -> EventLoopFuture)? + public init( tlsConfiguration: TLSConfiguration? = nil, redirectConfiguration: RedirectConfiguration? = nil, @@ -854,7 +866,13 @@ public class HTTPClient { connectionPool: ConnectionPool = ConnectionPool(), proxy: Proxy? = nil, ignoreUncleanSSLShutdown: Bool = false, - decompression: Decompression = .disabled + decompression: Decompression = .disabled, + http1_1ConnectionDebugInitializer: + (@Sendable (Channel) -> EventLoopFuture)? = nil, + http2ConnectionDebugInitializer: + (@Sendable (Channel) -> EventLoopFuture)? = nil, + http2StreamChannelDebugInitializer: + (@Sendable (Channel) -> EventLoopFuture)? = nil ) { self.tlsConfiguration = tlsConfiguration self.redirectConfiguration = redirectConfiguration ?? RedirectConfiguration() @@ -865,6 +883,9 @@ public class HTTPClient { self.httpVersion = .automatic self.networkFrameworkWaitForConnectivity = true self.enableMultipath = false + self.http1_1ConnectionDebugInitializer = http1_1ConnectionDebugInitializer + self.http2ConnectionDebugInitializer = http2ConnectionDebugInitializer + self.http2StreamChannelDebugInitializer = http2StreamChannelDebugInitializer } public init( @@ -873,7 +894,13 @@ public class HTTPClient { timeout: Timeout = Timeout(), proxy: Proxy? = nil, ignoreUncleanSSLShutdown: Bool = false, - decompression: Decompression = .disabled + decompression: Decompression = .disabled, + http1_1ConnectionDebugInitializer: + (@Sendable (Channel) -> EventLoopFuture)? = nil, + http2ConnectionDebugInitializer: + (@Sendable (Channel) -> EventLoopFuture)? = nil, + http2StreamChannelDebugInitializer: + (@Sendable (Channel) -> EventLoopFuture)? = nil ) { self.init( tlsConfiguration: tlsConfiguration, @@ -882,7 +909,10 @@ public class HTTPClient { connectionPool: ConnectionPool(), proxy: proxy, ignoreUncleanSSLShutdown: ignoreUncleanSSLShutdown, - decompression: decompression + decompression: decompression, + http1_1ConnectionDebugInitializer: http1_1ConnectionDebugInitializer, + http2ConnectionDebugInitializer: http2ConnectionDebugInitializer, + http2StreamChannelDebugInitializer: http2StreamChannelDebugInitializer ) } @@ -893,7 +923,13 @@ public class HTTPClient { maximumAllowedIdleTimeInConnectionPool: TimeAmount = .seconds(60), proxy: Proxy? = nil, ignoreUncleanSSLShutdown: Bool = false, - decompression: Decompression = .disabled + decompression: Decompression = .disabled, + http1_1ConnectionDebugInitializer: + (@Sendable (Channel) -> EventLoopFuture)? = nil, + http2ConnectionDebugInitializer: + (@Sendable (Channel) -> EventLoopFuture)? = nil, + http2StreamChannelDebugInitializer: + (@Sendable (Channel) -> EventLoopFuture)? = nil ) { var tlsConfig = TLSConfiguration.makeClientConfiguration() tlsConfig.certificateVerification = certificateVerification @@ -904,7 +940,10 @@ public class HTTPClient { connectionPool: ConnectionPool(idleTimeout: maximumAllowedIdleTimeInConnectionPool), proxy: proxy, ignoreUncleanSSLShutdown: ignoreUncleanSSLShutdown, - decompression: decompression + decompression: decompression, + http1_1ConnectionDebugInitializer: http1_1ConnectionDebugInitializer, + http2ConnectionDebugInitializer: http2ConnectionDebugInitializer, + http2StreamChannelDebugInitializer: http2StreamChannelDebugInitializer ) } @@ -916,7 +955,13 @@ public class HTTPClient { proxy: Proxy? = nil, ignoreUncleanSSLShutdown: Bool = false, decompression: Decompression = .disabled, - backgroundActivityLogger: Logger? + backgroundActivityLogger: Logger?, + http1_1ConnectionDebugInitializer: + (@Sendable (Channel) -> EventLoopFuture)? = nil, + http2ConnectionDebugInitializer: + (@Sendable (Channel) -> EventLoopFuture)? = nil, + http2StreamChannelDebugInitializer: + (@Sendable (Channel) -> EventLoopFuture)? = nil ) { var tlsConfig = TLSConfiguration.makeClientConfiguration() tlsConfig.certificateVerification = certificateVerification @@ -927,7 +972,10 @@ public class HTTPClient { connectionPool: ConnectionPool(idleTimeout: connectionPool), proxy: proxy, ignoreUncleanSSLShutdown: ignoreUncleanSSLShutdown, - decompression: decompression + decompression: decompression, + http1_1ConnectionDebugInitializer: http1_1ConnectionDebugInitializer, + http2ConnectionDebugInitializer: http2ConnectionDebugInitializer, + http2StreamChannelDebugInitializer: http2StreamChannelDebugInitializer ) } @@ -937,7 +985,13 @@ public class HTTPClient { timeout: Timeout = Timeout(), proxy: Proxy? = nil, ignoreUncleanSSLShutdown: Bool = false, - decompression: Decompression = .disabled + decompression: Decompression = .disabled, + http1_1ConnectionDebugInitializer: + (@Sendable (Channel) -> EventLoopFuture)? = nil, + http2ConnectionDebugInitializer: + (@Sendable (Channel) -> EventLoopFuture)? = nil, + http2StreamChannelDebugInitializer: + (@Sendable (Channel) -> EventLoopFuture)? = nil ) { self.init( certificateVerification: certificateVerification, @@ -946,7 +1000,10 @@ public class HTTPClient { maximumAllowedIdleTimeInConnectionPool: .seconds(60), proxy: proxy, ignoreUncleanSSLShutdown: ignoreUncleanSSLShutdown, - decompression: decompression + decompression: decompression, + http1_1ConnectionDebugInitializer: http1_1ConnectionDebugInitializer, + http2ConnectionDebugInitializer: http2ConnectionDebugInitializer, + http2StreamChannelDebugInitializer: http2StreamChannelDebugInitializer ) } } diff --git a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift index 546d1c3f4..1282a2687 100644 --- a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift +++ b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift @@ -4306,4 +4306,89 @@ final class HTTPClientTests: XCTestCaseHTTPClientTestsBaseClass { request.setBasicAuth(username: "foo", password: "bar") XCTAssertEqual(request.headers.first(name: "Authorization"), "Basic Zm9vOmJhcg==") } + + func testHTTP1ConnectionDebugInitializer() { + let connectionDebugInitializerUtil = DebugInitializerUtil() + + var config = HTTPClient.Configuration() + config.tlsConfiguration = .clientDefault + config.tlsConfiguration?.certificateVerification = .none + config.httpVersion = .http1Only + config.http1_1ConnectionDebugInitializer = connectionDebugInitializerUtil.operation + + let client = HTTPClient( + eventLoopGroupProvider: .singleton, + configuration: config, + backgroundActivityLogger: Logger( + label: "HTTPClient", + factory: StreamLogHandler.standardOutput(label:) + ) + ) + defer { XCTAssertNoThrow(client.shutdown()) } + + let bin = HTTPBin(.http1_1(ssl: true, compress: false)) + defer { XCTAssertNoThrow(try bin.shutdown()) } + + for _ in 0..<3 { + XCTAssertNoThrow(try client.get(url: "https://localhost:\(bin.port)/get").wait()) + } + + // Even though multiple requests were made, the connection debug initializer must be called + // only once. + XCTAssertEqual(connectionDebugInitializerUtil.executionCount, 1) + } + + func testHTTP2ConnectionAndStreamChannelDebugInitializers() { + let connectionDebugInitializerUtil = DebugInitializerUtil() + let streamChannelDebugInitializerUtil = DebugInitializerUtil() + + var config = HTTPClient.Configuration() + config.tlsConfiguration = .clientDefault + config.tlsConfiguration?.certificateVerification = .none + config.httpVersion = .automatic + config.http2ConnectionDebugInitializer = connectionDebugInitializerUtil.operation + config.http2StreamChannelDebugInitializer = streamChannelDebugInitializerUtil.operation + + let client = HTTPClient( + eventLoopGroupProvider: .singleton, + configuration: config, + backgroundActivityLogger: Logger( + label: "HTTPClient", + factory: StreamLogHandler.standardOutput(label:) + ) + ) + defer { XCTAssertNoThrow(client.shutdown()) } + + let bin = HTTPBin(.http2(compress: false)) + defer { XCTAssertNoThrow(try bin.shutdown()) } + + let numberOfRequests = 3 + + for _ in 0.. EventLoopFuture { + self.executionCount += 1 + + return channel.eventLoop.makeSucceededVoidFuture() + } + + init() { + self.executionCount = 0 + } } From f51ae1c9343dd5e005b9f65df29218f29dc82a75 Mon Sep 17 00:00:00 2001 From: cnkwocha Date: Tue, 21 Jan 2025 17:14:39 +0000 Subject: [PATCH 2/8] Implement feedback --- .../HTTP2/HTTP2Connection.swift | 35 ++++---- .../HTTPConnectionPool+Factory.swift | 13 +-- .../ConnectionPool/HTTPConnectionPool.swift | 22 +---- Sources/AsyncHTTPClient/HTTPClient.swift | 82 ++++++++----------- .../HTTPClientTests.swift | 55 +++++++++---- 5 files changed, 97 insertions(+), 110 deletions(-) diff --git a/Sources/AsyncHTTPClient/ConnectionPool/HTTP2/HTTP2Connection.swift b/Sources/AsyncHTTPClient/ConnectionPool/HTTP2/HTTP2Connection.swift index d6047901b..77e4835f6 100644 --- a/Sources/AsyncHTTPClient/ConnectionPool/HTTP2/HTTP2Connection.swift +++ b/Sources/AsyncHTTPClient/ConnectionPool/HTTP2/HTTP2Connection.swift @@ -35,6 +35,9 @@ final class HTTP2Connection { let multiplexer: HTTP2StreamMultiplexer let logger: Logger + /// A method with access to the stream channel that is called when creating the stream. + let streamChannelDebugInitializer: (@Sendable (Channel) -> EventLoopFuture)? + /// the connection pool that created the connection let delegate: HTTP2ConnectionDelegate @@ -95,7 +98,8 @@ final class HTTP2Connection { decompression: HTTPClient.Decompression, maximumConnectionUses: Int?, delegate: HTTP2ConnectionDelegate, - logger: Logger + logger: Logger, + streamChannelDebugInitializer: (@Sendable (Channel) -> EventLoopFuture)? = nil ) { self.channel = channel self.id = connectionID @@ -114,6 +118,7 @@ final class HTTP2Connection { ) self.delegate = delegate self.state = .initialized + self.streamChannelDebugInitializer = streamChannelDebugInitializer } deinit { @@ -128,7 +133,8 @@ final class HTTP2Connection { delegate: HTTP2ConnectionDelegate, decompression: HTTPClient.Decompression, maximumConnectionUses: Int?, - logger: Logger + logger: Logger, + streamChannelDebugInitializer: (@Sendable (Channel) -> EventLoopFuture)? = nil ) -> EventLoopFuture<(HTTP2Connection, Int)> { let connection = HTTP2Connection( channel: channel, @@ -136,26 +142,18 @@ final class HTTP2Connection { decompression: decompression, maximumConnectionUses: maximumConnectionUses, delegate: delegate, - logger: logger + logger: logger, + streamChannelDebugInitializer: streamChannelDebugInitializer ) return connection._start0().map { maxStreams in (connection, maxStreams) } } - func executeRequest( - _ request: HTTPExecutableRequest, - streamChannelDebugInitializer: (@Sendable (Channel) -> EventLoopFuture)? = nil - ) { + func executeRequest(_ request: HTTPExecutableRequest) { if self.channel.eventLoop.inEventLoop { - self.executeRequest0( - request, - streamChannelDebugInitializer: streamChannelDebugInitializer - ) + self.executeRequest0(request) } else { self.channel.eventLoop.execute { - self.executeRequest0( - request, - streamChannelDebugInitializer: streamChannelDebugInitializer - ) + self.executeRequest0(request) } } } @@ -227,10 +225,7 @@ final class HTTP2Connection { return readyToAcceptConnectionsPromise.futureResult } - private func executeRequest0( - _ request: HTTPExecutableRequest, - streamChannelDebugInitializer: (@Sendable (Channel) -> EventLoopFuture)? - ) { + private func executeRequest0(_ request: HTTPExecutableRequest) { self.channel.eventLoop.assertInEventLoop() switch self.state { @@ -271,7 +266,7 @@ final class HTTP2Connection { self.openStreams.remove(box) } - if let streamChannelDebugInitializer { + if let streamChannelDebugInitializer = self.streamChannelDebugInitializer { return streamChannelDebugInitializer(channel).map { _ in channel.write(request, promise: nil) } diff --git a/Sources/AsyncHTTPClient/ConnectionPool/HTTPConnectionPool+Factory.swift b/Sources/AsyncHTTPClient/ConnectionPool/HTTPConnectionPool+Factory.swift index db09e6ef7..04fb87bdc 100644 --- a/Sources/AsyncHTTPClient/ConnectionPool/HTTPConnectionPool+Factory.swift +++ b/Sources/AsyncHTTPClient/ConnectionPool/HTTPConnectionPool+Factory.swift @@ -85,10 +85,10 @@ extension HTTPConnectionPool.ConnectionFactory { logger: logger ) - if let debugInitializer + if let connectionDebugInitializer = self.clientConfiguration.http1_1ConnectionDebugInitializer { - debugInitializer(channel).whenComplete { debugInitializerResult in + connectionDebugInitializer(channel).whenComplete { debugInitializerResult in switch debugInitializerResult { case .success: requester.http1ConnectionCreated(connection) @@ -109,14 +109,17 @@ extension HTTPConnectionPool.ConnectionFactory { delegate: http2ConnectionDelegate, decompression: self.clientConfiguration.decompression, maximumConnectionUses: self.clientConfiguration.maximumUsesPerConnection, - logger: logger + logger: logger, + streamChannelDebugInitializer: + self.clientConfiguration.http2StreamChannelDebugInitializer ).whenComplete { result in switch result { case .success((let connection, let maximumStreams)): - if let debugInitializer + if let connectionDebugInitializer = self.clientConfiguration.http2ConnectionDebugInitializer { - debugInitializer(channel).whenComplete { debugInitializerResult in + connectionDebugInitializer(channel).whenComplete { + debugInitializerResult in switch debugInitializerResult { case .success: requester.http2ConnectionCreated( diff --git a/Sources/AsyncHTTPClient/ConnectionPool/HTTPConnectionPool.swift b/Sources/AsyncHTTPClient/ConnectionPool/HTTPConnectionPool.swift index 46b766781..ebcecbdc5 100644 --- a/Sources/AsyncHTTPClient/ConnectionPool/HTTPConnectionPool.swift +++ b/Sources/AsyncHTTPClient/ConnectionPool/HTTPConnectionPool.swift @@ -321,19 +321,11 @@ final class HTTPConnectionPool: private func runUnlockedRequestAction(_ action: Actions.RequestAction.Unlocked) { switch action { case .executeRequest(let request, let connection): - connection.executeRequest( - request.req, - http2StreamChannelDebugInitializer: - self.clientConfiguration.http2StreamChannelDebugInitializer - ) + connection.executeRequest(request.req) case .executeRequests(let requests, let connection): for request in requests { - connection.executeRequest( - request.req, - http2StreamChannelDebugInitializer: - self.clientConfiguration.http2StreamChannelDebugInitializer - ) + connection.executeRequest(request.req) } case .failRequest(let request, let error): @@ -661,18 +653,12 @@ extension HTTPConnectionPool { } } - fileprivate func executeRequest( - _ request: HTTPExecutableRequest, - http2StreamChannelDebugInitializer: (@Sendable (Channel) -> EventLoopFuture)? - ) { + fileprivate func executeRequest(_ request: HTTPExecutableRequest) { switch self._ref { case .http1_1(let connection): return connection.executeRequest(request) case .http2(let connection): - return connection.executeRequest( - request, - streamChannelDebugInitializer: http2StreamChannelDebugInitializer - ) + return connection.executeRequest(request) case .__testOnly_connection: break } diff --git a/Sources/AsyncHTTPClient/HTTPClient.swift b/Sources/AsyncHTTPClient/HTTPClient.swift index 009455420..aa84d1c83 100644 --- a/Sources/AsyncHTTPClient/HTTPClient.swift +++ b/Sources/AsyncHTTPClient/HTTPClient.swift @@ -866,13 +866,7 @@ public class HTTPClient { connectionPool: ConnectionPool = ConnectionPool(), proxy: Proxy? = nil, ignoreUncleanSSLShutdown: Bool = false, - decompression: Decompression = .disabled, - http1_1ConnectionDebugInitializer: - (@Sendable (Channel) -> EventLoopFuture)? = nil, - http2ConnectionDebugInitializer: - (@Sendable (Channel) -> EventLoopFuture)? = nil, - http2StreamChannelDebugInitializer: - (@Sendable (Channel) -> EventLoopFuture)? = nil + decompression: Decompression = .disabled ) { self.tlsConfiguration = tlsConfiguration self.redirectConfiguration = redirectConfiguration ?? RedirectConfiguration() @@ -883,9 +877,6 @@ public class HTTPClient { self.httpVersion = .automatic self.networkFrameworkWaitForConnectivity = true self.enableMultipath = false - self.http1_1ConnectionDebugInitializer = http1_1ConnectionDebugInitializer - self.http2ConnectionDebugInitializer = http2ConnectionDebugInitializer - self.http2StreamChannelDebugInitializer = http2StreamChannelDebugInitializer } public init( @@ -894,13 +885,7 @@ public class HTTPClient { timeout: Timeout = Timeout(), proxy: Proxy? = nil, ignoreUncleanSSLShutdown: Bool = false, - decompression: Decompression = .disabled, - http1_1ConnectionDebugInitializer: - (@Sendable (Channel) -> EventLoopFuture)? = nil, - http2ConnectionDebugInitializer: - (@Sendable (Channel) -> EventLoopFuture)? = nil, - http2StreamChannelDebugInitializer: - (@Sendable (Channel) -> EventLoopFuture)? = nil + decompression: Decompression = .disabled ) { self.init( tlsConfiguration: tlsConfiguration, @@ -909,10 +894,7 @@ public class HTTPClient { connectionPool: ConnectionPool(), proxy: proxy, ignoreUncleanSSLShutdown: ignoreUncleanSSLShutdown, - decompression: decompression, - http1_1ConnectionDebugInitializer: http1_1ConnectionDebugInitializer, - http2ConnectionDebugInitializer: http2ConnectionDebugInitializer, - http2StreamChannelDebugInitializer: http2StreamChannelDebugInitializer + decompression: decompression ) } @@ -923,13 +905,7 @@ public class HTTPClient { maximumAllowedIdleTimeInConnectionPool: TimeAmount = .seconds(60), proxy: Proxy? = nil, ignoreUncleanSSLShutdown: Bool = false, - decompression: Decompression = .disabled, - http1_1ConnectionDebugInitializer: - (@Sendable (Channel) -> EventLoopFuture)? = nil, - http2ConnectionDebugInitializer: - (@Sendable (Channel) -> EventLoopFuture)? = nil, - http2StreamChannelDebugInitializer: - (@Sendable (Channel) -> EventLoopFuture)? = nil + decompression: Decompression = .disabled ) { var tlsConfig = TLSConfiguration.makeClientConfiguration() tlsConfig.certificateVerification = certificateVerification @@ -940,10 +916,7 @@ public class HTTPClient { connectionPool: ConnectionPool(idleTimeout: maximumAllowedIdleTimeInConnectionPool), proxy: proxy, ignoreUncleanSSLShutdown: ignoreUncleanSSLShutdown, - decompression: decompression, - http1_1ConnectionDebugInitializer: http1_1ConnectionDebugInitializer, - http2ConnectionDebugInitializer: http2ConnectionDebugInitializer, - http2StreamChannelDebugInitializer: http2StreamChannelDebugInitializer + decompression: decompression ) } @@ -955,13 +928,7 @@ public class HTTPClient { proxy: Proxy? = nil, ignoreUncleanSSLShutdown: Bool = false, decompression: Decompression = .disabled, - backgroundActivityLogger: Logger?, - http1_1ConnectionDebugInitializer: - (@Sendable (Channel) -> EventLoopFuture)? = nil, - http2ConnectionDebugInitializer: - (@Sendable (Channel) -> EventLoopFuture)? = nil, - http2StreamChannelDebugInitializer: - (@Sendable (Channel) -> EventLoopFuture)? = nil + backgroundActivityLogger: Logger? ) { var tlsConfig = TLSConfiguration.makeClientConfiguration() tlsConfig.certificateVerification = certificateVerification @@ -972,10 +939,7 @@ public class HTTPClient { connectionPool: ConnectionPool(idleTimeout: connectionPool), proxy: proxy, ignoreUncleanSSLShutdown: ignoreUncleanSSLShutdown, - decompression: decompression, - http1_1ConnectionDebugInitializer: http1_1ConnectionDebugInitializer, - http2ConnectionDebugInitializer: http2ConnectionDebugInitializer, - http2StreamChannelDebugInitializer: http2StreamChannelDebugInitializer + decompression: decompression ) } @@ -985,6 +949,26 @@ public class HTTPClient { timeout: Timeout = Timeout(), proxy: Proxy? = nil, ignoreUncleanSSLShutdown: Bool = false, + decompression: Decompression = .disabled + ) { + self.init( + certificateVerification: certificateVerification, + redirectConfiguration: redirectConfiguration, + timeout: timeout, + maximumAllowedIdleTimeInConnectionPool: .seconds(60), + proxy: proxy, + ignoreUncleanSSLShutdown: ignoreUncleanSSLShutdown, + decompression: decompression + ) + } + + public init( + tlsConfiguration: TLSConfiguration? = nil, + redirectConfiguration: RedirectConfiguration? = nil, + timeout: Timeout = Timeout(), + connectionPool: ConnectionPool = ConnectionPool(), + proxy: Proxy? = nil, + ignoreUncleanSSLShutdown: Bool = false, decompression: Decompression = .disabled, http1_1ConnectionDebugInitializer: (@Sendable (Channel) -> EventLoopFuture)? = nil, @@ -994,17 +978,17 @@ public class HTTPClient { (@Sendable (Channel) -> EventLoopFuture)? = nil ) { self.init( - certificateVerification: certificateVerification, + tlsConfiguration: tlsConfiguration, redirectConfiguration: redirectConfiguration, timeout: timeout, - maximumAllowedIdleTimeInConnectionPool: .seconds(60), + connectionPool: connectionPool, proxy: proxy, ignoreUncleanSSLShutdown: ignoreUncleanSSLShutdown, - decompression: decompression, - http1_1ConnectionDebugInitializer: http1_1ConnectionDebugInitializer, - http2ConnectionDebugInitializer: http2ConnectionDebugInitializer, - http2StreamChannelDebugInitializer: http2StreamChannelDebugInitializer + decompression: decompression ) + self.http1_1ConnectionDebugInitializer = http1_1ConnectionDebugInitializer + self.http2ConnectionDebugInitializer = http2ConnectionDebugInitializer + self.http2StreamChannelDebugInitializer = http2StreamChannelDebugInitializer } } diff --git a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift index 1282a2687..de0e533f2 100644 --- a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift +++ b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift @@ -4308,13 +4308,18 @@ final class HTTPClientTests: XCTestCaseHTTPClientTestsBaseClass { } func testHTTP1ConnectionDebugInitializer() { - let connectionDebugInitializerUtil = DebugInitializerUtil() - - var config = HTTPClient.Configuration() + let connectionDebugInitializerUtil = CountingDebugInitializerUtil() + + // Initializing even with just `http1_1ConnectionDebugInitializer` (rather than manually + // modifying `config`) to ensure that the matching `init` actually wires up this argument + // with the respective property. This is necessary as these parameters are defaulted and can + // be easy to miss. + var config = HTTPClient.Configuration( + http1_1ConnectionDebugInitializer: connectionDebugInitializerUtil.initialize(channel:) + ) config.tlsConfiguration = .clientDefault config.tlsConfiguration?.certificateVerification = .none config.httpVersion = .http1Only - config.http1_1ConnectionDebugInitializer = connectionDebugInitializerUtil.operation let client = HTTPClient( eventLoopGroupProvider: .singleton, @@ -4335,19 +4340,26 @@ final class HTTPClientTests: XCTestCaseHTTPClientTestsBaseClass { // Even though multiple requests were made, the connection debug initializer must be called // only once. - XCTAssertEqual(connectionDebugInitializerUtil.executionCount, 1) + XCTAssertEqual(connectionDebugInitializerUtil.executionCount.withLockedValue { $0 }, 1) } func testHTTP2ConnectionAndStreamChannelDebugInitializers() { - let connectionDebugInitializerUtil = DebugInitializerUtil() - let streamChannelDebugInitializerUtil = DebugInitializerUtil() - - var config = HTTPClient.Configuration() + let connectionDebugInitializerUtil = CountingDebugInitializerUtil() + let streamChannelDebugInitializerUtil = CountingDebugInitializerUtil() + + // Initializing even with just `http2ConnectionDebugInitializer` and + // `http2StreamChannelDebugInitializer` (rather than manually modifying `config`) to ensure + // that the matching `init` actually wires up these arguments with the respective + // properties. This is necessary as these parameters are defaulted and can be easy to miss. + var config = HTTPClient.Configuration( + http2ConnectionDebugInitializer: + connectionDebugInitializerUtil.initialize(channel:), + http2StreamChannelDebugInitializer: + streamChannelDebugInitializerUtil.initialize(channel:) + ) config.tlsConfiguration = .clientDefault config.tlsConfiguration?.certificateVerification = .none config.httpVersion = .automatic - config.http2ConnectionDebugInitializer = connectionDebugInitializerUtil.operation - config.http2StreamChannelDebugInitializer = streamChannelDebugInitializerUtil.operation let client = HTTPClient( eventLoopGroupProvider: .singleton, @@ -4370,25 +4382,32 @@ final class HTTPClientTests: XCTestCaseHTTPClientTestsBaseClass { // Even though multiple requests were made, the connection debug initializer must be called // only once. - XCTAssertEqual(connectionDebugInitializerUtil.executionCount, 1) + XCTAssertEqual( + connectionDebugInitializerUtil.executionCount.withLockedValue { $0 }, + 1 + ) // The stream channel debug initializer must be called only as much as the number of // requests made. - XCTAssertEqual(streamChannelDebugInitializerUtil.executionCount, numberOfRequests) + XCTAssertEqual( + streamChannelDebugInitializerUtil.executionCount.withLockedValue { $0 }, + numberOfRequests + ) } } -class DebugInitializerUtil { - var executionCount: Int +final class CountingDebugInitializerUtil: Sendable { + let executionCount: NIOLockedValueBox + /// The acual debug initializer. @Sendable - func operation(channel: Channel) -> EventLoopFuture { - self.executionCount += 1 + func initialize(channel: Channel) -> EventLoopFuture { + self.executionCount.withLockedValue { $0 += 1 } return channel.eventLoop.makeSucceededVoidFuture() } init() { - self.executionCount = 0 + self.executionCount = .init(0) } } From a32379622c0a81b5f59f89ac64f25246f7425306 Mon Sep 17 00:00:00 2001 From: cnkwocha Date: Thu, 23 Jan 2025 09:42:16 +0000 Subject: [PATCH 3/8] Implement feedback --- .../HTTPClientTests.swift | 55 ++++++++++++++----- 1 file changed, 42 insertions(+), 13 deletions(-) diff --git a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift index de0e533f2..7141c3173 100644 --- a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift +++ b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift @@ -4307,7 +4307,41 @@ final class HTTPClientTests: XCTestCaseHTTPClientTestsBaseClass { XCTAssertEqual(request.headers.first(name: "Authorization"), "Basic Zm9vOmJhcg==") } - func testHTTP1ConnectionDebugInitializer() { + func testHTTP1PlainTextConnectionDebugInitializer() { + let connectionDebugInitializerUtil = CountingDebugInitializerUtil() + + // Initializing even with just `http1_1ConnectionDebugInitializer` (rather than manually + // modifying `config`) to ensure that the matching `init` actually wires up this argument + // with the respective property. This is necessary as these parameters are defaulted and can + // be easy to miss. + var config = HTTPClient.Configuration( + http1_1ConnectionDebugInitializer: connectionDebugInitializerUtil.initialize(channel:) + ) + config.httpVersion = .http1Only + + let client = HTTPClient( + eventLoopGroupProvider: .singleton, + configuration: config, + backgroundActivityLogger: Logger( + label: "HTTPClient", + factory: StreamLogHandler.standardOutput(label:) + ) + ) + defer { XCTAssertNoThrow(client.shutdown()) } + + let bin = HTTPBin(.http1_1(ssl: false, compress: false)) + defer { XCTAssertNoThrow(try bin.shutdown()) } + + for _ in 0..<3 { + XCTAssertNoThrow(try client.get(url: "http://localhost:\(bin.port)/get").wait()) + } + + // Even though multiple requests were made, the connection debug initializer must be called + // only once. + XCTAssertEqual(connectionDebugInitializerUtil.executionCount, 1) + } + + func testHTTP1EncryptedConnectionDebugInitializer() { let connectionDebugInitializerUtil = CountingDebugInitializerUtil() // Initializing even with just `http1_1ConnectionDebugInitializer` (rather than manually @@ -4340,7 +4374,7 @@ final class HTTPClientTests: XCTestCaseHTTPClientTestsBaseClass { // Even though multiple requests were made, the connection debug initializer must be called // only once. - XCTAssertEqual(connectionDebugInitializerUtil.executionCount.withLockedValue { $0 }, 1) + XCTAssertEqual(connectionDebugInitializerUtil.executionCount, 1) } func testHTTP2ConnectionAndStreamChannelDebugInitializers() { @@ -4382,32 +4416,27 @@ final class HTTPClientTests: XCTestCaseHTTPClientTestsBaseClass { // Even though multiple requests were made, the connection debug initializer must be called // only once. - XCTAssertEqual( - connectionDebugInitializerUtil.executionCount.withLockedValue { $0 }, - 1 - ) + XCTAssertEqual(connectionDebugInitializerUtil.executionCount, 1) // The stream channel debug initializer must be called only as much as the number of // requests made. - XCTAssertEqual( - streamChannelDebugInitializerUtil.executionCount.withLockedValue { $0 }, - numberOfRequests - ) + XCTAssertEqual(streamChannelDebugInitializerUtil.executionCount, numberOfRequests) } } final class CountingDebugInitializerUtil: Sendable { - let executionCount: NIOLockedValueBox + private let _executionCount: NIOLockedValueBox + var executionCount: Int { self._executionCount.withLockedValue { $0 } } /// The acual debug initializer. @Sendable func initialize(channel: Channel) -> EventLoopFuture { - self.executionCount.withLockedValue { $0 += 1 } + self._executionCount.withLockedValue { $0 += 1 } return channel.eventLoop.makeSucceededVoidFuture() } init() { - self.executionCount = .init(0) + self._executionCount = .init(0) } } From 0d22e92fc355417aabbf11f790620f572ae6d114 Mon Sep 17 00:00:00 2001 From: cnkwocha Date: Thu, 23 Jan 2025 16:52:05 +0000 Subject: [PATCH 4/8] Implement feedback --- .../HTTPClientTests.swift | 66 +++++++------------ 1 file changed, 24 insertions(+), 42 deletions(-) diff --git a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift index 7141c3173..7725e2d64 100644 --- a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift +++ b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift @@ -4307,7 +4307,7 @@ final class HTTPClientTests: XCTestCaseHTTPClientTestsBaseClass { XCTAssertEqual(request.headers.first(name: "Authorization"), "Basic Zm9vOmJhcg==") } - func testHTTP1PlainTextConnectionDebugInitializer() { + func runBaseTestForHTTP1ConnectionDebugInitializer(ssl: Bool) { let connectionDebugInitializerUtil = CountingDebugInitializerUtil() // Initializing even with just `http1_1ConnectionDebugInitializer` (rather than manually @@ -4315,10 +4315,17 @@ final class HTTPClientTests: XCTestCaseHTTPClientTestsBaseClass { // with the respective property. This is necessary as these parameters are defaulted and can // be easy to miss. var config = HTTPClient.Configuration( - http1_1ConnectionDebugInitializer: connectionDebugInitializerUtil.initialize(channel:) + http1_1ConnectionDebugInitializer: { channel in + return connectionDebugInitializerUtil.initialize(channel: channel) + } ) config.httpVersion = .http1Only + if ssl { + config.tlsConfiguration = .clientDefault + config.tlsConfiguration?.certificateVerification = .none + } + let client = HTTPClient( eventLoopGroupProvider: .singleton, configuration: config, @@ -4329,11 +4336,13 @@ final class HTTPClientTests: XCTestCaseHTTPClientTestsBaseClass { ) defer { XCTAssertNoThrow(client.shutdown()) } - let bin = HTTPBin(.http1_1(ssl: false, compress: false)) + let bin = HTTPBin(.http1_1(ssl: ssl, compress: false)) defer { XCTAssertNoThrow(try bin.shutdown()) } + let scheme = ssl ? "https" : "http" + for _ in 0..<3 { - XCTAssertNoThrow(try client.get(url: "http://localhost:\(bin.port)/get").wait()) + XCTAssertNoThrow(try client.get(url: "\(scheme)://localhost:\(bin.port)/get").wait()) } // Even though multiple requests were made, the connection debug initializer must be called @@ -4341,40 +4350,12 @@ final class HTTPClientTests: XCTestCaseHTTPClientTestsBaseClass { XCTAssertEqual(connectionDebugInitializerUtil.executionCount, 1) } - func testHTTP1EncryptedConnectionDebugInitializer() { - let connectionDebugInitializerUtil = CountingDebugInitializerUtil() - - // Initializing even with just `http1_1ConnectionDebugInitializer` (rather than manually - // modifying `config`) to ensure that the matching `init` actually wires up this argument - // with the respective property. This is necessary as these parameters are defaulted and can - // be easy to miss. - var config = HTTPClient.Configuration( - http1_1ConnectionDebugInitializer: connectionDebugInitializerUtil.initialize(channel:) - ) - config.tlsConfiguration = .clientDefault - config.tlsConfiguration?.certificateVerification = .none - config.httpVersion = .http1Only - - let client = HTTPClient( - eventLoopGroupProvider: .singleton, - configuration: config, - backgroundActivityLogger: Logger( - label: "HTTPClient", - factory: StreamLogHandler.standardOutput(label:) - ) - ) - defer { XCTAssertNoThrow(client.shutdown()) } - - let bin = HTTPBin(.http1_1(ssl: true, compress: false)) - defer { XCTAssertNoThrow(try bin.shutdown()) } - - for _ in 0..<3 { - XCTAssertNoThrow(try client.get(url: "https://localhost:\(bin.port)/get").wait()) - } + func testHTTP1PlainTextConnectionDebugInitializer() { + runBaseTestForHTTP1ConnectionDebugInitializer(ssl: false) + } - // Even though multiple requests were made, the connection debug initializer must be called - // only once. - XCTAssertEqual(connectionDebugInitializerUtil.executionCount, 1) + func testHTTP1EncryptedConnectionDebugInitializer() { + runBaseTestForHTTP1ConnectionDebugInitializer(ssl: true) } func testHTTP2ConnectionAndStreamChannelDebugInitializers() { @@ -4386,10 +4367,12 @@ final class HTTPClientTests: XCTestCaseHTTPClientTestsBaseClass { // that the matching `init` actually wires up these arguments with the respective // properties. This is necessary as these parameters are defaulted and can be easy to miss. var config = HTTPClient.Configuration( - http2ConnectionDebugInitializer: - connectionDebugInitializerUtil.initialize(channel:), - http2StreamChannelDebugInitializer: - streamChannelDebugInitializerUtil.initialize(channel:) + http2ConnectionDebugInitializer: { channel in + return connectionDebugInitializerUtil.initialize(channel: channel) + }, + http2StreamChannelDebugInitializer: { channel in + return streamChannelDebugInitializerUtil.initialize(channel: channel) + } ) config.tlsConfiguration = .clientDefault config.tlsConfiguration?.certificateVerification = .none @@ -4429,7 +4412,6 @@ final class CountingDebugInitializerUtil: Sendable { var executionCount: Int { self._executionCount.withLockedValue { $0 } } /// The acual debug initializer. - @Sendable func initialize(channel: Channel) -> EventLoopFuture { self._executionCount.withLockedValue { $0 += 1 } From 4847b6eff30a890711d654f6cbbfdd1518ed6c0a Mon Sep 17 00:00:00 2001 From: Clinton Nkwocha <32041805+clintonpi@users.noreply.github.com> Date: Fri, 24 Jan 2025 09:03:44 +0000 Subject: [PATCH 5/8] Correct typo Co-authored-by: Cory Benfield --- Tests/AsyncHTTPClientTests/HTTPClientTests.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift index 7725e2d64..c9e5a7061 100644 --- a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift +++ b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift @@ -4411,7 +4411,7 @@ final class CountingDebugInitializerUtil: Sendable { private let _executionCount: NIOLockedValueBox var executionCount: Int { self._executionCount.withLockedValue { $0 } } - /// The acual debug initializer. + /// The actual debug initializer. func initialize(channel: Channel) -> EventLoopFuture { self._executionCount.withLockedValue { $0 += 1 } From d47e75690c162fd605738c66be1526ba6b761c2e Mon Sep 17 00:00:00 2001 From: cnkwocha Date: Fri, 24 Jan 2025 11:20:11 +0000 Subject: [PATCH 6/8] Implement feedback --- .../HTTPClientTests.swift | 81 ++++++++++++++++--- 1 file changed, 68 insertions(+), 13 deletions(-) diff --git a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift index c9e5a7061..ae572268e 100644 --- a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift +++ b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift @@ -4326,15 +4326,19 @@ final class HTTPClientTests: XCTestCaseHTTPClientTestsBaseClass { config.tlsConfiguration?.certificateVerification = .none } - let client = HTTPClient( + let higherConnectTimeout = CountingDebugInitializerUtil.duration + .milliseconds(100) + var configWithHigherTimeout = config + configWithHigherTimeout.timeout = .init(connect: higherConnectTimeout) + + let clientWithHigherTimeout = HTTPClient( eventLoopGroupProvider: .singleton, - configuration: config, + configuration: configWithHigherTimeout, backgroundActivityLogger: Logger( label: "HTTPClient", factory: StreamLogHandler.standardOutput(label:) ) ) - defer { XCTAssertNoThrow(client.shutdown()) } + defer { XCTAssertNoThrow(try clientWithHigherTimeout.syncShutdown()) } let bin = HTTPBin(.http1_1(ssl: ssl, compress: false)) defer { XCTAssertNoThrow(try bin.shutdown()) } @@ -4342,12 +4346,34 @@ final class HTTPClientTests: XCTestCaseHTTPClientTestsBaseClass { let scheme = ssl ? "https" : "http" for _ in 0..<3 { - XCTAssertNoThrow(try client.get(url: "\(scheme)://localhost:\(bin.port)/get").wait()) + XCTAssertNoThrow( + try clientWithHigherTimeout.get(url: "\(scheme)://localhost:\(bin.port)/get").wait() + ) } // Even though multiple requests were made, the connection debug initializer must be called // only once. XCTAssertEqual(connectionDebugInitializerUtil.executionCount, 1) + + let lowerConnectTimeout = CountingDebugInitializerUtil.duration - .milliseconds(100) + var configWithLowerTimeout = config + configWithLowerTimeout.timeout = .init(connect: lowerConnectTimeout) + + let clientWithLowerTimeout = HTTPClient( + eventLoopGroupProvider: .singleton, + configuration: configWithLowerTimeout, + backgroundActivityLogger: Logger( + label: "HTTPClient", + factory: StreamLogHandler.standardOutput(label:) + ) + ) + defer { XCTAssertNoThrow(try clientWithLowerTimeout.syncShutdown()) } + + XCTAssertThrowsError( + try clientWithLowerTimeout.get(url: "\(scheme)://localhost:\(bin.port)/get").wait() + ) { + XCTAssertEqual($0 as? HTTPClientError, .connectTimeout) + } } func testHTTP1PlainTextConnectionDebugInitializer() { @@ -4378,15 +4404,19 @@ final class HTTPClientTests: XCTestCaseHTTPClientTestsBaseClass { config.tlsConfiguration?.certificateVerification = .none config.httpVersion = .automatic - let client = HTTPClient( + let higherConnectTimeout = CountingDebugInitializerUtil.duration + .milliseconds(100) + var configWithHigherTimeout = config + configWithHigherTimeout.timeout = .init(connect: higherConnectTimeout) + + let clientWithHigherTimeout = HTTPClient( eventLoopGroupProvider: .singleton, - configuration: config, + configuration: configWithHigherTimeout, backgroundActivityLogger: Logger( label: "HTTPClient", factory: StreamLogHandler.standardOutput(label:) ) ) - defer { XCTAssertNoThrow(client.shutdown()) } + defer { XCTAssertNoThrow(try clientWithHigherTimeout.syncShutdown()) } let bin = HTTPBin(.http2(compress: false)) defer { XCTAssertNoThrow(try bin.shutdown()) } @@ -4394,7 +4424,9 @@ final class HTTPClientTests: XCTestCaseHTTPClientTestsBaseClass { let numberOfRequests = 3 for _ in 0.. + private let _executionCount = NIOLockedValueBox(0) var executionCount: Int { self._executionCount.withLockedValue { $0 } } + /// The minimum time to spend running the debug initializer. + static let duration: TimeAmount = .milliseconds(300) + /// The actual debug initializer. func initialize(channel: Channel) -> EventLoopFuture { self._executionCount.withLockedValue { $0 += 1 } - return channel.eventLoop.makeSucceededVoidFuture() - } + let someScheduledTask = channel.eventLoop.scheduleTask(in: Self.duration) { + return channel.eventLoop.makeSucceededVoidFuture() + } - init() { - self._executionCount = .init(0) + return someScheduledTask.futureResult.flatMap { $0 } } } From fec1b802972f20fa025a2f3d825748410c9df407 Mon Sep 17 00:00:00 2001 From: cnkwocha Date: Fri, 24 Jan 2025 16:35:12 +0000 Subject: [PATCH 7/8] Fix formatting --- Tests/AsyncHTTPClientTests/HTTPClientTests.swift | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift index ae572268e..360632cdd 100644 --- a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift +++ b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift @@ -4316,7 +4316,7 @@ final class HTTPClientTests: XCTestCaseHTTPClientTestsBaseClass { // be easy to miss. var config = HTTPClient.Configuration( http1_1ConnectionDebugInitializer: { channel in - return connectionDebugInitializerUtil.initialize(channel: channel) + connectionDebugInitializerUtil.initialize(channel: channel) } ) config.httpVersion = .http1Only @@ -4394,10 +4394,10 @@ final class HTTPClientTests: XCTestCaseHTTPClientTestsBaseClass { // properties. This is necessary as these parameters are defaulted and can be easy to miss. var config = HTTPClient.Configuration( http2ConnectionDebugInitializer: { channel in - return connectionDebugInitializerUtil.initialize(channel: channel) + connectionDebugInitializerUtil.initialize(channel: channel) }, http2StreamChannelDebugInitializer: { channel in - return streamChannelDebugInitializerUtil.initialize(channel: channel) + streamChannelDebugInitializerUtil.initialize(channel: channel) } ) config.tlsConfiguration = .clientDefault @@ -4471,7 +4471,7 @@ final class CountingDebugInitializerUtil: Sendable { self._executionCount.withLockedValue { $0 += 1 } let someScheduledTask = channel.eventLoop.scheduleTask(in: Self.duration) { - return channel.eventLoop.makeSucceededVoidFuture() + channel.eventLoop.makeSucceededVoidFuture() } return someScheduledTask.futureResult.flatMap { $0 } From 77ffedf8aa8410ad60089026ef32e8c2f33c9b70 Mon Sep 17 00:00:00 2001 From: cnkwocha Date: Fri, 24 Jan 2025 16:47:31 +0000 Subject: [PATCH 8/8] Fix formatting --- .../HTTPConnectionPool+Factory.swift | 8 ++------ Sources/AsyncHTTPClient/HTTPClient.swift | 18 ++++++------------ 2 files changed, 8 insertions(+), 18 deletions(-) diff --git a/Sources/AsyncHTTPClient/ConnectionPool/HTTPConnectionPool+Factory.swift b/Sources/AsyncHTTPClient/ConnectionPool/HTTPConnectionPool+Factory.swift index 04fb87bdc..9a3d66a3a 100644 --- a/Sources/AsyncHTTPClient/ConnectionPool/HTTPConnectionPool+Factory.swift +++ b/Sources/AsyncHTTPClient/ConnectionPool/HTTPConnectionPool+Factory.swift @@ -85,9 +85,7 @@ extension HTTPConnectionPool.ConnectionFactory { logger: logger ) - if let connectionDebugInitializer - = self.clientConfiguration.http1_1ConnectionDebugInitializer - { + if let connectionDebugInitializer = self.clientConfiguration.http1_1ConnectionDebugInitializer { connectionDebugInitializer(channel).whenComplete { debugInitializerResult in switch debugInitializerResult { case .success: @@ -115,9 +113,7 @@ extension HTTPConnectionPool.ConnectionFactory { ).whenComplete { result in switch result { case .success((let connection, let maximumStreams)): - if let connectionDebugInitializer - = self.clientConfiguration.http2ConnectionDebugInitializer - { + if let connectionDebugInitializer = self.clientConfiguration.http2ConnectionDebugInitializer { connectionDebugInitializer(channel).whenComplete { debugInitializerResult in switch debugInitializerResult { diff --git a/Sources/AsyncHTTPClient/HTTPClient.swift b/Sources/AsyncHTTPClient/HTTPClient.swift index aa84d1c83..ff222bd6f 100644 --- a/Sources/AsyncHTTPClient/HTTPClient.swift +++ b/Sources/AsyncHTTPClient/HTTPClient.swift @@ -848,16 +848,13 @@ public class HTTPClient { public var enableMultipath: Bool /// A method with access to the HTTP/1 connection channel that is called when creating the connection. - public var http1_1ConnectionDebugInitializer: - (@Sendable (Channel) -> EventLoopFuture)? + public var http1_1ConnectionDebugInitializer: (@Sendable (Channel) -> EventLoopFuture)? /// A method with access to the HTTP/2 connection channel that is called when creating the connection. - public var http2ConnectionDebugInitializer: - (@Sendable (Channel) -> EventLoopFuture)? + public var http2ConnectionDebugInitializer: (@Sendable (Channel) -> EventLoopFuture)? /// A method with access to the HTTP/2 stream channel that is called when creating the stream. - public var http2StreamChannelDebugInitializer: - (@Sendable (Channel) -> EventLoopFuture)? + public var http2StreamChannelDebugInitializer: (@Sendable (Channel) -> EventLoopFuture)? public init( tlsConfiguration: TLSConfiguration? = nil, @@ -970,12 +967,9 @@ public class HTTPClient { proxy: Proxy? = nil, ignoreUncleanSSLShutdown: Bool = false, decompression: Decompression = .disabled, - http1_1ConnectionDebugInitializer: - (@Sendable (Channel) -> EventLoopFuture)? = nil, - http2ConnectionDebugInitializer: - (@Sendable (Channel) -> EventLoopFuture)? = nil, - http2StreamChannelDebugInitializer: - (@Sendable (Channel) -> EventLoopFuture)? = nil + http1_1ConnectionDebugInitializer: (@Sendable (Channel) -> EventLoopFuture)? = nil, + http2ConnectionDebugInitializer: (@Sendable (Channel) -> EventLoopFuture)? = nil, + http2StreamChannelDebugInitializer: (@Sendable (Channel) -> EventLoopFuture)? = nil ) { self.init( tlsConfiguration: tlsConfiguration,