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

Compilation errors are not handled properly #314

Closed
fatfisz opened this issue Jun 8, 2016 · 3 comments
Closed

Compilation errors are not handled properly #314

fatfisz opened this issue Jun 8, 2016 · 3 comments

Comments

@fatfisz
Copy link
Contributor

fatfisz commented Jun 8, 2016

I was running grunt-contrib-watch with grunt-contrib-less and other tasks, and I noticed that only grunt-contrib-less was behaving strangely: if a compilation error was encountered, everything just hung up.

After long search through grount-contrib-watch and grunt I am fairly certain that the problem lies with how grunt-contrib-less handles compilation errors.

So in the less task there is a compileLess function, which contains this code at the end:

return less.render(srcCode, options)
  .catch(function(err) {
    lessError(err, srcFile);
  });

As you can see, in case of an error a message is printed, and then - because nothing is returned from the function in catch, the promise is fulfilled with undefined.

The next step is this then:

compileLess(file, destFile, options)
  .then(function(output) {
    compiled.push(output.css);
    ...

But it should be obvious by now that this throws an error, because output is undefined (compileLess was fulfilled with it) and we can't get the css property of undefined.

Because there are no further error handlers, the error is just sent into nothingness and the nextFileObj function is not called. This seems like something that is not desired.

I'm not sure what's the appropriate fix here. Would rethrowing the error be ok?

  .catch(function(err) {
    lessError(err, srcFile);
    throw err;
  });
@fatfisz
Copy link
Contributor Author

fatfisz commented Jul 27, 2016

@XhmikosR Could you check this out please? This is a very annoying problem. And I have a fix ready for this, so almost no work from the maintainers required.

@XhmikosR
Copy link
Member

I'm still waiting for @vladikoff to look at #322 and then we can check #315.

@leigeber
Copy link
Contributor

Would love to see this one merged in. Bites me every time. Is there something outstanding any of us can help resolve or is this still waiting on @vladikoff? Thanks.

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