Skip to content

Commit

Permalink
Fix: don't classify plugins that throw errors as "missing" (fixes #6874
Browse files Browse the repository at this point in the history
…) (#8323)

* Fix: don't classify plugins that throw errors as "missing" (fixes #6874)

Previously, if a plugin threw an error on load, ESLint would display the "missing plugin" error message. This was confusing for plugin developers, because it would be difficult to distinguish between an installation error and a broken plugin.

* Use a getter instead of an external dependency
  • Loading branch information
not-an-aardvark committed Mar 30, 2017
1 parent 29f4ba5 commit 1b1046b
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 9 deletions.
27 changes: 19 additions & 8 deletions lib/config/plugins.js
Expand Up @@ -127,14 +127,25 @@ module.exports = {
if (!plugins[shortName]) {
try {
plugin = require(longName);
} catch (err) {
debug(`Failed to load plugin ${longName}.`);
err.message = `Failed to load plugin ${pluginName}: ${err.message}`;
err.messageTemplate = "plugin-missing";
err.messageData = {
pluginName: longName
};
throw err;
} catch (pluginLoadErr) {
try {

// Check whether the plugin exists
require.resolve(longName);
} catch (missingPluginErr) {

// If the plugin can't be resolved, display the missing plugin error (usually a config or install error)
debug(`Failed to load plugin ${longName}.`);
missingPluginErr.message = `Failed to load plugin ${pluginName}: ${missingPluginErr.message}`;
missingPluginErr.messageTemplate = "plugin-missing";
missingPluginErr.messageData = {
pluginName: longName
};
throw missingPluginErr;
}

// Otherwise, the plugin exists and is throwing on module load for some reason, so print the stack trace.
throw pluginLoadErr;
}

this.define(pluginName, plugin);
Expand Down
22 changes: 21 additions & 1 deletion tests/lib/config/plugins.js
Expand Up @@ -43,7 +43,12 @@ describe("Plugins", () => {
"eslint-plugin-example": plugin,
"@scope/eslint-plugin-example": scopedPlugin,
"./environments": Environments,
"../rules": Rules
"../rules": Rules,
"eslint-plugin-throws-on-load": {
get rules() {
throw new Error("error thrown while loading this module");
}
}
});
});

Expand Down Expand Up @@ -106,6 +111,21 @@ describe("Plugins", () => {
}, /Failed to load plugin/);
});

it("should rethrow an error that a plugin throws on load", () => {
try {
StubbedPlugins.load("throws-on-load");
} catch (err) {
assert.strictEqual(
err.message,
"error thrown while loading this module",
"should rethrow the same error that was thrown on plugin load"
);

return;
}
assert.fail(null, null, "should throw an error if a plugin fails to load");
});

it("should load a scoped plugin when referenced by short name", () => {
StubbedPlugins.load("@scope/example");
assert.equal(StubbedPlugins.get("@scope/example"), scopedPlugin);
Expand Down

0 comments on commit 1b1046b

Please sign in to comment.