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

request: better error message if eslint crashes #391

Closed
techieshark opened this issue Jan 14, 2018 · 4 comments
Closed

request: better error message if eslint crashes #391

techieshark opened this issue Jan 14, 2018 · 4 comments

Comments

@techieshark
Copy link

I started noticing an initially inexplicable error in vscode.

After updating from eslint 4.13 to 4.14, when I open a js file in vscode, the following error pops up: "Error: Cannot read propery 'type' of undefined [close]"

"Error: Cannot read propery 'type' of undefined [close]"

That's at the top of the editor window, and there's no obvious way to find out more information, so for a while I had no idea where the error came from.

Eventually I narrowed it down to the eslint vscode extension, and then I realized it was actually (apparently) the first part of the error that is being reported when I run eslint on the command line, which is crashing (that's a separate issue I'll file upstream).

My suggestion for eslint extension would be to improve the error code if eslint crashes, and say something like eslint command crashed: Cannot read property 'type' of undefined with an option to view the full crash message. That way, users would know to start looking at eslint command itself rather than digging around in vscode (or mistakenly thinking it is something caused by C++ extension, for example, from this issue - microsoft/vscode#37677).

The error from the command line is below:

Cannot read property 'type' of undefined
TypeError: Cannot read property 'type' of undefined
    at isForInRef (<my-project-dir>/node_modules/eslint/lib/rules/no-unused-vars.js:410:24)
    at variable.references.some.ref (<my-project-dir>/node_modules/eslint/lib/rules/no-unused-vars.js:447:21)
       at Array.some (<anonymous>)
    at isUsedVariable (<my-project-dir>/node_modules/eslint/lib/rules/no-unused-vars.js:446:40)
    at collectUnusedVariables (<my-project-dir>/node_modules/eslint/lib/rules/no-unused-vars.js:569:26)
    at collectUnusedVariables (<my-project-dir>/node_modules/eslint/lib/rules/no-unused-vars.js:576:17)
    at Program:exit (<my-project-dir>/node_modules/eslint/lib/rules/no-unused-vars.js:621:36)
    at listeners.(anonymous function).forEach.listener (<my-project-dir>/node_modules/eslint/lib/util/safe-emitter.js:47:58)
    at Array.forEach (<anonymous>)
    at Object.emit (<my-project-dir>/node_modules/eslint/lib/util/safe-emitter.js:47:38)
@techieshark
Copy link
Author

In case anyone else ends up running across that eslint issue, it was because I had updated from eslint 3.x to 4.x without updating from babel-eslint 7.x to 8.x. This fixed the eslint crash for me: yarn add -D eslint@4.x babel-eslint@8.

Would still be great to have that vscode error more verbose to help narrow down the problem faster.

@techieshark
Copy link
Author

This is great - now it shows:

improved vscode eslint error notification

However, in the case of the error I had, eslint crashed in such a way that that the error visible when running eslint on the command line isn't reported in the ESLint output channel within vscode, as shown below:

eslint output channel with no useful error shown

The reason no error was showing up was because ESLint itself crashed unexpectedly, and so all we have is the stack trace.

This can be observed/duplicated by adding some invalid code in your ./node_modules/eslint/lib/rules/, for example, I added undefined.type to rules/no-unused-vars.js, like so:

invalid code added, to make eslint crash

(Here's a diff from inside eslint dir):

diff --git a/lib/rules/no-unused-vars.js b/lib/rules/no-unused-vars.js
index 3ed278d..ae5ac1c 100644
--- a/lib/rules/no-unused-vars.js
+++ b/lib/rules/no-unused-vars.js
@@ -398,6 +398,9 @@ module.exports = {
          * @private
          */
         function isUsedVariable(variable) {
+
+            undefined.type; // added this obviously invalid code to duplicate the issue
+
             const functionNodes = variable.defs.filter(def => def.type === "FunctionName").map(def => def.node),
                 isFunctionDefinition = functionNodes.length > 0;
             let rhsNode = null;

As long as your ESLint config either "extends": "eslint:recommended" or includes

"rules": {
   "no-unused-vars": "error"
}

you should be able to duplicate the above screenshots (window warning but no output channel error).

Suggested improvement: also log the stack trace.

Something like this in server/src/server.ts:

 function showErrorMessage(error: any, document: TextDocument): Status {
        connection.window.showErrorMessage(`ESLint: ${getMessage(error, document)}. Please see the 'ESLint' output channel for details.`);
+       if (error.stack) {
+               connection.console.error('ESLint stack trace:');
+               connection.console.error(error.stack);
+       }
        return Status.error;
 }

Then in addition to the top of the window error we'd get a (somewhat) informative stack trace:
ESLint channel output showing stack trace

LMK if you'd like a PR (or just use the code above). Thanks!

@dbaeumer
Copy link
Member

Thanks for the great analysis. I add the code. Please have a look at this commit. If OK I will publish a new version.

339d507

@techieshark
Copy link
Author

Looks good and works. Thanks!

lottamus added a commit to stoplightio/eslint-config-stoplight that referenced this issue Jan 26, 2018
@vscodebot vscodebot bot locked and limited conversation to collaborators Mar 1, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants