Users are unable to differentiate between missing plugins and plugins that throw exceptions as they are loaded

[platinumazure]

Tell us about your environment
N/A (applies to all environments)

• ESLint Version: >=2.2.0
• Node Version: N/A
• npm Version: N/A

What parser (default, Babel-ESLint, etc.) are you using?
N/A

Please show your full configuration:
Any configuration which specifies a plugin which throws when trying to require the plugin module.

What did you do? Please include the actual source code causing the issue.
Run ESLint on files or text.

What did you expect to happen?
Error message says that the plugin was found but could not be loaded, plus possibly debugging information.

What actually happened? Please include the actual, raw output from ESLint.
The "messages/plugin-missing.txt" is rendered, same as if the plugin was missing.

---

How should we solve this? Here are the high-level options as I see them:

1. Make the try/catch around plugin loading smarter, try to detect module absent vs module throwing an exception, and then handle the latter case differently.
2. Consider showing stack traces on pretty messages (perhaps only behind a particular CLI flag).

[not-an-aardvark]

I think outputting a "plugin not found" error if a plugin throws an error while loading is a bug; it's giving the user incorrect information and making errors more difficult to identify. The current behavior is for the engine to catch all errors, even though the engine can only handle a module-not-found error. If plugin throws an error, it should be treated the same as an unexpected error within ESLint, and should crash the process with a stack trace.

We can distinguish module-loading errors from other errors with something like:

try {
    plugin = require(pluginFile);
} catch (err) {
    if (err.code === "MODULE_NOT_FOUND") {
        // output "missing plugin" message
    } else {
        // crash the process and output a stack trace
    }
}

[Slayer95]

Instead of checking err.code, the right way to do it is to call require.resolve

try {
    require.resolve(pluginFile);
} catch (err) {
    // output "missing plugin" message
}

Note that an error with code 'MODULE_NOT_FOUND' can be thrown on plugin load runtime as well.

[platinumazure]

@Slayer95 Does require.resolve hydrate the module cache? If not, I'd be worried about the performance impact from loading a module twice. (It wouldn't be too terrible for plugins since users use fewer of those compared to, say, rules; but it's worth checking anyway.)

[not-an-aardvark]

I don't think require.resolve actually loads the module, so it would not have anything to cache. It just finds the path at which a module would be loaded from, which is the same thing that happens internally when you call require on a string.

[Slayer95]

It does, however, cache the resolution of real paths, which is (one of) the most expensive operations when locating modules.

... I suppose there is still a trade-off between correctness and performance, though.
A benchmark would be convenient. What's the typical case? JSX + React?

[not-an-aardvark]

We could do the require.resolve check after catching a module-loading error:

try {
    plugin = require(pluginFile);
    // loaded plugin successfully, continue with linting process
} catch (err) {
    try {
        require.resolve(pluginFile);
        // plugin threw an error, crash with stack trace
    } catch (err2) {
        // Missing plugin
    }
}

That would avoid hurting performance on the hot path, and only do extra work when something is wrong.

[platinumazure]

I imagine it would be easy to set up a benchmark which checks this. Sorry for dragging us into the weeds.

@eslint/eslint-team Anyone else have any thoughts on this issue?

[kaicataldo]

If plugin throws an error, it should be treated the same as an unexpected error within ESLint, and should crash the process with a stack trace.

I agree with this. What @not-an-aardvark suggested seems like a good strategy to me.

[kaicataldo]

@platinumazure This issue seems very similar to #6874, which is already accepted. Can this all be handled together?

[platinumazure]

@kaicataldo I suppose so. I'll close this.