mafintosh / streamx

An iteration of the Node.js core streams with a series of improvements.
MIT License
226 stars 16 forks source link

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

Closed ehmicky closed 1 month ago

ehmicky commented 1 month ago

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 https://github.com/gulpjs/gulp/issues/2812

Solution

PR opened at #95.

mafintosh commented 1 month ago

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

mafintosh commented 1 month ago

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