Syntax error found by jscs lib is eaten by promise, then silently passes by ending with exit code 0

[Krinkle]

This is probably a combination of a bug in grunt-jscs-checker, a bug in upstream jscs and a bug in upstream grunt.

When a file being checked contains a syntax error (e.g. unexpected end on input), the following is the result of running grunt:

[0] $ grunt build jscs qunit
Running "build:all" (build) task
>> File "dist/oojs.js" created.

Running "jscs:src" (jscs) task
[0] $

Two issues:

• No where did it report an error.
• Grunt completely stopped, it doesn't report the task completed, it doesn't report the task failed or timed out, it doesn't start the next task. And the process ends, as if all tests passed, with exit code 0.

I've narrowed it down to the following code in /tasks/jscs.js:

        files.map( jscs.checkFile, jscs ).forEach(function( promise ) {
            if ( !promise ) {
                update();
                return;
            }

            promise.then(function( errors ) {
                ...

               update();
           });
        });

I tried adding an error handling (second argument of Promise.then), and that caught it. Alternatively, changing it from Promise.then to Promise.done makes Grunt handle by catching the uncaught exception there.

Adding the error handler (by passing to grunt.fail.warn), and calling grunt with --stack provides the following:

Error: Syntax error at src/foo.js: Line 14: Unexpected end of input
    at StringChecker.checkString (node_modules/grunt-jscs-checker/node_modules/jscs/lib/string-checker.js:156:19)
    at node_modules/grunt-jscs-checker/node_modules/jscs/lib/checker.js:58:26
    at Array.0 (node_modules/grunt-jscs-checker/node_modules/jscs/node_modules/vow/lib/vow.js:194:56)
    at callFns (node_modules/grunt-jscs-checker/node_modules/jscs/node_modules/vow/lib/vow.js:452:35)
    at process._tickCallback (node.js:415:13)

The reason this exception is actually able to strangle grunt is not in jscs. The jscs lib actually catches this exception internally and then passes it as data argument when it rejects the vow/promise.

However vow/promise will throw a global exception (using nextTick) if no error handler is given and the code uses .done instead of .then. When .then is used (as the code currently does), it is silently ignored.

I'm not sure why then, why the current code kills grunt. But either way, we should handle the error.

[Krinkle]

@markelog

When a file being checked contains a syntax error (e.g. unexpected end on input), the following is the result of running grunt:

So, for example:

--- src/intro.js
( function ( global ) {

'use strict';

Now, of course, such file should be excluded (which I was I did after finding out why grunt was crashing), but there's lots of syntax errors one could accidentally make that this should catch.

I do run jshint on the project in question, but it wasn't caught because the file was listed in .jshintignore already (so JSHint didn't trip on it), and jscs should (and can) handle invalid syntax without relying on jshint to filter it out first.

The jscs lib catches and rejects the promise, but on the grunt-jscs-checker end we weren't registering a fail handler for the promise, at which point vow throws the error globally.

[markelog]

@Krinkle thank you for the detailed test case!