Skip to content
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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Errors should emit error event #845

Open
agarzola opened this issue Feb 3, 2022 · 2 comments
Open

Errors should emit error event #845

agarzola opened this issue Feb 3, 2022 · 2 comments

Comments

@agarzola
Copy link

agarzola commented Feb 3, 2022

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:

gulp-sass/index.js

Lines 174 to 178 in c04bb67

gulpSass.logError = function logError(error) {
const message = new PluginError('sass', error.messageFormatted).toString();
process.stderr.write(`${message}\n`);
this.emit('end');
};

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
Copy link

cdfa commented Jul 14, 2022

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
Copy link

cr0ybot commented Sep 23, 2022

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants