Repository: mochajs/mocha

Problem statement:
🐛 Bug: mocha fails silently on invalid `package.json` section
### Bug Report Checklist

- [X] I have read and agree to Mocha's [Code of Conduct](https://github.com/mochajs/mocha/blob/master/.github/CODE_OF_CONDUCT.md) and [Contributing Guidelines](https://github.com/mochajs/mocha/blob/master/.github/CONTRIBUTING.md)
- [X] I have searched for [related issues](https://github.com/mochajs/mocha/issues?q=is%3Aissue) and [issues with the `faq` label](https://github.com/mochajs/mocha/issues?utf8=%E2%9C%93&q=is%3Aissue%20label%3Afaq%20), but none matched my issue.
- [X] I have '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, my usage of Mocha, or Mocha itself.
- [X] I want to provide a PR to resolve this

### Expected

I made an error in my `package.json` (an extra comma) because JSON is a bad format for configuration files ;-) like this:
```json
"mocha": {
    "spec": "./*.spec.js",
}
```
I expected `mocha` to give me an error, ideally a bit friendlier than the one from `npm`, but something like that:
```
$ npm install
npm ERR! code EJSONPARSE
...
npm ERR! JSON.parse Note: package.json must be actual JSON, not just JavaScript.
```

### Actual

Mocha just failed as if I hadn't configured it at all.  This was very confusing:
```
$ npx mocha
Error: No test files found: "test"
```

### Minimal, Reproducible Example

See "expected" above but basically:
```
npm install -D mocha
```
then add the offending section to `package.json`, then create `foo.spec.js`, then try to run `mocha`.

### Versions

`
10.4.0
10.4.0
v20.11.0
`

### Additional Info

I could definitely provide a PR if you could direct me to the appropriate place in the code (with which I am not familiar)


Hints:
🤔 I don't reproduce this in https://github.com/mochajs/mocha-examples/tree/4b00891d6c7886f2d451e962a974478e7d3c1aa9/packages/hello-world. After adding an invalid `abc` to the top of its `package.json`:

```plaintext
$ npm run test
npm ERR! code EJSONPARSE
npm ERR! JSON.parse Invalid package.json: JSONParseError: Unexpected token 'a', "abc
npm ERR! JSON.parse {
npm ERR! JSON.parse   "n"... is not valid JSON while parsing 'abc
npm ERR! JSON.parse {
npm ERR! JSON.parse   "name": "hello-world",
npm ERR! JSON.parse   "versio'
npm ERR! JSON.parse Failed to parse JSON data.
npm ERR! JSON.parse Note: package.json must be actual JSON, not just JavaScript.
```

Could you post a standalone reproduction please @dhdaines? 
Relevant piece of code in mocha:

https://github.com/mochajs/mocha/blob/c44653a3a04b8418ec24a942fa7513a4673f3667/lib/cli/options.js#L182-L201

If one hasn't specified a `package.json` explicitly then it simply ignores it when it can't be read, instead only doing a `debug()`, so yeah – it will completely ignore the config in these cases.

@JoshuaKGoldberg Your error might be from npm itself

Patch:
diff --git a/lib/cli/options.js b/lib/cli/options.js
--- a/lib/cli/options.js
+++ b/lib/cli/options.js
@@ -181,8 +181,24 @@ const loadPkgRc = (args = {}) => {
   result = {};
   const filepath = args.package || findUp.sync(mocharc.package);
   if (filepath) {
+    let configData;
     try {
-      const pkg = JSON.parse(fs.readFileSync(filepath, 'utf8'));
+      configData = fs.readFileSync(filepath, 'utf8');
+    } catch (err) {
+      // If `args.package` was explicitly specified, throw an error
+      if (filepath == args.package) {
+        throw createUnparsableFileError(
+          `Unable to read ${filepath}: ${err}`,
+          filepath
+        );
+      } else {
+        debug('failed to read default package.json at %s; ignoring',
+              filepath);
+        return result;
+      }
+    }
+    try {
+      const pkg = JSON.parse(configData);
       if (pkg.mocha) {
         debug('`mocha` prop of package.json parsed: %O', pkg.mocha);
         result = pkg.mocha;
@@ -190,13 +206,11 @@ const loadPkgRc = (args = {}) => {
         debug('no config found in %s', filepath);
       }
     } catch (err) {
-      if (args.package) {
-        throw createUnparsableFileError(
-          `Unable to read/parse ${filepath}: ${err}`,
-          filepath
-        );
-      }
-      debug('failed to read default package.json at %s; ignoring', filepath);
+      // If JSON failed to parse, throw an error.
+      throw createUnparsableFileError(
+        `Unable to parse ${filepath}: ${err}`,
+        filepath
+      );
     }
   }
   return result;


Test patch:
diff --git a/test/node-unit/cli/options.spec.js b/test/node-unit/cli/options.spec.js
--- a/test/node-unit/cli/options.spec.js
+++ b/test/node-unit/cli/options.spec.js
@@ -149,7 +149,7 @@ describe('options', function () {
                 loadOptions('--package /something/wherever --require butts');
               },
               'to throw',
-              'Unable to read/parse /something/wherever: bad file message'
+              'Unable to read /something/wherever: bad file message'
             );
           });
         });
@@ -199,6 +199,36 @@ describe('options', function () {
           });
         });
 
+        describe('when path to package.json unspecified and package.json exists but is invalid', function () {
+          beforeEach(function () {
+            const filepath = '/some/package.json';
+            readFileSync = sinon.stub();
+            // package.json
+            readFileSync
+              .onFirstCall()
+              .returns('{definitely-invalid');
+            findConfig = sinon.stub().returns('/some/.mocharc.json');
+            loadConfig = sinon.stub().returns({});
+            findupSync = sinon.stub().returns(filepath);
+            loadOptions = proxyLoadOptions({
+              readFileSync,
+              findConfig,
+              loadConfig,
+              findupSync
+            });
+          });
+
+          it('should throw', function () {
+            expect(
+              () => {
+                loadOptions();
+              },
+              'to throw',
+                /SyntaxError/,
+            );
+          });
+        });
+
         describe('when called with package = false (`--no-package`)', function () {
           let result;
           beforeEach(function () {
@@ -287,7 +317,7 @@ describe('options', function () {
           });
 
           it('should set config = false', function () {
-            expect(loadOptions(), 'to have property', 'config', false);
+            expect(result, 'to have property', 'config', false);
           });
         });