From 6e622d6337e98768faa400a6231e7bc4b55ab817 Mon Sep 17 00:00:00 2001
From: Guy Bedford <guybedford@gmail.com>
Date: Tue, 8 Sep 2020 22:27:55 -0700
Subject: [PATCH] esm: better package.json parser errors

PR-URL: https://github.com/nodejs/node/pull/35117
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
---
 lib/internal/errors.js                        |  2 +-
 lib/internal/modules/esm/resolve.js           | 21 ++++++++++++-------
 test/es-module/test-esm-invalid-pjson.js      |  8 +++----
 .../es-modules/pjson-invalid/package.json     |  1 +
 4 files changed, 18 insertions(+), 14 deletions(-)
 create mode 100644 test/fixtures/es-modules/pjson-invalid/package.json

diff --git a/lib/internal/errors.js b/lib/internal/errors.js
index 6a64eb24a854f4..b1601869d5e0a1 100644
--- a/lib/internal/errors.js
+++ b/lib/internal/errors.js
@@ -1119,7 +1119,7 @@ E('ERR_INVALID_OPT_VALUE', (name, value, reason = '') => {
 E('ERR_INVALID_OPT_VALUE_ENCODING',
   'The value "%s" is invalid for option "encoding"', TypeError);
 E('ERR_INVALID_PACKAGE_CONFIG', (path, base, message) => {
-  return `Invalid package config ${path}${base ? ` imported from ${base}` :
+  return `Invalid package config ${path}${base ? ` while importing ${base}` :
     ''}${message ? `. ${message}` : ''}`;
 }, Error);
 E('ERR_INVALID_PACKAGE_TARGET',
diff --git a/lib/internal/modules/esm/resolve.js b/lib/internal/modules/esm/resolve.js
index 0a828867692473..7091a55a5c41bf 100644
--- a/lib/internal/modules/esm/resolve.js
+++ b/lib/internal/modules/esm/resolve.js
@@ -82,7 +82,7 @@ function tryStatSync(path) {
   }
 }
 
-function getPackageConfig(path) {
+function getPackageConfig(path, specifier, base) {
   const existing = packageJSONCache.get(path);
   if (existing !== undefined) {
     return existing;
@@ -106,7 +106,11 @@ function getPackageConfig(path) {
   try {
     packageJSON = JSONParse(source);
   } catch (error) {
-    throw new ERR_INVALID_PACKAGE_CONFIG(path, null, error.message);
+    throw new ERR_INVALID_PACKAGE_CONFIG(
+      path,
+      (base ? `"${specifier}" from ` : '') + fileURLToPath(base || specifier),
+      error.message
+    );
   }
 
   let { imports, main, name, type } = packageJSON;
@@ -130,13 +134,14 @@ function getPackageConfig(path) {
   return packageConfig;
 }
 
-function getPackageScopeConfig(resolved, base) {
+function getPackageScopeConfig(resolved) {
   let packageJSONUrl = new URL('./package.json', resolved);
   while (true) {
     const packageJSONPath = packageJSONUrl.pathname;
     if (StringPrototypeEndsWith(packageJSONPath, 'node_modules/package.json'))
       break;
-    const packageConfig = getPackageConfig(fileURLToPath(packageJSONUrl), base);
+    const packageConfig = getPackageConfig(fileURLToPath(packageJSONUrl),
+                                           resolved);
     if (packageConfig.exists) return packageConfig;
 
     const lastPackageJSONUrl = packageJSONUrl;
@@ -497,7 +502,7 @@ function packageImportsResolve(name, base, conditions) {
     throw new ERR_INVALID_MODULE_SPECIFIER(name, reason, fileURLToPath(base));
   }
   let packageJSONUrl;
-  const packageConfig = getPackageScopeConfig(base, base);
+  const packageConfig = getPackageScopeConfig(base);
   if (packageConfig.exists) {
     packageJSONUrl = pathToFileURL(packageConfig.pjsonPath);
     const imports = packageConfig.imports;
@@ -535,7 +540,7 @@ function packageImportsResolve(name, base, conditions) {
 }
 
 function getPackageType(url) {
-  const packageConfig = getPackageScopeConfig(url, url);
+  const packageConfig = getPackageScopeConfig(url);
   return packageConfig.type;
 }
 
@@ -580,7 +585,7 @@ function packageResolve(specifier, base, conditions) {
     StringPrototypeSlice(specifier, separatorIndex));
 
   // ResolveSelf
-  const packageConfig = getPackageScopeConfig(base, base);
+  const packageConfig = getPackageScopeConfig(base);
   if (packageConfig.exists) {
     const packageJSONUrl = pathToFileURL(packageConfig.pjsonPath);
     if (packageConfig.name === packageName &&
@@ -608,7 +613,7 @@ function packageResolve(specifier, base, conditions) {
     }
 
     // Package match.
-    const packageConfig = getPackageConfig(packageJSONPath, base);
+    const packageConfig = getPackageConfig(packageJSONPath, specifier, base);
     if (packageConfig.exports !== undefined && packageConfig.exports !== null)
       return packageExportsResolve(
         packageJSONUrl, packageSubpath, packageConfig, base, conditions
diff --git a/test/es-module/test-esm-invalid-pjson.js b/test/es-module/test-esm-invalid-pjson.js
index 53ebd4962f0523..9f4711321230bf 100644
--- a/test/es-module/test-esm-invalid-pjson.js
+++ b/test/es-module/test-esm-invalid-pjson.js
@@ -19,11 +19,9 @@ child.on('close', mustCall((code, signal) => {
   strictEqual(signal, null);
   ok(
     stderr.includes(
-      [
-        '[ERR_INVALID_PACKAGE_CONFIG]: ',
-        `Invalid package config ${invalidJson}. `,
-        `Unexpected token } in JSON at position ${isWindows ? 16 : 14}`
-      ].join(''),
+      `[ERR_INVALID_PACKAGE_CONFIG]: Invalid package config ${invalidJson} ` +
+      `while importing "invalid-pjson" from ${entry}. ` +
+      `Unexpected token } in JSON at position ${isWindows ? 16 : 14}`
     ),
     stderr);
 }));
diff --git a/test/fixtures/es-modules/pjson-invalid/package.json b/test/fixtures/es-modules/pjson-invalid/package.json
new file mode 100644
index 00000000000000..c91736ab5c7dbc
--- /dev/null
+++ b/test/fixtures/es-modules/pjson-invalid/package.json
@@ -0,0 +1 @@
+syntax error