Handle errors from audit endpoint appropriately

If we're running the 'audit' command, then a failed endpoint means that
the command failed.  Error out in that case.

Otherwise, if it's a quick audit as part of another command, just return
a value to indicate that we should not print audit info.

This avoids showing '0 vulnerabilities found', which, while amusingly
technically correct, is misleading and not very helpful.

Fix: #1951

Credit: @isaacs
Close: #1956
Reviewed-by: @darcyclarke

1 files changed, 86 insertions, 6 deletions

diff --git a/test/lib/audit.js b/test/lib/audit.js
index cbbbcf56f..4918cb2fc 100644
--- a/test/lib/audit.js
+++ b/test/lib/audit.js
@@ -1,8 +1,8 @@
-const { test } = require('tap')
+const t = require('tap')
 const requireInject = require('require-inject')
 const audit = require('../../lib/audit.js')
 
-test('should audit using Arborist', t => {
+t.test('should audit using Arborist', t => {
   let ARB_ARGS = null
   let AUDIT_CALLED = false
   let REIFY_OUTPUT_CALLED = false
@@ -29,6 +29,7 @@ test('should audit using Arborist', t => {
       ARB_OBJ = this
       this.audit = () => {
         AUDIT_CALLED = true
+        this.auditReport = {}
       }
     },
     '../../lib/utils/reify-output.js': arb => {
@@ -62,7 +63,7 @@ test('should audit using Arborist', t => {
   t.end()
 })
 
-test('should audit - json', t => {
+t.test('should audit - json', t => {
   const audit = requireInject('../../lib/audit.js', {
     '../../lib/npm.js': {
       prefix: 'foo',
@@ -75,7 +76,9 @@ test('should audit - json', t => {
       exitCode: 0
     }),
     '@npmcli/arborist': function () {
-      this.audit = () => {}
+      this.audit = () => {
+        this.auditReport = {}
+      }
     },
     '../../lib/utils/reify-output.js': () => {},
     '../../lib/utils/output.js': () => {}
@@ -87,7 +90,84 @@ test('should audit - json', t => {
   })
 })
 
-test('completion', t => {
+t.test('report endpoint error', t => {
+  for (const json of [true, false]) {
+    t.test(`json=${json}`, t => {
+      const OUTPUT = []
+      const LOGS = []
+      const mocks = {
+        '../../lib/npm.js': {
+          prefix: 'foo',
+          command: 'audit',
+          flatOptions: {
+            json
+          },
+          log: {
+            warn: (...warning) => LOGS.push(warning)
+          }
+        },
+        'npm-audit-report': () => {
+          throw new Error('should not call audit report when there are errors')
+        },
+        '@npmcli/arborist': function () {
+          this.audit = () => {
+            this.auditReport = {
+              error: {
+                message: 'hello, this didnt work',
+                method: 'POST',
+                uri: 'https://example.com/',
+                headers: {
+                  head: ['ers']
+                },
+                statusCode: 420,
+                body: json ? { nope: 'lol' }
+                  : Buffer.from('i had a vuln but i eated it lol')
+              }
+            }
+          }
+        },
+        '../../lib/utils/reify-output.js': () => {},
+        '../../lib/utils/output.js': (...msg) => {
+          OUTPUT.push(msg)
+        }
+      }
+      // have to pass mocks to both to get the npm and output set right
+      const auditError = requireInject('../../lib/utils/audit-error.js', mocks)
+      const audit = requireInject('../../lib/audit.js', {
+        ...mocks,
+        '../../lib/utils/audit-error.js': auditError
+      })
+
+      audit([], (err) => {
+        t.equal(err, 'audit endpoint returned an error')
+        t.strictSame(OUTPUT, [
+          [
+            json ? '{\n' +
+              '  "message": "hello, this didnt work",\n' +
+              '  "method": "POST",\n' +
+              '  "uri": "https://example.com/",\n' +
+              '  "headers": {\n' +
+              '    "head": [\n' +
+              '      "ers"\n' +
+              '    ]\n' +
+              '  },\n' +
+              '  "statusCode": 420,\n' +
+              '  "body": {\n' +
+              '    "nope": "lol"\n' +
+              '  }\n' +
+              '}'
+              : 'i had a vuln but i eated it lol'
+          ]
+        ])
+        t.strictSame(LOGS, [['audit', 'hello, this didnt work']])
+        t.end()
+      })
+    })
+  }
+  t.end()
+})
+
+t.test('completion', t => {
   t.test('fix', t => {
     audit.completion({
       conf: { argv: { remain: ['npm', 'audit'] } }
@@ -117,4 +197,4 @@ test('completion', t => {
   })
 
   t.end()
-})
\ No newline at end of file
+})