fix missing connect timeout and make tests safer (swift-server#267)

* fix missing connect timeout and make tests safer

* swiftformat and linux tests

* fix timeout test

* speedup another test

* make tests safer

Author: artemredkin

Sources/AsyncHTTPClient/Utils.swift

@@ -79,7 +79,7 @@ extension NIOClientTCPBootstrap {
         requiresTLS: Bool,
         configuration: HTTPClient.Configuration
     ) throws -> NIOClientTCPBootstrap {
-        let bootstrap: NIOClientTCPBootstrap
+        var bootstrap: NIOClientTCPBootstrap
         #if canImport(Network)
             // if eventLoop is compatible with NIOTransportServices create a NIOTSConnectionBootstrap
             if #available(OSX 10.14, iOS 12.0, tvOS 12.0, watchOS 6.0, *), let tsBootstrap = NIOTSConnectionBootstrap(validatingGroup: eventLoop) {
@@ -106,10 +106,15 @@ extension NIOClientTCPBootstrap {
             }
         #endif
 
+        if let timeout = configuration.timeout.connect {
+            bootstrap = bootstrap.connectTimeout(timeout)
+        }
+
         // don't enable TLS if we have a proxy, this will be enabled later on
         if requiresTLS, configuration.proxy == nil {
             return bootstrap.enableTLS()
         }
+
         return bootstrap
     }
 

Tests/AsyncHTTPClientTests/HTTPClientNIOTSTests.swift

@@ -74,19 +74,19 @@ class HTTPClientNIOTSTests: XCTestCase {
     func testConnectionFailError() {
         guard isTestingNIOTS() else { return }
         let httpBin = HTTPBin(ssl: true)
-        let httpClient = HTTPClient(eventLoopGroupProvider: .shared(self.clientGroup))
+        let httpClient = HTTPClient(eventLoopGroupProvider: .shared(self.clientGroup),
+                                    configuration: .init(timeout: .init(connect: .milliseconds(100),
+                                                                        read: .milliseconds(100))))
+
         defer {
             XCTAssertNoThrow(try httpClient.syncShutdown(requiresCleanClose: true))
         }
+
         let port = httpBin.port
         XCTAssertNoThrow(try httpBin.shutdown())
 
-        do {
-            _ = try httpClient.get(url: "https://localhost:\(port)/get").wait()
-            XCTFail("This should have failed")
-        } catch ChannelError.connectTimeout {
-        } catch {
-            XCTFail("Error should have been ChannelError.connectTimeout not \(type(of: error))")
+        XCTAssertThrowsError(try httpClient.get(url: "https://localhost:\(port)/get").wait()) { error in
+            XCTAssertEqual(.connectTimeout(.milliseconds(100)), error as? ChannelError)
         }
     }
 
@@ -103,13 +103,8 @@ class HTTPClientNIOTSTests: XCTestCase {
                 XCTAssertNoThrow(try httpBin.shutdown())
             }
 
-            do {
-                _ = try httpClient.get(url: "https://localhost:\(httpBin.port)/get").wait()
-                XCTFail("This should have failed")
-            } catch let error as HTTPClient.NWTLSError {
-                XCTAssertEqual(error.status, errSSLHandshakeFail)
-            } catch {
-                XCTFail("Error should have been NWTLSError not \(type(of: error))")
+            XCTAssertThrowsError(try httpClient.get(url: "https://localhost:\(httpBin.port)/get").wait()) { error in
+                XCTAssertEqual((error as? HTTPClient.NWTLSError)?.status, errSSLHandshakeFail)
             }
         #endif
     }

Tests/AsyncHTTPClientTests/HTTPClientTests+XCTest.swift

@@ -46,6 +46,7 @@ extension HTTPClientTests {
             ("testStreaming", testStreaming),
             ("testRemoteClose", testRemoteClose),
             ("testReadTimeout", testReadTimeout),
+            ("testConnectTimeout", testConnectTimeout),
             ("testDeadline", testDeadline),
             ("testCancel", testCancel),
             ("testStressCancel", testStressCancel),

Tests/AsyncHTTPClientTests/HTTPClientTests.swift

@@ -422,6 +422,20 @@ class HTTPClientTests: XCTestCase {
         }
     }
 
+    func testConnectTimeout() throws {
+        let httpClient = HTTPClient(eventLoopGroupProvider: .shared(self.clientGroup),
+                                    configuration: .init(timeout: .init(connect: .milliseconds(100), read: .milliseconds(150))))
+
+        defer {
+            XCTAssertNoThrow(try httpClient.syncShutdown())
+        }
+
+        // This must throw as 198.51.100.254 is reserved for documentation only
+        XCTAssertThrowsError(try httpClient.get(url: "http://198.51.100.254:65535/get").wait()) { error in
+            XCTAssertEqual(.connectTimeout(.milliseconds(100)), error as? ChannelError)
+        }
+    }
+
     func testDeadline() throws {
         XCTAssertThrowsError(try self.defaultClient.get(url: self.defaultHTTPBinURLPrefix + "wait", deadline: .now() + .milliseconds(150)).wait(), "Should fail") { error in
             guard case let error = error as? HTTPClientError, error == .readTimeout else {
@@ -1661,7 +1675,7 @@ class HTTPClientTests: XCTestCase {
     }
 
     func testAvoidLeakingTLSHandshakeCompletionPromise() {
-        let localClient = HTTPClient(eventLoopGroupProvider: .shared(self.clientGroup))
+        let localClient = HTTPClient(eventLoopGroupProvider: .shared(self.clientGroup), configuration: .init(timeout: .init(connect: .milliseconds(100))))
         let localHTTPBin = HTTPBin()
         let port = localHTTPBin.port
         XCTAssertNoThrow(try localHTTPBin.shutdown())