view: Better errors when package.json is not JSON

isaacs · nlf · commit 3ee8f3b34055 · 2020-10-27T11:38:40.000-07:00
Previously, `npm view <noargs>` was just saying "Invalid package.json" if the package.json file was not JSON, or if it was missing. This errors out with the appropriate error in these cases. Also, no need to read the big clunky 'read-package-json' for this, we're literally just checking for a single field. We can be a bit more efficient here. PR-URL: #2051 Credit: @isaacs Close: #2051 Reviewed-by: @nlf
diff --git a/lib/view.js b/lib/view.js
@@ -9,12 +9,16 @@ const npm = require('./npm.js')
 const { packument } = require('pacote')
 const path = require('path')
 const { inspect, promisify } = require('util')
-const readJson = promisify(require('read-package-json'))
 const relativeDate = require('tiny-relative-date')
 const semver = require('semver')
 const style = require('ansistyles')
 const usageUtil = require('./utils/usage')
 
+const fs = require('fs')
+const readFile = promisify(fs.readFile)
+const jsonParse = require('json-parse-even-better-errors')
+const readJson = async file => jsonParse(await readFile(file, 'utf8'))
+
 const usage = usageUtil(
   'view',
   'npm view [<@scope>/]<pkg>[@<version>] [<field>[.subfield]...]'
@@ -86,8 +90,8 @@ const view = async args => {
   if (local) {
     const dir = npm.prefix
     const manifest = await readJson(path.resolve(dir, 'package.json'))
-    if (!manifest || !manifest.name)
-      throw new Error('Invalid package.json')
+    if (!manifest.name)
+      throw new Error('Invalid package.json, no "name" field')
     const p = manifest.name
     nv = npa(p)
     if (pkg && ~pkg.indexOf('@'))
diff --git a/test/lib/view.js b/test/lib/view.js
@@ -482,7 +482,43 @@ t.test('throw error if global mode', (t) => {
   })
 })
 
-t.test('throw error if invalid package.json', (t) => {
+t.test('throw ENOENT error if package.json misisng', (t) => {
+  const testDir = t.testdir({})
+
+  const view = requireInject('../../lib/view.js', {
+    '../../lib/npm.js': {
+      prefix: testDir,
+      flatOptions: {
+        global: false
+      }
+    }
+  })
+  view([], (err) => {
+    t.match(err, { code: 'ENOENT' })
+    t.end()
+  })
+})
+
+t.test('throw EJSONPARSE error if package.json not json', (t) => {
+  const testDir = t.testdir({
+    'package.json': 'not json, nope, not even a little bit!'
+  })
+
+  const view = requireInject('../../lib/view.js', {
+    '../../lib/npm.js': {
+      prefix: testDir,
+      flatOptions: {
+        global: false
+      }
+    }
+  })
+  view([], (err) => {
+    t.match(err, { code: 'EJSONPARSE' })
+    t.end()
+  })
+})
+
+t.test('throw error if package.json has no name', (t) => {
   const testDir = t.testdir({
     'package.json': '{}'
   })
@@ -496,7 +532,7 @@ t.test('throw error if invalid package.json', (t) => {
     }
   })
   view([], (err) => {
-    t.equals(err.message, 'Invalid package.json')
+    t.equals(err.message, 'Invalid package.json, no "name" field')
     t.end()
   })
 })
