Pipeline errors are not properly handled in Gulp v5

[ehmicky]

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

• 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.
• 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 (gulpjs/vinyl-fs#333 by @sttk) and to-through (gulpjs/to-through#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

What seems to be happening is:

• A stream in the pipeline errors
• This calls stream.destroy(error)
• Which itself calls stream.emit('error', error)
• However, for some reason, in Gulp v5 (not Gulp v4), the error event is not properly triggered on domain, which means async-done never completes.

[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) { ... })

[ehmicky]

I did some additional digging.

Background problem

In Node.js, source.pipe(dest) does not do proper error handling. In particular, any error in source is not propagated on dest. So dest does not properly end.

Node.js introduced the pipeline() and compose() methods to solve this.

So, actually, users should not do:

export default () => gulp.src(...)
  .pipe(...)
  .pipe(...)
  .pipe(...)

But:

export default () => pipeline(
  gulp.src(...),
  ...,
  ...,
  ...,
)

Or maybe:

export default () => gulp.src(...)
  .pipe(...)
  .compose(...)
  .compose(...)

Gulp v4 workaround

Unfortunately, the former pattern is documented everywhere by Gulp, and used by virtually all Gulp users. If any stream in the pipeline (except the last one) errors, this raises an uncaught exception at the process level.

Gulp 4 used to work around that by handling uncaught exceptions. It does this using node:domain (through async-done), which is a deprecated core API which can handle uncaught exceptions.

Gulp v5 bug

Gulp v5 switched from using node:stream to using a userland version of it called streamx. There is a bug in streamx which prevents uncaught exceptions from being emitted when piping streamx streams (such as gulp.src()) to node:stream streams (such as new Transform() in the example above).

I filed an issue mafintosh/streamx#94 and created a PR at mafintosh/streamx#95. If that PR is merged and released in a patch or minor release, Gulp should automatically get the fix, which would solve this issue.

[mcspr]

Also note https://github.com/orgs/gulpjs/discussions/2586
tbh, to someone not familiar with node, it is very surprising that the originally used method is not attempting to wrap the 'better' handler internally :/

Do you have an idea as to why the through2 continues to work, but not node:stream?

[ehmicky]

through2 is using readable-stream, which is itself an older version of node:stream. For me, through2 is not working either.

My PR on streamx was rejected, as apparently the bug (silently swallowing uncaught exceptions) is "intentional". They are recommending manually adding error event handlers on each stream that might fail, as opposed to having the library automatically propagating errors.

To me, it seems like the real problem is the switch to streamx. Users and Gulp plugins will provide streams using node:stream and use libraries expecting regular streams from node:stream, but Gulp provides streams using streamx, with slightly different behavior and shape. This will create bugs like this issue, and probably many more. Also, the benefits from streamx are not quite clear, especially as some behavior like "better error handling" is arguably actually worse.

But I understand that Gulp will probably not switch away from streamx. So, it seems to me the only workaround for users right now is to use pipeline() instead of .pipe().