Pipeline errors are not properly handled in Gulp v5

[ehmicky]

Before you open this issue, please complete the following tasks:

• [x] use the search bar at the top of the page to search this repository for similar issues or discussions that have already been opened.
• [ ] if you are looking for help from the gulp team or community, open a discussion.
• [ ] if you think there is a problem with the plugin you're using, open a discussion.
• [x] if you think there is a bug in our code, open this issue.

What were you expecting to happen?

When using gulp.src(...).pipe(stream).pipe(secondStream), any stream error should be shown by Gulp and complete the task (marking it as failed).

What actually happened?

stream errors in the pipeline are not shown by Gulp, and the task is shown as incomplete.

Please give us a sample of your gulpfile

// gulpfile.js
import gulp from 'gulp'
import {Transform, PassThrough} from 'node:stream'

export default () => gulp
  .src('./gulpfile.js')
  .pipe(new Transform({
    transform(file, encoding, done) {
      done(new Error('example'))
    },
    objectMode: true,
  }))
  .pipe(new PassThrough({objectMode: true}))

Terminal output / screenshots

With Gulp v5:

$ gulp
[01:46:48] Using gulpfile ~/Desktop/gulpfile.js
[01:46:48] Starting 'default'...
[01:46:48] The following tasks did not complete: default
Did you forget to signal async completion?

With Gulp v4, this worked correctly:

$ gulp
[01:45:39] Using gulpfile ~/Desktop/gulpfile.js
[01:45:39] Starting 'default'...
[01:45:39] 'default' errored after 15 ms
[01:45:39] Error: example
    at Transform.transform [as _transform] (file:///.../Desktop/gulpfile.js:8:12)
    at Transform._write (node:internal/streams/transform:171:8)
    at writeOrBuffer (node:internal/streams/writable:564:12)
    at _write (node:internal/streams/writable:493:10)
    at Writable.write (node:internal/streams/writable:502:10)
    at DestroyableTransform.ondata (/.../Desktop/node_modules/to-through/node_modules/readable-stream/lib/_stream_readable.js:619:20)
    at DestroyableTransform.emit (node:events:520:28)
    at DestroyableTransform.emit (node:domain:551:15)
    at addChunk (/.../Desktop/node_modules/to-through/node_modules/readable-stream/lib/_stream_readable.js:291:12)
    at readableAddChunk (/.../Desktop/node_modules/to-through/node_modules/readable-stream/lib/_stream_readable.js:278:11)

Please provide the following information:

• OS & version [e.g. MacOS Catalina 10.15.4]: Ubuntu 24.04
• node version (run node -v): 22.3.0
• npm version (run npm -v): 10.8.1
• gulp version (run gulp -v): 5.0.0

Additional information

This bug only happens when the stream that errors is not the last line in the pipeline. This means .pipe() must be called more than once.

It seems like this bug is related to vinyl-fs (https://github.com/gulpjs/vinyl-fs/pull/333 by @sttk) and to-through (https://github.com/gulpjs/to-through/pull/9 by @coreyfarrell) switching to streamx. The problem seems to be happening inside async-done, specifically the following line:

https://github.com/gulpjs/async-done/blob/4a7efae92c90ae6358412f2dc759561f0cb94ccc/index.js#L31

The error event is not properly triggered on domain, which means async-done never completes.

Looking at the Node.js source code:

• domain.once('error') is triggered in Node.js here: https://github.com/nodejs/node/blob/eb54c54bf5f7c077ec094d27cc1d93cd318bde06/lib/domain.js#L539
• However, that event is not triggered if the event emitter (here the stream) has any error event listener: https://github.com/nodejs/node/blob/eb54c54bf5f7c077ec094d27cc1d93cd318bde06/lib/domain.js#L481

What seems to be happening is:

• A stream in the pipeline errors
• This calls stream.destroy(error)
• Which itself calls stream.emit('error', error)

Then, in Gulp v4:

• The .pipe() logic from readable-stream catches the error event, removes the event listener then calls stream.emit('error', error) again, here: https://github.com/nodejs/readable-stream/blob/c85db76d4c41f64fd082c9263c3a918bec6f38a0/lib/_stream_readable.js#L640.
• Since there are no more error event listeners, domain.once('error') is properly triggered.
• This results in async-done completing properly.

However, in Gulp v5:

• The .pipe() logic from streamx does something similar: https://github.com/mafintosh/streamx/blob/625ce37f624aa51cda95fa1bffdc3fae2ecd03a5/index.js#L274 and https://github.com/mafintosh/streamx/blob/625ce37f624aa51cda95fa1bffdc3fae2ecd03a5/index.js#L443
• However, it does not remove the error event listener.
• This means domain.once('error') is not triggered due to https://github.com/nodejs/node/blob/eb54c54bf5f7c077ec094d27cc1d93cd318bde06/lib/domain.js#L481
• Therefore async-done does not complete, and Gulp shows the task as incomplete.

[mcspr]

However, it does not remove the error event listener.

Would that mean the problem is with streamx and not gulp? If I understood the flow correctly, streamx wraps original 'object' stream with itself. Looking at this part https://github.com/mafintosh/streamx/blob/625ce37f624aa51cda95fa1bffdc3fae2ecd03a5/index.js#L267-L277

isStreamx(...) is obviously false for node:stream Transform obj, so the 2nd branch is used. Should it instead use the 1st one? Or, some other event setup entirely

I've also noticed that issue with node:stream. However, gulpfile with through2 does seem to work correctly

through2.obj((chunk, enc, done) => { ... }) // original 'readable-stream' object is returned, things seem to work ok

Replacing the above with streamx also finishes the task and shows the error (minor issue having tsc warning about a type mismatch)

streamx.Transform({transform(chunk, done) { ... })