Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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.
  • Loading branch information
isaacs committed Aug 1, 2019
commit d8a143a9262a979ae897b92f15d3be80cdde77fb
1 change: 1 addition & 0 deletions bin/npm-cli.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
7 changes: 4 additions & 3 deletions lib/utils/error-handler.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
49 changes: 37 additions & 12 deletions lib/utils/error-message.js
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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':
Expand Down
38 changes: 38 additions & 0 deletions test/tap/cache-eacces-error-message.js
Original file line number Diff line number Diff line change
@@ -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')
})