Fix Stream Callback never being called in close #2905
Conversation
…esponse starts streaming
| @@ -50,13 +50,17 @@ final class HTTPServerHandler: ChannelInboundHandler, RemovableChannelHandler { | |||
| if !keepAlive { | |||
| context.close(mode: .output, promise: nil) | |||
| } | |||
| case .failure(ChannelError.ioOnClosedChannel): | |||
There was a problem hiding this comment.
quick question: why only when .ioOnClosedChannel? I would've thought that this needs to be done on any write error. And even if there's no write error but the connection goes away we should notify the user, right?
CC @Lukasa
There was a problem hiding this comment.
Shouldn't errorCaught(context:error:) function be triggered in all cases as well so the error can be bubbled up and handled?
There was a problem hiding this comment.
@0xTim the problem at hand isn't that Vapor doesn't know about the error, it's that the user doesn't learn about it, right? So I'm not sure why other errors aren't handled similarly. Not saying it's wrong, just don't quite follow
Co-authored-by: Dan Appel <Dan.appel00@gmail.com>
| if case .stream(let stream) = response.body.storage { | ||
| stream.callback(ErrorBodyStreamWriter(eventLoop: request.eventLoop, error: error)) | ||
| } |
There was a problem hiding this comment.
@weissi can you see any problem with this? I've taken a look at it and it seems to make sense we should call call errorCaught and trigger the stream callback if we have them
|
These changes are now available in 4.67.3 |
Fixes a bug where abandoned requests mean that the stream callback is never invoked. This happens when a client sends a request, the server starts processing it but hasn't constructed the
Responseand then the client closes early. The response is discarded, as is the stream callback so it never gets invoked. This fixes that issue