Errors should emit error event

[agarzola]

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:

https://github.com/dlmanning/gulp-sass/blob/c04bb67043fdbcb930152232ee9ba4caff0b2599/index.js#L174-L178

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?

[cdfa]

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);
}; 

[cr0ybot]

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 );
}