Allow for badly behaved plugins

[notmodified]

Hi there,

Thanks for all your hard work, keep it up!

I have been fiddling around with the tsify plugin and noticed whenever i messed up my syntax badly watchify would crash.

Turns out plugins pipelining multiple errors makes bad things happen.

What would the expected behaviour of the plugin be in this case? Should it not be emitting multiple errors in the first place? Should it be pumping these errors out elsewhere? If so I'll have a go at fixing this over in tsify.

My node is weak so I apologise if the following is a complete travesty. Either way it has been an interesting learning exercise.

thanks again.

[zertosh]

Hmm. I've had this problem before with multiple error events being emitted, but you still need to end the stream. How about just writing the first error only? Like this:

if (!didError) {
        didError = true;
        writer.end('console.error(' + JSON.stringify(String(err)) + ');');
}

[notmodified]

My bad for not understanding the semantics. With the stream ended I had to catch the associated error on the writer when it comes to packing time. My error messaging sucks a bit so please suggest alternative wording.

[notmodified]

Ok, I'm following you I think. How about this then? Writer errors should still bubble. But we catch the case where we cause one ourselves by trying to write when we have already .ended?

thanks for your patience.

[zertosh]

no problem. I don't know if I'm missing something but I just don't think the case in https://github.com/notmodified/watchify/commit/025e3fcd8dd4d5015995cd69e45dcd7eaf44d1f7 can ever happen. Or can it?

wb is the b.pipeline wrapped to be a read-only stream. Any error in pack would have surfaced through the pipeline if it happened before it became "readable" - and at that point it would never become "readable". If an error did occur after it became "readable", then https://github.com/notmodified/watchify/commit/025e3fcd8dd4d5015995cd69e45dcd7eaf44d1f7 still won't happen because that block is wrapped in a once (not an on).

Also, keep in mind that 99.99% of all errors will be emitted in the deps or syntax pipeline step ("deps" if it has a require, "syntax" if it doesn't). And no data will flow after sort unless all modules have been seen.

[notmodified]

It's probably me that's missing something. The case I aim to solve with https://github.com/notmodified/watchify/commit/025e3fcd8dd4d5015995cd69e45dcd7eaf44d1f7 is not to catch errors from wb but to catch the case where writer has already been ended by writer.end.

I think the reason I am seeing this behaviour might well the way tsify deals with its syntax errors. By doing b.pipeline.emit('error', '<something>'); and not using the pipeline as intended would that allow pack to become readable even though things had gone wrong further up?

I am happy to chop that bit out and see if I can fix the problem over on the tsify end if you think it's not something watchify should have to deal with.

[zertosh]

Got it. Ok, then leaving the didError check in the pack's on "readable" makes sense. The error output from watchify is relied on by many tools, so just don't add the "Browserfication failed..." message. Don't forget to squash and don't remove space that's not in your changeset :smile:

[notmodified]

Ok. Message removed. Commits squashed. Sneaky white space back where it belongs.

Thanks again for your patience, it's appreciated.

[zertosh]

np! thanks for the PR! published as watchify@3.6.1