http.IncomingMessage 'aborted' issues

[Jimbly]

Version

v16.13.0

Platform

Ubuntu 16.04.6 LTS

Subsystem

http

What steps will reproduce the bug?

It seems that in versions later than v15.5.0 the aborted event on incoming HTTP requests stopped working.

Disclaimer: I don't actually understand what's going on under the hood in Node.js, or fully grasp what's going on in http-proxy, or even quite understand how "new streams" work, only know "just enough to be dangerous" I guess =).

If this is an intentional change, it seems very dangerous, as it quietly creates pretty disastrous bugs. Specifically, I have a deployment using the popular http-proxy module, and due to this change, once Node v16 was deployed to production, the Node process was leaking resources (kernel TCP memory specifically) at a rate of hundreds of MBs per second, and doing so in a way that completely bricks the instance (cannot even SSH in to debug, since no TCP memory was available). If this kind of change/deprecation could be handled in what seems to be the usual way (display a console message as soon as someone attaches an event handler for aborted), this could have been caught in development/testing, instead of requiring days to track down. I'm sure I'll not be the last to run into this issue, and seems likely to affect any module that deals with http streaming (video requests from Chrome are created and aborted at a rate of 10s per second per browser).

Looking at the documentation it seems to indicate that aborted was "deprecated since v17.0.0", but should still actually be working. In practice it seems to have stopped working as of v15.5.0 (doing a quick diff of v15.4.0 where it worked, and v15.5.0 where it did not, points to #33035 being the culprit, but it looks like things may have changed since then as well).

I also found this deprecation notice, which seems related, however the text there seems relatively unhelpful and perhaps even incorrect. It seems to indicate the close event and readableEnded properties would provide similar functionality, but in practice it seemed that 'close' fires identically for aborted or non aborted requests. Through much poking the only way I could find to seemingly replicate the functionality of request.on('aborted') was to find the corresponding response object, and do a response.on('close') and in there check writableFinished. I'm guessing this is actually checking something subtly different, but I guess close enough to prevent http-proxy from burning to the ground. I've submitted a potential workaround to http-proxy at http-party/node-http-proxy#1559 however they have not had a release in 2 years so seems unlikely to get picked up.

For a very simple reproduction, load any video file over http-proxy, and seek backwards in Chrome. For example, run the following:

const httpProxy = require('http-proxy');

httpProxy.createProxyServer({
  target: 'http://files.dashingstrike.com.s3.amazonaws.com',
  changeOrigin: true,
}).listen(8080);

And then open http://localhost:8080/SplodyGameplayLong.mp4, and then seek backwards in the file. Chrome will abort hundreds of requests (each of which is requesting ~32MB), which, if not properly detected, will leak connections as well as hundreds of MBs of TCP kernel memory per second (quickly bricking your instance, making it unreachable even by SSH, as I found out -_-). On Linux, you can view TCP kernel memory used with cat /proc/net/sockstat (mem is count of 4K pages), or ss -atn state big if you have ss installed.

As for where to go from here...

Am I misunderstanding the documentation, and has aborted not actually been just deprecated but actually removed? If not, seems there's a bug here.

Whether it's already been removed or will be soon, can a deprecation warning be added (upon registering an 'aborted' handler) so developers get some kind of warning that points to the culprit? It's really hard to debug a bug that locks up your computer with nothing to go on -_-.

[jimmydief]

Thread with more discussion at #40775.

[bnoordhuis]

I believe this bug report is obsolete now that v16.x is out of support. A similar bug report for newer versions is tracked in #46666.