Errors should emit error event
Opened this issue · 2 comments
Hi! I am triaging an issue with our builds wherein our main Gulp task produces exit code 0 even if there was an error with the Sass plugin. I believe the issue is here:
Lines 174 to 178 in c04bb67
Instead of this.emit('end')
, I believe that the recommended solution is to do this.emit('error')
. I realize, however, that this may constitute a breaking change, so I think making this change opt-in might be for the best (default is end
, and you can opt-in for error
).
I will attempt to address this in a fork and submit a pull request for review, if you would be open to that. Thoughts?
If you emit an error
event in the handler, this will cause an infinite loop. Actually throwing the PluginError
works perfectly:
gulpSass.logError = function logError(error) {
throw new PluginError('sass', error.messageFormatted);
};
To @agarzola and @cdfa: THANK YOU!
I've been troubleshooting my gulp workflow all day, finally deciding to try to fix an issue that has been plaguing us for over a year! The watch task just hangs whenever there is a sass compilation error, and must be restarted. I've tried everything else, but throwing an error or this.emit('error')
both fix the issue.
EDIT: It does seem like in one of my attempts, this.emit('error')
did in fact result in an infinite loop of "undefined" logged to my console.
EDIT2: While throwing an error allows my watch task to continue working, plumber
no longer picks up the error. I do some logging/notifications for gulp errors, so this isn't ideal.
EDIT3: Passing the done
argument to the error handler seems to allow the error to get picked up by plumber
while also ending the task and allowing watch
to continue, but it also outputs to the console, so there's a bit of noise but it's better than it was before.
export function styles( done ) {
return gulp.src( src )
.pipe( plumber( errorHandler ) )
.pipe( sass.sync().on( 'error', done ) )
.pipe( gulp.dest( dest );
}