Could util:isStream recognize more varieties of node streams?

[schantaraud]

Hi,

util:isStream only returns 'node' if the input is an instance of stream.Readable, but there are many implementations of such streams that although compatible don't share the same prototype.

One of those cases is Fastify's mock HTTP requests that very conveniently allows tests to run without actually calling server.listen. It took me a long time to understand why all my encryption-enabled unit tests were failing with OpenPGP.js throwing "Error during parsing. This message / key probably does not conform to a valid OpenPGP format", as the same code worked fine outside of Jest.

Piping all requests to a Passthrough eventually fixed my problem, but it would be nice to just let OpenPGP recognize all compatible streams as such.

Could we change the test to something along the lines of:

if (NodeReadableStream &&
   (NodeReadableStream.prototype.isPrototypeOf(input) ||
    input.readable || typeof input._read === 'function')) {
    return 'node';
}

?

NodeReadableStream implies isNode so it seems safe to consider all compatible streams as being of the node kind.

Also (browser console):

> input = new ReadableStream()
> input.readable || typeof input._read === 'function'
> false

I'd be happy to submit a PR if we can agree on a good solution.

Cheers

[twiss]

Hey 👋
Do these mock streams generally implement the full API? We don't actually use the _read function itself, instead we do:

web-stream-tools/lib/node-conversions.js

Lines 21 to 42 in dc4b05e

	nodeToWeb = function(nodeStream) {
	return new ReadableStream({
	start(controller) {
	nodeStream.pause();
	nodeStream.on('data', chunk => {
	controller.enqueue(chunk);
	nodeStream.pause();
	});
	nodeStream.on('end', () => controller.close());
	nodeStream.on('error', e => controller.error(e));
	},
	pull() {
	nodeStream.resume();
	},
	cancel(reason) {
	nodeStream.pause();
	if (nodeStream.cancel) {
	return nodeStream.cancel(reason);
	}
	}
	});
	};

So I'd be slightly nervous to take _read as evidence that it's a stream. However, if by convention in Node.js, everyone who implements it also implements the events, I'd be fine with a PR implementing this, yeah.

[schantaraud]

This very popular library uses a similar method:

https://github.com/sindresorhus/is-stream/blob/3750505b0727f6df54324784fe369365ef78841e/index.js#L3-L6

https://github.com/sindresorhus/is-stream/blob/3750505b0727f6df54324784fe369365ef78841e/index.js#L14-L18

What about:

if (NodeReadableStream && // It is running in node
    input !== null && typeof input === 'object' &&
    typeof input.on === 'function' && // It has the function we need
    // The rest ensures it definitely is a readable stream
    typeof input.pipe === 'function' &&
    input.readable !== false &&
    typeof input._read === 'function' &&
    typeof input._readableState === 'object') {
    return 'node';
}