Detecting client request aborts in nodejs >= 15.50 is broken #46666
Description
Version
v18.13.0
Platform
Linux notebook 5.15.79.1-microsoft-standard-WSL2 #1 SMP Wed Nov 23 01:01:46 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux
Subsystem
http
What steps will reproduce the bug?
'use strict';
const http = require('http');
let clientReq;
const server = http.createServer(function (req, res) {
req.on('aborted', function () {
console.log('ABORTED');
server.close();
});
req.on('end', () => {
console.log('DESTROY');
clientReq.destroy();
});
req.resume(); // read all client data
});
server.listen(0, () => {
clientReq = http.request({
method: 'POST',
port: server.address().port,
headers: { connection: 'keep-alive' }
}, (res) => {
});
clientReq.on('error', (err) => console.log('CLIENT ERROR', err));
clientReq.end();
});shamefully stolen from #40775 which was closed with no apparent fix
How often does it reproduce? Is there a required condition?
always
What is the expected behavior?
get notified of client abort
What do you see instead?
Not being notified of client abort
Additional information
Up until nodejs 15.5.0 requests had an aborted event that emitted whenever the client aborted the request.
This sadly doesn't work in recent node versions.
Reacting to client aborts can be an important thing to consider when you are proxing requests, doing longer processing before responding or even just cancelling longer database queries if we're no longer able to do anything with the response...
The current docs don't make it clear how to get notified of client aborts instead it's missleading and causing real production issues for several people.
I'd kindly request to:
- Keep (and actually bring back) the
abortedevent. - Clearly state that
closeis emitted once the body has been read and doesn't imply the client aborted the request or the actual http request being completed
If you don't want to bring back the aborted event, please include the recommended way to be notified of client aborts.
I'm happy to contribute myself if that would speed things up.
Affected projects
Connected to that a breaking change in 16.0.0 that caused the close event to be emitted after the body has been read, has affected projects that depended on the older behavior and were caught by surprise.
- Missing 'aborted' on IncomingMessage (v16 regression) #40775
- Undocumented breaking change on v16.0.0 #38924
- http.IncomingMessage 'aborted' issues #41117
The largest project I guess is affected would be webpack-dev-server. It depends on http-proxy-middleware which itself depends on http-proxy. (~14 million weekly downloads)
There is a workaround, but it looks like http-proxy is no longer being maintained
Current documentation
I can understand that the changes were made to align IncomingMessage to regular streams, but IMO the current state is rather dangerous for users. #40775 (comment)
The docs on http.IncomingMessage are very sparse and I would even say missleading.
Event: 'close'
Emitted when the request has been completed.
I can't blame people jumping to the conclusion that is only completed once the response handling has also completed
Event: 'aborted'
Deprecated since: v17.0.0, v16.12.0 -- Listen for 'close' event instead.
Emitted when the request has been aborted.
I'm currently running 18.13.0 which appears to never actually emit aborted although it should be according to the docs.
Also the replacement suggested is now fundamently different to a client abort.
It looks like the best way to determine if a client has actually aborted a request is to use
ServerResponse.writableFinished
Is true if all data has been flushed to the underlying system, immediately before the 'finish' event is emitted.
AFAIK there is nothing you can do on the request object. Relying on the response object to detect request aborts is counterintuitive IMO.
To actually come up with that workaround I guess your only real chance is first to run into the issue and finding any of these while searching for solutions