Detecting client request aborts in nodejs >= 15.50 is broken

[simhnna]

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:

1. Keep (and actually bring back) the aborted event.
2. Clearly state that close is 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

• Fix 'aborted' detection on Node v15.5.0+ http-party/node-http-proxy#1559
• Missing 'aborted' on IncomingMessage (v16 regression) #40775 (comment)

[Niehno]

Ping: Is there any progress on this issue?

[rhinel]

any update ？

[laustdeleuran]

Shoutout to @simhnna for a really clearly formulated regression report. Wondering if there's any update from the project maintainers on attending to this?