Make sass errors stop Gulp to make them clearer

[joelanman]

attempt to fix #295

on a sass error, the Prototype Kit now exits (no prototype will appear in the browser)

please note there are 2 other approaches discussed in the comments below

it would be nice to clean up the error, I think we just need messageFormatted, it currently looks like this:

[joelanman]

slightly odd, when I force this error:

.test{
    background: url(
}

I get a segmentation fault from Gulp, which seems a bit wrong

Segmentation fault: 11

[joelanman]

Approach 2

tried a slightly different approach and it outputs this which I think is slightly better? Would prefer without the stack trace

code:

    .pipe(sass({ outputStyle: 'expanded' }).on('error', function (error) {
      done(new Error(error.message))
    }))

output:

[18:30:37] Finished 'copy-assets-documentation' after 294 ms
[18:30:37] 'sass' errored after 591 ms
[18:30:37] Error: app/assets/sass/application.scss
Error: no mixin named missing-mixin
        on line 22 of app/assets/sass/application.scss
        from line 3 of app/assets/sass/application-ie8.scss
>>     @include missing-mixin;

   -------------^

    at DestroyableTransform.<anonymous> (/Users/joelanman/projects/govuk-prototype-kit/gulp/sass.js:28:12)
    at DestroyableTransform.emit (events.js:215:7)
    at DestroyableTransform.EventEmitter.emit (domain.js:475:20)
    at onwriteError (/Users/joelanman/projects/govuk-prototype-kit/node_modules/readable-stream/lib/_stream_writable.js:449:12)
    at onwrite (/Users/joelanman/projects/govuk-prototype-kit/node_modules/readable-stream/lib/_stream_writable.js:470:11)
    at WritableState.onwrite (/Users/joelanman/projects/govuk-prototype-kit/node_modules/readable-stream/lib/_stream_writable.js:180:5)
    at DestroyableTransform.afterTransform (/Users/joelanman/projects/govuk-prototype-kit/node_modules/readable-stream/lib/_stream_transform.js:93:3)
    at errorM (/Users/joelanman/projects/govuk-prototype-kit/node_modules/gulp-sass/index.js:122:12)
    at Object.callback (/Users/joelanman/projects/govuk-prototype-kit/node_modules/gulp-sass/index.js:131:16)
    at options.error (/Users/joelanman/projects/govuk-prototype-kit/node_modules/node-sass/lib/index.js:294:32)
[18:30:37] 'generate-assets' errored after 643 ms
[18:30:37] 'default' errored after 645 ms
Segmentation fault: 11

[joelanman]

Approach 3

forcing process.exit in the error handler gives this, which is the cleanest so far, but also includes an unhelpful suggestion from gulp

code:

    .pipe(sass({ outputStyle: 'expanded' }).on('error', function (error) {
      console.error(error.message)
      process.exit(1)
    }))

output:

[18:36:28] Finished 'copy-assets-documentation' after 239 ms
app/assets/sass/application.scss
Error: no mixin named missing-mixin
        on line 22 of app/assets/sass/application.scss
        from line 3 of app/assets/sass/application-ie8.scss
>>     @include missing-mixin;

   -------------^

[18:36:29] The following tasks did not complete: default, generate-assets, <parallel>, sass, sass-documentation
[18:36:29] Did you forget to signal async completion?
Segmentation fault: 11

[joelanman]

Gulp has an open issue to document how to handle errors, but it's 6 years old now

gulpjs/gulp#359

[joelanman]

we discussed this on the team - concerned that a total exit might be annoying for users - they would have to fix the sass and restart the kit. Going to investigate options where users only have to fix the sass.

[joelanman]

I tried deleting the css before trying the compile. This fails, leaving no css. BrowserSync tries to load the new css and fails, however this does not make any difference to the loaded page. Refreshing the page manually does show a broken page as expected

[36degrees]

Closing in favour of #990