Created
January 5, 2014 12:43
-
-
Save floatdrop/8267745 to your computer and use it in GitHub Desktop.
Discussion about streams error handling and gulp
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
[15:39:55] <dashed> floatdrop: This is a bit crazy - but what if we implement "right" streams with "right" pipe? (ohmygodwhatiamsaying) | |
[15:39:55] <dashed> ??? | |
[15:43:34] <floatdrop> https://github.com/gulpjs/gulp/issues/75#issuecomment-31600182 | |
[15:46:34] <floatdrop> dashed generally this is bad-bad idea | |
[16:00:01] <nfroidure> Not sure it is a good idea to catch n drop every error events | |
[16:05:06] <dashed> depends on error. | |
[16:05:15] <dashed> ideally, we'll want something like this https://upload.wikimedia.org/wikipedia/commons/thumb/f/f6/Pipeline.svg/500px-Pipeline.svg.png Hide image by shift clicking. | |
[16:06:12] <floatdrop> I'll be whining about this line for years: .on('error', function(){}) | |
[16:06:44] <nfroidure> dashed, in fact, what we need is probably a better error categorization | |
[16:07:07] <dashed> that's what im thinking as well... define a gulp error event | |
[16:07:21] <nfroidure> distinguishing "fatal" errors from "file specific" errors. | |
[16:08:09] <dashed> does node do that in other areas of their API? | |
[16:08:17] <floatdrop> isn't fatal errors should be throwed and file specific is just `error` event/ | |
[16:09:14] <nfroidure> floatdrop, good point | |
[16:09:38] <dashed> currently plugin guidelines say to pass the instances of Error to error event | |
[16:09:57] <nfroidure> are there cases where a file error can compromise the whole result ? | |
[16:10:49] <dashed> ah.. im thinking of a different handling of error | |
[16:10:55] <nfroidure> dashed, i mean error categorization is done with console.error, console.warn and console.log | |
[16:10:55] <dashed> im talking about stream level | |
[16:11:02] <floatdrop> yeah. I would not launch builded folder to testing if one file is corrupted | |
[16:11:25] <floatdrop> testing/production | |
[16:12:08] <dashed> true | |
[16:12:55] <dashed> but, when you stream files, and a gulp plugin errors in the middle of the file stream | |
[16:13:00] <dashed> you get partial results anyways | |
[16:13:15] <dashed> it's just files after the errored file do not get processed | |
[16:13:47] <dashed> by stream, i meant coming from gulp.src | |
[16:14:42] <dashed> something like .on('error', handleError).on('error', gulp.beep) should notify user of the problem | |
[16:15:44] <dashed> but the entire point of continuing on error is treating each files independently | |
[16:16:18] <dashed> b/c currently, node stream in gulp considers the next file to be dependent on the current file | |
[16:16:36] <nfroidure> the exception terminate the process with an error code, if handling errors, there should be a mechanism to exit the process with an error code | |
[16:18:10] <dashed> interesting point | |
[16:19:16] <nfroidure> how about passing errors to the next stream so that listening for errors at the end of the whole pipeline would not throw exception ? | |
[16:19:26] <dashed> i think that if a gulp plugin errors on a particular file, but not others. only that particular file stops at that point in the stream. | |
[16:19:55] <dashed> all other files still pass downstream | |
[16:20:03] <floatdrop> nfroidure that is work for custom pipe function | |
[16:20:29] <floatdrop> but that what I expected from it | |
[16:20:38] <dashed> i wouldn't want to pass errors to the next stream | |
[16:20:51] <floatdrop> why? | |
[16:21:31] <dashed> node stream doesn't do this | |
[16:24:12] <dashed> if an error occurs on a particular file, that file stops at that point in the stream | |
[16:25:32] <dashed> http://pastie.org/8603131 | |
[16:25:35] <dashed> you want something like this? | |
[16:27:12] <dashed> i think i ran into a discussion regarding this | |
[16:27:33] <dashed> https://groups.google.com/forum/#!topic/nodejs/lJYT9hZxFu0 | |
[16:30:49] <floatdrop> heh, monkeypatching Stream.pipe from Isaac - https://groups.google.com/d/msg/nodejs/lJYT9hZxFu0/TF2jx4lQNR4J | |
[16:32:11] <dashed> if it's desirable to pipe errors to a single sink (like gutil.log), then gulp.src(...) should provide a method that wraps .pipe | |
[16:32:32] <dashed> and appends .on('error') | |
[16:33:22] <floatdrop> https://gist.github.com/floatdrop/8266618 | |
[16:33:57] <floatdrop> in response to pastie.org link | |
[16:34:47] <dashed> that looks ugly =[ | |
[16:34:49] <nfroidure> a bit verbose, but i like the idea. how about wrapping pipe and patching dest pipe inside the wrapper to propagate it through the pipeline ? | |
[16:35:57] <dashed> this looks like a potential useful use case, anyone want to open a gulp issue? | |
[16:36:30] <dashed> or should it be shimmed as a plugin? | |
[16:37:08] <nfroidure> floatdrop, commented the gist, why not just add an option to gulp.src forwardErrors:true | |
[16:37:09] <dashed> b/c if everyone is concerned that errors are dropped, then it should be baked into gulp to encourage it | |
[16:38:01] <dashed> nfroidure: looks great =] | |
[16:38:38] <floatdrop> well, this isn't solving, that plugins will throw on errors | |
[16:39:08] <floatdrop> or you want `pipe` function to patch destanation pipe function or something? | |
[16:39:33] <nfroidure> yep, monkey patching pipe functions of piped in streams | |
[16:39:41] <floatdrop> why am I do not see comment? | |
[16:39:50] <dashed> https://gist.github.com/nfroidure/8266634 | |
[16:40:27] <dashed> also to extend the idea further, should we be able to send 'error' to gulp.task? | |
[16:40:31] <floatdrop> yeah, looks much better | |
[16:40:49] <dashed> that way child gulp.tasks don't run? | |
[16:41:18] <dashed> b/c of deps option in gulp.task(name[, deps], fn) | |
[16:42:00] <dashed> so if an error occurs in a stream, then it should forward to gulp.task level as well | |
[16:42:41] <dashed> it would help to totally stop any production-related gulp tasks | |
[16:47:42] <dashed> another suggestion https://gist.github.com/nfroidure/8266634#comment-979686 | |
[16:48:05] <dashed> onErrorFunc: console.error option as a shorthand to hook .on('error', console.error) to all stream |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment