diff --git a/lib/commands/run-script.js b/lib/commands/run-script.js index dd00c98fc8b6e..422cca604685f 100644 --- a/lib/commands/run-script.js +++ b/lib/commands/run-script.js @@ -74,11 +74,13 @@ class RunScript extends BaseCommand { return } - const didYouMean = require('../utils/did-you-mean.js') - const suggestions = await didYouMean(path, event) - throw new Error( - `Missing script: "${event}"${suggestions}\n\nTo see a list of scripts, run:\n npm run` - ) + const suggestions = require('../utils/did-you-mean.js')(pkg, event) + throw new Error([ + `Missing script: "${event}"`, + ...suggestions ? ['', ...suggestions] : [], + 'To see a list of scripts, run:', + ' npm run', + ].join('\n')) } // positional args only added to the main event, not pre/post diff --git a/lib/commands/view.js b/lib/commands/view.js index 155288c96ea85..ad43b9ae2e691 100644 --- a/lib/commands/view.js +++ b/lib/commands/view.js @@ -11,6 +11,7 @@ const { inspect } = require('util') const { packument } = require('pacote') const Queryable = require('../utils/queryable.js') const BaseCommand = require('../base-cmd.js') +const { getError, jsonError, outputError } = require('../utils/error-message.js') const readJson = async file => jsonParse(await readFile(file, 'utf8')) @@ -77,11 +78,7 @@ class View extends BaseCommand { } async exec (args) { - if (!args.length) { - args = ['.'] - } - let pkg = args.shift() - const local = /^\.@/.test(pkg) || pkg === '.' + let { pkg, local, rest } = parseArgs(args) if (local) { if (this.npm.global) { @@ -96,92 +93,87 @@ class View extends BaseCommand { pkg = `${manifest.name}${pkg.slice(1)}` } - let wholePackument = false - if (!args.length) { - args = [''] - wholePackument = true - } - const [pckmnt, data] = await this.getData(pkg, args) - - if (!this.npm.config.get('json') && wholePackument) { - // pretty view (entire packument) - data.map((v) => this.prettyView(pckmnt, v[Object.keys(v)[0]][''])) - } else { - // JSON formatted output (JSON or specific attributes from packument) - let reducedData = data.reduce(reducer, {}) - if (wholePackument) { - // No attributes - reducedData = cleanBlanks(reducedData) - log.silly('view', reducedData) - } - - const msg = await this.jsonData(reducedData, pckmnt._id) - if (msg !== '') { - output.standard(msg) - } - } + await this.#viewPackage(pkg, rest) } async execWorkspaces (args) { - if (!args.length) { - args = ['.'] - } + const { pkg, local, rest } = parseArgs(args) - const pkg = args.shift() - - const local = /^\.@/.test(pkg) || pkg === '.' if (!local) { log.warn('Ignoring workspaces for specified package(s)') - return this.exec([pkg, ...args]) - } - let wholePackument = false - if (!args.length) { - wholePackument = true - args = [''] // getData relies on this + return this.exec([pkg, ...rest]) } - const results = {} + + const json = this.npm.config.get('json') await this.setWorkspaces() - for (const name of this.workspaceNames) { - const wsPkg = `${name}${pkg.slice(1)}` - const [pckmnt, data] = await this.getData(wsPkg, args) - - let reducedData = data.reduce(reducer, {}) - if (wholePackument) { - // No attributes - reducedData = cleanBlanks(reducedData) - log.silly('view', reducedData) - } - if (!this.npm.config.get('json')) { - if (wholePackument) { - data.map((v) => this.prettyView(pckmnt, v[Object.keys(v)[0]][''])) + let hasError = false + for (const name of this.workspaceNames) { + try { + await this.#viewPackage( + `${name}${pkg.slice(1)}`, + rest, + { multiple: !!this.workspaceNames.length } + ) + } catch (e) { + hasError = true + const err = getError(e, { npm: this.npm, command: this }) + if (json) { + output.buffer({ [name]: jsonError(err, this.npm) }) } else { - output.standard(`${name}:`) - const msg = await this.jsonData(reducedData, pckmnt._id) - if (msg !== '') { - output.standard(msg) - } - } - } else { - const msg = await this.jsonData(reducedData, pckmnt._id) - if (msg !== '') { - results[name] = JSON.parse(msg) + outputError(err) } } } - if (Object.keys(results).length > 0) { - output.standard(JSON.stringify(results, null, 2)) + + if (hasError) { + process.exitCode = 1 } } - async getData (pkg, args) { + async #viewPackage (name, args, { multiple = false } = {}) { + const wholePackument = !args.length const json = this.npm.config.get('json') - const opts = { - ...this.npm.flatOptions, - preferOnline: true, - fullMetadata: true, + + // If we are viewing many packages output the name before doing + // any async activity + if (!json && !wholePackument && multiple) { + output.standard(`${name}:`) } + const [pckmnt, data, version] = await this.#getData(name, wholePackument ? [''] : args) + if (wholePackument) { + pckmnt.version = version + } + + // This already outputs to the terminal + if (!json && wholePackument) { + // pretty view (entire packument) + for (const v of data) { + this.#prettyView(pckmnt, v[Object.keys(v)[0]]['']) + } + return + } + + // JSON formatted output (JSON or specific attributes from packument) + let reducedData = data.reduce(reducer, {}) + if (wholePackument) { + // No attributes + reducedData = cleanBlanks(reducedData) + log.silly('view', reducedData) + } + + const msg = this.#packageOutput(reducedData, pckmnt._id) + if (msg !== '') { + if (json && multiple) { + output.buffer({ [name]: JSON.parse(msg) }) + } else { + output.standard(msg) + } + } + } + + async #getData (pkg, args = []) { const spec = npa(pkg) // get the data about this package @@ -191,13 +183,17 @@ class View extends BaseCommand { version = spec.rawSpec } - const pckmnt = await packument(spec, opts) + const pckmnt = await packument(spec, { + ...this.npm.flatOptions, + preferOnline: true, + fullMetadata: true, + }) if (pckmnt['dist-tags']?.[version]) { version = pckmnt['dist-tags'][version] } - if (pckmnt.time && pckmnt.time.unpublished) { + if (pckmnt.time?.unpublished) { const u = pckmnt.time.unpublished const er = new Error(`Unpublished on ${u.time}`) er.statusCode = 404 @@ -240,7 +236,7 @@ class View extends BaseCommand { }) // No data has been pushed because no data is matching the specified version - if (data.length === 0 && version !== 'latest') { + if (!data.length && version !== 'latest') { const er = new Error(`No match found for version ${version}`) er.statusCode = 404 er.code = 'E404' @@ -248,14 +244,18 @@ class View extends BaseCommand { throw er } +<<<<<<< HEAD if (!json && args.length === 1 && args[0] === '') { pckmnt.version = version } return [pckmnt, data] +======= + return [pckmnt, data, version] +>>>>>>> b3de0dead (fix(view): output full json object with errors with workspaces) } - async jsonData (data, name) { + #packageOutput (data, name) { const versions = Object.keys(data) let msg = '' let msgJson = [] @@ -315,7 +315,7 @@ class View extends BaseCommand { return msg.trim() } - prettyView (packu, manifest) { + #prettyView (packu, manifest) { // More modern, pretty printing of default view const unicode = this.npm.config.get('unicode') const chalk = this.npm.chalk @@ -411,6 +411,20 @@ class View extends BaseCommand { module.exports = View +function parseArgs (args) { + if (!args.length) { + args = ['.'] + } + + const pkg = args.shift() + + return { + pkg, + local: /^\.@/.test(pkg) || pkg === '.', + rest: args, + } +} + function cleanBlanks (obj) { const clean = {} Object.keys(obj).forEach((version) => { diff --git a/lib/npm.js b/lib/npm.js index dcd6bdfc046df..bca26b56a2bc8 100644 --- a/lib/npm.js +++ b/lib/npm.js @@ -287,7 +287,12 @@ class Npm { async #handleError (err) { if (err) { - Object.assign(err, await this.#getError(err)) + // Get the local package if it exists for a more helpful error message + const localPkg = await require('@npmcli/package-json') + .normalize(this.localPrefix) + .then(p => p.content) + .catch(() => null) + Object.assign(err, this.#getError(err, { pkg: localPkg })) } // TODO: make this not need to be public @@ -295,11 +300,7 @@ class Npm { output.flush({ [META]: true, - jsonError: err && this.loaded && this.config.get('json') ? { - code: err.code, - summary: (err.summary || []).map(l => l.slice(1).join(' ')).join('\n'), - detail: (err.detail || []).map(l => l.slice(1).join(' ')).join('\n'), - } : null, + ...require('./utils/error-message.js').jsonError(err, this), }) if (err) { @@ -307,90 +308,30 @@ class Npm { } } - async #getError (err) { - const { errorMessage, getExitCodeFromError } = require('./utils/error-message.js') - - // 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? - if (this.#command?.constructor?.isShellout && typeof err.code === 'number' && err.code) { - return { - exitCode: err.code, - suppressError: true, - } - } - - // XXX: we should stop throwing strings - if (typeof err === 'string') { - log.error('', err) - return { - exitCode: 1, - suppressError: true, - } - } - - // XXX: we should stop throwing other non-errors - if (!(err instanceof Error)) { - log.error('weird error', err) - return { - exitCode: 1, - suppressError: true, - } - } - - if (err.code === 'EUNKNOWNCOMMAND') { - const didYouMean = require('./utils/did-you-mean.js') - const suggestions = await didYouMean(this.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') - return { - exitCode: 1, - suppressError: true, - } - } - - err.code ??= err.message.match(/^(?:Error: )?(E[A-Z]+)/)?.[1] - - for (const k of ['type', 'stack', 'statusCode', 'pkgid']) { - const v = err[k] - if (v) { - log.verbose(k, replaceInfo(v)) - } - } - - const exitCode = getExitCodeFromError(err) || 1 - const { summary, detail, files } = errorMessage(err, this) + #getError (rawErr, opts = {}) { + const { getError, outputError } = require('./utils/error-message.js') + const error = getError(rawErr, { + npm: this, + command: this.#command, + ...opts, + }) - const { writeFileSync } = require('node:fs') - for (const [file, content] of files) { + for (const [file, content] of (error.files || [])) { const filePath = `${this.logPath}${file}` const fileContent = `'Log files:\n${this.logFiles.join('\n')}\n\n${content.trim()}\n` try { - writeFileSync(filePath, fileContent) - detail.push(['', `\n\nFor a full report see:\n${filePath}`]) + require('node:fs').writeFileSync(filePath, fileContent) + error.detail = (error.detail || []).concat([ + ['', `\n\nFor a full report see:\n${filePath}`], + ]) } catch (fileErr) { log.warn('', `Could not write error message to ${file} due to ${fileErr}`) } } - for (const k of ['code', 'syscall', 'file', 'path', 'dest', 'errno']) { - const v = err[k] - if (v) { - log.error(k, v) - } - } + outputError(error) - for (const errline of [...summary, ...detail]) { - log.error(...errline) - } - - return { - exitCode, - summary, - detail, - suppressError: false, - } + return error } get title () { diff --git a/lib/utils/did-you-mean.js b/lib/utils/did-you-mean.js index 54c8ff2e35aa6..608a358aa5356 100644 --- a/lib/utils/did-you-mean.js +++ b/lib/utils/did-you-mean.js @@ -1,39 +1,36 @@ const Npm = require('../npm') const { distance } = require('fastest-levenshtein') -const pkgJson = require('@npmcli/package-json') const { commands } = require('./cmd-list.js') -const didYouMean = async (path, scmd) => { - const close = commands.filter(cmd => distance(scmd, cmd) < scmd.length * 0.4 && scmd !== cmd) - let best = [] - for (const str of close) { - const cmd = Npm.cmd(str) - best.push(` npm ${str} # ${cmd.description}`) - } - // We would already be suggesting this in `npm x` so omit them here - const runScripts = ['stop', 'start', 'test', 'restart'] - try { - const { content: { scripts, bin } } = await pkgJson.normalize(path) - best = best.concat( - Object.keys(scripts || {}) - .filter(cmd => distance(scmd, cmd) < scmd.length * 0.4 && !runScripts.includes(cmd)) - .map(str => ` npm run ${str} # run the "${str}" package script`), - Object.keys(bin || {}) - .filter(cmd => distance(scmd, cmd) < scmd.length * 0.4) - /* eslint-disable-next-line max-len */ - .map(str => ` npm exec ${str} # run the "${str}" command from either this or a remote npm package`) - ) - } catch { - // gracefully ignore not being in a folder w/ a package.json - } +const runScripts = ['stop', 'start', 'test', 'restart'] + +const isClose = (scmd, cmd) => distance(scmd, cmd) < scmd.length * 0.4 + +const didYouMean = (pkg, scmd) => { + const { scripts = {}, bin = {} } = pkg || {} + const close = commands.filter(cmd => isClose(scmd, cmd) && scmd !== cmd) + + const best = [ + ...close + .map(str => [str, Npm.cmd(str).description]), + ...Object.keys(scripts) + // We would already be suggesting this in `npm x` so omit them here + .filter(cmd => isClose(scmd, cmd) && !runScripts.includes(cmd)) + .map(str => [`run ${str}`, `run the "${str}" package script`]), + ...Object.keys(bin) + .filter(cmd => isClose(scmd, cmd)) + /* eslint-disable-next-line max-len */ + .map(str => [`exec ${str}`, `run the "${str}" command from either this or a remote npm package`]), + ] if (best.length === 0) { - return '' + return [] } - return best.length === 1 - ? `\n\nDid you mean this?\n${best[0]}` - : `\n\nDid you mean one of these?\n${best.slice(0, 3).join('\n')}` + return [ + `Did you mean ${best.length === 1 ? 'this' : 'one of these'}?`, + ...best.slice(0, 3).map(([msg, comment]) => ` npm ${msg} # ${comment}`), + ] } module.exports = didYouMean diff --git a/lib/utils/display.js b/lib/utils/display.js index 29a1f7951d506..5ca73bb345276 100644 --- a/lib/utils/display.js +++ b/lib/utils/display.js @@ -242,20 +242,28 @@ class Display { switch (level) { case output.KEYS.flush: this.#outputState.buffering = false - if (meta.jsonError && this.#json) { + if (this.#json) { const json = {} for (const item of this.#outputState.buffer) { // index 2 skips the level and meta Object.assign(json, tryJsonParse(item[2])) } - this.#writeOutput( - output.KEYS.standard, - meta, - JSON.stringify({ ...json, error: meta.jsonError }, null, 2) - ) - } else { - this.#outputState.buffer.forEach((item) => this.#writeOutput(...item)) + // XXX: in a future breaking change, all json output should be keyed on a new property + // such as `result`, so that we can be sure that `error` is not already a property on + // the json result which could be the case for things like `npm view . -ws` if there + // is a workspace named "error" + if (meta.error) { + json.error = meta.error + } + // Only write output if we have some json buffered + if (Object.keys(json).length) { + this.#writeOutput(output.KEYS.standard, meta, JSON.stringify(json, null, 2)) + this.#outputState.buffer.length = 0 + break + } } + + this.#outputState.buffer.forEach((item) => this.#writeOutput(...item)) this.#outputState.buffer.length = 0 break diff --git a/lib/utils/error-message.js b/lib/utils/error-message.js index 3d1b18f29dab6..4bf7108d4ef8a 100644 --- a/lib/utils/error-message.js +++ b/lib/utils/error-message.js @@ -1,24 +1,20 @@ const { format } = require('node:util') const { resolve } = require('node:path') const { redactLog: replaceInfo } = require('@npmcli/redact') -const { log } = require('proc-log') +const { log, output } = require('proc-log') const errorMessage = (er, npm) => { - const short = [] + const summary = [] const detail = [] const files = [] - if (er.message) { - er.message = replaceInfo(er.message) - } - if (er.stack) { - er.stack = replaceInfo(er.stack) - } + er.message &&= replaceInfo(er.message) + er.stack &&= replaceInfo(er.stack) switch (er.code) { case 'ERESOLVE': { const { report } = require('./explain-eresolve.js') - short.push(['ERESOLVE', er.message]) + summary.push(['ERESOLVE', er.message]) detail.push(['', '']) // XXX(display): error messages are logged so we use the logColor since that is based // on stderr. This should be handled solely by the display layer so it could also be @@ -31,18 +27,18 @@ const errorMessage = (er, npm) => { case 'ENOLOCK': { const cmd = npm.command || '' - short.push([cmd, 'This command requires an existing lockfile.']) + summary.push([cmd, 'This command requires an existing lockfile.']) detail.push([cmd, 'Try creating one first with: npm i --package-lock-only']) detail.push([cmd, `Original error: ${er.message}`]) break } case 'ENOAUDIT': - short.push(['audit', er.message]) + summary.push(['audit', er.message]) break case 'ECONNREFUSED': - short.push(['', er]) + summary.push(['', er]) detail.push([ '', [ @@ -62,7 +58,7 @@ const errorMessage = (er, npm) => { if (process.platform !== 'win32' && (isCachePath || isCacheDest)) { // user probably doesn't need this, but still add it to the debug log log.verbose(er.stack) - short.push([ + summary.push([ '', [ '', @@ -76,7 +72,7 @@ const errorMessage = (er, npm) => { ].join('\n'), ]) } else { - short.push(['', er]) + summary.push(['', er]) detail.push([ '', [ @@ -97,7 +93,7 @@ const errorMessage = (er, npm) => { } case 'ENOGIT': - short.push(['', er.message]) + summary.push(['', er.message]) detail.push([ '', ['', 'Failed using git.', 'Please check if you have git installed and in your PATH.'].join( @@ -123,7 +119,7 @@ const errorMessage = (er, npm) => { break } } - short.push(['JSON.parse', er.message]) + summary.push(['JSON.parse', er.message]) detail.push([ 'JSON.parse', [ @@ -137,7 +133,7 @@ const errorMessage = (er, npm) => { case 'E401': // E401 is for places where we accidentally neglect OTP stuff if (er.code === 'EOTP' || /one-time pass/.test(er.message)) { - short.push(['', 'This operation requires a one-time password from your authenticator.']) + summary.push(['', 'This operation requires a one-time password from your authenticator.']) detail.push([ '', [ @@ -155,7 +151,7 @@ const errorMessage = (er, npm) => { : er.headers['www-authenticate'].map(au => au.split(/[,\s]+/))[0] if (auth.includes('Bearer')) { - short.push([ + summary.push([ '', 'Unable to authenticate, your authentication token seems to be invalid.', ]) @@ -164,7 +160,7 @@ const errorMessage = (er, npm) => { ['To correct this please try logging in again with:', ' npm login'].join('\n'), ]) } else if (auth.includes('Basic')) { - short.push(['', 'Incorrect or missing password.']) + summary.push(['', 'Incorrect or missing password.']) detail.push([ '', [ @@ -180,14 +176,14 @@ const errorMessage = (er, npm) => { ].join('\n'), ]) } else { - short.push(['', er.message || er]) + summary.push(['', er.message || er]) } } break case 'E404': // There's no need to have 404 in the message as well. - short.push(['404', er.message.replace(/^404\s+/, '')]) + summary.push(['404', er.message.replace(/^404\s+/, '')]) if (er.pkgid && er.pkgid !== '-') { const pkg = er.pkgid.replace(/(?!^)@.*$/, '') @@ -210,7 +206,7 @@ const errorMessage = (er, npm) => { break case 'EPUBLISHCONFLICT': - short.push(['publish fail', 'Cannot publish over existing version.']) + summary.push(['publish fail', 'Cannot publish over existing version.']) detail.push(['publish fail', "Update the 'version' field in package.json and try again."]) detail.push(['publish fail', '']) detail.push(['publish fail', 'To automatically increment version numbers, see:']) @@ -218,8 +214,8 @@ const errorMessage = (er, npm) => { break case 'EISGIT': - short.push(['git', er.message]) - short.push(['git', ' ' + er.path]) + summary.push(['git', er.message]) + summary.push(['git', ' ' + er.path]) detail.push([ 'git', ['Refusing to remove it. Update manually,', 'or move it out of the way first.'].join('\n'), @@ -255,7 +251,7 @@ const errorMessage = (er, npm) => { detailEntry.push(`Actual ${key}:${' '.repeat(padding)}${actual[key]}`) } - short.push([ + summary.push([ 'notsup', [ format( @@ -274,14 +270,14 @@ const errorMessage = (er, npm) => { } case 'EEXIST': - short.push(['', er.message]) - short.push(['', 'File exists: ' + (er.dest || er.path)]) + summary.push(['', er.message]) + summary.push(['', 'File exists: ' + (er.dest || er.path)]) detail.push(['', 'Remove the existing file and try again, or run npm']) detail.push(['', 'with --force to overwrite files recklessly.']) break case 'ENEEDAUTH': - short.push(['need auth', er.message]) + summary.push(['need auth', er.message]) detail.push(['need auth', 'You need to authorize this machine using `npm adduser`']) break @@ -290,7 +286,7 @@ const errorMessage = (er, npm) => { case 'ETIMEDOUT': case 'ERR_SOCKET_TIMEOUT': case 'EAI_FAIL': - short.push(['network', er.message]) + summary.push(['network', er.message]) detail.push([ 'network', [ @@ -303,7 +299,7 @@ const errorMessage = (er, npm) => { break case 'ETARGET': - short.push(['notarget', er.message]) + summary.push(['notarget', er.message]) detail.push([ 'notarget', [ @@ -314,7 +310,7 @@ const errorMessage = (er, npm) => { break case 'E403': - short.push(['403', er.message]) + summary.push(['403', er.message]) detail.push([ '403', [ @@ -326,8 +322,8 @@ const errorMessage = (er, npm) => { break case 'EBADENGINE': - short.push(['engine', er.message]) - short.push(['engine', 'Not compatible with your version of node/npm: ' + er.pkgid]) + summary.push(['engine', er.message]) + summary.push(['engine', 'Not compatible with your version of node/npm: ' + er.pkgid]) detail.push([ 'notsup', [ @@ -343,7 +339,7 @@ const errorMessage = (er, npm) => { break case 'ENOSPC': - short.push(['nospc', er.message]) + summary.push(['nospc', er.message]) detail.push([ 'nospc', [ @@ -354,7 +350,7 @@ const errorMessage = (er, npm) => { break case 'EROFS': - short.push(['rofs', er.message]) + summary.push(['rofs', er.message]) detail.push([ 'rofs', [ @@ -365,7 +361,7 @@ const errorMessage = (er, npm) => { break case 'ENOENT': - short.push(['enoent', er.message]) + summary.push(['enoent', er.message]) detail.push([ 'enoent', [ @@ -379,7 +375,7 @@ const errorMessage = (er, npm) => { case 'EUNKNOWNTYPE': case 'EINVALIDTYPE': case 'ETOOMANYARGS': - short.push(['typeerror', er.stack]) + summary.push(['typeerror', er.stack]) detail.push([ 'typeerror', [ @@ -390,7 +386,7 @@ const errorMessage = (er, npm) => { break default: - short.push(['', er.message || er]) + summary.push(['', er.message || er]) if (er.cause) { detail.push(['cause', er.cause.message]) } @@ -408,7 +404,12 @@ const errorMessage = (er, npm) => { } break } - return { summary: short, detail, files } + + return { + summary, + detail, + files, + } } const getExitCodeFromError = (err) => { @@ -419,7 +420,105 @@ const getExitCodeFromError = (err) => { } } +const getError = (err, { npm, command, pkg } = {}) => { + // 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? + if (command?.constructor?.isShellout && typeof err.code === 'number' && err.code) { + return { + exitCode: err.code, + suppressError: true, + } + } + + // XXX: we should stop throwing strings + if (typeof err === 'string') { + return { + exitCode: 1, + suppressError: true, + summary: [['', err]], + } + } + + // XXX: we should stop throwing other non-errors + if (!(err instanceof Error)) { + return { + exitCode: 1, + suppressError: true, + summary: [['weird error', err]], + } + } + + if (err.code === 'EUNKNOWNCOMMAND') { + const suggestions = require('./did-you-mean.js')(pkg, err.command) + return { + exitCode: 1, + suppressError: true, + standard: [ + `Unknown command: "${err.command}"`, + ...suggestions ? ['', ...suggestions, ''] : [], + 'To see a list of supported npm commands, run:', + ' npm help', + ], + } + } + + // Anything after this is not suppressed and get more logged information + + // add a code to the error if it doesnt have one and mutate some properties + // so they have redacted information + err.code ??= err.message.match(/^(?:Error: )?(E[A-Z]+)/)?.[1] + // this mutates the error and redacts stack/message + const { summary, detail, files } = errorMessage(err, npm) + + return { + err, + code: err.code, + exitCode: getExitCodeFromError(err) || 1, + suppressError: false, + summary, + detail, + files, + verbose: ['type', 'stack', 'statusCode', 'pkgid'] + .filter(k => err[k]) + .map(k => [k, replaceInfo(err[k])]), + error: ['code', 'syscall', 'file', 'path', 'dest', 'errno'] + .filter(k => err[k]) + .map(k => [k, err[k]]), + } +} + +const outputError = ({ standard = [], verbose = [], error = [], summary = [], detail = [] }) => { + for (const line of standard) { + // Each output line is just a single string + output.standard(line) + } + for (const line of verbose) { + log.verbose(...line) + } + for (const line of [...error, ...summary, ...detail]) { + log.error(...line) + } +} + +const jsonError = (error, npm) => { + if (error && npm?.loaded && npm?.config.get('json')) { + return { + // error key is important, its the public API for getting error output from json mode + error: { + code: error.code, + summary: (error.summary || []).map(l => l.slice(1).join(' ')).join('\n').trim(), + detail: (error.detail || []).map(l => l.slice(1).join(' ')).join('\n').trim(), + }, + } + } +} + module.exports = { getExitCodeFromError, errorMessage, + getError, + jsonError, + outputError, } diff --git a/test/lib/cli/exit-handler.js b/test/lib/cli/exit-handler.js index a95ac75e56c14..0077596fab642 100644 --- a/test/lib/cli/exit-handler.js +++ b/test/lib/cli/exit-handler.js @@ -74,6 +74,10 @@ const mockExitHandler = async (t, { ...errorMessage.errorMessage(err), ...(files ? { files } : {}), }), + getError: (...args) => ({ + ...errorMessage.getError(...args), + ...(files ? { files } : {}), + }), } if (error) { diff --git a/test/lib/npm.js b/test/lib/npm.js index 6525708dc5795..a4f12f01c500a 100644 --- a/test/lib/npm.js +++ b/test/lib/npm.js @@ -561,9 +561,9 @@ t.test('usage', async t => { }) t.test('print usage if non-command param provided', async t => { - const { npm, outputs } = await loadMockNpm(t) + const { npm, joinedOutput } = await loadMockNpm(t) await t.rejects(npm.exec('tset'), { command: 'tset', exitCode: 1 }) - t.match(outputs[0], 'Unknown command: "tset"') - t.match(outputs[0], 'Did you mean this?') + t.match(joinedOutput(), 'Unknown command: "tset"') + t.match(joinedOutput(), 'Did you mean this?') }) diff --git a/test/lib/utils/did-you-mean.js b/test/lib/utils/did-you-mean.js index d111c2f002960..4ee7bd7ab654b 100644 --- a/test/lib/utils/did-you-mean.js +++ b/test/lib/utils/did-you-mean.js @@ -1,54 +1,47 @@ const t = require('tap') - const dym = require('../../../lib/utils/did-you-mean.js') t.test('did-you-mean', async t => { t.test('with package.json', async t => { - const testdir = t.testdir({ - 'package.json': JSON.stringify({ - bin: { - npx: 'exists', - }, - scripts: { - install: 'exists', - posttest: 'exists', - }, - }), - }) + const pkg = { + bin: { + npx: 'exists', + }, + scripts: { + install: 'exists', + posttest: 'exists', + }, + } t.test('nistall', async t => { - const result = await dym(testdir, 'nistall') + const result = dym(pkg, 'nistall').join('\n') t.match(result, 'npm install') }) t.test('sttest', async t => { - const result = await dym(testdir, 'sttest') + const result = dym(pkg, 'sttest').join('\n') t.match(result, 'npm test') t.match(result, 'npm run posttest') }) t.test('npz', async t => { - const result = await dym(testdir, 'npxx') + const result = dym(pkg, 'npxx').join('\n') t.match(result, 'npm exec npx') }) t.test('qwuijbo', async t => { - const result = await dym(testdir, 'qwuijbo') + const result = dym(pkg, 'qwuijbo').join('\n') t.match(result, '') }) }) t.test('with no package.json', t => { - const testdir = t.testdir({}) t.test('nistall', async t => { - const result = await dym(testdir, 'nistall') + const result = dym(null, 'nistall').join('\n') t.match(result, 'npm install') }) t.end() }) t.test('missing bin and script properties', async t => { - const testdir = t.testdir({ - 'package.json': JSON.stringify({ - name: 'missing-bin', - }), - }) - - const result = await dym(testdir, 'nistall') + const pkg = { + name: 'missing-bin', + } + const result = dym(pkg, 'nistall').join('\n') t.match(result, 'npm install') }) }) diff --git a/test/lib/utils/error-message.js b/test/lib/utils/error-message.js index 55b6053985a8c..c99971fd41233 100644 --- a/test/lib/utils/error-message.js +++ b/test/lib/utils/error-message.js @@ -93,17 +93,12 @@ t.test('just simple messages', async t => { t.test('replace message/stack sensistive info', async t => { const { errorMessage } = await loadMockNpm(t, { command: 'audit' }) - const path = '/some/path' - const pkgid = 'some@package' - const file = '/some/file' - const stack = 'dummy stack trace at https://user:pass@registry.npmjs.org/' - const message = 'Error at registry: https://user:pass@registry.npmjs.org/' - const er = Object.assign(new Error(message), { + const er = Object.assign(new Error('Error at registry: https://user:pass@registry.npmjs.org/'), { code: 'ENOAUDIT', - path, - pkgid, - file, - stack, + path: '/some/path', + pkgid: 'some@package', + file: '/some/file', + stack: 'dummy stack trace at https://user:pass@registry.npmjs.org/', }) t.matchSnapshot(errorMessage(er)) })