diff --git a/src/commands/base-command.ts b/src/commands/base-command.ts index b91d876f2a9..25196e16ed3 100644 --- a/src/commands/base-command.ts +++ b/src/commands/base-command.ts @@ -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' @@ -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' @@ -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 ').hideHelp(true)) .addOption( @@ -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 ', '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 diff --git a/src/commands/main.ts b/src/commands/main.ts index 2eea1a46796..86339c57d51 100644 --- a/src/commands/main.ts +++ b/src/commands/main.ts @@ -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' @@ -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' @@ -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', @@ -276,6 +288,13 @@ To ask a human for credentials: ${NETLIFY_CYAN('netlify login --request ')} 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) => { diff --git a/src/utils/command-error-handler.ts b/src/utils/command-error-handler.ts new file mode 100644 index 00000000000..7449d206634 --- /dev/null +++ b/src/utils/command-error-handler.ts @@ -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() + } +} diff --git a/src/utils/run-program.ts b/src/utils/run-program.ts index eadbbc940be..9b950e2dfcb 100644 --- a/src/utils/run-program.ts +++ b/src/utils/run-program.ts @@ -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 @@ -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 + } } diff --git a/tests/integration/commands/didyoumean/__snapshots__/didyoumean.test.ts.snap b/tests/integration/commands/didyoumean/__snapshots__/didyoumean.test.ts.snap index 6dbb7b3ed46..1f24269f778 100644 --- a/tests/integration/commands/didyoumean/__snapshots__/didyoumean.test.ts.snap +++ b/tests/integration/commands/didyoumean/__snapshots__/didyoumean.test.ts.snap @@ -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?" `; diff --git a/tests/integration/commands/error-handling/error-handling.test.ts b/tests/integration/commands/error-handling/error-handling.test.ts new file mode 100644 index 00000000000..7efeedbc4b5 --- /dev/null +++ b/tests/integration/commands/error-handling/error-handling.test.ts @@ -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') + } + }) + + 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') + }) +})