Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 35 additions & 11 deletions src/commands/base-command.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { NodeFS, NoopLogger } from '@netlify/build-info/node'
import { resolveConfig } from '@netlify/config'
import { getGlobalConfigStore, LocalState } from '@netlify/dev-utils'
import { isCI } from 'ci-info'
import { Command, Help, Option, type OptionValues } from 'commander'
import { Command, CommanderError, Help, Option, type OptionValues } from 'commander'
import debug from 'debug'
import { findUp } from 'find-up'
import inquirer from 'inquirer'
Expand All @@ -34,6 +34,7 @@ import {
warn,
logError,
} from '../utils/command-helpers.js'
import { handleOptionError, isOptionError } from '../utils/command-error-handler.js'
import type { FeatureFlags } from '../utils/feature-flags.js'
import { getFrameworksAPIPaths } from '../utils/frameworks-api.js'
import { getSiteByName } from '../utils/get-site.js'
Expand Down Expand Up @@ -236,12 +237,13 @@ export default class BaseCommand extends Command {
accountId?: string

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

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

return base.hook('preAction', async (_parentCommand, actionCommand) => {
base.hook('preAction', async (_parentCommand, actionCommand) => {
if (actionCommand.opts()?.debug) {
process.env.DEBUG = '*'
}
debug(`${name}:preAction`)('start')
debug(`${commandName}:preAction`)('start')
this.analytics.startTime = process.hrtime.bigint()
await this.init(actionCommand as BaseCommand)
debug(`${name}:preAction`)('end')
debug(`${commandName}:preAction`)('end')
})

// Wrap the command's action() to set exitOverride when action is registered.
// We set exitOverride here (rather than earlier) because Commander may clone
// or modify command instances during registration, so we need to set it on
// the final instance that will actually execute.
const originalAction = base.action.bind(base)
base.action = function (this: BaseCommand, fn: any) {
// Set exitOverride for option-related errors in non-interactive environments.
// In non-interactive mode, we show the full help output instead of just a
// brief error message, making it easier for users in CI/CD environments to
// understand what went wrong.
this.exitOverride((error: CommanderError) => {
if (isOptionError(error)) {
handleOptionError(this)
}

throw error
})

return originalAction(fn)
}

return base
}

#noBaseOptions = false
Expand Down
21 changes: 20 additions & 1 deletion src/commands/main.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import process from 'process'

import { Option } from 'commander'
import { Option, CommanderError } from 'commander'
import envinfo from 'envinfo'
import { closest } from 'fastest-levenshtein'
import inquirer from 'inquirer'
Expand All @@ -21,6 +21,8 @@ import {
import execa from '../utils/execa.js'
import getCLIPackageJson from '../utils/get-cli-package-json.js'
import { didEnableCompileCache } from '../utils/nodejs-compile-cache.js'
import { handleOptionError, isOptionError } from '../utils/command-error-handler.js'
import { isInteractive } from '../utils/scripted-commands.js'
import { track, reportError } from '../utils/telemetry/index.js'

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

// In non-interactive environments (CI/CD, scripts), show the suggestion
// without prompting, and display full help for available commands
if (!isInteractive()) {
log(`\nDid you mean ${chalk.blue(suggestion)}?`)
log()
command.outputHelp({ error: true })
log()
return logAndThrowError(`Run ${NETLIFY_CYAN(`${command.name()} help`)} for a list of available commands.`)
}

const applySuggestion = await new Promise((resolve) => {
const prompt = inquirer.prompt({
type: 'confirm',
Expand Down Expand Up @@ -276,6 +288,13 @@ To ask a human for credentials: ${NETLIFY_CYAN('netlify login --request <msg>')}
write(` ${chalk.red(BANG)} See more help with --help\n`)
},
})
.exitOverride(function (this: BaseCommand, error: CommanderError) {
if (isOptionError(error)) {
handleOptionError(this)
}

throw error
})
.action(mainCommand)

program.commands.forEach((cmd) => {
Expand Down
20 changes: 20 additions & 0 deletions src/utils/command-error-handler.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import { CommanderError, type HelpContext } from 'commander'

import { log } from './command-helpers.js'
import { isInteractive } from './scripted-commands.js'

const OPTION_ERROR_CODES = new Set([
'commander.unknownOption',
'commander.missingArgument',
'commander.excessArguments',
])

export const isOptionError = (error: CommanderError): boolean => OPTION_ERROR_CODES.has(error.code)

export const handleOptionError = (command: { outputHelp: (context?: HelpContext) => void }): void => {
if (!isInteractive()) {
log()
command.outputHelp({ error: true })
log()
}
}
13 changes: 11 additions & 2 deletions src/utils/run-program.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import { CommanderError } from 'commander'

import { injectForceFlagIfScripted } from './scripted-commands.js'
import { BaseCommand } from '../commands/index.js'
import { CI_FORCED_COMMANDS } from '../commands/main.js'
import { exit } from './command-helpers.js'

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

await program.parseAsync(argv)
try {
await program.parseAsync(argv)
} catch (error) {
if (error instanceof CommanderError) {
exit(error.exitCode)
}
throw error
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,24 @@

exports[`suggests closest matching command on typo 1`] = `
"β€Ί Warning: sta is not a netlify command.
? Did you mean api (y/N) "

Did you mean api?"
`;

exports[`suggests closest matching command on typo 2`] = `
"β€Ί Warning: opeen is not a netlify command.
? Did you mean open (y/N) "

Did you mean open?"
`;

exports[`suggests closest matching command on typo 3`] = `
"β€Ί Warning: hel is not a netlify command.
? Did you mean dev (y/N) "

Did you mean dev?"
`;

exports[`suggests closest matching command on typo 4`] = `
"β€Ί Warning: versio is not a netlify command.
? Did you mean serve (y/N) "

Did you mean serve?"
`;
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
import { describe, test } from 'vitest'

import { callCli } from '../../utils/call-cli.js'
import { normalize } from '../../utils/snapshots.js'

describe('error handling', () => {
test('unknown option shows error message', async (t) => {
try {
await callCli(['status', '--invalid-option'])
t.expect.fail('Expected callCli to throw')
} catch (error) {
const stderr = (error as { stderr: string }).stderr
const normalized = normalize(stderr)

// In interactive mode (test environment), shows brief error
t.expect(normalized).toContain('Error: unknown option')
t.expect(normalized).toContain('See more help with --help')
}
Comment on lines +7 to +18
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟑 Minor

Assert the new help-on-error contract from the same failed run.

These cases only check the generic error banner and then rerun the CLI, so a regression where the extra help text disappears or lands on the wrong stream would still pass. Capture the rejected ExecaError once and assert the USAGE/OPTIONS block from that same failure.

Also applies to: 22-48

πŸ€– Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/integration/commands/error-handling/error-handling.test.ts` around
lines 7 - 19, The test currently calls callCli(['status','--invalid-option'])
twice and only inspects the stderr from the second run; change the test (and the
similar cases in lines 22-48) to call callCli once and capture the thrown
ExecaError (e.g., assign the rejected value to a variable via await
expect(...).rejects.toThrow() replacement pattern or try/catch) and then assert
both the brief error banner and that the same error object's stderr contains the
full USAGE/OPTIONS help block (use normalize to compare), referencing the
existing callCli and normalize helpers and the 'unknown option shows error
message' test so all assertions are made against the single captured error
instance.

})

test('unknown command shows error', async (t) => {
try {
await callCli(['statuss'])
t.expect.fail('Expected callCli to throw')
} catch (error) {
const stderr = (error as { stderr: string }).stderr
const normalized = normalize(stderr)

// Shows error with help suggestion
t.expect(normalized).toContain('Error')
t.expect(normalized).toContain('netlify help')
}
})

test('missing required argument shows error', async (t) => {
try {
await callCli(['sites:delete'])
t.expect.fail('Expected callCli to throw')
} catch (error) {
const stderr = (error as { stderr: string }).stderr
const normalized = normalize(stderr)

t.expect(normalized).toContain('Error:')
t.expect(normalized).toContain('missing required argument')
}
})

test('help command works correctly', async (t) => {
const helpOutput = (await callCli(['status', '--help'])) as string
const normalized = normalize(helpOutput)

t.expect(normalized).toContain('Print status information')
t.expect(normalized).toContain('USAGE')
t.expect(normalized).toContain('OPTIONS')
})
})
Loading