vapor / http

🚀 Non-blocking, event-driven HTTP built on Swift NIO.
MIT License
238 stars 65 forks source link

Vapor should close the Channel when it receives an Error in the last ChannelHandler #334

Open weissi opened 5 years ago

weissi commented 5 years ago

In SwiftNIO it's generally a good idea that on errorCaught, the Channel should be closed to prevent leaking network connections that are in some error states. In Vapor this should be added to HTTPServerHandler. But this advice applies to any NIO ChannelPipeline: Generally the last handler (I usually call it the 'application handler') should close on unknown error.

Why? Generally, ChannelHandlers fire errors in cases where they don't know how to recover themselves. If another, later ChannelHandler knows how to recover, that ChannelHandler would consume the error. In other words: If an error reaches the end of the pipeline, nobody knew how to handle that error and closing the Channel is a good default.

This has always been true in SwiftNIO but in NIO 2 this will become especially important because ByteToMessageDecoders (and the HTTP decoders) won't close the connections themselves anymore. Why is that? If the decoders themselves close the connection, then there's no way for the user to opt out.

The code to implement this is very straightforward:

public func errorCaught(ctx: ChannelHandlerContext, error: Error) {
    switch error {
    case let e as SomeErrorTypeIKnowHowToHandle where e.someCondition == expectedValue:
        // handle error
    case let e as SomeOtherErrorIKnowHowToHandle:
        // handle that error
    default:
        // log error?
        ctx.close(promise: nil)
    }
}