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.

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

feat: friendlier error message when command not found #6887

Merged
merged 19 commits into from
Aug 4, 2023
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/chatty-houses-build.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@pnpm/plugin-commands-script-runners": patch
"pnpm": patch
---

Make the error message when user attempting to run a command that does not exist friendlier
Original file line number Diff line number Diff line change
@@ -1,12 +1,46 @@
import { type PackageScripts } from '@pnpm/types'
import didYouMean, { ReturnTypeEnums } from 'didyoumean2'
import { readdir } from 'fs/promises'
import path from 'path'

export function getNearest (name: string, list?: readonly string[]) {
return list && didYouMean(name, list ?? [], {
returnType: ReturnTypeEnums.FIRST_CLOSEST_MATCH,
})
}

export async function getNearestProgram (opts: {
programName: string,

Check failure on line 13 in exec/plugin-commands-script-runners/src/buildCommandNotFoundHint.ts

View workflow job for this annotation

GitHub Actions / ubuntu-latest / Node.js 16.14

Unexpected separator (,)

Check failure on line 13 in exec/plugin-commands-script-runners/src/buildCommandNotFoundHint.ts

View workflow job for this annotation

GitHub Actions / ubuntu-latest / Node.js 18

Unexpected separator (,)

Check failure on line 13 in exec/plugin-commands-script-runners/src/buildCommandNotFoundHint.ts

View workflow job for this annotation

GitHub Actions / windows-latest / Node.js 18

Unexpected separator (,)

Check failure on line 13 in exec/plugin-commands-script-runners/src/buildCommandNotFoundHint.ts

View workflow job for this annotation

GitHub Actions / windows-latest / Node.js 16.14

Unexpected separator (,)

Check failure on line 13 in exec/plugin-commands-script-runners/src/buildCommandNotFoundHint.ts

View workflow job for this annotation

GitHub Actions / ubuntu-latest / Node.js 20

Unexpected separator (,)

Check failure on line 13 in exec/plugin-commands-script-runners/src/buildCommandNotFoundHint.ts

View workflow job for this annotation

GitHub Actions / windows-latest / Node.js 20

Unexpected separator (,)

Check failure on line 13 in exec/plugin-commands-script-runners/src/buildCommandNotFoundHint.ts

View workflow job for this annotation

GitHub Actions / ubuntu-latest / Node.js 16.14

Unexpected separator (,)

Check failure on line 13 in exec/plugin-commands-script-runners/src/buildCommandNotFoundHint.ts

View workflow job for this annotation

GitHub Actions / windows-latest / Node.js 16.14

Unexpected separator (,)

Check failure on line 13 in exec/plugin-commands-script-runners/src/buildCommandNotFoundHint.ts

View workflow job for this annotation

GitHub Actions / ubuntu-latest / Node.js 18

Unexpected separator (,)

Check failure on line 13 in exec/plugin-commands-script-runners/src/buildCommandNotFoundHint.ts

View workflow job for this annotation

GitHub Actions / windows-latest / Node.js 18

Unexpected separator (,)

Check failure on line 13 in exec/plugin-commands-script-runners/src/buildCommandNotFoundHint.ts

View workflow job for this annotation

GitHub Actions / ubuntu-latest / Node.js 20

Unexpected separator (,)

Check failure on line 13 in exec/plugin-commands-script-runners/src/buildCommandNotFoundHint.ts

View workflow job for this annotation

GitHub Actions / windows-latest / Node.js 20

Unexpected separator (,)
dir: string,

Check failure on line 14 in exec/plugin-commands-script-runners/src/buildCommandNotFoundHint.ts

View workflow job for this annotation

GitHub Actions / ubuntu-latest / Node.js 16.14

Unexpected separator (,)

Check failure on line 14 in exec/plugin-commands-script-runners/src/buildCommandNotFoundHint.ts

View workflow job for this annotation

GitHub Actions / ubuntu-latest / Node.js 18

Unexpected separator (,)

Check failure on line 14 in exec/plugin-commands-script-runners/src/buildCommandNotFoundHint.ts

View workflow job for this annotation

GitHub Actions / windows-latest / Node.js 18

Unexpected separator (,)

Check failure on line 14 in exec/plugin-commands-script-runners/src/buildCommandNotFoundHint.ts

View workflow job for this annotation

GitHub Actions / windows-latest / Node.js 16.14

Unexpected separator (,)

Check failure on line 14 in exec/plugin-commands-script-runners/src/buildCommandNotFoundHint.ts

View workflow job for this annotation

GitHub Actions / ubuntu-latest / Node.js 20

Unexpected separator (,)

Check failure on line 14 in exec/plugin-commands-script-runners/src/buildCommandNotFoundHint.ts

View workflow job for this annotation

GitHub Actions / windows-latest / Node.js 20

Unexpected separator (,)

Check failure on line 14 in exec/plugin-commands-script-runners/src/buildCommandNotFoundHint.ts

View workflow job for this annotation

GitHub Actions / ubuntu-latest / Node.js 16.14

Unexpected separator (,)

Check failure on line 14 in exec/plugin-commands-script-runners/src/buildCommandNotFoundHint.ts

View workflow job for this annotation

GitHub Actions / windows-latest / Node.js 16.14

Unexpected separator (,)

Check failure on line 14 in exec/plugin-commands-script-runners/src/buildCommandNotFoundHint.ts

View workflow job for this annotation

GitHub Actions / ubuntu-latest / Node.js 18

Unexpected separator (,)

Check failure on line 14 in exec/plugin-commands-script-runners/src/buildCommandNotFoundHint.ts

View workflow job for this annotation

GitHub Actions / windows-latest / Node.js 18

Unexpected separator (,)

Check failure on line 14 in exec/plugin-commands-script-runners/src/buildCommandNotFoundHint.ts

View workflow job for this annotation

GitHub Actions / ubuntu-latest / Node.js 20

Unexpected separator (,)

Check failure on line 14 in exec/plugin-commands-script-runners/src/buildCommandNotFoundHint.ts

View workflow job for this annotation

GitHub Actions / windows-latest / Node.js 20

Unexpected separator (,)
workspaceDir: string | undefined,

Check failure on line 15 in exec/plugin-commands-script-runners/src/buildCommandNotFoundHint.ts

View workflow job for this annotation

GitHub Actions / ubuntu-latest / Node.js 16.14

Unexpected separator (,)

Check failure on line 15 in exec/plugin-commands-script-runners/src/buildCommandNotFoundHint.ts

View workflow job for this annotation

GitHub Actions / ubuntu-latest / Node.js 18

Unexpected separator (,)

Check failure on line 15 in exec/plugin-commands-script-runners/src/buildCommandNotFoundHint.ts

View workflow job for this annotation

GitHub Actions / windows-latest / Node.js 18

Unexpected separator (,)

Check failure on line 15 in exec/plugin-commands-script-runners/src/buildCommandNotFoundHint.ts

View workflow job for this annotation

GitHub Actions / windows-latest / Node.js 16.14

Unexpected separator (,)

Check failure on line 15 in exec/plugin-commands-script-runners/src/buildCommandNotFoundHint.ts

View workflow job for this annotation

GitHub Actions / ubuntu-latest / Node.js 20

Unexpected separator (,)

Check failure on line 15 in exec/plugin-commands-script-runners/src/buildCommandNotFoundHint.ts

View workflow job for this annotation

GitHub Actions / windows-latest / Node.js 20

Unexpected separator (,)

Check failure on line 15 in exec/plugin-commands-script-runners/src/buildCommandNotFoundHint.ts

View workflow job for this annotation

GitHub Actions / ubuntu-latest / Node.js 16.14

Unexpected separator (,)

Check failure on line 15 in exec/plugin-commands-script-runners/src/buildCommandNotFoundHint.ts

View workflow job for this annotation

GitHub Actions / windows-latest / Node.js 16.14

Unexpected separator (,)

Check failure on line 15 in exec/plugin-commands-script-runners/src/buildCommandNotFoundHint.ts

View workflow job for this annotation

GitHub Actions / ubuntu-latest / Node.js 18

Unexpected separator (,)

Check failure on line 15 in exec/plugin-commands-script-runners/src/buildCommandNotFoundHint.ts

View workflow job for this annotation

GitHub Actions / windows-latest / Node.js 18

Unexpected separator (,)

Check failure on line 15 in exec/plugin-commands-script-runners/src/buildCommandNotFoundHint.ts

View workflow job for this annotation

GitHub Actions / ubuntu-latest / Node.js 20

Unexpected separator (,)

Check failure on line 15 in exec/plugin-commands-script-runners/src/buildCommandNotFoundHint.ts

View workflow job for this annotation

GitHub Actions / windows-latest / Node.js 20

Unexpected separator (,)
}) {
try {
const { programName, dir, workspaceDir } = opts
const binDir = path.join(dir, 'node_modules', '.bin')
const programListPromise = readdir(binDir) // await is omitted to take advantage of concurrency
let programList!: string[]
if (workspaceDir && workspaceDir !== dir) {
const workspaceBinDir = path.join(workspaceDir, 'node_modules', '.bin')
const programListExtension = await readdir(workspaceBinDir)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is OK if you just use readdir synchronously here and make the function synchronous. We don't care about the performance here so much.

programList = await programListPromise
programList.push(...programListExtension)
} else {
programList = await programListPromise
}
return getNearest(programName, await programListPromise)
} catch (_err) {
return null
}
}

export function getNearestScript (scriptName: string, scripts?: PackageScripts | undefined) {
return getNearest(scriptName, scripts && Object.keys(scripts))
}

export function buildCommandNotFoundHint (scriptName: string, scripts?: PackageScripts | undefined) {
let hint = `Command "${scriptName}" not found.`

const nearestCommand = scripts && didYouMean(scriptName, Object.keys(scripts), {
returnType: ReturnTypeEnums.FIRST_CLOSEST_MATCH,
})
const nearestCommand = getNearestScript(scriptName, scripts)

if (nearestCommand) {
hint += ` Did you mean "pnpm run ${nearestCommand}"?`
Expand Down
32 changes: 30 additions & 2 deletions exec/plugin-commands-script-runners/src/exec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import {
import { PnpmError } from '@pnpm/error'
import which from 'which'
import writeJsonFile from 'write-json-file'
import { buildCommandNotFoundHint } from './buildCommandNotFoundHint'
import { getNearestProgram, getNearestScript } from './buildCommandNotFoundHint'

export const shorthands = {
parallel: runShorthands.parallel,
Expand Down Expand Up @@ -133,6 +133,7 @@ export async function handler (
shellMode?: boolean
resumeFrom?: string
reportSummary?: boolean
implicitlyFellbackFromRun?: boolean
} & Pick<Config, 'extraBinPaths' | 'extraEnv' | 'lockfileDir' | 'dir' | 'userAgent' | 'recursive' | 'workspaceDir'>,
params: string[]
) {
Expand Down Expand Up @@ -212,7 +213,34 @@ export async function handler (
result[prefix].duration = getExecutionDuration(startTime)
} catch (err: any) { // eslint-disable-line
if (await isErrorCommandNotFound(params[0], err)) {
err.hint = buildCommandNotFoundHint(params[0], (await readProjectManifestOnly(opts.dir)).scripts)
err.message = `Command "${params[0]}" not found`
if (opts.implicitlyFellbackFromRun) {
let nearestScript: string | null | undefined
try {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this try needed? There is already a try/catch in getNearestProgram

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because I think readProjectManifestOnly interacts with the filesystem and therefore can fail.

nearestScript = getNearestScript(params[0], (await readProjectManifestOnly(opts.dir)).scripts)
} catch (_err) {}
if (nearestScript) {
err.hint = `Did you mean "pnpm ${nearestScript}"?`
} else {
const nearestProgram = await getNearestProgram({
programName: params[0],
dir: opts.dir,
workspaceDir: opts.workspaceDir,
})
if (nearestProgram) {
err.hint = `Did you mean "pnpm ${nearestProgram}"?`
}
}
} else {
const nearestProgram = await getNearestProgram({
programName: params[0],
dir: opts.dir,
workspaceDir: opts.workspaceDir,
})
if (nearestProgram) {
err.hint = `Did you mean "pnpm exec ${nearestProgram}"?`
}
}
} else if (!opts.recursive && typeof err.exitCode === 'number') {
exitCode = err.exitCode
return
Expand Down
1 change: 1 addition & 0 deletions exec/plugin-commands-script-runners/src/run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,7 @@ export async function handler (
if (opts.argv == null) throw new Error('Could not fallback because opts.argv.original was not passed to the script runner')
return exec({
selectedProjectsGraph: {},
implicitlyFellbackFromRun: true,
...opts,
}, opts.argv.original.slice(1))
}
Expand Down
67 changes: 64 additions & 3 deletions exec/plugin-commands-script-runners/test/exec.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -812,25 +812,86 @@ test('pnpm recursive exec report summary with --bail', async () => {
expect(executionStatus[path.resolve('project-4')].status).toBe('queued')
})

test('pnpm exec command not found', async () => {
test('pnpm exec command not found (implicit fallback)', async () => {
prepare({
scripts: {
build: 'echo hello',
},
})

const { selectedProjectsGraph } = await readProjects(process.cwd(), [])
let error!: Error & { hint: string }
let error!: Error & { hint?: string }
try {
await exec.handler({
...DEFAULT_OPTS,
dir: process.cwd(),
recursive: false,
bail: true,
selectedProjectsGraph,
implicitlyFellbackFromRun: true,
}, ['buil'])
} catch (err: any) { // eslint-disable-line
error = err
}
expect(error?.hint).toBe('Command "buil" not found. Did you mean "pnpm run build"?')
expect(error?.message).toBe('Command "buil" not found')
expect(error?.hint).toBe('Did you mean "pnpm build"?')
})

test('pnpm exec command not found (explicit call, without near name packages)', async () => {
prepare({
scripts: {
cwsay: 'echo hello',
},
})

const { selectedProjectsGraph } = await readProjects(process.cwd(), [])
let error!: Error & { hint?: string }
try {
await exec.handler({
...DEFAULT_OPTS,
dir: process.cwd(),
recursive: false,
bail: true,
selectedProjectsGraph,
implicitlyFellbackFromRun: false,
}, ['cwsay'])
} catch (err: any) { // eslint-disable-line
error = err
}
expect(error?.message).toBe('Command "cwsay" not found')
expect(error?.hint).toBeFalsy()
})

test('pnpm exec command not found (explicit call, with a near name package)', async () => {
prepare({
dependencies: {
cowsay: '1.5.0',
},
})

const { selectedProjectsGraph } = await readProjects(process.cwd(), [])

await execa(pnpmBin, [
'install',
'--registry',
REGISTRY_URL,
'--store-dir',
path.resolve(DEFAULT_OPTS.storeDir),
])

let error!: Error & { hint?: string }
try {
await exec.handler({
...DEFAULT_OPTS,
dir: process.cwd(),
recursive: false,
bail: true,
selectedProjectsGraph,
implicitlyFellbackFromRun: false,
}, ['cwsay'])
} catch (err: any) { // eslint-disable-line
error = err
}
expect(error?.message).toBe('Command "cwsay" not found')
expect(error?.hint).toBe('Did you mean "pnpm exec cowsay"?')
})