Only support native ReadableStream streams

[kanongil]

Support plan

• is this issue currently blocking your project? (yes/no): no
• is this issue affecting a production system? (yes/no): no

Context

• node version: 14+
• module version: 20.x
• environment (e.g. node, browser, native):
• used with (e.g. hapi application, another framework, standalone, ...):
• any other relevant information:

Handlers currently support returning any ReadableStream, even v1 streams from the readable-stream module. However using such legacy streams is incompatible with modern streams and requires very careful integration to support.

What problem are you trying to solve?

Simpler, more robust internal stream handling. Eg. we can rely on all streams having the .destroy() method and remove tests like:

hapi/test/transmit.js

Line 1465 in 18495f7

it('close() stream when no destroy() method', async () => {

Do you have a new or modified API suggestion to solve the problem?

Drop node v12 support (#4279) and fail when the returned stream matches value instanceof Stream && !(value instanceof ReadableStream). The current detection logic relies on duck typing.

One drawback is that we increase the reliance on the node runtime, which means that the Chrome "support" as introduced in #3946 will be gone (unless they find a way to properly polyfill ReadableStream). I don't know why this was patched in, since we don't do any kind of testing on that platform.

For a more graceful approach, we could pipe() legacy streams through a Passthrough stream when returned from a handler. Then internal processing could still rely on the stream being native.

[Nargonath]

Thanks @kanongil for bringing this up. Overall this seems like a good idea, I'm not sure though regarding breaking the support for the Chrome use case. Reading through the PR you mentioned their use case seemed legitimate. FWIU if we pipe the legacy streams through a Passthrough stream we'd be able to keep the Chrome support?

[kanongil]

This will still break this specific Chrome support, since it most likely polyfills the internal ReadableStream using the readable-stream module, which is outdated and does not expose all modern APIs. We would likely query properties that aren't there.

[hueniverse]

This smells like the kind of change I would have made. Not sure what's the "Chrome support" use case...

[devinivy]

Resolved with v21 #4386