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

Invalid error handling when piping streamx with node:stream #94

Closed
ehmicky opened this issue Oct 29, 2024 · 2 comments
Closed

Invalid error handling when piping streamx with node:stream #94

ehmicky opened this issue Oct 29, 2024 · 2 comments

Comments

@ehmicky
Copy link

ehmicky commented Oct 29, 2024

The following does not raise an uncaughtException.

import {Readable} from 'streamx';
import {Writable} from 'node:stream';

process.once('uncaughtException', (error) => {
  console.log('uncaughtException', error)
})

const readable = new Readable({
  read() {
    this.push('.')
    this.push(null)
  }
})
const writable = new Writable({
  write (data, encoding, cb) {
    cb(new Error('test'))
  }
})
readable.pipe(writable)

On the other hand, when using Readable from node:stream (instead of streamx), an uncaughtException is thrown. This is the correct behavior, else the error would be silently ignored, despite not being properly handled by users.

Additional information

node:stream .pipe() has some code to handle that specific situation.

https://github.com/nodejs/node/blob/8aac7da7d66fc0b1426d8bf1e7b3e7b7208885bd/lib/internal/streams/readable.js#L1020-L1028

This logic is absent from streamx.

Related bugs

Gulp (which uses streamx) currently relies on an uncaughtException being thrown. This is because it uses async-done, which uses node:domain, which intercepts uncaught exceptions.

This leads to simple Gulp pipelines not handling errors properly, as described in gulpjs/gulp#2812

Solution

PR opened at #95.

@mafintosh
Copy link
Owner

This is working as intended. Pipes are fully error handled as they should be.

@mafintosh
Copy link
Owner

Simply listen for errors on the stream if you wanna observe failures

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants