diff --git a/.travis.yml b/.travis.yml index 55a6d2a..40db609 100644 --- a/.travis.yml +++ b/.travis.yml @@ -15,17 +15,17 @@ matrix: dist: xenial sudo: required services: docker - env: DOCKER_IMAGE=swift:5.0.3-xenial SWIFT_SNAPSHOT=5.0.3 + env: DOCKER_IMAGE=swift:5.0.3-xenial SWIFT_SNAPSHOT=5.0.3 DOCKER_PACKAGES="zlib1g-dev" - os: linux dist: xenial sudo: required services: docker - env: DOCKER_IMAGE=swift:5.1 + env: DOCKER_IMAGE=swift:5.1 DOCKER_PACKAGES="zlib1g-dev" - os: linux dist: xenial sudo: required services: docker - env: DOCKER_IMAGE=swift:5.1 SWIFT_SNAPSHOT=$SWIFT_DEVELOPMENT_SNAPSHOT + env: DOCKER_IMAGE=swift:5.1 SWIFT_SNAPSHOT=$SWIFT_DEVELOPMENT_SNAPSHOT DOCKER_PACKAGES="zlib1g-dev" - os: osx osx_image: xcode10.2 sudo: required diff --git a/Package.swift b/Package.swift index 999b690..e8da57f 100644 --- a/Package.swift +++ b/Package.swift @@ -29,7 +29,7 @@ let package = Package( dependencies: [ .package(url: "https://github.com/IBM-Swift/CircuitBreaker.git", from: "5.0.0"), .package(url: "https://github.com/IBM-Swift/LoggerAPI.git", from: "1.8.0"), - .package(url: "https://github.com/swift-server/async-http-client.git", .exact("1.0.0-alpha.4")), + .package(url: "https://github.com/swift-server/async-http-client.git", from: "1.0.0"), ], targets: [ .target( diff --git a/Sources/SwiftyRequest/RestRequest.swift b/Sources/SwiftyRequest/RestRequest.swift index 515a049..52f9fe6 100644 --- a/Sources/SwiftyRequest/RestRequest.swift +++ b/Sources/SwiftyRequest/RestRequest.swift @@ -22,8 +22,13 @@ import NIO import NIOHTTP1 import NIOSSL -fileprivate let globalELG = MultiThreadedEventLoopGroup(numberOfThreads: 1) +/// The default EventLoopGroup used by all RestRequest instances: a `MultiThreadedEventLoopGroup` +/// with one thread per available processor. +fileprivate let globalELG = MultiThreadedEventLoopGroup(numberOfThreads: System.coreCount) +// An object to represent the parameters of a request that can be mutated prior to +// a request being issued. This acts as an adapter between SwiftyRequest's API and +// async-http-client's (immutable) `Request` type, and can vend new `Request`s. fileprivate class MutableRequest { /// Request HTTP method var method: NIOHTTP1.HTTPMethod @@ -365,9 +370,9 @@ public class RestRequest { } } - @available(*, deprecated, renamed: "init(method:url:insecure:clientCertificate:)") - public convenience init(method: HTTPMethod = .get, url: String, containsSelfSignedCert: Bool? = false, clientCertificate: ClientCertificate? = nil) { - self.init(method: method, url: url, insecure: containsSelfSignedCert ?? false, clientCertificate: clientCertificate) + @available(*, deprecated, renamed: "init(method:url:insecure:clientCertificate:timeout:eventLoopGroup:)") + public convenience init(method: HTTPMethod = .get, url: String, containsSelfSignedCert: Bool? = false, clientCertificate: ClientCertificate? = nil, timeout: HTTPClient.Configuration.Timeout? = nil, eventLoopGroup: EventLoopGroup? = nil) { + self.init(method: method, url: url, insecure: containsSelfSignedCert ?? false, clientCertificate: clientCertificate, timeout: timeout, eventLoopGroup: eventLoopGroup) } /// Initialize a `RestRequest` instance. @@ -382,9 +387,11 @@ public class RestRequest { /// - url: URL string to use for the network request. /// - insecure: Pass `True` to accept invalid or self-signed certificates. /// - clientCertificate: An optional `ClientCertificate` for client authentication. - public init(method: HTTPMethod = .get, url: String, insecure: Bool = false, clientCertificate: ClientCertificate? = nil) { + /// - timeout: An optional `HTTPClient.Configuration.Timeout` specifying how long to wait for connection or response from a remote service before timing out. Defaults to `nil`, which means no timeout. + /// - eventLoopGroup: An optional `EventLoopGroup` that should be used for requests, instead of the default `MultiThreadedEventLoopGroup` shared between all RestRequest instances. + public init(method: HTTPMethod = .get, url: String, insecure: Bool = false, clientCertificate: ClientCertificate? = nil, timeout: HTTPClient.Configuration.Timeout? = nil, eventLoopGroup: EventLoopGroup? = nil) { self.mutableRequest = MutableRequest(method: method, url: url) - self.session = RestRequest.createHTTPClient(insecure: insecure, clientCertificate: clientCertificate) + self.session = RestRequest.createHTTPClient(insecure: insecure, clientCertificate: clientCertificate, timeout: timeout, eventLoopGroup: eventLoopGroup) // Set initial headers self.acceptType = "application/json" @@ -392,7 +399,7 @@ public class RestRequest { } - private static func createHTTPClient(insecure: Bool, clientCertificate: ClientCertificate? = nil) -> HTTPClient { + private static func createHTTPClient(insecure: Bool, clientCertificate: ClientCertificate? = nil, timeout: HTTPClient.Configuration.Timeout?, eventLoopGroup: EventLoopGroup?) -> HTTPClient { let chain: [NIOSSLCertificateSource] let key: NIOSSLPrivateKeySource? if let clientCertificate = clientCertificate { @@ -405,8 +412,9 @@ public class RestRequest { let tlsConfiguration = TLSConfiguration.forClient( certificateVerification: (insecure ? .none : .fullVerification), certificateChain: chain, privateKey: key) - let config = HTTPClient.Configuration(tlsConfiguration: tlsConfiguration) - return HTTPClient(eventLoopGroupProvider: .shared(globalELG), configuration: config) + let config = HTTPClient.Configuration(tlsConfiguration: tlsConfiguration, + timeout: timeout ?? HTTPClient.Configuration.Timeout()) + return HTTPClient(eventLoopGroupProvider: .shared(eventLoopGroup ?? globalELG), configuration: config) } /// Convenience function to encode an `Encodable` type as the request body, using a diff --git a/TestServer/Sources/TestServer/main.swift b/TestServer/Sources/TestServer/main.swift index 76a02bb..68e7fdc 100644 --- a/TestServer/Sources/TestServer/main.swift +++ b/TestServer/Sources/TestServer/main.swift @@ -14,6 +14,7 @@ * limitations under the License. */ import Kitura +import Socket import HeliumLogger import FileKit import Foundation @@ -112,6 +113,22 @@ router.get("/users/:id") { (id: Int, respondWith: (User?, RequestError?) -> Void respondWith(userStore[id], nil) } +// MARK: Timeout tests + +router.get("/timeout") { request, response, next in + guard let param = request.queryParameters["delay"], let delay = Int(param) else { + return try response.status(.badRequest).end() + } + DispatchQueue.global().asyncAfter(deadline: .now() + .milliseconds(delay)) { + response.status(.OK) + next() + } +} + +// A socket that listens but never accepts connections +let sleepyServerSocket = try Socket.create(family: .inet) +try sleepyServerSocket.listen(on: 8079, maxBacklogSize: 0, allowPortReuse: false) + // MARK: Start server // Add an HTTP server and connect it to the router diff --git a/Tests/SwiftyRequestTests/SwiftyRequestTests.swift b/Tests/SwiftyRequestTests/SwiftyRequestTests.swift index dc0eb7f..3dbf7e2 100644 --- a/Tests/SwiftyRequestTests/SwiftyRequestTests.swift +++ b/Tests/SwiftyRequestTests/SwiftyRequestTests.swift @@ -1,6 +1,8 @@ import XCTest import CircuitBreaker import NIOSSL +import NIO +import AsyncHTTPClient @testable import SwiftyRequest #if swift(>=4.1) @@ -71,6 +73,9 @@ class SwiftyRequestTests: XCTestCase { ("testBasicAuthenticationFails", testBasicAuthenticationFails), ("testTokenAuthentication", testTokenAuthentication), ("testHeaders", testHeaders), + ("testEventLoopGroup", testEventLoopGroup), + ("testRequestTimeout", testRequestTimeout), +// ("testConnectTimeout", testConnectTimeout), ] // Enable logging output for tests @@ -1092,4 +1097,103 @@ class SwiftyRequestTests: XCTestCase { waitForExpectations(timeout: 10) } + // MARK: Test configuration parameters + + /// Tests that a custom EventLoopGroup can be specified and that RestRequest will use this + /// instead of the default group. + /// Because there is no way to compare an EventLoopGroup or inspect its properties to determine + /// whether it is the one we expected, we instead supply a custom group containing only one + /// thread, and then assert that multiple request handlers cannot run in parallel. + func testEventLoopGroup() { + let expectation = self.expectation(description: "All outstanding requests have completed") + let myELG = MultiThreadedEventLoopGroup(numberOfThreads: 1) + let request = RestRequest(method: .get, url: "http://localhost:8080/", eventLoopGroup: myELG) + let sema = DispatchSemaphore(value: 0) + let requests = DispatchSemaphore(value: 0) + request.responseVoid { _ in + requests.signal() + request.responseVoid { _ in + requests.signal() + expectation.fulfill() + } + // Intentionally block the EventLoop thread, so that we can tell that the second + // request has not been processed (detecting that our single-threaded group is + // in use). + XCTAssertEqual(sema.wait(timeout: .distantFuture), .success) + } + + // Assert that only a single request has been executed by our single-threaded ELG + XCTAssertEqual(requests.wait(timeout: .now() + .seconds(1)), .success, "First request should have been issued") + XCTAssertEqual(requests.wait(timeout: .now() + .seconds(1)), .timedOut, "Second request should not have been issued") + + // Unblock the EventLoop thread, allowing the queued request to complete + sema.signal() + waitForExpectations(timeout: 1) + } + + // MARK: Timeout tests + + /// Makes a request to a route that delays its response for longer than the configured timeout, causing a failure. + /// Then tests that a request with the same configuration succeeds if the route responds within the timeout. + func testRequestTimeout() { + let timeoutExpectation = self.expectation(description: "Request times out") + let successExpectation = self.expectation(description: "Request succeeds") + + let request = RestRequest(method: .get, url: "http://localhost:8080/timeout", timeout: HTTPClient.Configuration.Timeout(connect: nil, read: .milliseconds(500))) + + let delay1s = URLQueryItem(name: "delay", value: "1000") + + request.responseVoid(queryItems: [delay1s]) { result in + switch result { + case .success(let response): + XCTFail("Request should have timed out, but status was \(response.status)") + case .failure(let error): + XCTAssertEqual(error, RestError.httpClientError(HTTPClientError.readTimeout)) + } + timeoutExpectation.fulfill() + } + + let delayNone = URLQueryItem(name: "delay", value: "100") + + request.responseVoid(queryItems: [delayNone]) { result in + switch result { + case .success(let response): + XCTAssertEqual(response.status, .ok) + case .failure(let error): + XCTFail("Request should have succeeded, but produced: \(error)") + } + successExpectation.fulfill() + } + + waitForExpectations(timeout: 3) + } + +/** + * Note: Disabling this test because it seems unreliable in a CI environment + /// Connects to a socket that listens but never accepts a connection, and verifies that the client + /// times out with a failure after a specified connect timeout. + func testConnectTimeout() { + let timeoutExpectation = self.expectation(description: "Request times out") + let timeout: TimeAmount = .milliseconds(500) + + let request = RestRequest(method: .get, url: "http://localhost:8079/", timeout: HTTPClient.Configuration.Timeout(connect: timeout, read: nil)) + + request.responseVoid { result in + switch result { + case .success(let response): + XCTFail("Connection should have timed out, but status was \(response.status)") + case .failure(let error): + XCTAssertEqual(error, RestError.otherError(NIO.ChannelError.connectTimeout(timeout))) + if let underlyingError = error.error, case let NIO.ChannelError.connectTimeout(timeAmount) = underlyingError { + XCTAssertEqual(timeAmount, timeout, "Timeout amount was incorrect") + } else { + XCTFail("Underlying error was not NIO.ChannelError.connectTimeout, it was: \(error.error ?? error)") + } + } + timeoutExpectation.fulfill() + } + + waitForExpectations(timeout: 3) + } +*/ }