vitkarpov / grunt-nunjucks-2-html

Compiles nunjucks templates *right* into HTML
MIT License
35 stars 11 forks source link

Better handling of errors #28

Closed ArmorDarks closed 9 years ago

ArmorDarks commented 9 years ago

So far we can get templates-related errors in two cases:

  1. When we've launched watch task, which recompiles templates on changes, and non of templates had errors. But duringwatch task we've introduced error.
  2. When we launch whole task (let's say, grunt test) and one of templates contains error

In first case, Task's behaviour will be according to expected — it will grunt.log.error(err) and stop compilation of template until we'll fix error

In second case Task will behave not according to Grunt guidelines: if any template has error, Task will grunt.log.error(err) and continue compilation. Do you see problem here? Failed to compile templates means that some pages won't be generated, but in same time we won't receive failed build. It is especially crucial for deployment, since during deploy it will emit error only in log, and won't fail whole build (while should, since we won't receive failed to compile page on production).

Instead it have to do grunt.fail.warn('Something went wrong') right after grunt.log.error(err) or throw only grunt.fail.warn(err) (depending on your preference of colors).

In other words, Task have to throw an warning, which will fail task, but in same time will allow to use --force when we want ignore that error. We're using grunt.fail.warn here istead of grunt.fail.fatal, since it isn't fatal error and we can continue to go on when needed.

In fact, in first case Task should throw grunt.fail.warn(...) too, but due to nature of Grunt, Task will be stopped due to error anyway, but it won't happen in second case.

Also note, that due to changes, introduced in 0.3.2, if one of templates will fail, whole task will fail due to Fatal error: callback was already called. It shouldn't happen — it should fail because of grunt.fail.warn(...), error from compilation, but not from error from async, otherwise we won't be able to use --force to compile all templates despite present error.

3

vitkarpov commented 9 years ago

@ArmorDarks Okay, I see. Maybe you can make a pr? Seems you understand what's need to be done.

ArmorDarks commented 9 years ago

Done.

I didn't make PR because I've stuck with Fatal error: callback was already called — wasn't able to figure out why it thought that callback has been already called by the time we get error. Now I don't receive that error... probably just messed my code somewhere.

Anyway, now everything seems to be fine

vitkarpov commented 9 years ago

@ArmorDarks you make a lot for the project, so I think you should make a pr with including youself as contributor inside package.json — the link will appear on the npm's page :)