tschaub / gulp-newer

Pass through newer source files only
https://npmjs.org/package/gulp-newer
226 stars 24 forks source link

Errors swallowed #7

Closed karlvr closed 9 years ago

karlvr commented 10 years ago

I believe I've discovered that the gulp-newer plugin swallows errors from further down the pipe.

This appears to occur in the final then in the promise chain in _transform. We call self.push and in there the pipe may throw an error. We don't catch it, and execution stops. I'm not sure exactly what's happening - the gulp task never completes and no error is shown on the console.

I have experimentally added a try {} catch (e) {} around the contents of the final then block, which stores the error in a variable scoped in _transform and then throws it if it exists at the end of the _transform method. This appears to give the desired behaviour: showing an error on the console. However I'm not sure if this is the right solution, hence I haven't provided a pull request. I'm happy to do so! But I'm not confident I have the best solution as my understanding of the plugin is pretty fresh!

Best, Karl

ai commented 9 years ago

@tschaub any updates?

michael42 commented 9 years ago

We're having the same issue here. The build isn't failing and no errors are printed.

Here's an example:

var gulp = require('gulp');

gulp.task('default', function() {
    require('fs').writeFileSync('test', '123');

    return gulp.src('test')
        .pipe(require('gulp-newer')('foo'))
        .on('data', function() { throw new Error(); })
        .pipe(gulp.dest('.'));
});

Produces output (with exit code 0):

[11:18:07] Using gulpfile gulpfile.js
[11:18:07] Starting 'default'...
richardolsson commented 9 years ago

This bug just bit me. Is this project alive and should a bug like this be expected to be fixed? Would a pull request be accepted?

lddubeau commented 9 years ago

I've put in a pull request with a fix. By the way, @michael42, thanks for the code that demonstrates the problem. It was helpful in figuring out the issue.