Repository: mochajs/mocha

Problem statement:
Correctly diagnose errors from required reporter module
### Prerequisites

- [x] Checked that your issue hasn't already been filed by cross-referencing [issues with the `faq` label](https://github.com/mochajs/mocha/issues?utf8=%E2%9C%93&q=is%3Aissue%20label%3Afaq%20)
- [x] Checked next-gen ES issues and syntax problems by using the same environment and/or transpiler configuration without Mocha to ensure it isn't just a feature that actually isn't supported in the environment in question or a bug in your code.
- [x] 'Smoke tested' the code to be tested by running it outside the real test suite to get a better sense of whether the problem is in the code under test, your usage of Mocha, or Mocha itself
- [x] Ensured that there is no discrepancy between the locally and globally installed versions of Mocha. You can find them with: `node node_modules/.bin/mocha --version`(Local) and `mocha --version`(Global). We recommend avoiding the use of globally installed Mocha.

### Description

When developing a third party reporter, mocha misdiagnoses an error coming from within the reporter as the reporter module not being found.

### Steps to Reproduce

[Minimal reproduction](https://github.com/aido179/mocha-err)
1. Create a parent project that uses mocha.
2. Create a custom reporter, that doesn't require mocha anywhere. 
3. Install that reporter in the parent project (in this case, as a local package using the file location).
4. When mocha `require`s the reporter, node throws an error. The `require` works correctly, but mocha erroneously `console.warn`s that the reporter wasn't found.

**Expected behavior:**

Mocha says that the reporter package caused the error.

**Actual behavior:** 

Mocha erroneously `console.warn`s that the reporter wasn't found.

**Reproduces how often:** 100%

### Versions

Mocha version: 5.2.0 (not installed globally)
Node version: v8.11.1
OS: MacOs High Sierra v10.13.6
Shell: iTerm2 build 3.2.4beta2

### Additional Information
As discussed in #3530, initially thought the package was not being required correctly.



Hints:
While I can see how a better error message could be helpful, the _real_ bug here is in _your_ reporter  depending on missing project dependencies.

Patch:
diff --git a/lib/cli/run-helpers.js b/lib/cli/run-helpers.js
--- a/lib/cli/run-helpers.js
+++ b/lib/cli/run-helpers.js
@@ -225,18 +225,18 @@ exports.validateLegacyPlugin = (opts, pluginType, map = {}) => {
 
   // if this exists, then it's already loaded, so nothing more to do.
   if (!map[pluginId]) {
+    let foundId;
     try {
-      map[pluginId] = require(pluginId);
+      foundId = require.resolve(pluginId);
+      map[pluginId] = require(foundId);
     } catch (err) {
-      if (err.code === 'MODULE_NOT_FOUND') {
-        // Try to load reporters from a path (absolute or relative)
-        try {
-          map[pluginId] = require(path.resolve(pluginId));
-        } catch (err) {
-          throw createUnknownError(err);
-        }
-      } else {
-        throw createUnknownError(err);
+      if (foundId) throw createUnknownError(err);
+
+      // Try to load reporters from a cwd-relative path
+      try {
+        map[pluginId] = require(path.resolve(pluginId));
+      } catch (e) {
+        throw createUnknownError(e);
       }
     }
   }
diff --git a/lib/mocha.js b/lib/mocha.js
--- a/lib/mocha.js
+++ b/lib/mocha.js
@@ -16,7 +16,6 @@ var Suite = require('./suite');
 var esmUtils = require('./nodejs/esm-utils');
 var createStatsCollector = require('./stats-collector');
 const {
-  warn,
   createInvalidReporterError,
   createInvalidInterfaceError,
   createMochaInstanceAlreadyDisposedError,
@@ -335,35 +334,26 @@ Mocha.prototype.reporter = function (reporterName, reporterOptions) {
     }
     // Try to load reporters from process.cwd() and node_modules
     if (!reporter) {
+      let foundReporter;
       try {
-        reporter = require(reporterName);
+        foundReporter = require.resolve(reporterName);
+        reporter = require(foundReporter);
       } catch (err) {
-        if (err.code === 'MODULE_NOT_FOUND') {
-          // Try to load reporters from a path (absolute or relative)
-          try {
-            reporter = require(path.resolve(utils.cwd(), reporterName));
-          } catch (_err) {
-            _err.code === 'MODULE_NOT_FOUND'
-              ? warn(`'${reporterName}' reporter not found`)
-              : warn(
-                  `'${reporterName}' reporter blew up with error:\n ${err.stack}`
-                );
-          }
-        } else {
-          warn(`'${reporterName}' reporter blew up with error:\n ${err.stack}`);
+        if (foundReporter) {
+          throw createInvalidReporterError(err.message, foundReporter);
+        }
+        // Try to load reporters from a cwd-relative path
+        try {
+          reporter = require(path.resolve(reporterName));
+        } catch (e) {
+          throw createInvalidReporterError(e.message, reporterName);
         }
       }
     }
-    if (!reporter) {
-      throw createInvalidReporterError(
-        `invalid reporter '${reporterName}'`,
-        reporterName
-      );
-    }
     this._reporter = reporter;
   }
   this.options.reporterOption = reporterOptions;
-  // alias option name is used in public reporters xunit/tap/progress
+  // alias option name is used in built-in reporters xunit/tap/progress
   this.options.reporterOptions = reporterOptions;
   return this;
 };


Test patch:
diff --git a/test/browser-specific/fixtures/webpack/webpack.config.js b/test/browser-specific/fixtures/webpack/webpack.config.js
--- a/test/browser-specific/fixtures/webpack/webpack.config.js
+++ b/test/browser-specific/fixtures/webpack/webpack.config.js
@@ -17,7 +17,7 @@ module.exports = {
   plugins: [
     new FailOnErrorsPlugin({
       failOnErrors: true,
-      failOnWarnings: true
+      failOnWarnings: false
     })
   ]
 };
diff --git a/test/node-unit/cli/run-helpers.spec.js b/test/node-unit/cli/run-helpers.spec.js
--- a/test/node-unit/cli/run-helpers.spec.js
+++ b/test/node-unit/cli/run-helpers.spec.js
@@ -77,6 +77,23 @@ describe('helpers', function () {
           {message: /wonky/, code: 'ERR_MOCHA_INVALID_REPORTER'}
         );
       });
+
+      it('should fail and report the original "MODULE_NOT_FOUND" error.message', function () {
+        expect(
+          () =>
+            validateLegacyPlugin(
+              {
+                reporter: require.resolve('./fixtures/bad-require.fixture.js')
+              },
+              'reporter'
+            ),
+          'to throw',
+          {
+            message: /Error: Cannot find module 'fake'/,
+            code: 'ERR_MOCHA_INVALID_REPORTER'
+          }
+        );
+      });
     });
   });
 
diff --git a/test/node-unit/mocha.spec.js b/test/node-unit/mocha.spec.js
--- a/test/node-unit/mocha.spec.js
+++ b/test/node-unit/mocha.spec.js
@@ -246,7 +246,7 @@ describe('Mocha', function () {
 
         it('should load from current working directory', function () {
           expect(function () {
-            mocha.reporter('./spec.js');
+            mocha.reporter('./lib/reporters/spec.js');
           }, 'not to throw');
         });
 
@@ -255,7 +255,7 @@ describe('Mocha', function () {
             expect(
               function () {
                 mocha.reporter(
-                  '../../test/node-unit/fixtures/wonky-reporter.fixture.js'
+                  './test/node-unit/fixtures/wonky-reporter.fixture.js'
                 );
               },
               'to throw',
@@ -264,19 +264,6 @@ describe('Mocha', function () {
               }
             );
           });
-
-          it('should warn about the error before throwing', function () {
-            try {
-              mocha.reporter(
-                '../../test/node-unit/fixtures/wonky-reporter.fixture.js'
-              );
-            } catch (ignored) {
-            } finally {
-              expect(stubs.errors.warn, 'to have a call satisfying', [
-                expect.it('to match', /reporter blew up/)
-              ]);
-            }
-          });
         });
       });
 
@@ -292,7 +279,7 @@ describe('Mocha', function () {
             expect(
               function () {
                 mocha.reporter(
-                  './test/node-unit/fixtures/wonky-reporter.fixture.js'
+                  '../test/node-unit/fixtures/wonky-reporter.fixture.js'
                 );
               },
               'to throw',
@@ -301,19 +288,6 @@ describe('Mocha', function () {
               }
             );
           });
-
-          it('should warn about the error before throwing', function () {
-            try {
-              mocha.reporter(
-                './test/node-unit/fixtures/wonky-reporter.fixture.js'
-              );
-            } catch (ignored) {
-            } finally {
-              expect(stubs.errors.warn, 'to have a call satisfying', [
-                expect.it('to match', /reporter blew up/)
-              ]);
-            }
-          });
         });
       });
     });