http server does not support pipelined requests

[t089]

Maybe I don't use HTTPClient correctly, but from the docs it seems that the idea is, to reuse a single client for multiple requests in order to reuse the existing connection.

However, there is an issue if you send out multiple requests one after another and/or if the server takes some undefined time to process the request(s). The underlying QueueHandler will associate responses from the server with responses from the client in the order they arrive. Yet if the server takes different time to process the events this assumption is not true anymore. In the end you will get unexpected responses for your requests.

In the description of QueueHandler you write:

This handler works great with client protocols that support pipelining.

Edit: I previously assumed that HTTP pipelining does not exists for HTTP/1.1. Of course, it does indeed. However it notes:

the server must send its responses in the same order that the requests were received

I don't think that HTTPClient and HTTPServer handle this very well.

So the problem is actually in the implementation of HTTPServer: it should obey the order of requests and only respond in the same order.

Example

See the following example:

import HTTP

let clientElg = MultiThreadedEventLoopGroup(numberOfThreads: 1)
let serverElg = MultiThreadedEventLoopGroup(numberOfThreads: 1)

let SERVER_PORT = 8338

final class HelloResponder: HTTPServerResponder {
    func respond(to request: HTTPRequest, on worker: Worker) -> EventLoopFuture<HTTPResponse> {
        let lastPathComponent = request.url.pathComponents.last!
        let randomWaitTime : Int = 5 + Int(arc4random_uniform(10))
        let p: Promise<String> = worker.eventLoop.newPromise()
        DispatchQueue.global().asyncAfter(deadline: .now() + .seconds(randomWaitTime) ) {
            worker.eventLoop.submit({ () -> String in
                let reply = "Hello, " + String(lastPathComponent) + "!"
                // print("replying to req /hello/\(lastPathComponent): \(reply)")
                return reply
            }).cascade(promise: p)
        }
        return p.futureResult.map({ string -> HTTPResponse in
            let body = Data(bytes: string.utf8)
            var res = HTTPResponse(status: .ok, body: body)
            res.headers.add(name: "Content-Type", value: "text/plain")
            return res
        })
    }
}


let server = try HTTPServer.start(hostname: "localhost", port: SERVER_PORT, responder: HelloResponder(),  on: serverElg) { (error) in
    print("server error: \(error)")
    }.wait()

do {
    let client = try HTTPClient.connect(hostname: "localhost", port: SERVER_PORT, on: clientElg).wait()
    
    var requestFutures = [Future<()>]()
    for i in 0..<5 {
        let req = HTTPRequest(url: "/hello/" + String(i))
        let f: Future<Void> = client.send(req)
            .map { (res)  in
                let text = String(data: res.body.data!, encoding: .utf8)!
                print("req to /hello/\(i) got response: \(text)")
            }.catch { error in
                print("req to /hello/\(i) got error: \(error)")
        }
        requestFutures.append(f)
    }
    
    Future<Void>.andAll(requestFutures, eventLoop: clientElg.eventLoop).whenSuccess {
        let _ = client.close()
    }
    
    try client.onClose.wait()
    try server.close().wait()
} catch {
    print("error: \(error)")
}

Here we send out 5 requests in a loop to a path /hello/:number. The server is expected to respond with: Hello, :number.

But because the server is a bit lazy, it waits a random number of seconds before it actually
sends out the response. This completely messes up the client. One output of this program is:

req to /hello/0 got response: Hello, 2!
req to /hello/1 got response: Hello, 3!
req to /hello/2 got response: Hello, 4!
req to /hello/3 got response: Hello, 0!
req to /hello/4 got response: Hello, 1!
Program ended with exit code: 0

[t089]

If you enable the pipeliningAssistance of swift-nio the responses arrive in the correct order!

// configure the pipeline
return channel.pipeline.configureHTTPServerPipeline(
    withPipeliningAssistance: true, /*false*/
    withServerUpgrade: upgrade,
    withErrorHandling: false

HTTPServer line 62

[t089]

Here is a test case that shows this problem:

func testPipelining() throws {
    struct SlowResponder: HTTPServerResponder {
        func respond(to request: HTTPRequest, on worker: Worker) -> EventLoopFuture<HTTPResponse> {
            let timeout : Int = 5 - Int(request.url.lastPathComponent)!
            let scheduled = worker.eventLoop.scheduleTask(in: .milliseconds(timeout * 100)) { () -> HTTPResponse in
                let res = HTTPResponse(
                    status: .ok,
                    body: request.url.lastPathComponent
                )
                return res
            }
            
            return scheduled.futureResult
        }
    }
    let worker = MultiThreadedEventLoopGroup(numberOfThreads: 1)
    let server = try HTTPServer.start(
        hostname: "localhost",
        port: 8080,
        responder: SlowResponder(),
        on: worker
    ) { error in
        XCTFail("\(error)")
        }.wait()
    
    let client = try HTTPClient.connect(hostname: "localhost", port: 8080, on: worker).wait()
    
    var responses = [String]()
    var futures : [Future<()>] = []
    
    for i in 0..<5 {
        var req = HTTPRequest(method: .GET, url: "/\(i)")
        req.headers.replaceOrAdd(name: .connection, value: "keep-alive")
        let resFuture = client.send(req).map({ res in
            let body = String(data: res.body.data!, encoding: .utf8)!
            responses.append(body)
        })
        futures.append(resFuture)
    }
    
    try Future<()>.andAll(futures, eventLoop: worker.eventLoop).wait()
    
    XCTAssertEqual([ "0", "1", "2", "3", "4" ], responses)
    
    try server.close().wait()
    try server.onClose.wait()
}

-[HTTPTests.HTTPTests testPipelining] : XCTAssertEqual failed: ("["0", "1", "2", "3", "4"]") is not equal to ("["4", "3", "2", "1", "0"]") - 

[t089]

Hello, is anybody acknowledging this issue? Am I missing something? It seems pretty serious to me. Especially with a service under load.

[tanner0101]

The reasoning behind not enabling pipelining assistance is that it has been mostly abandoned due to issues. See the wikipedia page for HTTP pipelining:

As of 2018, HTTP pipelining is not enabled by default in modern browsers, due to several issues including buggy proxy servers and HOL blocking.[2]

A much better alternative to HTTP/1 pipelining is to use HTTP/2 which supports proper multiplexing. That said, I think we could improve the current state of the HTTPServer by:

• allowing the user to optionally enable pipelining assistance
• providing a better error upon receiving pipelined requests if pipelining is not supported

Given the non-trivial amount of work this would require, I think this is out of scope for vapor/http 3.x. Until this is resolved in the next version, HTTP pipelining should be considered undefined behavior.

[vzsg]

For a practical workaround, I think you should forget that HTTPClient exists, and instead stick with URLSession – or it's Vapor wrapper Client, if you're inside a Vapor app.

[tanner0101]

@vzsg I renamed the issue since it was misleading. The issue is actually with HTTPServer not supporting pipelined requests.

[t089]

Thanks @tanner0101 for the write-up. I'm still a bit worried about my micro service running on the current Vapor 3 HTTP server without pipelining. As it is a micro service, it receives requests from other micro services written in other languages and using all kinds of HTTP clients. I cannot guarantee that none of them expects pipelining to work correctly. Have you reached out to swift-nio and clarified why you might have chosen to opt-out of their pipelining implementation?

I would be super happy about a patch release of Vapor that exposes the option to enable pipelining in NIOServerConfig.

[tanner0101]

@t089 after discussing with the NIO team, I think it might actually not be too hard to implement in a minor release. I'm going to take a crack at it today and I'll update you here.

[tanner0101]

@t089 I just put up PRs to add support to this package (#320) and vapor (vapor/vapor#1852). I'd love if you could test these out and verify they fix the issue you are having.

Update your Package.swift to use the http-pipelining branch for Vapor.

.package(url: "https://github.com/vapor/vapor.git", .branch("http-pipelining"),

You may also need to specify the http-pipelining branch for the HTTP package. I'm not sure if this is required, but to be on the safe side add this to your Package.swift.

.package(url: "https://github.com/vapor/http.git", .branch("http-pipelining"),

Update and regenerate your Xcode project.

swift package update
swift package generate-xcodeproj

Register a custom NIOServerConfig that specifies pipelining should be enabled.

services.register(NIOServerConfig.self) { c in 
    var config = NIOServerConfig.default()
    config.supportPipelining = true
    return config
}