Handle unhandledRejections, help with cache eacces

Suggested by @godmar in
https://npm.community/t/npm-err-cb-never-called-permission-denied/9167/5

Incidentally, this turned up that we're catching uncaughtExceptions in
the main npm functions, but not unhandledRejections!

Tracing this through, it seems like node-fetch-npm's use of cacache is
particularly brittle.  Any throw that comes from cacache is not caught
properly, since node-fetch-npm is all streams and callbacks.  The naive
approach (just adding a catch and failing the callback) doesn't work,
because then make-fetch-happen and npm-registry-fetch interpret the
failure as an invalid response, when actually it was a local cache
error.

So, a bit more love and polish is definitely still needed in the
guts of npm's fetching and caching code paths.  In the meantime, though,
handling any unhandledRejection at the top level prevents at least the
worst and most useless type of error message.

PR-URL: https://github.com/npm/cli/pull/227
Credit: @isaacs
Close: #227
Reviewed-by: @isaacs

4 files changed, 80 insertions, 15 deletions

diff --git a/bin/npm-cli.js b/bin/npm-cli.js
index 705aa472e..93eddc7a3 100755
--- a/bin/npm-cli.js
+++ b/bin/npm-cli.js
@@ -62,6 +62,7 @@
   log.info('using', 'node@%s', process.version)
 
   process.on('uncaughtException', errorHandler)
+  process.on('unhandledRejection', errorHandler)
 
   if (conf.usage && npm.command !== 'help') {
     npm.argv.unshift(npm.command)
diff --git a/lib/utils/error-handler.js b/lib/utils/error-handler.js
index 7cb43be29..39e0035c2 100644
--- a/lib/utils/error-handler.js
+++ b/lib/utils/error-handler.js
@@ -187,11 +187,12 @@ function errorHandler (er) {
   log.verbose('npm ', 'v' + npm.version)
 
   ;[
+    'code',
+    'syscall',
     'file',
     'path',
-    'code',
-    'errno',
-    'syscall'
+    'dest',
+    'errno'
   ].forEach(function (k) {
     var v = er[k]
     if (v) log.error(k, v)
diff --git a/lib/utils/error-message.js b/lib/utils/error-message.js
index bf5d65c0d..ea8b05938 100644
--- a/lib/utils/error-message.js
+++ b/lib/utils/error-message.js
@@ -2,6 +2,7 @@
 var npm = require('../npm.js')
 var util = require('util')
 var nameValidator = require('validate-npm-package-name')
+var npmlog = require('npmlog')
 
 module.exports = errorMessage
 
@@ -33,18 +34,42 @@ function errorMessage (er) {
 
     case 'EACCES':
     case 'EPERM':
-      short.push(['', er])
-      detail.push([
-        '',
-        [
-          '\nThe operation was rejected by your operating system.',
-          (process.platform === 'win32'
-            ? 'It\'s possible that the file was already in use (by a text editor or antivirus),\nor that you lack permissions to access it.'
-            : 'It is likely you do not have the permissions to access this file as the current user'),
-          '\nIf you believe this might be a permissions issue, please double-check the',
-          'permissions of the file and its containing directories, or try running',
-          'the command again as root/Administrator (though this is not recommended).'
-        ].join('\n')])
+      const isCachePath = typeof er.path === 'string' &&
+        er.path.startsWith(npm.config.get('cache'))
+      const isCacheDest = typeof er.dest === 'string' &&
+        er.dest.startsWith(npm.config.get('cache'))
+
+      const isWindows = process.platform === 'win32'
+
+      if (!isWindows && (isCachePath || isCacheDest)) {
+        // user probably doesn't need this, but still add it to the debug log
+        npmlog.verbose(er.stack)
+        short.push([
+          '',
+          [
+            '',
+            'Your cache folder contains root-owned files, due to a bug in',
+            'previous versions of npm which has since been addressed.',
+            '',
+            'To permanently fix this problem, please run:',
+            `  sudo chown -R ${process.getuid()}:${process.getgid()} ${JSON.stringify(npm.config.get('cache'))}`
+          ].join('\n')
+        ])
+      } else {
+        short.push(['', er])
+        detail.push([
+          '',
+          [
+            '\nThe operation was rejected by your operating system.',
+            (process.platform === 'win32'
+              ? 'It\'s possible that the file was already in use (by a text editor or antivirus),\n' +
+                'or that you lack permissions to access it.'
+              : 'It is likely you do not have the permissions to access this file as the current user'),
+            '\nIf you believe this might be a permissions issue, please double-check the',
+            'permissions of the file and its containing directories, or try running',
+            'the command again as root/Administrator.'
+          ].join('\n')])
+      }
       break
 
     case 'ELIFECYCLE':
diff --git a/test/tap/cache-eacces-error-message.js b/test/tap/cache-eacces-error-message.js
new file mode 100644
index 000000000..aa112eba4
--- /dev/null
+++ b/test/tap/cache-eacces-error-message.js
@@ -0,0 +1,38 @@
+const npm = require('../../lib/npm.js')
+const t = require('tap')
+
+if (process.platform === 'win32') {
+  t.plan(0, 'this is a unix-only thing')
+  process.exit(0)
+}
+
+const errorMessage = require('../../lib/utils/error-message.js')
+
+const common = require('../common-tap.js')
+
+t.plan(1)
+
+npm.load({ cache: common.cache }, () => {
+  npm.config.set('cache', common.cache)
+  const er = new Error('access is e, i am afraid')
+  er.code = 'EACCES'
+  er.errno = -13
+  er.path = common.cache + '/src'
+  er.dest = common.cache + '/to'
+
+  t.match(errorMessage(er), {
+    summary: [
+      [
+        '',
+        new RegExp('\n' +
+          'Your cache folder contains root-owned files, due to a bug in\n' +
+          'previous versions of npm which has since been addressed.\n' +
+          '\n' +
+          'To permanently fix this problem, please run:\n' +
+          '  sudo chown -R [0-9]+:[0-9]+ ".*npm_cache_cache-eacces-error-message"'
+        )
+      ]
+    ],
+    detail: []
+  }, 'get the helpful error message')
+})