Malformed chunked response detector creates false-positive ‘premature close’ errors when HTTP Agent involved

[steveluscher]

When sockets get reused for multiple requests to endpoints that support chunked transfer encoding, the code in node-fetch that's designed to detect malformed chunked responses gets confused and throws false-positive 'premature close' errors.

Sockets get reused in such a way when you use an http.Agent or an https.Agent in ‘keep alive’ mode.

Reproduction

Steps to reproduce the behavior:

1. Add this logging to node-fetch (diff link)
2. Create an HTTPS Agent with keepAlive: true and maxSockets of 5.
3. Make 6 requests in a row for an endpoint that supports chunked transfer encoding

In that repro, 6 requests are made with a limit of 5 sockets in keep-alive mode. One of those sockets gets reused for the 6th request. The presence of the 6th request as a pending listener for that reused socket confuses the logic in node-fetch that is designed to detect corrupt chunked responses.

Here's a repro project that you can clone that does all of the above: https://github.com/steveluscher/repro-node-fetch-premature-close-with-agent

Expected behavior

No premature close error should be thrown, since the chunked responses are well-formed.

Your Environment

software	version
node-fetch	2.6.8-2.6.12
node	18+

Additional context

This bug is the root of solana-labs/solana-web3.js#1418.

cc/ @achingbrain.

[Yogu]

I needed to debug the issue in order to understand this issue, but I think I do now. Just posting this comment to re-phrase the issue in my words, maybe it helps someone else. You mentioned it in #1576 with this comment

If you folks are using an http.Agent with your connections in keepAlive mode with a fixed number of maxSockets, #1767 is the problem.

and this actually means "if these conditions are met, you will always encounter the issue" (so this issue here) regardless of chunk/content lengths (what was discussed in #1576)

The reasons seems to be: node-fetch assumes that the body 'data' event fires before response 'close' event. This is not true when an agent wants to reuse a connection where the request already completed, but the consumer has not read the body yet. Seems like node does not wait for the consumer to actually consume the response body and instead reads and buffers it.