fix: do not abort when stream payload consumed #63
Conversation
| @@ -93,7 +93,7 @@ module.exports = fp( | |||
| if (raw.socket.destroyed) { | |||
| throw new Errors.SOCKET_CLOSED(reqId) | |||
| } else { | |||
| raw.once( | |||
| raw.socket.once( | |||
| 'close', | |||
| function () { | |||
| if (controllers.has(this)) { | |||
There was a problem hiding this comment.
I tried checking if the request was completed by doing
| if (controllers.has(this)) { | |
| if (!raw.completed && controllers.has(this)) { |
but that doesn't seem to work as intended for a http1.1 client like unidici.
Do we need some other check, as i'm not sure if it will work for http2 clients, as they can close their sending side of the socket while keeping the receiving side open?
There was a problem hiding this comment.
This check is a must, otherwise we can abort the request as soon as the transmission has finished (request payload received and parsed).
A keep-alive connection is independent of a request lifecycle; the keep alive is handled by the socket while the request lifecycle by the IncomingMessage API (or HTTPServerRequest for HTTP/2).
Let's add it, I believe your tests already covers these cases. Maybe one more for HTTP/2 can be helpful
addresses #62