fix: Format JSON parse errors so that they provide file context (#594)

Author: shubheksha

package.json

@@ -58,6 +58,7 @@
     "mz": "2.6.0",
     "node-firefox-connect": "1.2.0",
     "regenerator-runtime": "0.10.0",
+    "parse-json": "2.2.0",
     "sign-addon": "0.2.0",
     "source-map-support": "0.4.6",
     "stream-to-promise": "2.2.0",

src/cmd/build.js

@@ -4,6 +4,7 @@ import minimatch from 'minimatch';
 import {createWriteStream} from 'fs';
 import streamToPromise from 'stream-to-promise';
 import {fs} from 'mz';
+import parseJSON from 'parse-json';
 
 import defaultSourceWatcher from '../watcher';
 import {zipDir} from '../util/zip-dir';
@@ -59,14 +60,23 @@ export async function getDefaultLocalizedName(
 ): Promise<string> {
 
   let messageData: LocalizedMessageData;
+  let messageContents: string | Buffer;
   let extensionName: string = manifestData.name;
 
   try {
-    messageData = JSON.parse(await fs.readFile(messageFile));
+    messageContents = await fs.readFile(messageFile);
   } catch (error) {
     throw new UsageError(
-      `Error reading or parsing file ${messageFile}: ${error}`);
+      `Error reading messages.json file at ${messageFile}: ${error}`);
   }
+
+  try {
+    messageData = parseJSON(messageContents, messageFile);
+  } catch (error) {
+    throw new UsageError(
+      `Error parsing messages.json ${error}`);
+  }
+
   extensionName = manifestData.name.replace(/__MSG_([A-Za-z0-9@_]+?)__/g,
     (match, messageName) => {
       if (!(messageData[messageName]

src/util/manifest.js

@@ -4,6 +4,7 @@ import path from 'path';
 import {fs} from 'mz';
 import {InvalidManifest} from '../errors';
 import {createLogger} from './logger';
+import parseJSON from 'parse-json';
 
 
 const log = createLogger(__filename);
@@ -45,7 +46,7 @@ export default async function getValidatedManifest(
   let manifestData;
 
   try {
-    manifestData = JSON.parse(manifestContents);
+    manifestData = parseJSON(manifestContents, manifestFile);
   } catch (error) {
     throw new InvalidManifest(
       `Error parsing manifest.json at ${manifestFile}: ${error}`);

tests/unit/test-cmd/test.build.js

@@ -102,9 +102,9 @@ describe('build', () => {
           .then(makeSureItFails())
           .catch((error) => {
             assert.instanceOf(error, UsageError);
-            assert.match(
-              error.message,
-              /Error .* file .*messages\.json: SyntaxError: Unexpected string/);
+            assert.match(error.message, /Unexpected token '"' at 1:15/);
+            assert.match(error.message, /^Error parsing messages.json/);
+            assert.include(error.message, messageFileName);
           });
       }
     );
@@ -150,7 +150,10 @@ describe('build', () => {
         assert.instanceOf(error, UsageError);
         assert.match(
           error.message,
-          /Error .* file .*messages\.json: .*: no such file or directory/);
+          /Error: ENOENT: no such file or directory, open .*messages.json/);
+        assert.match(error.message, /^Error reading messages.json/);
+        assert.include(error.message,
+          '/path/to/non-existent-dir/messages.json');
       });
   });
 

tests/unit/test-util/test.manifest.js

@@ -70,7 +70,8 @@ describe('util/manifest', () => {
           .then(() => getValidatedManifest(tmpDir.path()))
           .then(makeSureItFails())
           .catch(onlyInstancesOf(InvalidManifest, (error) => {
-            assert.match(error.message, /Error parsing manifest\.json/);
+            assert.match(error.message, /Error parsing manifest\.json at /);
+            assert.include(error.message, 'Unexpected token \' \' at 2:49');
             assert.include(error.message, manifestFile);
           }));
       }