New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fails on return #8

Open
thasmo opened this Issue Jan 26, 2014 · 21 comments

Comments

Projects
None yet
@thasmo

thasmo commented Jan 26, 2014

Hey there!

AWESOME 馃悞 patch, love it so much!

I noticed if I return the stream in a task, 'watch' freezes after a stream error:

gulp.task('styles', function() {
    return gulp.src('application/static/style/*.scss')
        .pipe(plumber())
        .pipe(sass())
        .pipe(gulp.dest('public/static/style/'));
});

If I remove 'return' it doesn't freeze, when an error occurs:

gulp.task('styles', function() {
    gulp.src('application/static/style/*.scss')
        .pipe(plumber())
        .pipe(sass())
        .pipe(gulp.dest('public/static/style/'));
});

Returning the stream is mentioned in the gulp docs here:
https://github.com/gulpjs/gulp/blob/master/docs/API.md#async-task-support

Thanks! :)

@floatdrop

This comment has been minimized.

Show comment
Hide comment
@floatdrop

floatdrop Jan 27, 2014

Owner

Thank you! But while I was writing a test for it, it turns out, that without plumber I got same behavior (not working, when returning a stream). Could you remove plumber and check this too? If so, this issue related with orchestrator.

Owner

floatdrop commented Jan 27, 2014

Thank you! But while I was writing a test for it, it turns out, that without plumber I got same behavior (not working, when returning a stream). Could you remove plumber and check this too? If so, this issue related with orchestrator.

@splodingsocks

This comment has been minimized.

Show comment
Hide comment
@splodingsocks

splodingsocks Mar 19, 2014

I'm seeing this behavior but only when my stream uses the gulp-sass plugin. I throw an error in the stream and plumber notifies me, but the watch task for sass stops completely. I also tried returning a promise instead of the stream, and using a callback instead of returning a stream. Neither of those worked either.

However, my coffeescript task works fine. I make an error there and I'm notified, but the watch task continues and I can fix the error without restarting gulp.

splodingsocks commented Mar 19, 2014

I'm seeing this behavior but only when my stream uses the gulp-sass plugin. I throw an error in the stream and plumber notifies me, but the watch task for sass stops completely. I also tried returning a promise instead of the stream, and using a callback instead of returning a stream. Neither of those worked either.

However, my coffeescript task works fine. I make an error there and I'm notified, but the watch task continues and I can fix the error without restarting gulp.

@laurelnaiad

This comment has been minimized.

Show comment
Hide comment
@laurelnaiad

laurelnaiad Apr 26, 2014

I've just discovered gulp-sass' propensity to kill my watch, as well. It's easy enough to have one syntax error from a designer and the whole thing blows up. I've figured out how to handle it.

Basically, you pass plumber a custom error handler. That error handler logs the problem (I just copied plumber's default handlers log message, plus a few key newlines)... and then your function closes down the watch that is now defunct.

(I keep track of the gulp-watch object and my connect servers, all of which have a close function, so I call that on each of them)

Then the function reinvokes the watcher function that I use to initiate the watch in the first place ,along with a message saying that I'm doing so. It actually works out pretty well. It just takes managing the state of a few things in order to end the current watch and start up a new one in good health. I have it set up so that it doesn't matter which plugin crashes the party, this behavior will kick in if plumber's patch detects and prevents me from the China syndrome.

laurelnaiad commented Apr 26, 2014

I've just discovered gulp-sass' propensity to kill my watch, as well. It's easy enough to have one syntax error from a designer and the whole thing blows up. I've figured out how to handle it.

Basically, you pass plumber a custom error handler. That error handler logs the problem (I just copied plumber's default handlers log message, plus a few key newlines)... and then your function closes down the watch that is now defunct.

(I keep track of the gulp-watch object and my connect servers, all of which have a close function, so I call that on each of them)

Then the function reinvokes the watcher function that I use to initiate the watch in the first place ,along with a message saying that I'm doing so. It actually works out pretty well. It just takes managing the state of a few things in order to end the current watch and start up a new one in good health. I have it set up so that it doesn't matter which plugin crashes the party, this behavior will kick in if plumber's patch detects and prevents me from the China syndrome.

@jtomaszewski

This comment has been minimized.

Show comment
Hide comment
@jtomaszewski

jtomaszewski Apr 26, 2014

So you're restarting gulp watch in the onError handler? Heh, seems like the only way to do it right now.

Could you show us some source of it ?

jtomaszewski commented Apr 26, 2014

So you're restarting gulp watch in the onError handler? Heh, seems like the only way to do it right now.

Could you show us some source of it ?

@heikki

This comment has been minimized.

Show comment
Hide comment
@heikki

heikki Apr 26, 2014

I had same issue with gulp-stylus. Emitting end from the custom error handler seems to fix it:

gulp.task('styles', function() {
    return gulp.src(paths.styles.src)
        .pipe(plumber(function(error) {
            gutil.log(gutil.colors.red(error.message));
            this.emit('end');
        }))
        .pipe(stylus({ use: [ nib() ] }))
        .pipe(concat('all.css'))
        .pipe(gulp.dest(buildDir + '/styles'));
});

heikki commented Apr 26, 2014

I had same issue with gulp-stylus. Emitting end from the custom error handler seems to fix it:

gulp.task('styles', function() {
    return gulp.src(paths.styles.src)
        .pipe(plumber(function(error) {
            gutil.log(gutil.colors.red(error.message));
            this.emit('end');
        }))
        .pipe(stylus({ use: [ nib() ] }))
        .pipe(concat('all.css'))
        .pipe(gulp.dest(buildDir + '/styles'));
});
@laurelnaiad

This comment has been minimized.

Show comment
Hide comment
@laurelnaiad

laurelnaiad Apr 26, 2014

@jtomaszewski my project is scheduled to go open source later this year and I'm not in control of that schedule. I'd organize a snippet for you, but I think @heikki's method makes more sense. I knew that my problem was the lack of an end event because it was obvious that watch didn't think its handler was completing, but it didn't occur to me to emit end from this in the error handler. That's going to wind up being cleaner, I can just feel it. In fact I'm going to undo some of my twists and turns right now. Thanks, @heikki.

laurelnaiad commented Apr 26, 2014

@jtomaszewski my project is scheduled to go open source later this year and I'm not in control of that schedule. I'd organize a snippet for you, but I think @heikki's method makes more sense. I knew that my problem was the lack of an end event because it was obvious that watch didn't think its handler was completing, but it didn't occur to me to emit end from this in the error handler. That's going to wind up being cleaner, I can just feel it. In fact I'm going to undo some of my twists and turns right now. Thanks, @heikki.

@frxnz

This comment has been minimized.

Show comment
Hide comment
@frxnz

frxnz commented May 14, 2014

+1 @heikki

@mmrko

This comment has been minimized.

Show comment
Hide comment
@mmrko

mmrko commented Jun 13, 2014

@heikki 馃憤

@gaving

This comment has been minimized.

Show comment
Hide comment
@gaving

gaving commented Aug 2, 2014

@heikki 馃憤

@pronebird

This comment has been minimized.

Show comment
Hide comment
@pronebird

pronebird Oct 13, 2014

Insane, this is really annoying, why does not plumber catch this or less/sass plugins automatically close pipe on error? That seems odd.. Why would plumber exist if I can add .on("error", ...) instead of plumber and handle error myself.

pronebird commented Oct 13, 2014

Insane, this is really annoying, why does not plumber catch this or less/sass plugins automatically close pipe on error? That seems odd.. Why would plumber exist if I can add .on("error", ...) instead of plumber and handle error myself.

@laurelnaiad

This comment has been minimized.

Show comment
Hide comment
@laurelnaiad

laurelnaiad Oct 13, 2014

gulp-sass has a configurable onError handler which works for me (unless node-sass segfaults, which I hope is a thing of the past).

laurelnaiad commented Oct 13, 2014

gulp-sass has a configurable onError handler which works for me (unless node-sass segfaults, which I hope is a thing of the past).

@floatdrop

This comment has been minimized.

Show comment
Hide comment
@floatdrop

floatdrop Oct 14, 2014

Owner

@pronebird when you have long pipeline - then you must add .on('error', ...) after each pipe which is very ugly.

We started to use gulp-less for a while and sooner or later I will get to this bug (if I can reproduce it).

Owner

floatdrop commented Oct 14, 2014

@pronebird when you have long pipeline - then you must add .on('error', ...) after each pipe which is very ugly.

We started to use gulp-less for a while and sooner or later I will get to this bug (if I can reproduce it).

@pronebird

This comment has been minimized.

Show comment
Hide comment
@pronebird

pronebird Oct 14, 2014

@floatdrop thanks for explanation. I encounter this error on gulp-less. Custom plumber error handler solved the problem.

Do you think it would make sense to fix plumber to close pipe on error by default?

pronebird commented Oct 14, 2014

@floatdrop thanks for explanation. I encounter this error on gulp-less. Custom plumber error handler solved the problem.

Do you think it would make sense to fix plumber to close pipe on error by default?

@floatdrop

This comment has been minimized.

Show comment
Hide comment
@floatdrop

floatdrop Oct 14, 2014

Owner

@pronebird well, plumber purpose is to keep pipes working on errors. If you want to close pipe on errors - I think you should not use plumber at all.

Owner

floatdrop commented Oct 14, 2014

@pronebird well, plumber purpose is to keep pipes working on errors. If you want to close pipe on errors - I think you should not use plumber at all.

@jmcbee

This comment has been minimized.

Show comment
Hide comment
@jmcbee

jmcbee Oct 22, 2014

How do I close pipe on errors? I'm experiencing this on gulp-sass as well. cheers!

jmcbee commented Oct 22, 2014

How do I close pipe on errors? I'm experiencing this on gulp-sass as well. cheers!

@pronebird

This comment has been minimized.

Show comment
Hide comment
@pronebird

pronebird Oct 22, 2014

@fbm-static call this.emit('end'); in error handler for plumber. @heikki posted example above.

pronebird commented Oct 22, 2014

@fbm-static call this.emit('end'); in error handler for plumber. @heikki posted example above.

@heikki

This comment has been minimized.

Show comment
Hide comment
@heikki

heikki commented Oct 22, 2014

@jmcbee

This comment has been minimized.

Show comment
Hide comment
@jmcbee

jmcbee Oct 22, 2014

@heikki when I do this.end() it does not seem to finish the task.

jmcbee commented Oct 22, 2014

@heikki when I do this.end() it does not seem to finish the task.

@callumacrae

This comment has been minimized.

Show comment
Hide comment
@callumacrae

callumacrae Jan 12, 2015

I'm having the same problem with gulp-compass, but calling this.emit('end') doesn't fix it.

callumacrae commented Jan 12, 2015

I'm having the same problem with gulp-compass, but calling this.emit('end') doesn't fix it.

@derekgreer

This comment has been minimized.

Show comment
Hide comment
@derekgreer

derekgreer Sep 11, 2015

this.emit('end') isn't working for me either.

After skimming the article referenced by heikki, I modified my code to the following which does work:

.pipe($.if(isWatched, $.plumber(function (error) {
  $.util.log($.util.colors.red(error.message));
  this.end();
})))

Note: I'm setting isWatched within my watch task because I really only want plumber to kick in when I'm watching stuff, not during my CI build.

derekgreer commented Sep 11, 2015

this.emit('end') isn't working for me either.

After skimming the article referenced by heikki, I modified my code to the following which does work:

.pipe($.if(isWatched, $.plumber(function (error) {
  $.util.log($.util.colors.red(error.message));
  this.end();
})))

Note: I'm setting isWatched within my watch task because I really only want plumber to kick in when I'm watching stuff, not during my CI build.

@fanpei91

This comment has been minimized.

Show comment
Hide comment
@fanpei91

fanpei91 commented Jan 10, 2016

@heikki Thank you!

PaulPorfiroff added a commit to PaulPorfiroff/atom-language-tiddlywiki5 that referenced this issue Mar 4, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment