[bug] POST -> Premature close stream

[lmeyerov]

🐛 Bug Report

Cross-posting of an unclear got <- arrow stream -> fastify bug: sindresorhus/got#1556

Any pointers as we try to refine this one would be appreciated. While we finally have a user input that is deterministically reproducible with our broader system, it's going to be awhile to keep chiseling away at a tiny reproduction. Our guess is it is how we are handling sockets or eos (esp. given changes around eos in the ecosystem), but there's a lot going on with the dependencies here so we're quite puzzled right now.

Your Environment

• node version: 14.14
• fastify version: 3.7.0
• os: Ubuntu 18.04 LTS
• We are generally up-to-date within 1-3mo, and this cropped up in the last month, so potentially fastify 2-5mo back, with our main changes here around keeping up w/ got/fastify

[mcollina]

This needs a full example to reproduce. There can be many cases - likely there is something off in the Node.js stream implementation of arrow.

[lmeyerov]

Thanks! We're working on narrowing + extracting , starting with identifying if a version change of got/fastify/arrow, and then to more precise reproduction. If anything is sparking ideas meanwhile, would be appreciated.

[lmeyerov]

As a brief update:

• We updated to fastify 3.9.2, just in case
• We updated node 14.14 -> 14.15.2 b/c stream: fix regression on duplex end nodejs/node#35941
• We narrowed down the conditions to:

Server:

fastify.post('endpoint',
   async function()  {
    stream = pump(..., function (error) { logger.info('done 1'); })
    reply....compress(stream)
    await(eos(stream))
    logger.info('done 2')
  })

Client:

got.extend({'method': 'POST', 'timeout': 5, ...})('endpoint')

Three interesting things in the above:

1. The await(eos(stream)) + pump finish fast, yet 5s later, the Timeout propagates back
2. If we change reply....compress(stream) to reply....send(stream), it works
3. This is part of a keepalive got http socket, and the client does 2 successive calls like this, w/ only the 2nd failing. Setting http({keepalive: false}) (so I believe a new socket?) does not prevent a 2nd client call from failing.

We're slowly whittling it down, including to a standalone test case, but surprising already.

[mcollina]

Can you make a runnable example?

[lmeyerov]

Yes, we need to get the hotfix out with the workaround (dropping fastify-compress <> got for streams), and then will revisit

[lmeyerov]

@mcollina I think we're good to close this

More of a writeup here: sindresorhus/got#1556 . There's a lingering fastify-compress-relevant issue on why br is getting picked despite encodings order, so we'll be digging into that on one of our next perf sweeps.