From 54eae3063eeb197225ee930525a1316e34ecf34c Mon Sep 17 00:00:00 2001 From: Gar Date: Mon, 14 Jun 2021 10:39:53 -0700 Subject: [PATCH] chore(errorHandler): rename to exit handler Cleaned the code up too PR-URL: https://github.com/npm/cli/pull/3416 Credit: @wraithgar Close: #3416 Reviewed-by: @nlf --- lib/cli.js | 45 +++---- .../{error-handler.js => exit-handler.js} | 118 ++++++++---------- lib/utils/explain-eresolve.js | 2 +- ...r.js.test.cjs => exit-handler.js.test.cjs} | 6 +- test/lib/cli.js | 72 ++++------- test/lib/explore.js | 21 ---- test/lib/load-all.js | 8 +- .../{error-handler.js => exit-handler.js} | 82 +++++------- 8 files changed, 140 insertions(+), 214 deletions(-) rename lib/utils/{error-handler.js => exit-handler.js} (71%) rename tap-snapshots/test/lib/utils/{error-handler.js.test.cjs => exit-handler.js.test.cjs} (66%) rename test/lib/utils/{error-handler.js => exit-handler.js} (86%) diff --git a/lib/cli.js b/lib/cli.js index fbceb459db97c..106dd629d4e19 100644 --- a/lib/cli.js +++ b/lib/cli.js @@ -19,8 +19,8 @@ module.exports = (process) => { checkForUnsupportedNode() const npm = require('../lib/npm.js') - const errorHandler = require('../lib/utils/error-handler.js') - errorHandler.setNpm(npm) + const exitHandler = require('../lib/utils/exit-handler.js') + exitHandler.setNpm(npm) // if npm is called as "npmg" or "npm_g", then // run in global mode. @@ -32,20 +32,22 @@ module.exports = (process) => { log.info('using', 'npm@%s', npm.version) log.info('using', 'node@%s', process.version) - process.on('uncaughtException', errorHandler) - process.on('unhandledRejection', errorHandler) + process.on('uncaughtException', exitHandler) + process.on('unhandledRejection', exitHandler) + + const updateNotifier = require('../lib/utils/update-notifier.js') // now actually fire up npm and run the command. // this is how to use npm programmatically: - const updateNotifier = require('../lib/utils/update-notifier.js') npm.load(async er => { + // Any exceptions here will be picked up by the uncaughtException handler if (er) - return errorHandler(er) + return exitHandler(er) // npm --version=cli if (npm.config.get('version', 'cli')) { npm.output(npm.version) - return errorHandler.exit(0) + return exitHandler() } // npm --versions=cli @@ -57,22 +59,21 @@ module.exports = (process) => { updateNotifier(npm) const cmd = npm.argv.shift() + if (!cmd) { + npm.output(npm.usage) + process.exitCode = 1 + return exitHandler() + } + const impl = npm.commands[cmd] - if (impl) - impl(npm.argv, errorHandler) - else { - try { - if (cmd) { - const didYouMean = require('./utils/did-you-mean.js') - const suggestions = await didYouMean(npm, npm.localPrefix, cmd) - npm.output(`Unknown command: "${cmd}"${suggestions}\n\nTo see a list of supported npm commands, run:\n npm help`) - } else - npm.output(npm.usage) - process.exitCode = 1 - return errorHandler() - } catch (err) { - errorHandler(err) - } + if (!impl) { + const didYouMean = require('./utils/did-you-mean.js') + const suggestions = await didYouMean(npm, npm.localPrefix, cmd) + npm.output(`Unknown command: "${cmd}"${suggestions}\n\nTo see a list of supported npm commands, run:\n npm help`) + process.exitCode = 1 + return exitHandler() } + + impl(npm.argv, exitHandler) }) } diff --git a/lib/utils/error-handler.js b/lib/utils/exit-handler.js similarity index 71% rename from lib/utils/error-handler.js rename to lib/utils/exit-handler.js index f40e1f04fb180..dc499f526a208 100644 --- a/lib/utils/error-handler.js +++ b/lib/utils/exit-handler.js @@ -1,17 +1,20 @@ -let npm // set by the cli -let cbCalled = false const log = require('npmlog') -let itWorked = false +const os = require('os') const path = require('path') const writeFileAtomic = require('write-file-atomic') const mkdirp = require('mkdirp-infer-owner') const fs = require('graceful-fs') -let wroteLogFile = false -let exitCode = 0 + const errorMessage = require('./error-message.js') const replaceInfo = require('./replace-info.js') +let exitHandlerCalled = false let logFileName +let npm // set by the cli +let wroteLogFile = false + +const timings = {} + const getLogFile = () => { // we call this multiple times, so we need to treat it as a singleton because // the date is part of the name @@ -21,7 +24,6 @@ const getLogFile = () => { return logFileName } -const timings = {} process.on('timing', (name, value) => { if (timings[name]) timings[name] += value @@ -53,22 +55,20 @@ process.on('exit', code => { } } - if (code) - itWorked = false - if (itWorked) + if (!code) log.info('ok') else { - if (!cbCalled) { - log.error('', 'cb() never called!') + if (!exitHandlerCalled) { + log.error('', 'Exit handler never called!') console.error('') log.error('', 'This is an error with npm itself. Please report this error at:') log.error('', ' ') + // TODO this doesn't have an npm.config.loaded guard writeLogFile() } - - if (code) - log.verbose('code', code) + log.verbose('code', code) } + // In timing mode we always write the log file if (npm.config && npm.config.loaded && npm.config.get('timing') && !wroteLogFile) writeLogFile() if (wroteLogFile) { @@ -83,52 +83,46 @@ process.on('exit', code => { ' ' + getLogFile(), ].join('\n') ) - wroteLogFile = false } - // actually exit. - if (exitCode === 0 && !itWorked) - exitCode = 1 + // these are needed for the tests to have a clean slate in each test case + exitHandlerCalled = false + wroteLogFile = false - if (exitCode !== 0) - process.exit(exitCode) + // actually exit. + process.exit(code) }) const exit = (code, noLog) => { - exitCode = exitCode || process.exitCode || code - - log.verbose('exit', code) + log.verbose('exit', code || 0) if (log.level === 'silent') noLog = true - const reallyExit = () => { - itWorked = !code - - // Exit directly -- nothing in the CLI should still be running in the - // background at this point, and this makes sure anything left dangling - // for whatever reason gets thrown away, instead of leaving the CLI open - // - // Commands that expect long-running actions should just delay `cb()` - process.stdout.write('', () => { - process.exit(code) - }) - } - + // noLog is true if there was an error, including if config wasn't loaded, so + // this doesn't need a config.loaded guard if (code && !noLog) writeLogFile() - reallyExit() + + // Exit directly -- nothing in the CLI should still be running in the + // background at this point, and this makes sure anything left dangling + // for whatever reason gets thrown away, instead of leaving the CLI open + process.stdout.write('', () => { + // `|| process.exitCode` supports a single use case, where we set the exit + // code to 1 if npm is called with no arguments + process.exit(code) + }) } -const errorHandler = (er) => { +const exitHandler = (err) => { log.disableProgress() if (!npm.config || !npm.config.loaded) { // logging won't work unless we pretend that it's ready - er = er || new Error('Exit prior to config file resolving.') - console.error(er.stack || er.message) + err = err || new Error('Exit prior to config file resolving.') + console.error(err.stack || err.message) } - if (cbCalled) - er = er || new Error('Callback called more than once.') + if (exitHandlerCalled) + err = err || new Error('Exit handler called more than once.') // only show the notification if it finished before the other stuff we // were doing. no need to hang on `npm -v` or something. @@ -139,40 +133,39 @@ const errorHandler = (er) => { log.level = level } - cbCalled = true - if (!er) - return exit(0) + exitHandlerCalled = true + if (!err) + return exit() // 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.shelloutCommands.includes(npm.command) - const quietShellout = isShellout && typeof er.code === 'number' && er.code + const quietShellout = isShellout && typeof err.code === 'number' && err.code if (quietShellout) - return exit(er.code, true) - else if (typeof er === 'string') { - log.error('', er) + return exit(err.code, true) + else if (typeof err === 'string') { + log.error('', err) return exit(1, true) - } else if (!(er instanceof Error)) { - log.error('weird error', er) + } else if (!(err instanceof Error)) { + log.error('weird error', err) return exit(1, true) } - if (!er.code) { - const matchErrorCode = er.message.match(/^(?:Error: )?(E[A-Z]+)/) - er.code = matchErrorCode && matchErrorCode[1] + if (!err.code) { + const matchErrorCode = err.message.match(/^(?:Error: )?(E[A-Z]+)/) + err.code = matchErrorCode && matchErrorCode[1] } for (const k of ['type', 'stack', 'statusCode', 'pkgid']) { - const v = er[k] + const v = err[k] if (v) log.verbose(k, replaceInfo(v)) } log.verbose('cwd', process.cwd()) - const os = require('os') const args = replaceInfo(process.argv) log.verbose('', os.type() + ' ' + os.release()) log.verbose('argv', args.map(JSON.stringify).join(' ')) @@ -180,19 +173,19 @@ const errorHandler = (er) => { log.verbose('npm ', 'v' + npm.version) for (const k of ['code', 'syscall', 'file', 'path', 'dest', 'errno']) { - const v = er[k] + const v = err[k] if (v) log.error(k, v) } - const msg = errorMessage(er, npm) + const msg = errorMessage(err, npm) for (const errline of [...msg.summary, ...msg.detail]) log.error(...errline) if (npm.config && npm.config.get('json')) { const error = { error: { - code: er.code, + code: err.code, summary: messageText(msg.summary), detail: messageText(msg.detail), }, @@ -200,7 +193,7 @@ const errorHandler = (er) => { console.error(JSON.stringify(error, null, 2)) } - exit(typeof er.errno === 'number' ? er.errno : typeof er.code === 'number' ? er.code : 1) + exit(typeof err.errno === 'number' ? err.errno : typeof err.code === 'number' ? err.code : 1) } const messageText = msg => msg.map(line => line.slice(1).join(' ')).join('\n') @@ -209,8 +202,6 @@ const writeLogFile = () => { if (wroteLogFile) return - const os = require('os') - try { let logOutput = '' log.record.forEach(m => { @@ -243,8 +234,7 @@ const writeLogFile = () => { } } -module.exports = errorHandler -module.exports.exit = exit +module.exports = exitHandler module.exports.setNpm = (n) => { npm = n } diff --git a/lib/utils/explain-eresolve.js b/lib/utils/explain-eresolve.js index b35a32c6f935d..fa3c6bda52293 100644 --- a/lib/utils/explain-eresolve.js +++ b/lib/utils/explain-eresolve.js @@ -1,4 +1,4 @@ -// this is called when an ERESOLVE error is caught in the error-handler, +// this is called when an ERESOLVE error is caught in the exit-handler, // or when there's a log.warn('eresolve', msg, explanation), to turn it // into a human-intelligible explanation of what's wrong and how to fix. const { writeFileSync } = require('fs') diff --git a/tap-snapshots/test/lib/utils/error-handler.js.test.cjs b/tap-snapshots/test/lib/utils/exit-handler.js.test.cjs similarity index 66% rename from tap-snapshots/test/lib/utils/error-handler.js.test.cjs rename to tap-snapshots/test/lib/utils/exit-handler.js.test.cjs index 78a9eef217f35..0a5ed59a1157c 100644 --- a/tap-snapshots/test/lib/utils/error-handler.js.test.cjs +++ b/tap-snapshots/test/lib/utils/exit-handler.js.test.cjs @@ -5,14 +5,14 @@ * Make sure to inspect the output below. Do not ignore changes! */ 'use strict' -exports[`test/lib/utils/error-handler.js TAP handles unknown error > should have expected log contents for unknown error 1`] = ` +exports[`test/lib/utils/exit-handler.js TAP handles unknown error > should have expected log contents for unknown error 1`] = ` 0 verbose code 1 1 error foo A complete log of this run can be found in: -1 error foo {CWD}/test/lib/utils/tap-testdir-error-handler/_logs/expecteddate-debug.log +1 error foo {CWD}/test/lib/utils/tap-testdir-exit-handler/_logs/expecteddate-debug.log 2 verbose stack Error: ERROR 3 verbose cwd {CWD} 4 verbose Foo 1.0.0 -5 verbose argv "/node" "{CWD}/test/lib/utils/error-handler.js" +5 verbose argv "/node" "{CWD}/test/lib/utils/exit-handler.js" 6 verbose node v1.0.0 7 verbose npm v1.0.0 8 error foo code ERROR diff --git a/test/lib/cli.js b/test/lib/cli.js index d0a9e0bd49200..132d4224b6b79 100644 --- a/test/lib/cli.js +++ b/test/lib/cli.js @@ -24,20 +24,16 @@ const unsupportedMock = { checkForUnsupportedNode: () => {}, } -let errorHandlerCalled = null -let errorHandlerNpm = null -let errorHandlerCb -const errorHandlerMock = (...args) => { - errorHandlerCalled = args - if (errorHandlerCb) - errorHandlerCb() +let exitHandlerCalled = null +let exitHandlerNpm = null +let exitHandlerCb +const exitHandlerMock = (...args) => { + exitHandlerCalled = args + if (exitHandlerCb) + exitHandlerCb() } -let errorHandlerExitCalled = null -errorHandlerMock.exit = code => { - errorHandlerExitCalled = code -} -errorHandlerMock.setNpm = npm => { - errorHandlerNpm = npm +exitHandlerMock.setNpm = npm => { + exitHandlerNpm = npm } const logs = [] @@ -52,7 +48,7 @@ const cli = t.mock('../../lib/cli.js', { '../../lib/utils/update-notifier.js': async () => null, '../../lib/utils/did-you-mean.js': () => '\ntest did you mean', '../../lib/utils/unsupported.js': unsupportedMock, - '../../lib/utils/error-handler.js': errorHandlerMock, + '../../lib/utils/exit-handler.js': exitHandlerMock, npmlog: npmlogMock, }) @@ -64,7 +60,7 @@ t.test('print the version, and treat npm_g to npm -g', t => { proc.argv.length = 0 logs.length = 0 npmOutputs.length = 0 - errorHandlerExitCalled = null + exitHandlerCalled = null }) const { argv } = process @@ -87,7 +83,7 @@ t.test('print the version, and treat npm_g to npm -g', t => { ['info', 'using', 'node@%s', '420.69.lol'], ]) t.strictSame(npmOutputs, [['99.99.99']]) - t.strictSame(errorHandlerExitCalled, 0) + t.strictSame(exitHandlerCalled, []) t.end() }) @@ -108,7 +104,7 @@ t.test('calling with --versions calls npm version with no args', t => { proc.argv.length = 0 logs.length = 0 npmOutputs.length = 0 - errorHandlerExitCalled = null + exitHandlerCalled = null delete npmock.commands.version }) @@ -124,7 +120,7 @@ t.test('calling with --versions calls npm version with no args', t => { ]) t.strictSame(npmOutputs, []) - t.strictSame(errorHandlerExitCalled, null) + t.strictSame(exitHandlerCalled, null) t.strictSame(args, []) t.end() @@ -175,39 +171,17 @@ t.test('gracefully handles error printing usage', t => { const { output } = npmock t.teardown(() => { npmock.output = output - errorHandlerCb = null - errorHandlerCalled = null - }) - const proc = { - argv: ['node', 'npm'], - on: () => {}, - } - npmock.argv = [] - errorHandlerCb = () => { - t.match(errorHandlerCalled, [], 'should call errorHandler with no args') - t.match(errorHandlerNpm, npmock, 'errorHandler npm is set') - t.end() - } - cli(proc) -}) - -t.test('handles output error', t => { - const { output } = npmock - t.teardown(() => { - npmock.output = output - errorHandlerCb = null - errorHandlerCalled = null + exitHandlerCb = null + exitHandlerCalled = null }) const proc = { argv: ['node', 'npm'], on: () => {}, } npmock.argv = [] - npmock.output = () => { - throw new Error('ERR') - } - errorHandlerCb = () => { - t.match(errorHandlerCalled, /ERR/, 'should call errorHandler with error') + exitHandlerCb = () => { + t.match(exitHandlerCalled, [], 'should call exitHandler with no args') + t.match(exitHandlerNpm, npmock, 'exitHandler npm is set') t.end() } cli(proc) @@ -215,8 +189,8 @@ t.test('handles output error', t => { t.test('load error calls error handler', t => { t.teardown(() => { - errorHandlerCb = null - errorHandlerCalled = null + exitHandlerCb = null + exitHandlerCalled = null LOAD_ERROR = null }) @@ -226,8 +200,8 @@ t.test('load error calls error handler', t => { argv: ['node', 'npm', 'asdf'], on: () => {}, } - errorHandlerCb = () => { - t.strictSame(errorHandlerCalled, [er]) + exitHandlerCb = () => { + t.strictSame(exitHandlerCalled, [er]) t.end() } cli(proc) diff --git a/test/lib/explore.js b/test/lib/explore.js index b49c885cbcb26..fd9949e73fc4c 100644 --- a/test/lib/explore.js +++ b/test/lib/explore.js @@ -43,15 +43,11 @@ const mockRunScript = ({ pkg, banner, path, event, stdio }) => { } const output = [] -let ERROR_HANDLER_CALLED = null const logs = [] const getExplore = (windows) => { const Explore = t.mock('../../lib/explore.js', { '../../lib/utils/is-windows.js': windows, path: require('path')[windows ? 'win32' : 'posix'], - '../../lib/utils/error-handler.js': er => { - ERROR_HANDLER_CALLED = er - }, 'read-package-json-fast': mockRPJ, '@npmcli/run-script': mockRunScript, }) @@ -83,11 +79,9 @@ t.test('basic interactive', t => { throw er t.strictSame({ - ERROR_HANDLER_CALLED, RPJ_CALLED, RUN_SCRIPT_EXEC, }, { - ERROR_HANDLER_CALLED: null, RPJ_CALLED: 'c:\\npm\\dir\\pkg\\package.json', RUN_SCRIPT_EXEC: 'shell-command', }) @@ -102,11 +96,9 @@ t.test('basic interactive', t => { throw er t.strictSame({ - ERROR_HANDLER_CALLED, RPJ_CALLED, RUN_SCRIPT_EXEC, }, { - ERROR_HANDLER_CALLED: null, RPJ_CALLED: '/npm/dir/pkg/package.json', RUN_SCRIPT_EXEC: 'shell-command', }) @@ -136,11 +128,9 @@ t.test('interactive tracks exit code', t => { throw er t.strictSame({ - ERROR_HANDLER_CALLED, RPJ_CALLED, RUN_SCRIPT_EXEC, }, { - ERROR_HANDLER_CALLED: null, RPJ_CALLED: 'c:\\npm\\dir\\pkg\\package.json', RUN_SCRIPT_EXEC: 'shell-command', }) @@ -156,11 +146,9 @@ t.test('interactive tracks exit code', t => { throw er t.strictSame({ - ERROR_HANDLER_CALLED, RPJ_CALLED, RUN_SCRIPT_EXEC, }, { - ERROR_HANDLER_CALLED: null, RPJ_CALLED: '/npm/dir/pkg/package.json', RUN_SCRIPT_EXEC: 'shell-command', }) @@ -224,11 +212,9 @@ t.test('basic non-interactive', t => { throw er t.strictSame({ - ERROR_HANDLER_CALLED, RPJ_CALLED, RUN_SCRIPT_EXEC, }, { - ERROR_HANDLER_CALLED: null, RPJ_CALLED: 'c:\\npm\\dir\\pkg\\package.json', RUN_SCRIPT_EXEC: 'ls', }) @@ -241,11 +227,9 @@ t.test('basic non-interactive', t => { throw er t.strictSame({ - ERROR_HANDLER_CALLED, RPJ_CALLED, RUN_SCRIPT_EXEC, }, { - ERROR_HANDLER_CALLED: null, RPJ_CALLED: '/npm/dir/pkg/package.json', RUN_SCRIPT_EXEC: 'ls', }) @@ -310,7 +294,6 @@ t.test('signal fails non-interactive', t => { t.test('usage if no pkg provided', t => { t.teardown(() => { output.length = 0 - ERROR_HANDLER_CALLED = null }) const noPkg = [ [], @@ -326,11 +309,9 @@ t.test('usage if no pkg provided', t => { posixExplore.exec(args, er => { t.match(er, 'Usage:') t.strictSame({ - ERROR_HANDLER_CALLED: null, RPJ_CALLED, RUN_SCRIPT_EXEC, }, { - ERROR_HANDLER_CALLED: null, RPJ_CALLED: '/npm/dir/pkg/package.json', RUN_SCRIPT_EXEC: 'ls', }) @@ -345,11 +326,9 @@ t.test('pkg not installed', t => { posixExplore.exec(['pkg', 'ls'], er => { t.strictSame({ - ERROR_HANDLER_CALLED, RPJ_CALLED, RUN_SCRIPT_EXEC, }, { - ERROR_HANDLER_CALLED: null, RPJ_CALLED: '/npm/dir/pkg/package.json', RUN_SCRIPT_EXEC: 'ls', }) diff --git a/test/lib/load-all.js b/test/lib/load-all.js index 4a975d49a490e..e6e407805346d 100644 --- a/test/lib/load-all.js +++ b/test/lib/load-all.js @@ -22,10 +22,10 @@ else { t.end() }) - t.test('call the error handle so we dont freak out', t => { - const errorHandler = require('../../lib/utils/error-handler.js') - errorHandler.setNpm(npm) - errorHandler() + t.test('call the exit handler so we dont freak out', t => { + const exitHandler = require('../../lib/utils/exit-handler.js') + exitHandler.setNpm(npm) + exitHandler() t.end() }) } diff --git a/test/lib/utils/error-handler.js b/test/lib/utils/exit-handler.js similarity index 86% rename from test/lib/utils/error-handler.js rename to test/lib/utils/exit-handler.js index 9a681e52ce5db..beb1ef8c56690 100644 --- a/test/lib/utils/error-handler.js +++ b/test/lib/utils/exit-handler.js @@ -121,8 +121,8 @@ const mocks = { }), } -let errorHandler = t.mock('../../../lib/utils/error-handler.js', mocks) -errorHandler.setNpm(npm) +let exitHandler = t.mock('../../../lib/utils/exit-handler.js', mocks) +exitHandler.setNpm(npm) t.test('default exit code', (t) => { t.plan(1) @@ -165,7 +165,7 @@ t.test('handles unknown error', (t) => { writeFileAtomic.sync = (filename, content) => { t.equal( redactCwd(filename), - '{CWD}/test/lib/utils/tap-testdir-error-handler/_logs/expecteddate-debug.log', + '{CWD}/test/lib/utils/tap-testdir-exit-handler/_logs/expecteddate-debug.log', 'should use expected log filename' ) t.matchSnapshot( @@ -174,7 +174,7 @@ t.test('handles unknown error', (t) => { ) } - errorHandler(err) + exitHandler(err) t.teardown(() => { writeFileAtomic.sync = sync @@ -197,7 +197,7 @@ t.test('npm.config not ready', (t) => { ) } - errorHandler() + exitHandler() t.teardown(() => { console.error = _error @@ -219,7 +219,7 @@ t.test('fail to write logfile', (t) => { }) t.doesNotThrow( - () => errorHandler(err), + () => exitHandler(err), 'should not throw on cache write failure' ) }) @@ -244,7 +244,7 @@ t.test('console.log output using --json', (t) => { ) } - errorHandler(new Error('Error: EBADTHING Something happened')) + exitHandler(new Error('Error: EBADTHING Something happened')) t.teardown(() => { console.error = _error @@ -271,7 +271,7 @@ t.test('throw a non-error obj', (t) => { t.equal(code, 1, 'should exit with code 1') } - errorHandler(weirdError) + exitHandler(weirdError) t.teardown(() => { process.exit = _exit @@ -295,7 +295,7 @@ t.test('throw a string error', (t) => { t.equal(code, 1, 'should exit with code 1') } - errorHandler(error) + exitHandler(error) t.teardown(() => { process.exit = _exit @@ -315,7 +315,7 @@ t.test('update notification', (t) => { t.equal(msg, updateMsg, 'should show update message') } - errorHandler(err) + exitHandler(err) t.teardown(() => { npmlog.notice = _notice @@ -355,7 +355,7 @@ t.test('it worked', (t) => { const _exit = process.exit process.exit = (code) => { process.exit = _exit - t.equal(code, 0, 'should exit with code 0') + t.false(code, 'should exit with no code') const _info = npmlog.info npmlog.info = (msg) => { @@ -371,14 +371,14 @@ t.test('it worked', (t) => { config.values.timing = true }) - errorHandler.exit(0) + exitHandler() }) t.test('uses code from errno', (t) => { t.plan(1) - errorHandler = t.mock('../../../lib/utils/error-handler.js', mocks) - errorHandler.setNpm(npm) + exitHandler = t.mock('../../../lib/utils/exit-handler.js', mocks) + exitHandler.setNpm(npm) npmlog.level = 'silent' const _exit = process.exit @@ -386,7 +386,7 @@ t.test('uses code from errno', (t) => { t.equal(code, 127, 'should use set errno') } - errorHandler(Object.assign( + exitHandler(Object.assign( new Error('Error with errno'), { errno: 127, @@ -402,8 +402,8 @@ t.test('uses code from errno', (t) => { t.test('uses exitCode as code if using a number', (t) => { t.plan(1) - errorHandler = t.mock('../../../lib/utils/error-handler.js', mocks) - errorHandler.setNpm(npm) + exitHandler = t.mock('../../../lib/utils/exit-handler.js', mocks) + exitHandler.setNpm(npm) npmlog.level = 'silent' const _exit = process.exit @@ -411,7 +411,7 @@ t.test('uses exitCode as code if using a number', (t) => { t.equal(code, 404, 'should use code if a number') } - errorHandler(Object.assign( + exitHandler(Object.assign( new Error('Error with code type number'), { code: 404, @@ -424,25 +424,25 @@ t.test('uses exitCode as code if using a number', (t) => { }) }) -t.test('call errorHandler with no error', (t) => { +t.test('call exitHandler with no error', (t) => { t.plan(1) - errorHandler = t.mock('../../../lib/utils/error-handler.js', mocks) - errorHandler.setNpm(npm) + exitHandler = t.mock('../../../lib/utils/exit-handler.js', mocks) + exitHandler.setNpm(npm) const _exit = process.exit process.exit = (code) => { - t.equal(code, 0, 'should exit with code 0') + t.equal(code, undefined, 'should exit with code undefined') } t.teardown(() => { process.exit = _exit }) - errorHandler() + exitHandler() }) -t.test('callback called twice', (t) => { +t.test('exit handler called twice', (t) => { t.plan(2) const _verbose = npmlog.verbose @@ -450,13 +450,13 @@ t.test('callback called twice', (t) => { t.equal(key, 'stack', 'should log stack in verbose level') t.match( value, - /Error: Callback called more than once./, + /Error: Exit handler called more than once./, 'should have expected error msg' ) npmlog.verbose = _verbose } - errorHandler() + exitHandler() }) t.test('defaults to log error msg if stack is missing', (t) => { @@ -480,35 +480,17 @@ t.test('defaults to log error msg if stack is missing', (t) => { t.equal(msg, 'Error with no stack', 'should use error msg') } - errorHandler(noStackErr) + exitHandler(noStackErr) }) -t.test('set it worked', (t) => { - t.plan(1) - - errorHandler = t.mock('../../../lib/utils/error-handler.js', mocks) - errorHandler.setNpm(npm) - - const _exit = process.exit - process.exit = () => { - t.ok('ok') - } - - t.teardown(() => { - process.exit = _exit - }) - - errorHandler.exit(0, true) -}) - -t.test('use exitCode when emitting exit event', (t) => { +t.test('exits cleanly when emitting exit event', (t) => { t.plan(1) npmlog.level = 'silent' const _exit = process.exit process.exit = (code) => { process.exit = _exit - t.equal(code, 1, 'should exit with code 1') + t.same(code, null, 'should exit with code null') } t.teardown(() => { @@ -549,7 +531,7 @@ t.test('do no fancy handling for shellouts', t => { t.test('shellout with a numeric error code', t => { EXPECT_EXIT = 5 - errorHandler(Object.assign(new Error(), { code: 5 })) + exitHandler(Object.assign(new Error(), { code: 5 })) t.equal(EXPECT_EXIT, 0, 'called process.exit') // should log no warnings or errors, verbose/silly is fine. t.strictSame(loudNoises(), [], 'no noisy warnings') @@ -558,7 +540,7 @@ t.test('do no fancy handling for shellouts', t => { t.test('shellout without a numeric error code (something in npm)', t => { EXPECT_EXIT = 1 - errorHandler(Object.assign(new Error(), { code: 'banana stand' })) + exitHandler(Object.assign(new Error(), { code: 'banana stand' })) t.equal(EXPECT_EXIT, 0, 'called process.exit') // should log some warnings and errors, because something weird happened t.strictNotSame(loudNoises(), [], 'bring the noise') @@ -567,7 +549,7 @@ t.test('do no fancy handling for shellouts', t => { t.test('shellout with code=0 (extra weird?)', t => { EXPECT_EXIT = 1 - errorHandler(Object.assign(new Error(), { code: 0 })) + exitHandler(Object.assign(new Error(), { code: 0 })) t.equal(EXPECT_EXIT, 0, 'called process.exit') // should log some warnings and errors, because something weird happened t.strictNotSame(loudNoises(), [], 'bring the noise')