Add handleErrors option to avoid Gulp crashing on Stylus parse errors

[distracteddev]

This allows a user to avoid having to restart Gulp every time a syntax error is made and saved. Furthermore, it places the error cleanly into the CSS so that it appears in the browser. This ensures that when using LiveReload, users are not forced to constantly watch their gulp output while editing their styles.

[stephenlacy]

How many other plugins use this type of error reporting?
Inserting error data into the output files breaks normal gulp usage.

CC @Jenius

[distracteddev]

First off, I'm not sure how many other Gulp plugins use this type of error reporting. However, I got the idea from my SASS days.

Second, I believe that Gulp has two distinct use-cases: Production Builds and Dev Builds. They are clearly different and if you look at any Gulp file you will see different tasks being run for each type of build.

This is why I made sure to add this functionality as an optional piece of sugar that is disabled by default

Keeping the above in mind, this is my take on your second comment:

• Inserting error data into output files breaks gulp usage for "Deploy Builds" .
• Inserting error data into output files for "Dev Builds" just makes it easier to rapidly update your stylus files and view the changes without having to worry about re-running gulp or checking its output for errors.

That being said, I have no idea how many others would find this useful/desirable so feel free to reject this PR if you do not see the value in it.

[stephenlacy]

I may actually use these changes, but am not sure who else will.

@contra Thoughts?

[distracteddev]

Also I should mention that the only reason I made this change INSIDE gulp-stylus is because this module uses the 'map-stream' module which causes all errors to be thrown in such a way that they cannot be caught from outside the module.

e.g:

gulp.task('mainStyles', function () {
  try {
    return gulp.src('src/app/styles/main.styl')
        .pipe(stylus())
        .pipe(gulp.dest('dist/css'))
  } catch(e) {
    // Stylus Render Errors are never caught
  }
});

[jescalan]

I'd have to disagree with this way of handling errors. IMHO, there should be no situation where a stylus compiling plugin will produce invalid css.

If you have a pipeline in which you want to show the error in-browser, you can set one up where you run a local server, watch, and catch the gulp error and display it in browser through a socket (like I mentioned in the other issue, I do something like this with a library I maintain so I know that it does work), but that still means gulp functions as it should and you handle the error differently at a different layer.

[distracteddev]

@Jenius Could you please expand on what you mean by invalid css? This is perfectly valid css and infact more accurately represents the current state of the stylus since if we didn't replace the CSS with the error message the browser would be using a css file based on an outdated stylus file.

[distracteddev]

@Jenius Also your solution does not solve the main inconvenience here which is that fact that you have to go to your console and restart gulp every time you make a small syntax error and save your stylus file.

Although this might not seem like such an inconvenience, imagine you have several tasks and watchers in your Gulpfile. Restarting gulp from scratch (vs just a watch task run through) takes considerably longer. For my very small project that uses webpack + react the first Gulp run through takes 5-7 seconds whereas a watch-cycle takes < 200ms.

[jescalan]

@distracteddev ah yeah you're right, sorry I am tired haha, it is valid css. Still am not a fan of the pattern in general, but if other people like it and as long as there is an option to toggle it on or off, I suppose it can do no harm.

[andyjansson]

Isn't this what .on() is for?

the only reason I made this change INSIDE gulp-stylus is because this module uses the 'map-stream' module which causes all errors to be thrown in such a way that they cannot be caught from outside the module.

I don't think that's true. I glanced at the source of map-stream and it does emit events for errors: https://github.com/dominictarr/map-stream/blob/master/index.js#L75

[yocontra]

No this is not good. This whole mess will be solved with gulpjs/gulp#358 and gulpjs/gulp#359

[yocontra]

@distracteddev That time on first launch may be due to blocking plugins. When all of your stuff is running at once it's possible that certain plugins (like uglify) will block the main event loop while they process a file which then causes all other tasks to take longer. This is a known thing we are looking at solving

[yocontra]

@distracteddev Just opened this up for you gulpjs/gulp#364

[stephenlacy]

Excellent, I will close this now since it will be fixed in gulp

[distracteddev]

@contra Sweeeet. Glad to see that Gulp is looking to imrpove its error handling. This will definitely help solve my problem in the future.

@andyjansson It seems that only works when map-stream is used in a certain way. See https://github.com/dominictarr/map-stream/blob/master/index.js#L103 for the line that is actually being used by gulp-stylus when an error occurs. It simply throws the error.

[distracteddev]

@contra @stevelacy Also, although the error handling will be improved and Gulp might not crash, that only solves half of the problem that this PR fixes. It still will not help deliver the error message directly to the browser when stylus errors are created.

This wouldn't be that much of an issue in most cases, but most people expect to be able to edit their stylus, hit save, and see a change. If we don't pass the error to the browser this pattern breaks down. When I'm not working on my giant display, I don't have the real-estate to have my browser + editor + terminal open. Furthermore, for teams that have dedicated designers that style their project who may not be familiar with gulp or the need to constantly check it for errors, I believe delivering the stylus error directly to the browser (in addition to a gulp error log) is still the better user-experience.

Thoughts?

[yocontra]

@distracteddev It isn't a bad idea to show the error in the browser but this is something we would have to be able to implement generically across all applicable plugins without actually putting it in the plugin.

When the new error management system is in place and there is a central place to collect errors, it will be easy to do this. You could make a chrome extension that gulp triggers, figure out how to wire into devtools to insert css on the page, etc. all with one handler

Pseudocode:

gulp.on('errorOrsomething', sendToBrowser);

Implementing it per plugin is never good and leads to headaches of "who has what"

[stephenlacy]

@distracteddev @contra
I like that, it goes with the plugin/extension I was thinking about which would show the errors emitted from jade/CSS(less,sass,stylus) plugins.

[distracteddev]

Sounds good to me. Just wanted to add that I <3 Gulp and really appreciate the effort to continue improving it.