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

2 files changed, 54 insertions, 10 deletions

diff --git a/lib/utils/audit-error.js b/lib/utils/audit-error.js
new file mode 100644
index 000000000..91e880c46
--- /dev/null
+++ b/lib/utils/audit-error.js
@@ -0,0 +1,39 @@
+// print an error or just nothing if the audit report has an error
+// this is called by the audit command, and by the reify-output util
+// prints a JSON version of the error if it's --json
+// returns 'true' if there was an error, false otherwise
+
+const output = require('./output.js')
+const npm = require('../npm.js')
+const auditError = (report) => {
+  if (!report || !report.error) {
+    return false
+  }
+
+  if (npm.command !== 'audit') {
+    return true
+  }
+
+  const { error } = report
+
+  // ok, we care about it, then
+  npm.log.warn('audit', error.message)
+  const { body: errBody } = error
+  const body = Buffer.isBuffer(errBody) ? errBody.toString() : errBody
+  if (npm.flatOptions.json) {
+    output(JSON.stringify({
+      message: error.message,
+      method: error.method,
+      uri: error.uri,
+      headers: error.headers,
+      statusCode: error.statusCode,
+      body
+    }, null, 2))
+  } else {
+    output(body)
+  }
+
+  throw 'audit endpoint returned an error'
+}
+
+module.exports = auditError
diff --git a/lib/utils/reify-output.js b/lib/utils/reify-output.js
index 09b31fa94..10b276cd9 100644
--- a/lib/utils/reify-output.js
+++ b/lib/utils/reify-output.js
@@ -16,6 +16,7 @@ const { depth } = require('treeverse')
 const ms = require('ms')
 const auditReport = require('npm-audit-report')
 const { readTree: getFundingInfo } = require('libnpmfund')
+const auditError = require('./audit-error.js')
 
 // TODO: output JSON if flatOptions.json is true
 const reifyOutput = arb => {
@@ -24,13 +25,18 @@ const reifyOutput = arb => {
     return
   }
 
-  const { diff, auditReport, actualTree } = arb
+  const { diff, actualTree } = arb
+
+  // note: fails and crashes if we're running audit fix and there was an error
+  // which is a good thing, because there's no point printing all this other
+  // stuff in that case!
+  const auditReport = auditError(arb.auditReport) ? null : arb.auditReport
 
   const summary = {
     added: 0,
     removed: 0,
     changed: 0,
-    audited: auditReport ? actualTree.inventory.size : 0,
+    audited: auditReport && !auditReport.error ? actualTree.inventory.size : 0,
     funding: 0
   }
 
@@ -64,15 +70,15 @@ const reifyOutput = arb => {
   }
 
   if (npm.flatOptions.json) {
-    if (arb.auditReport) {
-      summary.audit = npm.command === 'audit' ? arb.auditReport
-        : arb.auditReport.toJSON().metadata
+    if (auditReport) {
+      summary.audit = npm.command === 'audit' ? auditReport
+        : auditReport.toJSON().metadata
     }
     output(JSON.stringify(summary, 0, 2))
   } else {
     packagesChangedMessage(summary)
     packagesFundingMessage(summary)
-    printAuditReport(arb)
+    printAuditReport(auditReport)
   }
 }
 
@@ -80,16 +86,15 @@ const reifyOutput = arb => {
 // at the end if there's still stuff, because it's silly for `npm audit`
 // to tell you to run `npm audit` for details.  otherwise, use the summary
 // report.  if we get here, we know it's not quiet or json.
-const printAuditReport = arb => {
-  if (!arb.auditReport) {
+const printAuditReport = report => {
+  if (!report) {
     return
   }
-
   const reporter = npm.command !== 'audit' ? 'install' : 'detail'
   const defaultAuditLevel = npm.command !== 'audit' ? 'none' : 'low'
   const auditLevel = npm.flatOptions.auditLevel || defaultAuditLevel
 
-  const res = auditReport(arb.auditReport, {
+  const res = auditReport(report, {
     reporter,
     ...npm.flatOptions,
     auditLevel