Bring error unpipe overrides into core

[yocontra]

• Document this option

This will be off by default, but I think this is a common enough case that people would like this.

I envision the API looking like this:

gulp.src(['**/*.js'], {partial: true})

will block unpipe events downstream

[tomasdev]

This might be 100% relevant to this post. https://www.npmjs.org/package/gulp-plumber - the origins at #91

[yocontra]

@tomasdev Yeah it would be what gulp-plumber is doing but hidden behind a flag and in a different module that is slightly more generic

[mietek]

👍

[yocontra]

cc @chrisdickinson who is working on some pipelineError stuff that would also solve this

[phated]

More of his work on pipeline error can be found at https://github.com/chrisdickinson/node/commits/feature/adderrorhandler - I think we should implement the shim when there is a PR for this functionality

[phated]

This has officially been opened as a PR on node - please chime in at nodejs/node-v0.x-archive#8306

[floatdrop]

@contra until it is done, are you interested in moving gulp-plumber into gulpjs org?

[gagern]

The nodejs/node-v0.x-archive#8306 referenced above has been closed with the observation that the error event for streams is intended for fatal errors only, and that such fatal errors should always unpipe the pipeline. In the light of this, it seems to me as if the continued use of plumber or its semantics would indicate a deliberate deviation of how streams are meant to be used by those who maintain their implementation.

In nodejs/node-v0.x-archive#8306 (comment) @dominictarr also wrote:

If gulp just used a different error name for non fatal errors then think this would solve all the problems.

I wonder, could we let gulp stream stages emit non-fatal-error, problem, report or whatever events and use these for error reporting? Of course, such errors wouldn't get forwarded along the pipeline, so gulp would have to install a listener for each constituent step of a pipeline. In #1433 I've suggested providing a function to do so, so that would integrate nicely with what we have here.

With a bit of work, plugins could also detect whether a listener is installed for any of the new events, and if not issue a classic error for the sake of compatibility with older versions of gulp, with older styles of how to build gulp pipelines, and with stream handling libraries which are unaware of any gulp-specific event types. The converse problem, of plugins not switched over to generating these new events, would probably be more problematic in the near future.

I realize that what I'm suggesting here is not a step towards what the current title of this issue is requesting. It's more a discussion of alternative approaches to address the same underlying problem. If you feel an urgent need, you could combine the above with the monkey-patching approach for gulp.src, i.e. have the patched .pipe event handle the non fatal error events. But I feel a new pipeline-building syntax would be cleaner than monkey patching.

[hildjj]

I'm envisioning a vinyl object marked as an error going into the stream. You could then filter those off in by using .errorPipe, handling them however you like (including forwarding them along after printing). Errors would not be passed into .pipe, nor would the input object that caused the error. This could be hidden behind a new option, as suggested above.

Effectively, this makes a gulp stream be two node streams, one for vinyl objects, one for errors.

[phated]

@contra it seems that https://www.npmjs.com/package/pump solves all of the problems. Do you think we should expose that as a method in gulp? I could see there being a much more complex solution, but maybe going the simplest route is better. Otherwise, we can just document the recommendation to use pump everywhere.

[yocontra]

@phated How does pump solve this problem? Doesn't seem to do anything for error management.

[phated]

@contra The complex solution would be for gulp.src to return a mock stream that exposed a pipe method that queued streams into pump or pumpify but that seems overkill and makes gulp.src streams behave differently than browserify or other node streams.

[phated]

Node's recommended pattern is to use streams.pipeline() now - We'll be documenting this as "Getting Started - Error Management" so I'm going to close this.