swift-server / async-http-client Goto Github PK

View Code? Open in Web Editor NEW

879.0 26.0 107.0 2.3 MB

HTTP client library built on SwiftNIO

Home Page: https://swift-server.github.io/async-http-client

License: Apache License 2.0

Swift 99.02% Dockerfile 0.08% Shell 0.77% C 0.13%

swift-server http-client swift5 swift-nio

async-http-client's People

Contributors

Stargazers

Watchers

Forkers

ianpartridge tomerd dokun1 andrew-lees11 glbrntt ldewailly vkill kyleishie adam-fowler gradientlabapp s2ler anil291987 awadodeh weissi armandgrillet seriyvolk83 yasumoto strangedays ggthedev lukasa taavi013 marinofelipe puneetchd jonlund shekhar-rajak mtdev2 phpan iainsmith artemredkin ktoso fabianfett dimitribouniol kuwerty bikram990 karwa slashmo peteradams-a pokryfka val-verde brightenai igormuzyka ya-pulser vinnyt thantthet 0xtim 0xpablo kyungkoo winsmith huayunzhangke madsodgaard davidde94 modestgoblin fcobia yakavenkam popflamingo culturedcode dnadoba 993695620 karkutsi fundaev mahdibm acche illusionofchaos vlm kevints franzbusch edadealmobiledevelopment proxymanapp cpriebe approov pureswift felixschlegel fananek tachyonics oliverletterer xygkevin cloewen8 gjcairo vudat081299 carolinacass mihaelisaev higher08 yim-lee isabella232 cbaker6 gregggreg natikgadzhi rnro baarde appsnitch locnguyenhuu martialln simonjbeaumont wangx0404 nishant-dani al45tair michaelmcguire nicfontana ker0beros hamzahrmalik

async-http-client's Issues

Crash when using very tight or past deadline

The following code causes a crash:

_ = try httpClient.get(url: "https://github.com", deadline: .now() + .milliseconds(1)).wait()

The backtrace is as follows:

* thread #4, name = 'NIO-ELT-#0', stop reason = EXC_BAD_INSTRUCTION (code=EXC_I386_INVOP, subcode=0x0)
  * frame #0: 0x00000001070cabe1 AsyncHTTPClientTests`static NIODeadline.- infix(lhs=(uptimeNanoseconds = 247397817094645), rhs=(uptimeNanoseconds = 247397825576649), self=NIO.NIODeadline) at EventLoop.swift:397:68
    frame #1: 0x00000001073d47a3 AsyncHTTPClientTests`HTTPClient.resolve(timeout=nil, deadline=(uptimeNanoseconds = 247397817094645), self=0x0000000100e05270) at HTTPClient.swift:175:29
    frame #2: 0x00000001073d3dff AsyncHTTPClientTests`closure #2 in closure #2 in HTTPClient.execute<A>(self=0x0000000100e05270, deadline=(uptimeNanoseconds = 247397817094645), channel=(instance = 0x0000000100c1f450 -> 0x00000001074c0718 type metadata for NIO.SocketChannel, witness_table_Channel = 0x0000000107499d18 AsyncHTTPClientTests`protocol witness table for NIO.BaseSocketChannel<A> : NIO.Channel in NIO)) at HTTPClient.swift:138:43
    frame #3: 0x00000001073de2ae AsyncHTTPClientTests`partial apply for closure #2 in closure #2 in HTTPClient.execute<A>(request:delegate:deadline:) at <compiler-generated>:0
    frame #4: 0x00000001073d3d1c AsyncHTTPClientTests`thunk for @escaping @callee_guaranteed () -> (@owned EventLoopFuture<()>) at <compiler-generated>:0
    frame #5: 0x00000001073de2f1 AsyncHTTPClientTests`thunk for @escaping @callee_guaranteed () -> (@owned EventLoopFuture<()>)partial apply at <compiler-generated>:0
    frame #6: 0x00000001070df330 AsyncHTTPClientTests`closure #1 in EventLoopFuture.flatMap<Value>(self=0x0000000100c23bf0, callback=0x00000001073de2e0 AsyncHTTPClientTests`reabstraction thunk helper from @escaping @callee_guaranteed () -> (@owned NIO.EventLoopFuture<()>) to @escaping @callee_guaranteed (@in_guaranteed ()) -> (@owned NIO.EventLoopFuture<()>)partial apply forwarder with unmangled suffix ".28" at <compiler-generated>, next=NIO.EventLoopPromise<()> @ 0x0000700007b469f8) at EventLoopFuture.swift:468:31
    frame #7: 0x00000001070e4c68 AsyncHTTPClientTests`partial apply for closure #1 in EventLoopFuture.flatMap<A>(file:line:_:) at <compiler-generated>:0
    frame #8: 0x00000001070dfbe8 AsyncHTTPClientTests`EventLoopFuture._addCallback(callback=0x00000001070e4c40 AsyncHTTPClientTests`partial apply forwarder for closure #1 () -> NIO.CallbackList in NIO.EventLoopFuture.flatMap<A>(file: Swift.StaticString, line: Swift.UInt, _: (A) -> NIO.EventLoopFuture<A1>) -> NIO.EventLoopFuture<A1> at <compiler-generated>, self=0x0000000100c23bf0) at EventLoopFuture.swift:634:16
    frame #9: 0x00000001070dfd42 AsyncHTTPClientTests`EventLoopFuture._whenComplete(callback=0x00000001070e4c40 AsyncHTTPClientTests`partial apply forwarder for closure #1 () -> NIO.CallbackList in NIO.EventLoopFuture.flatMap<A>(file: Swift.StaticString, line: Swift.UInt, _: (A) -> NIO.EventLoopFuture<A1>) -> NIO.EventLoopFuture<A1> at <compiler-generated>, self=0x0000000100c23bf0) at EventLoopFuture.swift:641:18
    frame #10: 0x00000001070def9d AsyncHTTPClientTests`EventLoopFuture.flatMap<Value>(file="/Users/ldewailly/devex/async-http-client/Sources/AsyncHTTPClient/HTTPClient.swift", line=137, callback=0x00000001073de2e0 AsyncHTTPClientTests`reabstraction thunk helper from @escaping @callee_guaranteed () -> (@owned NIO.EventLoopFuture<()>) to @escaping @callee_guaranteed (@in_guaranteed ()) -> (@owned NIO.EventLoopFuture<()>)partial apply forwarder with unmangled suffix ".28" at <compiler-generated>, self=0x0000000100c23bf0) at EventLoopFuture.swift:465:14
    frame #11: 0x00000001073d2820 AsyncHTTPClientTests`closure #2 in HTTPClient.execute<T>(channel=(instance = 0x0000000100c1f450 -> 0x00000001074c0718 type metadata for NIO.SocketChannel, witness_table_Channel = 0x0000000107499d18 AsyncHTTPClientTests`protocol witness table for NIO.BaseSocketChannel<A> : NIO.Channel in NIO), self=0x0000000100e05270, request=AsyncHTTPClient.HTTPClient.Request @ 0x0000000100b93cc0, deadline=(uptimeNanoseconds = 247397817094645), task=0x0000000100b92dd0, delegate=0x0000700007b46fa0, redirectHandler=nil) at HTTPClient.swift:137:19
    frame #12: 0x00000001073d8f9a AsyncHTTPClientTests`partial apply for closure #2 in HTTPClient.execute<A>(request:delegate:deadline:) at <compiler-generated>:0
    frame #13: 0x0000000107058b00 AsyncHTTPClientTests`thunk for @escaping @callee_guaranteed (@guaranteed Channel) -> (@owned EventLoopFuture<()>) at <compiler-generated>:0
    frame #14: 0x0000000107062a01 AsyncHTTPClientTests`thunk for @escaping @callee_guaranteed (@guaranteed Channel) -> (@owned EventLoopFuture<()>)partial apply at <compiler-generated>:0
    frame #15: 0x0000000107058bf8 AsyncHTTPClientTests`thunk for @escaping @callee_guaranteed (@in_guaranteed Channel) -> (@out EventLoopFuture<()>) at <compiler-generated>:0
    frame #16: 0x00000001070627b1 AsyncHTTPClientTests`thunk for @escaping @callee_guaranteed (@in_guaranteed Channel) -> (@out EventLoopFuture<()>)partial apply at <compiler-generated>:0
    frame #17: 0x000000010705f102 AsyncHTTPClientTests`setupChannel #1 (eventLoop=(instance = 0x0000000100b91430 -> 0x00000001074bf628 type metadata for NIO.SelectableEventLoop, witness_table_EventLoop = 0x000000010749d9e0 AsyncHTTPClientTests`protocol witness table for NIO.SelectableEventLoop : NIO.EventLoop in NIO), channelInitializer=0x00000001070627a0 AsyncHTTPClientTests`reabstraction thunk helper from @escaping @callee_guaranteed (@in_guaranteed NIO.Channel) -> (@out NIO.EventLoopFuture<()>) to @escaping @callee_guaranteed (@guaranteed NIO.Channel) -> (@owned NIO.EventLoopFuture<()>)partial apply forwarder with unmangled suffix ".126" at <compiler-generated>, channel=0x0000000100c1f450, channelOptions=NIO.ChannelOptions.Storage @ 0x0000700007b472b8, body=0x000000010705c6f0 AsyncHTTPClientTests`closure #1 (NIO.Channel) -> NIO.EventLoopFuture<()> in closure #1 (NIO.EventLoop, Swift.Int32) -> NIO.EventLoopFuture<NIO.Channel> in NIO.ClientBootstrap.connect(host: Swift.String, port: Swift.Int) -> NIO.EventLoopFuture<NIO.Channel> at Bootstrap.swift:433, promise=NIO.EventLoopPromise<Channel> @ 0x0000700007b472a0) in ClientBootstrap.execute(eventLoop:protocolFamily:_:) at Bootstrap.swift:520:13
    frame #18: 0x000000010705ce2e AsyncHTTPClientTests`ClientBootstrap.execute(eventLoop=(instance = 0x0000000100b91430 -> 0x00000001074bf628 type metadata for NIO.SelectableEventLoop, witness_table_EventLoop = 0x000000010749d9e0 AsyncHTTPClientTests`protocol witness table for NIO.SelectableEventLoop : NIO.EventLoop in NIO), protocolFamily=2, body=0x000000010705c6f0 AsyncHTTPClientTests`closure #1 (NIO.Channel) -> NIO.EventLoopFuture<()> in closure #1 (NIO.EventLoop, Swift.Int32) -> NIO.EventLoopFuture<NIO.Channel> in NIO.ClientBootstrap.connect(host: Swift.String, port: Swift.Int) -> NIO.EventLoopFuture<NIO.Channel> at Bootstrap.swift:433, self=0x0000000100b934f0) at Bootstrap.swift:534:20
    frame #19: 0x000000010705c699 AsyncHTTPClientTests`closure #1 in ClientBootstrap.connect(eventLoop=(instance = 0x0000000100b91430 -> 0x00000001074bf628 type metadata for NIO.SelectableEventLoop, witness_table_EventLoop = 0x000000010749d9e0 AsyncHTTPClientTests`protocol witness table for NIO.SelectableEventLoop : NIO.EventLoop in NIO), protocolFamily=2, self=0x0000000100b934f0) at Bootstrap.swift:433:25
    frame #20: 0x000000010705c6bc AsyncHTTPClientTests`partial apply for closure #1 in ClientBootstrap.connect(host:port:) at <compiler-generated>:0
    frame #21: 0x00000001070f87ed AsyncHTTPClientTests`HappyEyeballsConnector.connectToTarget(target=v4, self=0x0000000100b94370) at HappyEyeballs.swift:529:29
    frame #22: 0x00000001070f7f6d AsyncHTTPClientTests`HappyEyeballsConnector.beginConnecting(self=0x0000000100b94370) at HappyEyeballs.swift:459:9
    frame #23: 0x00000001070f815e AsyncHTTPClientTests`HappyEyeballsConnector.connectToNewTargets(self=0x0000000100b94370) at HappyEyeballs.swift:474:9
    frame #24: 0x00000001070f73f1 AsyncHTTPClientTests`HappyEyeballsConnector.processInput(input=resolverACompleted, self=0x0000000100b94370) at HappyEyeballs.swift:374:13
    frame #25: 0x00000001070fa54a AsyncHTTPClientTests`closure #3 in HappyEyeballsConnector.whenALookupComplete(self=0x0000000100b94370) at HappyEyeballs.swift:611:18
    frame #26: 0x00000001070fc945 AsyncHTTPClientTests`partial apply for closure #3 in HappyEyeballsConnector.whenALookupComplete(future:) at <compiler-generated>:0
    frame #27: 0x000000010705da90 AsyncHTTPClientTests`thunk for @escaping @callee_guaranteed (@guaranteed Result<(), Error>) -> () at <compiler-generated>:0
    frame #28: 0x00000001070fc991 AsyncHTTPClientTests`thunk for @escaping @callee_guaranteed (@guaranteed Result<(), Error>) -> ()partial apply at <compiler-generated>:0
    frame #29: 0x00000001070e27fa AsyncHTTPClientTests`closure #1 in EventLoopFuture.whenComplete(callback=0x00000001070fc980 AsyncHTTPClientTests`reabstraction thunk helper from @escaping @callee_guaranteed (@guaranteed Swift.Result<(), Swift.Error>) -> () to @escaping @callee_guaranteed (@in_guaranteed Swift.Result<(), Swift.Error>) -> ()partial apply forwarder with unmangled suffix ".18" at <compiler-generated>, self=0x0000000100c1ec40) at EventLoopFuture.swift:709:13
    frame #30: 0x00000001070e5175 AsyncHTTPClientTests`partial apply for closure #1 in EventLoopFuture.whenComplete(_:) at <compiler-generated>:0
    frame #31: 0x00000001070dfbe8 AsyncHTTPClientTests`EventLoopFuture._addCallback(callback=0x00000001070e5160 AsyncHTTPClientTests`partial apply forwarder for closure #1 () -> NIO.CallbackList in NIO.EventLoopFuture.whenComplete((Swift.Result<A, Swift.Error>) -> ()) -> () at <compiler-generated>, self=0x0000000100c1ec40) at EventLoopFuture.swift:634:16
    frame #32: 0x00000001070dfd42 AsyncHTTPClientTests`EventLoopFuture._whenComplete(callback=0x00000001070e5160 AsyncHTTPClientTests`partial apply forwarder for closure #1 () -> NIO.CallbackList in NIO.EventLoopFuture.whenComplete((Swift.Result<A, Swift.Error>) -> ()) -> () at <compiler-generated>, self=0x0000000100c1ec40) at EventLoopFuture.swift:641:18
    frame #33: 0x00000001070e25eb AsyncHTTPClientTests`EventLoopFuture.whenComplete(callback=0x00000001070fc980 AsyncHTTPClientTests`reabstraction thunk helper from @escaping @callee_guaranteed (@guaranteed Swift.Result<(), Swift.Error>) -> () to @escaping @callee_guaranteed (@in_guaranteed Swift.Result<(), Swift.Error>) -> ()partial apply forwarder with unmangled suffix ".18" at <compiler-generated>, self=0x0000000100c1ec40) at EventLoopFuture.swift:708:14
    frame #34: 0x00000001070fa35f AsyncHTTPClientTests`HappyEyeballsConnector.whenALookupComplete(future=0x0000000100b941c0, self=0x0000000100b94370) at HappyEyeballs.swift:609:11
    frame #35: 0x00000001070f79dc AsyncHTTPClientTests`HappyEyeballsConnector.beginDNSResolution(self=0x0000000100b94370) at HappyEyeballs.swift:432:9
    frame #36: 0x00000001070f6fa0 AsyncHTTPClientTests`HappyEyeballsConnector.processInput(input=resolve, self=0x0000000100b94370) at HappyEyeballs.swift:322:13
    frame #37: 0x00000001070f6e2c AsyncHTTPClientTests`closure #1 in HappyEyeballsConnector.resolveAndConnect(self=0x0000000100b94370) at HappyEyeballs.swift:308:18
    frame #38: 0x00000001070f6e4c AsyncHTTPClientTests`partial apply for closure #1 in HappyEyeballsConnector.resolveAndConnect() at <compiler-generated>:0
    frame #39: 0x00000001070d0bec AsyncHTTPClientTests`thunk for @escaping @callee_guaranteed () -> () at <compiler-generated>:0
    frame #40: 0x00000001070d9541 AsyncHTTPClientTests`partial apply for thunk for @escaping @callee_guaranteed () -> () at <compiler-generated>:0
    frame #41: 0x00000001070d0c0c AsyncHTTPClientTests`thunk for @escaping @callee_guaranteed () -> (@out ()) at <compiler-generated>:0
    frame #42: 0x00000001070d67f1 AsyncHTTPClientTests`partial apply for thunk for @escaping @callee_guaranteed () -> (@out ()) at <compiler-generated>:0
    frame #43: 0x00000001070d0c57 AsyncHTTPClientTests`closure #3 in SelectableEventLoop.run(task=0x00000001070d67e0 AsyncHTTPClientTests`partial apply forwarder for reabstraction thunk helper from @escaping @callee_guaranteed () -> (@out ()) to @escaping @callee_guaranteed () -> () at <compiler-generated>) at EventLoop.swift:873:25
    frame #44: 0x00000001070d6771 AsyncHTTPClientTests`partial apply for closure #3 in SelectableEventLoop.run() at <compiler-generated>:0
    frame #45: 0x00000001070484c6 AsyncHTTPClientTests`thunk for @callee_guaranteed () -> (@error @owned Error) at <compiler-generated>:0
    frame #46: 0x00000001070d679b AsyncHTTPClientTests`thunk for @callee_guaranteed () -> (@error @owned Error)partial apply at <compiler-generated>:0
    frame #47: 0x00000001070cc4a9 AsyncHTTPClientTests`closure #1 in withAutoReleasePool<T>(execute=0x00000001070d6780 AsyncHTTPClientTests`reabstraction thunk helper from @callee_guaranteed () -> (@error @owned Swift.Error) to @escaping @callee_guaranteed () -> (@out (), @error @owned Swift.Error)partial apply forwarder with unmangled suffix ".23" at <compiler-generated>) at EventLoop.swift:587:13
    frame #48: 0x00000001070d9576 AsyncHTTPClientTests`partial apply for closure #1 in withAutoReleasePool<A>(_:) at <compiler-generated>:0
    frame #49: 0x00007fff6e8acf0e libswiftObjectiveC.dylib`ObjectiveC.autoreleasepool<A>(invoking: () throws -> A) throws -> A + 46
    frame #50: 0x00000001070cc440 AsyncHTTPClientTests`withAutoReleasePool<T>(execute=0x00000001070d6780 AsyncHTTPClientTests`reabstraction thunk helper from @callee_guaranteed () -> (@error @owned Swift.Error) to @escaping @callee_guaranteed () -> (@out (), @error @owned Swift.Error)partial apply forwarder with unmangled suffix ".23" at <compiler-generated>) at EventLoop.swift:586:16
    frame #51: 0x00000001070cfbd5 AsyncHTTPClientTests`SelectableEventLoop.run(self=0x0000000100b91430) at EventLoop.swift:872:21
    frame #52: 0x00000001070d327b AsyncHTTPClientTests`closure #1 in static MultiThreadedEventLoopGroup.setupThreadAndEventLoop(t=(pthread = 0x0000700007b49000), initializer=0x00000001070d9000 AsyncHTTPClientTests`partial apply forwarder for reabstraction thunk helper from @escaping @callee_guaranteed (@in_guaranteed NIO.NIOThread) -> (@out ()) to @escaping @callee_guaranteed (@guaranteed NIO.NIOThread) -> () at <compiler-generated>, self=0x00000001074bf738, lock=(mutex = 0x0000000100e01950), _loop=0x0000000100b91430, loopUpAndRunningGroup=0x0000000100e03070) at EventLoop.swift:1039:23
    frame #53: 0x00000001070d90da AsyncHTTPClientTests`partial apply for closure #1 in static MultiThreadedEventLoopGroup.setupThreadAndEventLoop(name:initializer:) at <compiler-generated>:0
    frame #54: 0x00000001070d382f AsyncHTTPClientTests`thunk for @escaping @callee_guaranteed (@guaranteed NIOThread) -> () at <compiler-generated>:0
    frame #55: 0x000000010717c381 AsyncHTTPClientTests`partial apply for thunk for @escaping @callee_guaranteed (@guaranteed NIOThread) -> () at <compiler-generated>:0
    frame #56: 0x000000010717c73b AsyncHTTPClientTests`closure #1 in static NIOThread.spawnAndRun(p=(_rawValue = 0x0000000100e0aba0 -> 0x00007fff9b424cf0 libswiftCore.dylib`InitialAllocationPool + 2808)) at Thread.swift:104:13
    frame #57: 0x000000010717c7b9 AsyncHTTPClientTests`@objc closure #1 in static NIOThread.spawnAndRun(name:body:) at <compiler-generated>:0
    frame #58: 0x00007fff6edfedaa libsystem_pthread.dylib`_pthread_start + 125
    frame #59: 0x00007fff6edfb6af libsystem_pthread.dylib`thread_start + 15

And the reason is that HTTPClient.resolve() computes a negative NIODeadline which is not legal (UInt64)

cancel should be done with an outbound user event

async-http-client/Sources/AsyncHTTPClient/HTTPHandler.swift

Lines 398 to 402 in e9d0dd2

 self.lock.withLock { 

 if !cancelled { 

 cancelled = true 

 channel?.pipeline.fireUserInboundEventTriggered(TaskCancelEvent()) 

 }

the above code would be better done with an outbound user event, so something like

channel.triggerOutboundUserEvent(CancelEvent)

as an added benefit, that returns a promise so we can know if that was successful or not. The promise will be failed automatically if that has been handled before or the channel is already closed.

request body streaming

apart from response body streaming, we should also be able to stream request bodies.

Requests are executed only synchronously

I'm writing a simple HTTP test client that communicates with Apache Tomcat server on localhost. It operates with simple HEAD requests.
If I execute request the suggested way it does effectively nothing

let httpClient = HTTPClient(eventLoopGroupProvider: .createNew)

...

var request = try HTTPClient.Request(url: "http://localhost:8080/...", method: .HEAD)
request.headers.add(name: "User-Agent", value: "Swift HTTPClient")
request.body = .none
httpClient.execute(request: request).whenComplete() { result in
    switch result {
    case .failure(let error):
        // process error
        print("ERR \(error)")
    case .success(let response):
        if response.status == .ok {
            // handle response
            print("RESP: \(response)")
        } else {
            // handle remote error
            print(response.status)
        }
    }
}

Callback never called.

But if I wait for the result it will complete.

if let result = try? httpClient.execute(request: request).wait() {
    print(result.status)
}
// prints ok

What am I doing wrong? Is it possible that event loop is not running?

I'm using Swift 5.1.2 in Arch Linux.

Connection pool design advice

I’m seeking for design advice on how to build a connection pool, I would like to spend some time implementing it and hopefully contribute them if I get good results.
I don't know exactly where to start so I thought it might be good to expose what I understand on how the connection pool should work, and show design ideas I found by exploring already existing implementations.

First, the general concept of the connection pool: on each new request, the client asks its connection pool for a connection to the server. When it receives a new connection request, the pool has essentially three choices:

Use an already opened, unused connection
Open a new connection if no other connection is available
If there are already too many concurrent connections to the server, wait until an already opened connection becomes available
In practice the connection is a NIO Channel that is either directly a TCP connection in the case of HTTP/1.1, or a stream for an HTTP/2 multiplexed connection.

After some research, I stumbled upon the Apache Foundation HttpClient. It uses an HttpClientConnectionManager interface that lets the user provide the manager they wish. A PoolingHttpClientConnectionManager class implements this interface.
I wonder wether the principle of a user provided connection manager makes sense for the Swift NIO HTTP client? In the case of the Apache, I see the other class that implements HttpClientConnectionManager is BasicHttpClientConnectionManager, a connection manager that maintains only a single active connection at a time, but if I understand it correctly, this manager purpose is to enable to have one client per thread, which doesn’t seem appliable/useful with NIO? We can also imagine concurrent yet unpooled connection management, which is how the client currently works, but I don’t see the advantage of this over a pooled connection management?

Speaking more specifically about the connection pool, I have a few questions:

How to give back a connection to the pool? Should we pass it as an argument to a pool method (such as this) , or could we have a design where the pool passes an object (maybe a promise?) to the client on which it calls the appropriate method when it has finished leasing the connection?
How should we implement the thread safety of the pool? The already existing APIs based on Future/Promises should make this easy for many parts, but the data structure storing the connections must still be protected. Regarding the threading model of NIO, is this a good practice to hold a reference to a specific event loop and to use .execute to ensure what we want is executed on the right thread?
Speaking of the data structure storing the connections, could this be as simple as a dictionary that keys the connections per host/scheme?
I saw that some pools have a global cleanup delay where the manager will loop over all opened connections to see which ones have been idle for too long in order to evict them. How does this « global cleanup interval » strategy compare to scheduling timeouts per connection using NIO tasks? From what I see, for instance with the IdleStateHandler, NIO doesn’t hesitate to schedule tasks frequently, so I guess the performance impact isn’t too high?
Some pool designs enable specifying a delay after which a connection request is failed if the pool didn’t provide one in time, is this something we would also want for a pool?

Thank you!

Connect to IP but send a user-specified hostname in the SNI field [was: Do we have feature like curl's `--resolve` option?]

With curl's --resolve option, it should supports IP based SNI.
Ref https://curl.haxx.se/docs/manpage.html#--resolve

Revisit settings/configuration Timeout type naming

Hi there, was skimming the APIs a little bit and found the configuration type names a bit weird.

Specifically, the fields of Configuration using short names looks fun, but is also quite confusing imho, e.g. Timeout:

        var config = HTTPClient.Configuration()
        config.timeout = HTTPClient.Timeout() // this is not "a" Timeout, but configuration

        // a) rename? TimeoutConfiguration
        let t = HTTPClient.TimeoutConfiguration()
        // b) move? 
        let t = HTTPClient.Configuration.Timeout()

The types are nested in HTTPClient which makes it read like "this is the http client timeout", which it is not, it is just the settings what time timeouts shall be.

Looking at types:

public struct Configuration {
    public var tlsConfiguration: TLSConfiguration?
    public var followRedirects: Bool
    public var timeout: Timeout
    public var proxy: Proxy?
}

Timeout should likely be called TimeoutConfiguration; WDYT?

Proxy is likely fine, since that indeed is the value of the proxy to be used... though tbh future proof would be to make it a Configuration as well IMHO (in case it'd get per destination proxying or not etc)

remove all deprecations

Whilst deprecations in the 1.0.0-alpha.X time frame aren't strictly necessary, they make it much easier for our users to adopt the latest alpha.

Before the 1.0.0 release however, we should remove all of them because we don't want to start a new major release with deprecated API in there :).

thread sanitizer sees a threading issue in NIOHTTPClientTests.SwiftHTTPTests testUploadStreamingBackpressure

hey @artemredkin , nio release tools just ran and TSan found this:

Test Case '-[NIOHTTPClientTests.SwiftHTTPTests testUploadStreamingBackpressure]' started.
==================
WARNING: ThreadSanitizer: data race (pid=88805)
  Read of size 8 at 0x7b08000092f0 by main thread:
    #0 implicit closure #1 in SwiftHTTPTests.testUploadStreamingBackpressure() <compiler-generated> (swift-nio-http-clientPackageTests:x86_64+0x631e2b)
    #1 partial apply for implicit closure #1 in SwiftHTTPTests.testUploadStreamingBackpressure() <compiler-generated> (swift-nio-http-clientPackageTests:x86_64+0x631e8e)
    #2 thunk for @callee_guaranteed () -> (@unowned Int, @error @owned Error) <compiler-generated> (swift-nio-http-clientPackageTests:x86_64+0x50db4d)
    #3 thunk for @callee_guaranteed () -> (@unowned Int, @error @owned Error)partial apply <compiler-generated> (swift-nio-http-clientPackageTests:x86_64+0x631f0e)
    #4 closure #1 in XCTAssertEqual<A>(_:_:_:file:line:) <null> (libswiftXCTest.dylib:x86_64+0xb0b3)
    #5 @objc SwiftHTTPTests.testUploadStreamingBackpressure() <compiler-generated> (swift-nio-http-clientPackageTests:x86_64+0x632257)
    #6 __invoking___ <null> (CoreFoundation:x86_64h+0x49adb)

  Previous write of size 8 at 0x7b08000092f0 by thread T20:
    #0 didReceivePart(task:_:) in BackpressureTestDelegate #1 in SwiftHTTPTests.testUploadStreamingBackpressure() SwiftNIOHTTPTests.swift:416 (swift-nio-http-clientPackageTests:x86_64+0x632eaf)
    #1 protocol witness for HTTPClientResponseDelegate.didReceivePart(task:_:) in conformance BackpressureTestDelegate #1 in SwiftHTTPTests.testUploadStreamingBackpressure() <compiler-generated> (swift-nio-http-clientPackageTests:x86_64+0x6333e4)
    #2 TaskHandler.channelRead(context:data:) HTTPHandler.swift:405 (swift-nio-http-clientPackageTests:x86_64+0x5be25b)
    #3 protocol witness for _ChannelInboundHandler.channelRead(context:data:) in conformance TaskHandler<A> <compiler-generated> (swift-nio-http-clientPackageTests:x86_64+0x5c3608)
    #4 ChannelHandlerContext.invokeChannelRead(_:) ChannelPipeline.swift:1328 (swift-nio-http-clientPackageTests:x86_64+0x3a88c0)
    #5 ChannelHandlerContext.fireChannelRead(_:) ChannelPipeline.swift:1141 (swift-nio-http-clientPackageTests:x86_64+0x3af8d6)
    #6 HTTPDecoder.didReceiveBody(_:) HTTPDecoder.swift:493 (swift-nio-http-clientPackageTests:x86_64+0x54612a)
    #7 protocol witness for HTTPDecoderDelegate.didReceiveBody(_:) in conformance HTTPDecoder<A, B> <compiler-generated> (swift-nio-http-clientPackageTests:x86_64+0x54d11e)
    #8 BetterHTTPParser.didReceiveBodyData(_:) HTTPDecoder.swift:142 (swift-nio-http-clientPackageTests:x86_64+0x53d529)
    #9 closure #2 in BetterHTTPParser.init(kind:) HTTPDecoder.swift:71 (swift-nio-http-clientPackageTests:x86_64+0x53a414)
    #10 @objc closure #2 in BetterHTTPParser.init(kind:) <compiler-generated> (swift-nio-http-clientPackageTests:x86_64+0x53a490)
    #11 c_nio_http_parser_execute c_nio_http_parser.c:2041 (swift-nio-http-clientPackageTests:x86_64+0x307c7e)
    #12 c_nio_http_parser_execute_swift CNIOHTTPParser.h:26 (swift-nio-http-clientPackageTests:x86_64+0x550078)
    #13 closure #1 in BetterHTTPParser.feedInput(_:) HTTPDecoder.swift:348 (swift-nio-http-clientPackageTests:x86_64+0x5421dc)
    #14 partial apply for closure #1 in BetterHTTPParser.feedInput(_:) <compiler-generated> (swift-nio-http-clientPackageTests:x86_64+0x54ff4c)
    #15 thunk for @callee_guaranteed (@unowned UnsafeMutablePointer<http_parser>) -> (@unowned Int) <compiler-generated> (swift-nio-http-clientPackageTests:x86_64+0x5424d3)
    #16 partial apply for thunk for @callee_guaranteed (@unowned UnsafeMutablePointer<http_parser>) -> (@unowned Int) <compiler-generated> (swift-nio-http-clientPackageTests:x86_64+0x54ffb8)
    #17 BetterHTTPParser.withExclusiveHTTPParser<A>(_:) HTTPDecoder.swift:336 (swift-nio-http-clientPackageTests:x86_64+0x54092a)
    #18 BetterHTTPParser.feedInput(_:) HTTPDecoder.swift:341 (swift-nio-http-clientPackageTests:x86_64+0x540e60)
    #19 closure #1 in HTTPDecoder.feedInput(context:buffer:) HTTPDecoder.swift:602 (swift-nio-http-clientPackageTests:x86_64+0x54be77)
    #20 partial apply for closure #1 in HTTPDecoder.feedInput(context:buffer:) <compiler-generated> (swift-nio-http-clientPackageTests:x86_64+0x550601)
    #21 thunk for @callee_guaranteed (@unowned UnsafeRawBufferPointer) -> (@unowned Int, @error @owned Error) <compiler-generated> (swift-nio-http-clientPackageTests:x86_64+0x36330d)
    #22 thunk for @callee_guaranteed (@unowned UnsafeRawBufferPointer) -> (@unowned Int, @error @owned Error)partial apply <compiler-generated> (swift-nio-http-clientPackageTests:x86_64+0x5506a1)
    #23 ByteBuffer.withUnsafeReadableBytes<A>(_:) ByteBuffer-core.swift:530 (swift-nio-http-clientPackageTests:x86_64+0x37117d)
    #24 HTTPDecoder.feedInput(context:buffer:) HTTPDecoder.swift:601 (swift-nio-http-clientPackageTests:x86_64+0x54b9d3)
    #25 HTTPDecoder.decode(context:buffer:) HTTPDecoder.swift:609 (swift-nio-http-clientPackageTests:x86_64+0x54c0b5)
    #26 protocol witness for ByteToMessageDecoder.decode(context:buffer:) in conformance HTTPDecoder<A, B> <compiler-generated> (swift-nio-http-clientPackageTests:x86_64+0x54cd90)
    #27 closure #1 in ByteToMessageHandler.decodeLoop(context:decodeMode:) Codec.swift:545 (swift-nio-http-clientPackageTests:x86_64+0x3dad9d)
    #28 partial apply for closure #1 in ByteToMessageHandler.decodeLoop(context:decodeMode:) <compiler-generated> (swift-nio-http-clientPackageTests:x86_64+0x3e6f91)
    #29 ByteToMessageHandler.withNextBuffer(allowEmptyBuffer:_:) Codec.swift:507 (swift-nio-http-clientPackageTests:x86_64+0x3d8df4)
    #30 ByteToMessageHandler.decodeLoop(context:decodeMode:) Codec.swift:541 (swift-nio-http-clientPackageTests:x86_64+0x3da2dd)
    #31 ByteToMessageHandler.channelRead(context:data:) Codec.swift:613 (swift-nio-http-clientPackageTests:x86_64+0x3dd294)
    #32 protocol witness for _ChannelInboundHandler.channelRead(context:data:) in conformance ByteToMessageHandler<A> <compiler-generated> (swift-nio-http-clientPackageTests:x86_64+0x3de242)
    #33 ChannelHandlerContext.invokeChannelRead(_:) ChannelPipeline.swift:1328 (swift-nio-http-clientPackageTests:x86_64+0x3a88c0)
    #34 ChannelHandlerContext.invokeChannelRead(_:) ChannelPipeline.swift:1330 (swift-nio-http-clientPackageTests:x86_64+0x3a896b)
    #35 ChannelPipeline.fireChannelRead0(_:) ChannelPipeline.swift:824 (swift-nio-http-clientPackageTests:x86_64+0x39fc91)
    #36 SocketChannel.readFromSocket() SocketChannel.swift:144 (swift-nio-http-clientPackageTests:x86_64+0x4eb777)
    #37 BaseSocketChannel.readable0() BaseSocketChannel.swift:962 (swift-nio-http-clientPackageTests:x86_64+0x341412)
    #38 BaseSocketChannel.readable() BaseSocketChannel.swift:946 (swift-nio-http-clientPackageTests:x86_64+0x343505)
    #39 protocol witness for SelectableChannel.readable() in conformance BaseSocketChannel<A> <compiler-generated> (swift-nio-http-clientPackageTests:x86_64+0x34530f)
    #40 SelectableEventLoop.handleEvent<A>(_:channel:) EventLoop.swift:784 (swift-nio-http-clientPackageTests:x86_64+0x40d36c)
    #41 closure #1 in closure #1 in SelectableEventLoop.run() EventLoop.swift:833 (swift-nio-http-clientPackageTests:x86_64+0x40e9ec)
    #42 partial apply for closure #1 in closure #1 in SelectableEventLoop.run() <compiler-generated> (swift-nio-http-clientPackageTests:x86_64+0x41dab8)
    #43 thunk for @callee_guaranteed (@guaranteed SelectorEvent<NIORegistration>) -> (@error @owned Error) <compiler-generated> (swift-nio-http-clientPackageTests:x86_64+0x40eb28)
    #44 partial apply for thunk for @callee_guaranteed (@guaranteed SelectorEvent<NIORegistration>) -> (@error @owned Error) <compiler-generated> (swift-nio-http-clientPackageTests:x86_64+0x41db21)
    #45 Selector.whenReady(strategy:_:) Selector.swift:588 (swift-nio-http-clientPackageTests:x86_64+0x4c855e)
    #46 closure #1 in SelectableEventLoop.run() EventLoop.swift:828 (swift-nio-http-clientPackageTests:x86_64+0x40e6db)
    #47 partial apply for closure #1 in SelectableEventLoop.run() <compiler-generated> (swift-nio-http-clientPackageTests:x86_64+0x418f36)
    #48 thunk for @callee_guaranteed () -> (@error @owned Error) <compiler-generated> (swift-nio-http-clientPackageTests:x86_64+0x332d56)
    #49 thunk for @callee_guaranteed () -> (@error @owned Error)partial apply <compiler-generated> (swift-nio-http-clientPackageTests:x86_64+0x418fae)
    #50 closure #1 in withAutoReleasePool<A>(_:) EventLoop.swift:587 (swift-nio-http-clientPackageTests:x86_64+0x40831f)
    #51 partial apply for closure #1 in withAutoReleasePool<A>(_:) <compiler-generated> (swift-nio-http-clientPackageTests:x86_64+0x41da19)
    #52 autoreleasepool<A>(invoking:) <null> (libswiftObjectiveC.dylib:x86_64+0x2f0d)
    #53 SelectableEventLoop.run() EventLoop.swift:827 (swift-nio-http-clientPackageTests:x86_64+0x40d8ed)
    #54 closure #1 in static MultiThreadedEventLoopGroup.setupThreadAndEventLoop(name:initializer:) EventLoop.swift:1039 (swift-nio-http-clientPackageTests:x86_64+0x4136fa)
    #55 partial apply for closure #1 in static MultiThreadedEventLoopGroup.setupThreadAndEventLoop(name:initializer:) <compiler-generated> (swift-nio-http-clientPackageTests:x86_64+0x41d139)
    #56 thunk for @escaping @callee_guaranteed (@guaranteed NIOThread) -> () <compiler-generated> (swift-nio-http-clientPackageTests:x86_64+0x4140cb)
    #57 partial apply for thunk for @escaping @callee_guaranteed (@guaranteed NIOThread) -> () <compiler-generated> (swift-nio-http-clientPackageTests:x86_64+0x514f98)
    #58 closure #1 in static NIOThread.spawnAndRun(name:body:) Thread.swift:104 (swift-nio-http-clientPackageTests:x86_64+0x515487)
    #59 @objc closure #1 in static NIOThread.spawnAndRun(name:body:) <compiler-generated> (swift-nio-http-clientPackageTests:x86_64+0x515540)

  Location is heap block of size 32 at 0x7b08000092e0 allocated by main thread:
    #0 malloc <null> (libclang_rt.tsan_osx_dynamic.dylib:x86_64h+0x4e67a)
    #1 swift_slowAlloc <null> (libswiftCore.dylib:x86_64+0x2d73a8)
    #2 SwiftHTTPTests.testUploadStreamingBackpressure() SwiftNIOHTTPTests.swift:433 (swift-nio-http-clientPackageTests:x86_64+0x62f958)
    #3 @objc SwiftHTTPTests.testUploadStreamingBackpressure() <compiler-generated> (swift-nio-http-clientPackageTests:x86_64+0x632257)
    #4 __invoking___ <null> (CoreFoundation:x86_64h+0x49adb)

  Thread T20 (tid=2352443, running) created by main thread at:
    #0 pthread_create <null> (libclang_rt.tsan_osx_dynamic.dylib:x86_64h+0x2a9ed)
    #1 static NIOThread.spawnAndRun(name:body:) Thread.swift:90 (swift-nio-http-clientPackageTests:x86_64+0x514463)
    #2 static MultiThreadedEventLoopGroup.setupThreadAndEventLoop(name:initializer:) EventLoop.swift:1025 (swift-nio-http-clientPackageTests:x86_64+0x41306c)
    #3 closure #1 in MultiThreadedEventLoopGroup.init(threadInitializers:) EventLoop.swift:1070 (swift-nio-http-clientPackageTests:x86_64+0x4146a8)
    #4 partial apply for closure #1 in MultiThreadedEventLoopGroup.init(threadInitializers:) <compiler-generated> (swift-nio-http-clientPackageTests:x86_64+0x419d24)
    #5 thunk for @callee_guaranteed (@guaranteed @escaping @callee_guaranteed (@guaranteed NIOThread) -> ()) -> (@owned SelectableEventLoop, @error @owned Error) <compiler-generated> (swift-nio-http-clientPackageTests:x86_64+0x414837)
    #6 partial apply for thunk for @callee_guaranteed (@guaranteed @escaping @callee_guaranteed (@guaranteed NIOThread) -> ()) -> (@owned SelectableEventLoop, @error @owned Error) <compiler-generated> (swift-nio-http-clientPackageTests:x86_64+0x419da9)
    #7 Collection.map<A>(_:) <null> (libswiftCore.dylib:x86_64+0xfd6e)
    #8 MultiThreadedEventLoopGroup.__allocating_init(threadInitializers:) EventLoop.swift (swift-nio-http-clientPackageTests:x86_64+0x41414f)
    #9 MultiThreadedEventLoopGroup.__allocating_init(numberOfThreads:) EventLoop.swift:1059 (swift-nio-http-clientPackageTests:x86_64+0x413fff)
    #10 HTTPClient.init(eventLoopGroupProvider:configuration:) SwiftNIOHTTP.swift:33 (swift-nio-http-clientPackageTests:x86_64+0x5d0862)
    #11 HTTPClient.__allocating_init(eventLoopGroupProvider:configuration:) SwiftNIOHTTP.swift (swift-nio-http-clientPackageTests:x86_64+0x5d05bf)
    #12 SwiftHTTPTests.testUploadStreamingBackpressure() SwiftNIOHTTPTests.swift:423 (swift-nio-http-clientPackageTests:x86_64+0x62ef2f)
    #13 @objc SwiftHTTPTests.testUploadStreamingBackpressure() <compiler-generated> (swift-nio-http-clientPackageTests:x86_64+0x632257)
    #14 __invoking___ <null> (CoreFoundation:x86_64h+0x49adb)

SUMMARY: ThreadSanitizer: data race <compiler-generated> in implicit closure #1 in SwiftHTTPTests.testUploadStreamingBackpressure()
==================
Test Case '-[NIOHTTPClientTests.SwiftHTTPTests testUploadStreamingBackpressure]' passed (1.067 seconds).

Take Retry-After value into account on 3xx responses

A server might specify a minimum amount of time to wait before redirection using the Retry-After header, it should probably be taken into account by the client.

Changing HTTPClient.Request doesn't change other url components

You initialize HTTPClient.Request by providing a URL. This then sets the scheme and host. If you change one of these, the others don't update which can lead to unexpected behavior when the host on the request doesn't match your updated URL.

Support NIO1 (ver 0.0.0)

`(sync)Shutdown` should cancel (or wait for) all in-flight requests

At the moment, if you shut down HTTPClient with in-flight requests, they'll just never terminate. We should cancel all the inflight requests with a sensible error.

Option B would be to wait for all of them to complete but I think that'd be harder to implement and less useful.

programming model questions

Right now, the delegate's methods are calling out on essentially a random EventLoop.
I think we have the guarantee, that each delegate method will at least be called on the same EventLoop?

I realise that I proposed the change to have task.currentEventLoop (which can change) but I'm not sure anymore that was a good idea. I didn't quite realise that this means that the user has no idea on what EventLoop their delegate's methods will be called on.

Maybe it would be better to go back to having a fixed EventLoop on the task and hop over in case we get a Channel from a different EventLoop from a connection pool.

The other dimension where this matters is the EventLoopPreference, that also kind of means a random EventLoop. Maybe we should just have the user specify the EventLoop they want (if they care, random one otherwise) and use that. If we get a Channel from a different EventLoop then let's just hop(to:) the correct one.

This is just an idea really but I was thinking about the programming model earlier today and 'essentially a random EventLoop' sounds like it might lead to a lot of threading bugs?

Tagging the relevant people @adtrevor / @ianpartridge / @tanner0101 / @Lukasa / @artemredkin

Support gzip/deflate decompression

It would be great if NIOHTTPClient could provide the machinery to decompress (gzip and deflate) response bodies.

swift-nio-extras provides compression support: https://github.com/apple/swift-nio-extras/blob/master/Sources/NIOHTTPCompression/HTTPResponseCompressor.swift

redirects ignore EventLoop preference

@t089

Note, that redirects will ignore the preference parameter, won't they?

async-http-client/Sources/AsyncHTTPClient/HTTPClient.swift

Line 218 in 8fa354d

self.execute(request: newRequest, delegate: delegate, deadline: deadline)

Indeed looks like this part is missing?

Originally posted by @t089 in #79 (comment)

Investigate TLS mutual auth support

should we rename `didReceivePart` to `didReceiveBodyPart` ?

async-http-client/Sources/AsyncHTTPClient/HTTPHandler.swift

Line 322 in e0eeb04

 func didReceivePart(task: HTTPClient.Task<Response>, _ buffer: ByteBuffer) -> EventLoopFuture<Void> 

this is a little unclear, a part of what? I'd say we should rename to didReceiveBodyPart

EventLoop of Task and channel might not match

This test triggers a precondition failure: preconditionInEventLoop

func testRequestWithSharedEventLoopGroup() throws {
    let httpBin = HttpBin()
    let eventLoopGroup = MultiThreadedEventLoopGroup(numberOfThreads: 8)
    let httpClient = HTTPClient(eventLoopGroupProvider: .shared(eventLoopGroup))
    defer {
        try! eventLoopGroup.syncShutdownGracefully()
        httpBin.shutdown()
    }
    
    let response = try httpClient.get(url: "http://localhost:\(httpBin.port)/events/10/1").wait()
    XCTAssertEqual(.ok, response.status)
}

Note that I am using my own EventLoopGroup to run the HTTPClient.

The problem is in execute method. In the beginning of the function, we ask the event loop group for an EventLoop and associate this event loop with the Task. But, the ChannelBootstrap will ask the event loop group again for an event loop which will likely be a different one. So in the end the Channel will actually live on a different event loop than the Task.

Then in

self.delegate.didReceivePart(task: self.task, body).whenComplete { result in
                    self.handleBackpressureResult(context: context, result: result)
                }

The delegate will create a completion future from the eventLoop property of Task. So self.handleBackpressureResult will be executed on this event loop and thus context.read() will be executed on a different event loop than what the context expects -> 💥

Possible fixes

Make sure that event loop of Task is the same as the one from the channel.
and/or: make sure that delegate callbacks hop back to the EventLoop of the channel

convert error struct to enum

It's currently a struct with nested structs. An enum could be easier to work w/ in Swift.

Channel doesn't close on TaskCancelEvent/IdleStateEvent

Currently, the TaskHandler doesn't close the channel when it receives an IdleStateHandler.IdleStateEvent or a TaskCancelEvent.
This is something I'll probably fix in #31 because the way those events are handled will depend on wether or not a connection is currently leased.

Accept EventLoop in HTTPClient.execute()

In some cases it's desirable to be able to control which EventLoop a request is executed on. For instance, if uploading a file using NIO's NonBlockingFileIO we currently incur the cost of hopping between the event loop reading from disk and the event loop writing to the network on every chunk.

HTTPClient.execute could be updated as follows:

public func execute<T: HTTPClientResponseDelegate>(request: Request, delegate: T, eventLoop: EventLoop? = nil, deadline: NIODeadline? = nil) -> Task<T.Response> {
    let eventLoop = eventLoop ?? self.eventLoopGroup.next()
    ...
}

Investigate multipart support for request body

HTTPCookie conflicts with Foundation

The HTTPCookie type conflicts with the one from Foundation:

import NIOHTTPClient
import Foundation
let cookie: HTTPCookie // Error: 'HTTPCookie' is ambiguous for type lookup in this context

Maybe it should be made a subtype of HTTPClient, to have something such as HTTPClient.Cookie?

Implement backpressure test using embedded channel

Remove eventLoop property from Task

Currently, Task has an eventLoop stored property, and its channel must be on this same event loop.
In practice, eventLoop can be used by the http client delegate in the following ways:

Return an immediately succeeded future in the appropriate methods when they don’t want to apply back pressure.
Schedule backpressure related work on this event loop and return the backpressure control future.
In case backpressure work is done on another event loop (which might be more often the case than #2), hop the resulting backpressure future to eventLoop.

In these three cases, the goal to guarantee to the http client that it can assume all futures it gets from its delegate are already on the right event loop.

Potential shortcomings

Errors on user side

When applying back pressure, users getting futures from different event loops might forget to hop them back to the right event loop. On the other hand, if the client stopped relying on the assumption that futures returned from the delegate are already on the right event loop, and instead always took care of hopping them, this source of bugs would disappear entirely for a minimal performance cost (hopTo isn’t too heavy, and in many cases the user will need to use it either way, so why force them to do it when we could do this on the library side?).

Hard to compose with future evolutions

The eventLoop property might not compose very well with future evolutions of the client such as pooling (tldr: the associated event loop is hard to determine synchronously like Task needs, and Task might change of event loop during its lifetime):

It complicates the connection generation logic on the pool side. For instance a connection pool would likely contain a method such as getConnection(for request: HTTPClient.Request -> EventLoopFuture<Connection>, this is logical for it to return a future because a connection might not be instantly available, but it couldn’t be used as such with the current AsyncClient public API: execute() synchronously returns Task, but Task needs an eventLoop on initialization, which couldn’t be instantaneously determined due to getConnection being an async method. There would of be workarounds for this, but they would complicate the connection pool logic (maybe not a really good sign when two isolated components start to interact this way) and may also force creating new connections (instead of using released ones) just to satisfy the event loop identity constraint.
Pooling will also need a retry logic for when a connection dies too soon, this means associating the Task with a new channel that will probably not be on the same event loop as before except if we always made a new connection for retries, but it would be a ressource waste.

Proposed solution

Remove eventLoop property entirely, always hop futures provided by the delegate, additionally, update HTTPClientResponseDelegate backpressure controlling methods to make them return EventLoopFuture<Void>? instead of a non-optional EventLoopFuture<Void>.

This change would enable the following:

Reduces the risk of threading issue in delegate implementations
Makes Task less dependent on a specific event loop, making it easier to evolve in the future.
Delegate implementations that want to schedule work on the same event loop as channel can still make use of task.channel.eventLoop knowing it might evolve during the Task lifetime.
Delegate implementations that don't wish to exerce backpressure can express it more clearly/easily by returning nil instead of having to reference eventLoop just to return an immediately completed void future.

What do you think about this? Am I missing some important part?

Thank you

URL.uri removes trailing slash

Connect timeout defaults to 10 seconds, not indefinite

The documentation for HTTPClient.Configuration.timeout suggests that the default behavior is not to time out.

However, in practice the client will produce a NIO.ChannelError.connectTimeout(.nanoseconds(10000000000)) - ie. 10 seconds - by default if the connection is not accepted within that time.

Setting this to a larger (or smaller) value does seem to work however.

Stack overflow in promise chain when performing web scraping.

I've written a little scraper which can use both the foundation URL class and the HTTPClient classes. Requests go through many different proxies pulled from proxy lists on the web.

The HTTPClient classes are failing.
Do you want bugs yet, or should I check back in a couple months?

public API breakage checker

this is quite an active project where we expect quite a bit of API to be added. We need to be sure the public API isn't broken now that we've released 1.0.0.

CC @tomerd

Clarify semantics of `shutdown`

We should clarify what the semantics of shutdown are.

I think the following definition could make sense:

When a shutdown signal is received by the client

It keeps processing already inflight tasks (following redirect, downloading / uploading, etc ...)
It cancels tasks that are waiting to be processed
It errors new tasks that are being scheduled after the signal was received
Once all tasks have either failed or finished, if it owns the event loop it stops the event loop.

I think at the moment there is no queue of pending tasks, so point two is maybe not relevant yet.

client needs to tolerate futures from arbitrary EventLoops

For back-pressure we've got a bunch of callbacks where the user can return futures to make the client continue. Those futures, at the moment, are required to be on the 'right' EventLoop but the client should definitely not require this. Instead, the client should always hop(to:) user futures to the correct EL if it requires it for a certain operation.

In most cases, the user will happen to return futures from the right EL anyway so hop(to:) is pretty much free (it fast paths eventLoop === future.eventLoop and just returns).

Percent encoding is lost in the path

When you make a request using with percent encoding in the path, the encoding is lost causing the request to fail. This is because at HTTPHandler line 200 the path and query are used to get the url and these unencode the URL.

Investigate support for nio-transport-services

configurable channel options

Make the ClientBootstrap channel options configurable.

Make URLRequest as a parameter

Why it uses custom class HTTPRequest when there is a perfect URLRequest?

types should be nested in HTTPClient

HTTPClientConfiguration should be HTTPClient.Configuration
HTTPClientErrors should be HTTPClient.Errors

Cocoapod support

In order for other Cocoapod-enabled packages to adopt this one (such as SwiftyRequest), an async-http-client pod is needed.

Swift-NIO et. al is already available in pod form, so I believe this should be relatively straightforward.

Options to cancel a Task.

Hi, the AsyncHTTPClient currently has an interface, that makes the consumer choose between

shorthands to get/post/execute, where the consumer doesn’t have to implement the delegate but has no chance to cancel a task
full access to the task but one has to implement the delegate.

I would like to be able to cancel my request but I don’t want to implement the delegate protocol myself. Currently the ResponseAccumulator is an internal class and therefore can’t be used by a consumer.

Would it be possible to make the ResponseAccumulator public? Or can we build an execute method that doesn't need a delegate but returns a Task instead of a Response?

Whatever the preferred way is, if there is a decision on how to move forward, I'd be interested in creating a potential PR.

README code snippets not compiling

A few of the code snippets provided in README.md are not quite correct. E.g.

var request = try HTTPClient.HTTPRequest(url: "https://swift.org", method: .POST)

should be

var request = try HTTPClient.Request(url: "https://swift.org", method: .POST)

Declarative API

I wonder if when function builders have formally been added to Swift a declarative API with function builders could be made akin to this library: https://github.com/carson-katri/swift-request

Fire `didTransmitRequestBody` only when write future is filfilled

Improvement ideas

Hello and thank you for this package!
I have some ideas for improvement and would appreciate to get feedback on those! 🙂

Performance

Pooling

Users who make a large number of concurrent requests would strongly benefit from having a pool from which the HTTP client would pick tasks in order to optimise them.

Fallback to multiple concurrent connections on HTTP/1.1

When HTTP/2 multiplexing isn't available, the client could open multiple HTTP/1.1 connections to a same server just like what web browsers do.

DNS caching

SwiftNIO already lets users provide their own DNS resolver, which also allows implementing a local DNS cache. The client could let users to specify their own resolver, and why not provide a higher-level mechanism that would still take care of the actual resolving as the default resolver does, but ask the user to inject their persistence mechanism.

Redirection improvements

Currently, the client doesn't detect redirect cycles. The HTTP/1.1 standard, section 10.3 specifies that:

A client SHOULD detect infinite redirection loops, since such loops generate network traffic for each redirection.

Enabling additional redirection options would also be interesting, for instance:

Specifying a max redirection count
Allowing to choose between different redirection policies, for example:

public enum Policy {
                /// Refuse all redirections
                case refuseAll
                
                /// Accepts scheme replacement from `http` to `https`
                /// but only if URI stays the same
                case acceptSameURISecurityUpgrade
                
                /// Accept strictly the same domain
                case acceptSameDomain
                
                /// Accept also subdomains
                case acceptSameDomainAndSubdomains
                
                /// Accept any domain
                case acceptAll
                
                /// Choose manually
                case custom(handler: (URL)->EventLoopFuture<Bool>)
            }

If it ever makes sense for some users to access the content of the redirect pages, maybe provide a mechanism for this?

Configuration options

Request specific options

Just as timeout and redirect options can already be set per request, I hope all configuration options can be specified per request as long as it makes sense.

Output traffic to `.pcap` files

Use what's been done in apple/swift-nio-extras#46 to allow debug inspection of TLS traffic with tools such as Wireshark or tcpdump.
(Thanks @tanner0101 for showing me this)

Allow users to inject their custom behaviour / NIO handlers

If possible, this would provide greater customisation options without making the client overly complex for rarely used features.

Cannot cancel inside delegate

To cancel an in-progress request, the user can call cancel() on the HTTPTask.

However, if they are doing streaming there is no way to cancel from inside their HTTPResponseDelegate.

public protocol HTTPResponseDelegate : class {
    associatedtype Response

    func didTransmitRequestBody()
    func didReceiveHead(_ head: HTTPResponseHead)
    func didReceivePart(_ buffer: ByteBuffer)
    func didReceiveError(_ error: Error)
    func didFinishRequest() throws -> Response
}

Functions like didReceivePart() do not have access to the task, so they can't call cancel(). The delegate can't store a reference to the task because the delegate is passed in to execute() (for example), which happens before the task is created.

Redundant Naming

The name for this repo is rather redundant. Because the org is called swift-server, it can be safely assumed that Swift is the language that the projects will be built in. I suggest that the swift- prefix gets removed from the repo name. This makes the names more concise.

There are a couple of reasons I can think of that people might have against this, and why I think they aren't issues:

Discoverability: GitHub already handles this for us, by marking each repo with the language it is primarily written in.
Apple uses it: The Apple org also has repos that use other languages, such as C and C++. Swift Server is purely Swift.

no asynchronous shutdown method

async-http-client only has a synchronous shutdown method but it'd be great to have an asynchronous one too.

Investigate SNI support

should support HTTP/2

HTTP/2 support. Partial example can be found in swift-nio-examples/http2-client. This does HTTP/2 with 'prior knowledge', for this package we also need to support HTTP/2 upgrade

Fatal error if server responds with Payload Too Large during upload streaming

I've been playing with implementing a "max body size" policy in my NIO based web-server and noticed that this can trigger a fatalError in the state machine of ResponseAccumulator.

You can reproduce this as follows:

In HttpBinHandler change the .body handler to:

var response = self.resps.removeFirst()
guard response.body?.readableBytes ?? 0 < self.maxBodySize else {
    context.close(mode: .input).whenSuccess {
        context.write(self.wrapOutboundOut(.head(HTTPResponseHead(version: HTTPVersion(major: 1, minor: 1), status: .payloadTooLarge))), promise: nil)
        context.writeAndFlush(self.wrapOutboundOut(.end(nil)), promise: nil)
    }
    self.resps.prepend(response)
    return
}
response.add(body)
self.resps.prepend(response)

Here self.maxBodySize is a constant let maxBodySize: Int = 1_024 * 1_024 // 1 MB.

When the server receives more than the allowed body size it will close the input stream and send a payloadTooLarge response to the client. Not sure if this is the correct way to implement this, though?!?

Anyhow, with this change this test case will trigger a fatalError:

func testUploadStreamingTooMuch() throws {
    let httpBin = HttpBin()
    let httpClient = HTTPClient(eventLoopGroupProvider: .createNew)
    defer {
        try! httpClient.syncShutdown()
        httpBin.shutdown()
    }
    
    let body: HTTPClient.Body = .stream(length: nil) { writer in
        var bytesWritten: Int = 0
        let chunkSize = 1_024 * 100
        let totalSize = 1_024 * 1_024 * 10
        var buffer = ByteBufferAllocator().buffer(capacity: chunkSize)
        buffer.writeBytes([UInt8](repeating: 0, count: chunkSize))
        func step() -> EventLoopFuture<Void> {
            let f = writer.write(.byteBuffer(buffer))
            let eventLoop = f.eventLoop
            return f.flatMap {
                print("Uploaded: \(bytesWritten) / \(totalSize) (\(round(Float(bytesWritten) / Float(totalSize) * 1000) / 10)%)")
                bytesWritten += chunkSize
                if bytesWritten < totalSize {
                    return step()
                } else {
                    return eventLoop.makeSucceededFuture(())
                }
            }
        }
        
        return step()
    }
    
    var request = try Request(url: "http://localhost:\(httpBin.port)/post")
    request.method = .POST
    request.headers.add(name: "Transfer-Encoding", value: "chunked")
    request.body = body
    let response = try httpClient.execute(request: request).wait()
    let bytes = response.body.flatMap { $0.getData(at: 0, length: $0.readableBytes) }
    
    
    XCTAssertEqual(.payloadTooLarge, response.status)
    XCTAssertNil(bytes)
}

Fatal error: head already set: file /Users/tobias/Developing/swift-nio-http-client/Sources/NIOHTTPClient/HTTPHandler.swift, line 145
2019-06-24 09:43:23.371626+0200 xctest[99350:18225045] Fatal error: head already set: file /Users/tobias/Developing/swift-nio-http-client/Sources/NIOHTTPClient/HTTPHandler.swift, line 145

The reason for this is, that the server will actually send a second Bad Request response to the client. This response comes from the HTTPServerProtocolErrorHandler which receives a invalidEOFState error and translates this into a Bad Response error.

So I guess this might also indicate that the way I've implemented the max body size policy might be wrong if this leads to a protocol error?

You can check out a branch with the changes here: https://github.com/t089/swift-nio-http-client/tree/maxBodysize

figure out how delegates vs. retries

Once we have a channel pool (#105), we need to think about retries because there's a slightly higher chance that a request may fail.

We'll need to have the delegates take part in that retry dance. @adtrevor & @Lukasa discussed this here:

	self.lock.withLock {
	if !cancelled {
	cancelled = true
	channel?.pipeline.fireUserInboundEventTriggered(TaskCancelEvent())
	}