Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: create exit handler class #7442

Merged
merged 1 commit into from
May 6, 2024
Merged

feat: create exit handler class #7442

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
29 changes: 1 addition & 28 deletions lib/base-cmd.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
const { log, output } = require('proc-log')
const { log } = require('proc-log')

class BaseCommand {
static workspaces = false
Expand Down Expand Up @@ -111,33 +111,6 @@ class BaseCommand {
})
}

async cmdExec (args) {
const { config } = this.npm

if (config.get('usage')) {
return output.standard(this.usage)
}

const hasWsConfig = config.get('workspaces') || config.get('workspace').length
// if cwd is a workspace, the default is set to [that workspace]
const implicitWs = config.get('workspace', 'default').length

// (-ws || -w foo) && (cwd is not a workspace || command is not ignoring implicit workspaces)
if (hasWsConfig && (!implicitWs || !this.constructor.ignoreImplicitWorkspace)) {
if (this.npm.global) {
throw new Error('Workspaces not supported for global packages')
}
if (!this.constructor.workspaces) {
throw Object.assign(new Error('This command does not support workspaces.'), {
code: 'ENOWORKSPACES',
})
}
return this.execWorkspaces(args)
}

return this.exec(args)
}

// Compare the number of entries with what was expected
checkExpected (entries) {
if (!this.npm.config.isDefault('expect-results')) {
Expand Down
22 changes: 7 additions & 15 deletions lib/cli/entry.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ module.exports = async (process, validateEngines) => {
require('graceful-fs').gracefulify(require('fs'))

const satisfies = require('semver/functions/satisfies')
const exitHandler = require('./exit-handler.js')
const ExitHandler = require('./exit-handler.js')
const exitHandler = new ExitHandler({ process })
const Npm = require('../npm.js')
const npm = new Npm()
exitHandler.setNpm(npm)
Expand All @@ -28,8 +29,7 @@ module.exports = async (process, validateEngines) => {

// At this point we've required a few files and can be pretty sure we dont contain invalid syntax for this version of node. It's possible a lazy require would, but that's unlikely enough that it's not worth catching anymore and we attach the more important exit handlers.
validateEngines.off()
process.on('uncaughtException', exitHandler)
process.on('unhandledRejection', exitHandler)
exitHandler.registerUncaughtHandlers()

// It is now safe to log a warning if they are using a version of node that is not going to fail on syntax errors but is still unsupported and untested and might not work reliably. This is safe to use the logger now which we want since this will show up in the error log too.
if (!satisfies(validateEngines.node, validateEngines.engines)) {
Expand All @@ -42,13 +42,13 @@ module.exports = async (process, validateEngines) => {
const { exec, command, args } = await npm.load()

if (!exec) {
return exitHandler()
return exitHandler.exit()
}

if (!command) {
output.standard(npm.usage)
process.exitCode = 1
return exitHandler()
return exitHandler.exit()
}

// Options are prefixed by a hyphen-minus (-, \u2d).
Expand All @@ -72,16 +72,8 @@ module.exports = async (process, validateEngines) => {
updateNotifier(npm).then((msg) => (npm.updateNotification = msg))

await execPromise
return exitHandler()
return exitHandler.exit()
} catch (err) {
if (err.code === 'EUNKNOWNCOMMAND') {
const didYouMean = require('../utils/did-you-mean.js')
const suggestions = await didYouMean(npm.localPrefix, err.command)
output.standard(`Unknown command: "${err.command}"${suggestions}\n`)
output.standard('To see a list of supported npm commands, run:\n npm help')
process.exitCode = 1
return exitHandler()
}
return exitHandler(err)
return exitHandler.exit(err)
}
}
263 changes: 138 additions & 125 deletions lib/cli/exit-handler.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,160 +1,173 @@
const { log, output, META } = require('proc-log')
const errorMessage = require('../utils/error-message.js')
const { redactLog: replaceInfo } = require('@npmcli/redact')
const { errorMessage, getExitCodeFromError } = require('../utils/error-message.js')

let npm = null // set by the cli
let exitHandlerCalled = false
let showLogFileError = false
class ExitHandler {
#npm = null
#process = null
#exited = false
#exitErrorMessage = false

process.on('exit', code => {
const hasLoadedNpm = npm?.config.loaded
#noNpmError = false

if (!code) {
log.info('ok')
} else {
log.verbose('code', code)
get #hasNpm () {
return !!this.#npm
}

if (!exitHandlerCalled) {
process.exitCode = code || 1
log.error('', 'Exit handler never called!')
log.error('', 'This is an error with npm itself. Please report this error at:')
log.error('', ' <https://github.com/npm/cli/issues>')
// eslint-disable-next-line no-console
console.error('')
showLogFileError = true
get #loaded () {
return !!this.#npm?.loaded
}

// npm must be loaded to know where the log file was written
if (hasLoadedNpm) {
npm.finish({ showLogFileError })
// This removes any listeners npm setup, mostly for tests to avoid max listener warnings
npm.unload()
get #showExitErrorMessage () {
if (!this.#loaded) {
return false
}
if (!this.#exited) {
return true
}
return this.#exitErrorMessage
}

// these are needed for the tests to have a clean slate in each test case
exitHandlerCalled = false
showLogFileError = false
})
get #notLoadedOrExited () {
return !this.#loaded && !this.#exited
}

const exitHandler = err => {
exitHandlerCalled = true
setNpm (npm) {
this.#npm = npm
}

const hasLoadedNpm = npm?.config.loaded
constructor ({ process }) {
this.#process = process
this.#process.on('exit', this.#handleProcesExitAndReset)
}

if (!npm) {
err = err || new Error('Exit prior to setting npm in exit handler')
// Don't use proc-log here since npm was never set
// eslint-disable-next-line no-console
console.error(err.stack || err.message)
return process.exit(1)
registerUncaughtHandlers () {
this.#process.on('uncaughtException', this.#handleExit)
this.#process.on('unhandledRejection', this.#handleExit)
}

if (!hasLoadedNpm) {
err = err || new Error('Exit prior to config file resolving.')
// Don't use proc-log here since npm was never loaded
// eslint-disable-next-line no-console
console.error(err.stack || err.message)
exit (err) {
this.#handleExit(err)
}

// only show the notification if it finished.
if (typeof npm.updateNotification === 'string') {
log.notice('', npm.updateNotification, { [META]: true, force: true })
#handleProcesExitAndReset = (code) => {
this.#handleProcessExit(code)

// Reset all the state. This is only relevant for tests since
// in reality the process fully exits here.
this.#process.off('exit', this.#handleProcesExitAndReset)
this.#process.off('uncaughtException', this.#handleExit)
this.#process.off('unhandledRejection', this.#handleExit)
if (this.#loaded) {
this.#npm.unload()
}
this.#npm = null
this.#exited = false
this.#exitErrorMessage = false
}

let exitCode = process.exitCode || 0
let noLogMessage = exitCode !== 0
let jsonError

if (err) {
exitCode = 1
// if we got a command that just shells out to something else, then it
// will presumably print its own errors and exit with a proper status
// code if there's a problem. If we got an error with a code=0, then...
// something else went wrong along the way, so maybe an npm problem?
const isShellout = npm.isShellout
const quietShellout = isShellout && typeof err.code === 'number' && err.code
if (quietShellout) {
exitCode = err.code
noLogMessage = true
} else if (typeof err === 'string') {
// XXX: we should stop throwing strings
log.error('', err)
noLogMessage = true
} else if (!(err instanceof Error)) {
log.error('weird error', err)
noLogMessage = true
} else {
const os = require('node:os')
const fs = require('node:fs')
if (!err.code) {
const matchErrorCode = err.message.match(/^(?:Error: )?(E[A-Z]+)/)
err.code = matchErrorCode && matchErrorCode[1]
}
#handleProcessExit (code) {
// Force exit code to a number if it has not been set
const exitCode = typeof code === 'number' ? code : (this.#exited ? 0 : 1)
this.#process.exitCode = exitCode

for (const k of ['type', 'stack', 'statusCode', 'pkgid']) {
const v = err[k]
if (v) {
log.verbose(k, replaceInfo(v))
}
}
if (this.#notLoadedOrExited) {
// Exit handler was not called and npm was not loaded so we have to log something
this.#logConsoleError(new Error(`Process exited unexpectedly with code: ${exitCode}`))
return
}

log.verbose('cwd', process.cwd())
log.verbose('', os.type() + ' ' + os.release())
log.verbose('node', process.version)
log.verbose('npm ', 'v' + npm.version)
if (this.#logNoNpmError()) {
return
}

for (const k of ['code', 'syscall', 'file', 'path', 'dest', 'errno']) {
const v = err[k]
if (v) {
log.error(k, v)
}
}
const os = require('node:os')
log.verbose('cwd', this.#process.cwd())
log.verbose('os', `${os.type()} ${os.release()}`)
log.verbose('node', this.#process.version)
log.verbose('npm ', `v${this.#npm.version}`)

const { summary, detail, json, files = [] } = errorMessage(err, npm)
jsonError = json

for (let [file, content] of files) {
file = `${npm.logPath}${file}`
content = `'Log files:\n${npm.logFiles.join('\n')}\n\n${content.trim()}\n`
try {
fs.writeFileSync(file, content)
detail.push(['', `\n\nFor a full report see:\n${file}`])
} catch (logFileErr) {
log.warn('', `Could not write error message to ${file} due to ${logFileErr}`)
}
}
// only show the notification if it finished
if (typeof this.#npm.updateNotification === 'string') {
log.notice('', this.#npm.updateNotification, { [META]: true, force: true })
}

for (const errline of [...summary, ...detail]) {
log.error(...errline)
if (!this.#exited) {
log.error('', 'Exit handler never called!')
log.error('', 'This is an error with npm itself. Please report this error at:')
log.error('', ' <https://github.com/npm/cli/issues>')
if (this.#npm.silent) {
output.error('')
}
}

if (typeof err.errno === 'number') {
exitCode = err.errno
} else if (typeof err.code === 'number') {
exitCode = err.code
}
log.verbose('exit', exitCode)

if (exitCode) {
log.verbose('code', exitCode)
} else {
log.info('ok')
}

if (this.#showExitErrorMessage) {
log.error('', this.#npm.exitErrorMessage())
}
}

if (hasLoadedNpm) {
output.flush({ [META]: true, jsonError })
#logConsoleError (err) {
// Run our error message formatters on all errors even if we
// have no npm or an unloaded npm. This will clean the error
// and possible return a formatted message about EACCESS or something.
const { summary, detail } = errorMessage(err, this.#npm)
const formatted = [...new Set([...summary, ...detail].flat().filter(Boolean))].join('\n')
// If we didn't get anything from the formatted message then just display the full stack
// eslint-disable-next-line no-console
console.error(formatted === err.message ? err.stack : formatted)
}

log.verbose('exit', exitCode || 0)
#logNoNpmError (err) {
if (this.#hasNpm) {
return false
}
// Make sure we only log this error once
if (!this.#noNpmError) {
this.#noNpmError = true
this.#logConsoleError(
new Error(`Exit prior to setting npm in exit handler`, err ? { cause: err } : {})
)
}
return true
}

#handleExit = (err) => {
this.#exited = true

// No npm at all
if (this.#logNoNpmError(err)) {
return this.#process.exit(this.#process.exitCode || getExitCodeFromError(err) || 1)
}

showLogFileError = (hasLoadedNpm && npm.silent) || noLogMessage
? false
: !!exitCode
// npm was never loaded but we still might have a config loading error or
// something similar that we can run through the error message formatter
// to give the user a clue as to what happened.s
if (!this.#loaded) {
this.#logConsoleError(new Error('Exit prior to config file resolving', { cause: err }))
return this.#process.exit(this.#process.exitCode || getExitCodeFromError(err) || 1)
}

this.#exitErrorMessage = err?.suppressError === true ? false : !!err

// explicitly call process.exit now so we don't hang on things like the
// update notifier, also flush stdout/err beforehand because process.exit doesn't
// wait for that to happen.
let flushed = 0
const flush = [process.stderr, process.stdout]
const exit = () => ++flushed === flush.length && process.exit(exitCode)
flush.forEach((f) => f.write('', exit))
// Prefer the exit code of the error, then the current process exit code,
// then set it to 1 if we still have an error. Otherwise we call process.exit
// with undefined so that it can determine the final exit code
const exitCode = err?.exitCode ?? this.#process.exitCode ?? (err ? 1 : undefined)

// explicitly call process.exit now so we don't hang on things like the
// update notifier, also flush stdout/err beforehand because process.exit doesn't
// wait for that to happen.
this.#process.stderr.write('', () => this.#process.stdout.write('', () => {
this.#process.exit(exitCode)
}))
}
}

module.exports = exitHandler
module.exports.setNpm = n => (npm = n)
module.exports = ExitHandler
Loading
Loading