http: emit aborted event on requests - #64693
Conversation
Sorry, something went wrong.
|
Review requested:
|
Sorry, something went wrong.
Fixes: nodejs#46666 Signed-off-by: Simon Hanna <simon.hanna@aeb.com>
|
I don't really understand; aborted is deprecated so it shouldn't be used. |
Sorry, something went wrong.
|
Aborted is currently never emitted and the docs don't really mention a way to react to client aborts. If you wish to keep the aborted deprecation, all the related code might be removed, because it hasn't been working since node 15. If that's the case I'll drop the code changes and just request merging the docs. |
Sorry, something went wrong.
| --> | ||
|
|
||
| > Stability: 0 - Deprecated. Listen for `'close'` event instead. | ||
| > Stability: 0 - Deprecated. Use the `'close'` event on the response. If it was aborted `writableFinished` will be false. |
There was a problem hiding this comment.
What does this have to do with the deprecation?
Sorry, something went wrong.
There was a problem hiding this comment.
If the deprecation should be kept, then afaik this is the only way to know if a request was aborted or not.
Sorry, something went wrong.
Reviewers
At least 1 approving review is required to merge this pull request.
Currently the aborted event is never emitted on the request object. This merge request fixes that by ensuring the abort event is emitted on the request if the server hasn't finished reading the request or the response sending the response hasn't finished yet. It also improved the documentation to make it clear when close and abort events are emitted.
Fixes: #46666
I would welcome reverting the deprecation from the req aborted event, but didn't do that here. IMO it's much nicer for devs to be able to listen for client aborts in a single place (the request object).