Skip to content
Permalink

Comparing changes

This is a direct comparison between two commits made in this repository or its related repositories. View the default comparison for this range or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: npm/cli
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: 71d5e0a10c3b00ea69e754fd42fcfa238b91faaf
Choose a base ref
..
head repository: npm/cli
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: c462312825ba4fb08f72ee2d17283585f8ce80eb
Choose a head ref
12 changes: 5 additions & 7 deletions lib/commands/run-script.js
View file
Original file line number Diff line number Diff line change
@@ -98,13 +98,11 @@ class RunScript extends BaseCommand {
return
}

const suggestions = require('../utils/did-you-mean.js')(pkg, event)
throw new Error([
`Missing script: "${event}"`,
suggestions ? `\n\n${suggestions}` : '',
'To see a list of scripts, run:',
' npm run',
].join('\n'))
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`
)
}

// positional args only added to the main event, not pre/post
96 changes: 78 additions & 18 deletions lib/npm.js
View file
Original file line number Diff line number Diff line change
@@ -11,7 +11,6 @@ const { log, time, output, META } = require('proc-log')
const { redactLog: replaceInfo } = require('@npmcli/redact')
const pkg = require('../package.json')
const { deref } = require('./utils/cmd-list.js')
const { jsonError, outputError } = require('./utils/output-error.js')

class Npm {
static get version () {
@@ -288,49 +287,110 @@ class Npm {

async #handleError (err) {
if (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 }))
Object.assign(err, await this.#getError(err))
}

// TODO: make this not need to be public
this.finish()

output.flush({
[META]: true,
jsonError: jsonError(err, this),
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,
})

if (err) {
throw err
}
}

#getError (rawErr, opts) {
const error = require('./utils/error-message.js').getError(rawErr, {
npm: this,
command: this.#command,
...opts,
})
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)

const { writeFileSync } = require('node:fs')
for (const [file, content] of (error.files || [])) {
for (const [file, content] of files) {
const filePath = `${this.logPath}${file}`
const fileContent = `'Log files:\n${this.logFiles.join('\n')}\n\n${content.trim()}\n`
try {
writeFileSync(filePath, fileContent)
error.detail.push(['', `\n\nFor a full report see:\n${filePath}`])
detail.push(['', `\n\nFor a full report see:\n${filePath}`])
} catch (fileErr) {
log.warn('', `Could not write error message to ${file} due to ${fileErr}`)
}
}

outputError(error)
for (const k of ['code', 'syscall', 'file', 'path', 'dest', 'errno']) {
const v = err[k]
if (v) {
log.error(k, v)
}
}

return error
for (const errline of [...summary, ...detail]) {
log.error(...errline)
}

return {
exitCode,
summary,
detail,
suppressError: false,
}
}

get title () {
49 changes: 27 additions & 22 deletions lib/utils/did-you-mean.js
View file
Original file line number Diff line number Diff line change
@@ -1,34 +1,39 @@
const Npm = require('../npm')
const { distance } = require('fastest-levenshtein')
const pkgJson = require('@npmcli/package-json')
const { commands } = require('./cmd-list.js')

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 best = [
...commands
.filter(cmd => isClose(scmd, cmd) && scmd !== cmd)
.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`]),
]
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
}

if (best.length === 0) {
return ''
}

return `Did you mean ${best.length === 1 ? 'this' : 'one of these'}?\n` +
best.slice(0, 3).map(([msg, comment]) => ` npm ${msg} # ${comment}`).join('\n')
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')}`
}

module.exports = didYouMean
69 changes: 13 additions & 56 deletions lib/utils/display.js
View file
Original file line number Diff line number Diff line change
@@ -70,7 +70,7 @@ const tryJsonParse = (value) => {
try {
return JSON.parse(value)
} catch {
return
return {}
}
}
return value
@@ -86,56 +86,6 @@ const setBlocking = (stream) => {
return stream
}

// These are important
// This is the key that is returned to the user for errors
const ERROR_KEY = 'error'
// This is the key producers use to indicate that there
// is a json error that should be merged into the finished output
const JSON_ERROR_KEY = 'jsonError'

const mergeJson = (meta, buffer) => {
const buffered = buffer.reduce((acc, i) => {
// index 2 is the logged argument
acc[0].push(tryJsonParse(i[2]))
// index 1 is the meta object
acc[1].push(i[1][JSON_ERROR_KEY])
return acc
}, [
// meta also contains the meta object passed to flush
[], [meta[JSON_ERROR_KEY]],
])

const items = buffered[0].filter(Boolean)
const errors = buffered[1].filter(Boolean)

// If all items are keyed with array indexes, then we return the
// array. This skips any error checking since we cant really set
// an error property on an array in a way that can be stringified
// XXX(BREAKING_CHANGE): remove this in favor of always returning an object
if (items.length && items.every((o, i) => Object.hasOwn(o, i))) {
return Object.assign([], ...items)
}

const res = Object.assign({}, ...items)

if (errors.length) {
// This is not ideal. JSON output has always been keyed at the root with an `error`
// key, so we cant change that without it being a breaking change. At the same time
// some commands output arbitrary keys at the top level of the output, such as package
// names. So the output could already have the same key. The choice here is to overwrite
// it with our error since that is (probably?) more important.
// XXX(BREAKING_CHANGE): all json output should be keyed under well known keys, eg `result` and `error`
/* istanbul ignore next */
if (res[ERROR_KEY]) {
log.warn('display', `overwriting existing ${ERROR_KEY} on json output`)
}
res[ERROR_KEY] = Object.assign({}, ...errors)
}

// Only write output if we have some json buffered
return Object.keys(res).length ? res : null
}

const withMeta = (handler) => (level, ...args) => {
let meta = {}
const last = args.at(-1)
@@ -290,17 +240,24 @@ class Display {
// directly as a listener and still reference "this"
#outputHandler = withMeta((level, meta, ...args) => {
switch (level) {
case output.KEYS.flush: {
case output.KEYS.flush:
this.#outputState.buffering = false
const json = this.#json ? mergeJson(meta, this.#outputState.buffer) : null
if (json) {
this.#writeOutput(output.KEYS.standard, meta, JSON.stringify(json, null, 2))
if (meta.jsonError && 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))
}
this.#outputState.buffer.length = 0
break
}

case output.KEYS.buffer:
this.#outputState.buffer.push([output.KEYS.standard, meta, ...args])
Loading