Skip to content

Commit 0715310

Browse files
authored
fix: improve ax error handling around missing commands or unknown flags
1 parent 056268a commit 0715310

File tree

6 files changed

+150
-18
lines changed

6 files changed

+150
-18
lines changed

src/commands/base-command.ts

Lines changed: 35 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import { NodeFS, NoopLogger } from '@netlify/build-info/node'
99
import { resolveConfig } from '@netlify/config'
1010
import { getGlobalConfigStore, LocalState } from '@netlify/dev-utils'
1111
import { isCI } from 'ci-info'
12-
import { Command, Help, Option, type OptionValues } from 'commander'
12+
import { Command, CommanderError, Help, Option, type OptionValues } from 'commander'
1313
import debug from 'debug'
1414
import { findUp } from 'find-up'
1515
import inquirer from 'inquirer'
@@ -34,6 +34,7 @@ import {
3434
warn,
3535
logError,
3636
} from '../utils/command-helpers.js'
37+
import { handleOptionError, isOptionError } from '../utils/command-error-handler.js'
3738
import type { FeatureFlags } from '../utils/feature-flags.js'
3839
import { getFrameworksAPIPaths } from '../utils/frameworks-api.js'
3940
import { getSiteByName } from '../utils/get-site.js'
@@ -236,12 +237,13 @@ export default class BaseCommand extends Command {
236237
accountId?: string
237238

238239
/**
239-
* IMPORTANT this function will be called for each command!
240-
* Don't do anything expensive in there.
240+
* Override Commander's createCommand to return BaseCommand instances with our setup.
241+
* This is called by .command() to create subcommands.
242+
* IMPORTANT: This function is called for each command! Don't do anything expensive here.
241243
*/
242-
createCommand(name: string): BaseCommand {
243-
const base = new BaseCommand(name)
244-
// .addOption(new Option('--force', 'Force command to run. Bypasses prompts for certain destructive commands.'))
244+
createCommand(name?: string): BaseCommand {
245+
const commandName = name || ''
246+
const base = new BaseCommand(commandName)
245247
.addOption(new Option('--silent', 'Silence CLI output').hideHelp(true))
246248
.addOption(new Option('--cwd <cwd>').hideHelp(true))
247249
.addOption(
@@ -262,20 +264,42 @@ export default class BaseCommand extends Command {
262264
)
263265
.option('--debug', 'Print debugging information')
264266

265-
// only add the `--filter` option to commands that are workspace aware
266-
if (!COMMANDS_WITHOUT_WORKSPACE_OPTIONS.has(name)) {
267+
if (!COMMANDS_WITHOUT_WORKSPACE_OPTIONS.has(commandName)) {
267268
base.option('--filter <app>', 'For monorepos, specify the name of the application to run the command in')
268269
}
269270

270-
return base.hook('preAction', async (_parentCommand, actionCommand) => {
271+
base.hook('preAction', async (_parentCommand, actionCommand) => {
271272
if (actionCommand.opts()?.debug) {
272273
process.env.DEBUG = '*'
273274
}
274-
debug(`${name}:preAction`)('start')
275+
debug(`${commandName}:preAction`)('start')
275276
this.analytics.startTime = process.hrtime.bigint()
276277
await this.init(actionCommand as BaseCommand)
277-
debug(`${name}:preAction`)('end')
278+
debug(`${commandName}:preAction`)('end')
278279
})
280+
281+
// Wrap the command's action() to set exitOverride when action is registered.
282+
// We set exitOverride here (rather than earlier) because Commander may clone
283+
// or modify command instances during registration, so we need to set it on
284+
// the final instance that will actually execute.
285+
const originalAction = base.action.bind(base)
286+
base.action = function (this: BaseCommand, fn: any) {
287+
// Set exitOverride for option-related errors in non-interactive environments.
288+
// In non-interactive mode, we show the full help output instead of just a
289+
// brief error message, making it easier for users in CI/CD environments to
290+
// understand what went wrong.
291+
this.exitOverride((error: CommanderError) => {
292+
if (isOptionError(error)) {
293+
handleOptionError(this)
294+
}
295+
296+
throw error
297+
})
298+
299+
return originalAction(fn)
300+
}
301+
302+
return base
279303
}
280304

281305
#noBaseOptions = false

src/commands/main.ts

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import process from 'process'
22

3-
import { Option } from 'commander'
3+
import { Option, CommanderError } from 'commander'
44
import envinfo from 'envinfo'
55
import { closest } from 'fastest-levenshtein'
66
import inquirer from 'inquirer'
@@ -21,6 +21,8 @@ import {
2121
import execa from '../utils/execa.js'
2222
import getCLIPackageJson from '../utils/get-cli-package-json.js'
2323
import { didEnableCompileCache } from '../utils/nodejs-compile-cache.js'
24+
import { handleOptionError, isOptionError } from '../utils/command-error-handler.js'
25+
import { isInteractive } from '../utils/scripted-commands.js'
2426
import { track, reportError } from '../utils/telemetry/index.js'
2527

2628
import { createAgentsCommand } from './agents/index.js'
@@ -180,6 +182,16 @@ const mainCommand = async function (options, command) {
180182
const allCommands = command.commands.map((cmd) => cmd.name())
181183
const suggestion = closest(command.args[0], allCommands)
182184

185+
// In non-interactive environments (CI/CD, scripts), show the suggestion
186+
// without prompting, and display full help for available commands
187+
if (!isInteractive()) {
188+
log(`\nDid you mean ${chalk.blue(suggestion)}?`)
189+
log()
190+
command.outputHelp({ error: true })
191+
log()
192+
return logAndThrowError(`Run ${NETLIFY_CYAN(`${command.name()} help`)} for a list of available commands.`)
193+
}
194+
183195
const applySuggestion = await new Promise((resolve) => {
184196
const prompt = inquirer.prompt({
185197
type: 'confirm',
@@ -276,6 +288,13 @@ To ask a human for credentials: ${NETLIFY_CYAN('netlify login --request <msg>')}
276288
write(` ${chalk.red(BANG)} See more help with --help\n`)
277289
},
278290
})
291+
.exitOverride(function (this: BaseCommand, error: CommanderError) {
292+
if (isOptionError(error)) {
293+
handleOptionError(this)
294+
}
295+
296+
throw error
297+
})
279298
.action(mainCommand)
280299

281300
program.commands.forEach((cmd) => {

src/utils/command-error-handler.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
import { CommanderError, type HelpContext } from 'commander'
2+
3+
import { log } from './command-helpers.js'
4+
import { isInteractive } from './scripted-commands.js'
5+
6+
const OPTION_ERROR_CODES = new Set([
7+
'commander.unknownOption',
8+
'commander.missingArgument',
9+
'commander.excessArguments',
10+
])
11+
12+
export const isOptionError = (error: CommanderError): boolean => OPTION_ERROR_CODES.has(error.code)
13+
14+
export const handleOptionError = (command: { outputHelp: (context?: HelpContext) => void }): void => {
15+
if (!isInteractive()) {
16+
log()
17+
command.outputHelp({ error: true })
18+
log()
19+
}
20+
}

src/utils/run-program.ts

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
1+
import { CommanderError } from 'commander'
2+
13
import { injectForceFlagIfScripted } from './scripted-commands.js'
24
import { BaseCommand } from '../commands/index.js'
35
import { CI_FORCED_COMMANDS } from '../commands/main.js'
6+
import { exit } from './command-helpers.js'
47

5-
// This function is used to run the program with the correct flags
68
export const runProgram = async (program: BaseCommand, argv: string[]) => {
79
const cmdName = argv[2]
810
// checks if the command has a force option
@@ -12,5 +14,12 @@ export const runProgram = async (program: BaseCommand, argv: string[]) => {
1214
injectForceFlagIfScripted(argv)
1315
}
1416

15-
await program.parseAsync(argv)
17+
try {
18+
await program.parseAsync(argv)
19+
} catch (error) {
20+
if (error instanceof CommanderError) {
21+
exit(error.exitCode)
22+
}
23+
throw error
24+
}
1625
}

tests/integration/commands/didyoumean/__snapshots__/didyoumean.test.ts.snap

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,20 +2,24 @@
22

33
exports[`suggests closest matching command on typo 1`] = `
44
"› Warning: sta is not a netlify command.
5-
? Did you mean api (y/N) "
5+
6+
Did you mean api?"
67
`;
78

89
exports[`suggests closest matching command on typo 2`] = `
910
"› Warning: opeen is not a netlify command.
10-
? Did you mean open (y/N) "
11+
12+
Did you mean open?"
1113
`;
1214

1315
exports[`suggests closest matching command on typo 3`] = `
1416
"› Warning: hel is not a netlify command.
15-
? Did you mean dev (y/N) "
17+
18+
Did you mean dev?"
1619
`;
1720

1821
exports[`suggests closest matching command on typo 4`] = `
1922
"› Warning: versio is not a netlify command.
20-
? Did you mean serve (y/N) "
23+
24+
Did you mean serve?"
2125
`;
Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
import { describe, test } from 'vitest'
2+
3+
import { callCli } from '../../utils/call-cli.js'
4+
import { normalize } from '../../utils/snapshots.js'
5+
6+
describe('error handling', () => {
7+
test('unknown option shows error message', async (t) => {
8+
try {
9+
await callCli(['status', '--invalid-option'])
10+
t.expect.fail('Expected callCli to throw')
11+
} catch (error) {
12+
const stderr = (error as { stderr: string }).stderr
13+
const normalized = normalize(stderr)
14+
15+
// In interactive mode (test environment), shows brief error
16+
t.expect(normalized).toContain('Error: unknown option')
17+
t.expect(normalized).toContain('See more help with --help')
18+
}
19+
})
20+
21+
test('unknown command shows error', async (t) => {
22+
try {
23+
await callCli(['statuss'])
24+
t.expect.fail('Expected callCli to throw')
25+
} catch (error) {
26+
const stderr = (error as { stderr: string }).stderr
27+
const normalized = normalize(stderr)
28+
29+
// Shows error with help suggestion
30+
t.expect(normalized).toContain('Error')
31+
t.expect(normalized).toContain('netlify help')
32+
}
33+
})
34+
35+
test('missing required argument shows error', async (t) => {
36+
try {
37+
await callCli(['sites:delete'])
38+
t.expect.fail('Expected callCli to throw')
39+
} catch (error) {
40+
const stderr = (error as { stderr: string }).stderr
41+
const normalized = normalize(stderr)
42+
43+
t.expect(normalized).toContain('Error:')
44+
t.expect(normalized).toContain('missing required argument')
45+
}
46+
})
47+
48+
test('help command works correctly', async (t) => {
49+
const helpOutput = (await callCli(['status', '--help'])) as string
50+
const normalized = normalize(helpOutput)
51+
52+
t.expect(normalized).toContain('Print status information')
53+
t.expect(normalized).toContain('USAGE')
54+
t.expect(normalized).toContain('OPTIONS')
55+
})
56+
})

0 commit comments

Comments
 (0)