Stream is not destroyed on error

[szmarczak]

Instead, an error event is emitted:

decompress-response/index.js

Line 32 in a31bde7

stream.emit('error', error);

This causes getStream() to hang on Node.js 14.1.0.

[sindresorhus]

Which stream should be destroyed? I thought streams are automatically destroyed on error unless there's a handler.

[szmarczak]

I thought streams are automatically destroyed on error unless there's a handler.

The thing is that there is a handler. If there's an error of any kind, Got tries to read the response. But since the stream is not destroyed, it just hangs on Node.js >= 14.0.0.

I'd just replace

decompress-response/index.js

Lines 25 to 35 in a31bde7

	decompress.on('error', error => {
	// Ignore empty response
	if (error.code === 'Z_BUF_ERROR') {
	stream.end();
	return;
	}
	stream.emit('error', error);
	});
	const finalStream = streamPipeline(response, decompress, stream, () => {});

with

	const finalStream = streamPipeline(response, decompress, stream, error => {
		if (error) {
			if (error.code === 'Z_BUF_ERROR') {
				stream.end();
				return;
			}

			stream.destroy(error);
		}
	});

[sindresorhus]

Sounds good. You know streams better than me.

[szmarczak]

Nah, I just randomly stumbled upon a Got issue and found a culprit :P But it doesn't hang on Node.js 13 so probably something has changed under the hood.

[szmarczak]

I think it's either https://github.com/nodejs/node/pull/31182/files or https://github.com/nodejs/node/pull/32158/files or maybe both. But it's nice to see stream improvements.