Adds try/catch to the ESLint processor preprocess task

[therealgilles]

Description

Adds try/catch to the ESLint processor preprocess task, to bail and return the original code if an error is thrown by the underlying parser (e.g. a parsing error outside the template strings).

The ESLint crew told me processors are not supposed to throw errors. See discussion here: eslint/eslint#16015

Fixes # (issue)

Type of change

• Bug fix (non-breaking change which fixes an issue)

Screenshots/Sandbox (if appropriate/relevant):

Here is a simple repo that reproduces the issue: https://github.com/therealgilles/graphql-config-code-file-ts

How Has This Been Tested?

I'm not sure how to build the repo and run existing tests. If you have documentation on this, I'll be happy to do it.

Test Environment:

• OS: MacOS Ventura 13.0 Beta
• @graphql-eslint/...: 3.10.4
• NodeJS: v16.15.0

Checklist:

• I have followed the CONTRIBUTING doc and the style guidelines of this project
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• [N/A] I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests and linter rules pass locally with my changes
• [N/A] Any dependent changes have been merged and published in downstream modules

[JounQin]

An error indicates bug, you should never suppress unexpected errors.

[therealgilles]

@JounQin The syntax error will be reported by ESLint so it will be caught.

Ideally @babel/parser would be called with the ErrorRecovery flag set to true. But graphql-tag-pluck does not seem to give any control on that. Additionally it's not clear that this would be enough in all parsing error cases.

I think the right thing to do is to suppress the error so that ESLint can proceed and the syntax error can be reported. I would add a warning message when running ESLint in debug mode, so that debugging is easier.

[JounQin]

This is similar situation for all processors.

If the syntax is correct, then it's a bug of this plugin.

If the syntax is incorrect, then a clearer crash error should help the user to understand.

Maybe ESLint should support SyntaxError with loc info from processors. (This is a ESLint proposal, if your description from ESLint team's view is correct.)

[therealgilles]

See ESLint discussion here: eslint/eslint#16015 (reply in thread)

[JounQin]

See ESLint discussion here: eslint/eslint#16015 (reply in thread)

So if I don't understand incorrectly, @btmills agreed to try/catch in ESLint's process logic instead of handling it by processor itself?

eslint/eslint#16015 (reply in thread)

[therealgilles]

I think it is under consideration. I still think it's bad form for a preprocessor to throw, but not sure how preprocessor would be intended to report errors to ESLint core.

[dotansimha]

@B2o5T what do you think?

[JounQin]

See eslint/eslint#16104 and my PR eslint/eslint#16105

[dimaMachina]

Sorry for the long response, closing it here since this will be fixed in ESLint directly