Send close header if the request 'keep alive' is false

[enricenrich]

Following the NIOHTTP1Server example of SwiftNIO, we send the close header if the request "keep alive" is false.

Tested it in the conditions that it was throwing an error (attached below), with this change it's working as expected.

[ ERROR ] data received after completed connection: close message ([...]/Sources/Vapor/Logging/Logger+LogError.swift:32)

Fixes: https://github.com/vapor/http/issues/305

[enricenrich]

@tanner0101 Based on my test, it was enough just calling ctx.close(promise: nil). It's also how SwiftNIO does this in the example.

About putting this at the end of whenSuccess, I just tried it and it also works. 👍

[enricenrich]

@tanner0101 Makes sense to move the ctx.close(promise: nil) to the whenSuccess block at the end I think, specially because as you said the whenFailure block also calls it.

Do you want me to do the change?

[weissi]

Indeed @tanner0101 on top of the actual close (as in the PR) you should also set connection: close. NIO’s example server has an example for that: https://github.com/apple/swift-nio/blob/master/Sources/NIOHTTP1Server/main.swift#L36

But tbh that’s probably overly complicated, think setting connection: close iff request.isKeepAlive == false sounds good enough. The reason the code is more complicated is that it predates request.isKeepAlive.

[enricenrich]

@tanner0101 @weissi @joscdk I've just pushed two commits to implement the suggestions mentioned in this PR.

The ctx.close(promise: nil) is now called at the end of the whenSuccess block instead of the whenComplete block to avoid being called twice.

And on serializing the HTTPResponse we add the Connection header if needed.

[enricenrich]

@tanner0101 @weissi @joscdk Ok, I've pushed one more commit to simplify a bit the code as suggested. I think that it should be good now. 🙂

[joscdk]

@tanner0101 should we get this merged and tagged?

[penny-coin]

Hey @enricenrich, you just merged a pull request, have a coin!

You now have 1 coins.