Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

stream: Readable iterator unhandled error when piping #28194

Closed
marcosc90 opened this issue Jun 12, 2019 · 2 comments
Closed

stream: Readable iterator unhandled error when piping #28194

marcosc90 opened this issue Jun 12, 2019 · 2 comments
Labels
stream Issues and PRs related to the stream subsystem.

Comments

@marcosc90
Copy link
Contributor

marcosc90 commented Jun 12, 2019

  • Version: 12.4
  • Platform: Ubuntu 18.04

When using asyncIterator on a piped stream, the error is not handled correctly.

const fs = require('fs');
const { PassThrough } = require('stream');

async function print() {

	const read = fs.createReadStream('file');
	const iterator = read.pipe(new PassThrough())

	for await (const k of iterator) {
		console.log(k);
	}
}

print()
  .then(() => console.log('done')) // never called
  .catch(console.log); // never called

In the above example, the .catch is not catching the error, and the script crashes.

I know, that I should catch the error of each stream, but if I do:

read.on('error', console.log);

The print function never resolves nor rejects. The only solution I've found is to emit the error to the piped stream.

async function print() {

	const read = fs.createReadStream('file');
	const stream = new PassThrough();
	
	read.on('error', (err) => stream.emit('error', err));

	const iterator = read.pipe(stream);

	for await (const k of iterator) {
		console.log(k);
	}
}

When you have multiple pipes, this can get very ugly. I don't know if this is the intended behaviour, but makes it hard & ugly to work with async iterators.

@Fishrock123 Fishrock123 added the stream Issues and PRs related to the stream subsystem. label Jun 13, 2019
@Fishrock123
Copy link
Contributor

I don't know for sure, but does stream.pipeline() solve this?

@marcosc90
Copy link
Contributor Author

marcosc90 commented Jun 13, 2019

No, it doesn't unfortunately, because the error is passed to the .pipeline callback, so the for await does not throw, and the print function resolves without iterating anything, instead of throwing.

async function print() {

	const read = fs.createReadStream('file');
	const stream = new PassThrough();

	const iterator = pipeline(read, stream, (err) => {
           // I get the error here
           // print will resolve, without iterating.
        });

	for await (const k of iterator) {
		console.log(k);
	}
}

marcosc90 added a commit to marcosc90/node that referenced this issue Dec 9, 2019
mcollina added a commit to mcollina/node that referenced this issue Dec 12, 2019
This changes makes all stream in a pipeline emit 'error' in
case of an abnormal termination of the pipeline. If the last stream
is currently being async iterated, this change will make the iteration
reject accordingly.

See: nodejs#30861
Fixes: nodejs#28194
MylesBorins pushed a commit that referenced this issue Dec 17, 2019
This changes makes all stream in a pipeline emit 'error' in
case of an abnormal termination of the pipeline. If the last stream
is currently being async iterated, this change will make the iteration
reject accordingly.

See: #30861
Fixes: #28194

PR-URL: #30869
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
targos pushed a commit to targos/node that referenced this issue Apr 25, 2020
This changes makes all stream in a pipeline emit 'error' in
case of an abnormal termination of the pipeline. If the last stream
is currently being async iterated, this change will make the iteration
reject accordingly.

See: nodejs#30861
Fixes: nodejs#28194

PR-URL: nodejs#30869
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
codebytere pushed a commit that referenced this issue Jun 6, 2020
This changes makes all stream in a pipeline emit 'error' in
case of an abnormal termination of the pipeline. If the last stream
is currently being async iterated, this change will make the iteration
reject accordingly.

See: #30861
Fixes: #28194

PR-URL: #30869
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stream Issues and PRs related to the stream subsystem.
Projects
None yet
2 participants