Do not call .end() in case of an error

[mcollina]

Do not call .end() in case of an error.

@phated can you please check if this fixes your issue and does not cause any regressions in gulp? I'm not 100% about semver, an opinion in that regard would be awesome.

Fixes #18

[vweevers]

LGTM. Semver-patch, I think.

[phated]

@mcollina this looks great but can it be backported to the 1.x release line? Even though my new module will be targeting node 6+, the Vinyl objects being produced by gulp will be cloneable-readables from the 1.x line - us having to major bump would cause a cascade of major bumps leading to a gulp 5 (edit) way sooner than we are prepared for.

I tried reaching into the _original property but that doesn't seem to work either.

[vweevers]

leading to a gulp 5 with no real changes

If it has to bump major then that's a real change.

[phated]

I am happy to take on the backporting and release stream of 1.x

[vweevers]

Agree to disagree on the node policy (I wish you would have worded it differently than "for no reason", which I find disrespectful to other people's management styles and the time that they're able and willing to invest in supporting EOL node versions - that said, I understand your position).

You're gonna be stuck with an old readable-stream as well. That path seems more painful to me than upgrading and bumping accordingly. WDYT?

[phated]

Sorry for the phrasing. I've been formulating my upgrade plan in my head for so long that I forget that I need to describe it better.

We have to accept that gulp 4 will need to be on readable-stream 2.x for it's lifespan because we promised our users 0.10 support, since that was the newest version when development began.

The current plan is to begin gulp 5 work with the intent to support node 10 as the minimum version, for some very specific features. That means we have quite some time before the LTS of node 8 falls off. Which is sort of good because we have a massive amount of work to do.

I think having this patch backported would be super helpful. I've been tackling this sort of work for many things and would be happy to help here.

[mcollina]

I’m ok in getting a PR for a backport. The reason why this was bumped to 2.x is to get the updated readable-stream v3, which adds pipeline, async iterators and some behavior change on destroy that is close to this fix.

Given that pipeline is not going to be available in readable-stream v2, how do you plan to test it in old node versions?

I would highly recommend updating to readable-stream v3 everywhere to get async iterator support. They are amazing. I understand your reasoning anyway and I’m happy to take a backport PR.

[mcollina]

Released as 2.0.1. I'm happy to get a backport!