diff --git a/AGENTS.md b/AGENTS.md index 49289325..9e87b531 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -57,6 +57,9 @@ exit-code table, lives in `docs/output-contract.md`. answer from stdin and stderr, not stdout. - Never print anything to stdout around a `--output json` document. A failure oclif raises leaves stdout empty; see the contract's exit codes. +- Invalid input exits 2, other failures 1. Validation helpers throw + `InvalidInputError`; a catch block that reports the error itself sets + `process.exitCode = exitCodeFor(err)` rather than a literal 1. - Terminal users see both streams, so moving chatter to stderr does not change the interactive experience — that is the default for new output. diff --git a/docs/output-contract.md b/docs/output-contract.md index 76d93946..a7e1c480 100644 --- a/docs/output-contract.md +++ b/docs/output-contract.md @@ -99,13 +99,18 @@ change in the PR. | Code | Meaning | |---|---| | 0 | Success. Also a declined confirmation, `--dry-run` previews, and runs whose checks passed or only degraded (a degraded check does not fail the run). A run stopped with Ctrl-C has no code of its own: it exits 1 if a result that arrived failed, and 0 otherwise. | -| 1 | Failure: a failed check or run error, no matching checks under `--fail-on-no-matching` (the default), API errors, invalid configuration, or a failure reported only on stderr. | -| 2 | Usage error: unknown flags or arguments, oclif parse errors, or invalid flag or argument values a command rejects with `this.error`. The message is on stderr and stdout is empty. | +| 1 | Failure: a failed check or run error, no matching checks under `--fail-on-no-matching` (the default), API errors, invalid configuration, well-formed input that names something that does not exist (an unknown location, plan or entitlement), or a failure reported only on stderr. | +| 2 | Usage error: unknown flags or arguments, oclif parse errors, invalid flag or argument values, or flags that cannot be combined. The message is on stderr; stdout is empty, except that some commands print their JSON error document under `--output json`. | | 3 | `confirmation_required` (agent and CI mode): a write command stopped for approval and printed the envelope on stdout. | | 127 | Unknown command. | -oclif's `this.error` exits 2 by default, so commands report any other failure -through it with `{ exit: 1 }`; a bare `this.error` is for invalid input. +Commands reject invalid input with a bare `this.error` (oclif's default exit +is 2), by throwing `InvalidInputError` (`helpers/invalid-input-error.ts`), or +with `style.longError`/`style.fatal` followed by `this.exit(2)`. Validation +helpers throw `InvalidInputError`, since they cannot call `this.error`; a catch +block that reports an error itself takes the code from `exitCodeFor(err)`, so +invalid input caught there still exits 2. Any other failure exits 1, through +`this.error` with `{ exit: 1 }` when it uses oclif's. ## Notes diff --git a/packages/cli/e2e/__tests__/cli-parse-error.spec.ts b/packages/cli/e2e/__tests__/cli-parse-error.spec.ts index ca149156..06f49d5a 100644 --- a/packages/cli/e2e/__tests__/cli-parse-error.spec.ts +++ b/packages/cli/e2e/__tests__/cli-parse-error.spec.ts @@ -43,4 +43,25 @@ describe('cli parse error', () => { } } }) + + // Input a command rejects itself, before any API call, exits with the + // usage-error code like oclif's own parse errors. + it.each([ + ['a malformed api field', ['api', '/v1/checks', '-F', 'broken'], 'Invalid field format'], + ['a custom flag parse failure', ['deploy', '--schedule-on-deploy-threshold', 'abc'], + 'Expected a non-negative whole number or "auto"'], + ['an unsupported import resource type', ['import', 'plan', 'bogus:1'], 'Unsupported resource type'], + ])('exits 2 for %s', async (_, args, message) => { + try { + await runCheckly(fixt, args) + expect.unreachable('Expected command to fail') + } catch (err) { + if (err instanceof ExecaError) { + expect(err.exitCode).toBe(2) + expect(err.stderr).toContain(message) + } else { + throw err + } + } + }) }) diff --git a/packages/cli/src/commands/__tests__/assets.spec.ts b/packages/cli/src/commands/__tests__/assets.spec.ts index c814b2f8..af43b569 100644 --- a/packages/cli/src/commands/__tests__/assets.spec.ts +++ b/packages/cli/src/commands/__tests__/assets.spec.ts @@ -127,7 +127,7 @@ describe('assets commands', () => { }) await expect(AssetsList.prototype.run.call(missing as any)) .rejects - .toThrow('Use exactly one of --check-id or --test-session-id.') + .toMatchObject({ message: 'Use exactly one of --check-id or --test-session-id.', oclif: { exit: 2 } }) const both = createCommandContext({ flags: { @@ -139,7 +139,7 @@ describe('assets commands', () => { }) await expect(AssetsList.prototype.run.call(both as any)) .rejects - .toThrow('Use exactly one of --check-id or --test-session-id, not both.') + .toMatchObject({ message: 'Use exactly one of --check-id or --test-session-id, not both.', oclif: { exit: 2 } }) }) it('lists check result assets as a table with copyable Asset values and truncation warning', async () => { @@ -743,7 +743,7 @@ describe('assets commands', () => { await expect(AssetsDownload.prototype.run.call(ctx as any)) .rejects - .toThrow('--force and --skip-existing are mutually exclusive.') + .toMatchObject({ message: '--force and --skip-existing are mutually exclusive.', oclif: { exit: 2 } }) }) it('does not expose archive extraction before it is implemented', () => { diff --git a/packages/cli/src/commands/__tests__/confirm-flow-deploy.spec.ts b/packages/cli/src/commands/__tests__/confirm-flow-deploy.spec.ts index 4e57272c..57ad7f60 100644 --- a/packages/cli/src/commands/__tests__/confirm-flow-deploy.spec.ts +++ b/packages/cli/src/commands/__tests__/confirm-flow-deploy.spec.ts @@ -1187,7 +1187,7 @@ describe('deploy with --no-plan', () => { vi.mocked(parseProject).mockClear() const ctx = createCommandContext({ plan: false, force: true, ...flags }) - await expect(Deploy.prototype.run.call(ctx as any)).rejects.toThrow('EXIT_1') + await expect(Deploy.prototype.run.call(ctx as any)).rejects.toThrow('EXIT_2') expect(ctx.style.longError).toHaveBeenCalledWith( `${named} applies to a planned deploy only.`, @@ -1999,7 +1999,7 @@ describe('deploy of a plan with nothing to apply', () => { vi.mocked(parseProject).mockClear() const ctx = createCommandContext({ force: true, ...flags }) - await expect(Deploy.prototype.run.call(ctx as any)).rejects.toThrow('EXIT_1') + await expect(Deploy.prototype.run.call(ctx as any)).rejects.toThrow('EXIT_2') expect(ctx.style.longError).toHaveBeenCalledWith( `${named} applies only when checks are scheduled after the deploy.`, diff --git a/packages/cli/src/commands/account/usage/__tests__/usage.spec.ts b/packages/cli/src/commands/account/usage/__tests__/usage.spec.ts index 3a01342a..6609bf58 100644 --- a/packages/cli/src/commands/account/usage/__tests__/usage.spec.ts +++ b/packages/cli/src/commands/account/usage/__tests__/usage.spec.ts @@ -110,6 +110,7 @@ describe('usage terms commands', () => { expect(api.usage.getTerm).not.toHaveBeenCalled() expect(bad.style.longError).toHaveBeenCalledWith('Failed to load usage term.', expect.objectContaining({ message: 'Usage term ID must be a valid UUID.' })) + expect(process.exitCode).toBe(2) vi.mocked(api.usage.getTerm).mockRejectedValue(new NotFoundError({ statusCode: 404, error: 'Not Found', message: 'No usage terms found', code: 'NO_USAGE_TERMS', })) @@ -331,7 +332,7 @@ describe('report failure paths', () => { expect(api.usage.getSeries).not.toHaveBeenCalled() expect(ctx.style.longError).toHaveBeenCalledWith(expect.any(String), expect.objectContaining({ message: expect.stringContaining('does not support --check-type') })) - expect(process.exitCode).toBe(1) + expect(process.exitCode).toBe(2) }) it('reports usage:read permission failures', async () => { diff --git a/packages/cli/src/commands/account/usage/projections.ts b/packages/cli/src/commands/account/usage/projections.ts index 10bd8578..35d9488e 100644 --- a/packages/cli/src/commands/account/usage/projections.ts +++ b/packages/cli/src/commands/account/usage/projections.ts @@ -2,6 +2,7 @@ import { Args, Flags } from '@oclif/core' import { AuthCommand } from '../../authCommand.js' import { outputFlag } from '../../../helpers/flags.js' import { describeUsageError, projectionsParams, validateUsageTermId } from '../../../helpers/usage.js' +import { exitCodeFor } from '../../../helpers/invalid-input-error.js' import * as api from '../../../rest/api.js' import { formatUsageProjections } from '../../../formatters/usage.js' @@ -33,7 +34,7 @@ export default class UsageProjectionsCommand extends AuthCommand { this.log(formatUsageProjections(projections, flags.output === 'md' ? 'md' : 'terminal', term)) } catch (err: unknown) { this.style.longError('Failed to load usage projections.', describeUsageError(err, { termId: args.id })) - process.exitCode = 1 + process.exitCode = exitCodeFor(err) } } } diff --git a/packages/cli/src/commands/account/usage/series.ts b/packages/cli/src/commands/account/usage/series.ts index 9a9f9d53..e2f99972 100644 --- a/packages/cli/src/commands/account/usage/series.ts +++ b/packages/cli/src/commands/account/usage/series.ts @@ -1,6 +1,7 @@ import { AuthCommand } from '../../authCommand.js' import { outputFlag } from '../../../helpers/flags.js' import { describeUsageError, loadUsageNames, usageSeriesFlags, usageSeriesParams } from '../../../helpers/usage.js' +import { exitCodeFor } from '../../../helpers/invalid-input-error.js' import * as api from '../../../rest/api.js' import { heading, type OutputFormat } from '../../../formatters/render.js' import { @@ -50,7 +51,7 @@ export default class UsageSeriesCommand extends AuthCommand { ].filter(Boolean).join('\n\n')) } catch (err: unknown) { this.style.longError('Failed to load usage series.', describeUsageError(err)) - process.exitCode = 1 + process.exitCode = exitCodeFor(err) } } } diff --git a/packages/cli/src/commands/account/usage/summary.ts b/packages/cli/src/commands/account/usage/summary.ts index 5682d43d..4a2582f0 100644 --- a/packages/cli/src/commands/account/usage/summary.ts +++ b/packages/cli/src/commands/account/usage/summary.ts @@ -1,6 +1,7 @@ import { AuthCommand } from '../../authCommand.js' import { outputFlag } from '../../../helpers/flags.js' import { describeUsageError, usageRangeFlags, usageRangeParams } from '../../../helpers/usage.js' +import { exitCodeFor } from '../../../helpers/invalid-input-error.js' import * as api from '../../../rest/api.js' import { type OutputFormat, renderCommandHints } from '../../../formatters/render.js' import { formatUsageSummary, usageListCommand } from '../../../formatters/usage.js' @@ -36,7 +37,7 @@ export default class UsageSummaryCommand extends AuthCommand { this.log(output.join('\n\n')) } catch (err: unknown) { this.style.longError('Failed to load usage summary.', describeUsageError(err)) - process.exitCode = 1 + process.exitCode = exitCodeFor(err) } } } diff --git a/packages/cli/src/commands/account/usage/term.ts b/packages/cli/src/commands/account/usage/term.ts index d055f7b4..4eab2431 100644 --- a/packages/cli/src/commands/account/usage/term.ts +++ b/packages/cli/src/commands/account/usage/term.ts @@ -2,6 +2,7 @@ import { Args } from '@oclif/core' import { AuthCommand } from '../../authCommand.js' import { outputFlag } from '../../../helpers/flags.js' import { describeUsageError, validateUsageTermId } from '../../../helpers/usage.js' +import { exitCodeFor } from '../../../helpers/invalid-input-error.js' import * as api from '../../../rest/api.js' import { formatUsageTermDetail } from '../../../formatters/usage.js' @@ -25,7 +26,7 @@ export default class UsageTermCommand extends AuthCommand { : formatUsageTermDetail(term, flags.output === 'md' ? 'md' : 'terminal')) } catch (err: unknown) { this.style.longError('Failed to load usage term.', describeUsageError(err, { termId: args.id })) - process.exitCode = 1 + process.exitCode = exitCodeFor(err) } } } diff --git a/packages/cli/src/commands/account/usage/terms.ts b/packages/cli/src/commands/account/usage/terms.ts index afb5075b..a5147b59 100644 --- a/packages/cli/src/commands/account/usage/terms.ts +++ b/packages/cli/src/commands/account/usage/terms.ts @@ -1,6 +1,7 @@ import { AuthCommand } from '../../authCommand.js' import { outputFlag } from '../../../helpers/flags.js' import { describeUsageError, usageTermsListFlags, usageTermsParams } from '../../../helpers/usage.js' +import { exitCodeFor } from '../../../helpers/invalid-input-error.js' import * as api from '../../../rest/api.js' import type { OutputFormat } from '../../../formatters/render.js' import { @@ -41,7 +42,7 @@ export default class UsageTermsCommand extends AuthCommand { ].filter(Boolean).join('\n\n')) } catch (err: unknown) { this.style.longError('Failed to load usage terms.', describeUsageError(err)) - process.exitCode = 1 + process.exitCode = exitCodeFor(err) } } } diff --git a/packages/cli/src/commands/alert-channels/__tests__/alert-channels.spec.ts b/packages/cli/src/commands/alert-channels/__tests__/alert-channels.spec.ts index 2616f89f..fc82253f 100644 --- a/packages/cli/src/commands/alert-channels/__tests__/alert-channels.spec.ts +++ b/packages/cli/src/commands/alert-channels/__tests__/alert-channels.spec.ts @@ -89,6 +89,32 @@ describe('alert-channels commands', () => { expect(JSON.parse(ctx.logged[0])).toEqual(alertChannelFixture) }) + it('get rejects an invalid ID with the usage-error exit code', async () => { + const ctx = createCommandContext({ + args: { id: 'abc' }, + flags: { output: 'json' }, + }) + + await AlertChannelsGet.prototype.run.call(ctx as any) + + expect(api.alertChannels.get).not.toHaveBeenCalled() + expect(ctx.style.longError).toHaveBeenCalledWith('Failed to get alert channel details.', + expect.objectContaining({ message: 'Alert channel ID must be a positive integer.' })) + expect(process.exitCode).toBe(2) + }) + + it('get keeps exit 1 for an API failure', async () => { + vi.mocked(api.alertChannels.get).mockRejectedValue(new Error('boom')) + const ctx = createCommandContext({ + args: { id: '123' }, + flags: { output: 'json' }, + }) + + await AlertChannelsGet.prototype.run.call(ctx as any) + + expect(process.exitCode).toBe(1) + }) + it('get includes navigation hints in terminal output', async () => { const ctx = createCommandContext({ args: { id: '123' }, diff --git a/packages/cli/src/commands/alert-channels/get.ts b/packages/cli/src/commands/alert-channels/get.ts index aa088e07..58613e79 100644 --- a/packages/cli/src/commands/alert-channels/get.ts +++ b/packages/cli/src/commands/alert-channels/get.ts @@ -3,6 +3,7 @@ import chalk from 'chalk' import { AuthCommand } from '../authCommand.js' import { outputFlag } from '../../helpers/flags.js' import { parsePositiveInteger } from '../../helpers/number.js' +import { exitCodeFor } from '../../helpers/invalid-input-error.js' import * as api from '../../rest/api.js' import type { OutputFormat } from '../../formatters/render.js' import { formatAlertChannelDetail } from '../../formatters/alert-channels.js' @@ -53,7 +54,7 @@ export default class AlertChannelsGet extends AuthCommand { this.log(output.join('\n')) } catch (err: any) { this.style.longError('Failed to get alert channel details.', err) - process.exitCode = 1 + process.exitCode = exitCodeFor(err) } } } diff --git a/packages/cli/src/commands/alert-channels/list.ts b/packages/cli/src/commands/alert-channels/list.ts index 0e7baa95..84ce49e8 100644 --- a/packages/cli/src/commands/alert-channels/list.ts +++ b/packages/cli/src/commands/alert-channels/list.ts @@ -2,6 +2,7 @@ import { Flags } from '@oclif/core' import { AuthCommand } from '../authCommand.js' import { outputFlag } from '../../helpers/flags.js' import { validateIntegerRange } from '../../helpers/number.js' +import { exitCodeFor } from '../../helpers/invalid-input-error.js' import * as api from '../../rest/api.js' import type { OutputFormat } from '../../formatters/render.js' import { @@ -71,7 +72,7 @@ export default class AlertChannelsList extends AuthCommand { this.log(output.join('\n')) } catch (err: any) { this.style.longError('Failed to list alert channels.', err) - process.exitCode = 1 + process.exitCode = exitCodeFor(err) } } } diff --git a/packages/cli/src/commands/alert-channels/logs.ts b/packages/cli/src/commands/alert-channels/logs.ts index 5fea77ea..0111e4b7 100644 --- a/packages/cli/src/commands/alert-channels/logs.ts +++ b/packages/cli/src/commands/alert-channels/logs.ts @@ -8,6 +8,7 @@ import { validateIntegerRange, } from '../../helpers/number.js' import * as api from '../../rest/api.js' +import { exitCodeFor } from '../../helpers/invalid-input-error.js' import type { AlertNotificationListParams } from '../../rest/alert-notifications.js' import type { OutputFormat } from '../../formatters/render.js' import { formatAlertNotificationLogs } from '../../formatters/alert-channels.js' @@ -98,7 +99,7 @@ export default class AlertChannelsLogs extends AuthCommand { this.log(output.join('\n')) } catch (err: any) { this.style.longError('Failed to list alert channel logs.', err) - process.exitCode = 1 + process.exitCode = exitCodeFor(err) } } } diff --git a/packages/cli/src/commands/assets/download.ts b/packages/cli/src/commands/assets/download.ts index b46f8b39..0342efb4 100644 --- a/packages/cli/src/commands/assets/download.ts +++ b/packages/cli/src/commands/assets/download.ts @@ -1,6 +1,7 @@ import { Flags } from '@oclif/core' import path from 'node:path' import { AuthCommand } from '../authCommand.js' +import { InvalidInputError } from '../../helpers/invalid-input-error.js' import { outputFlag } from '../../helpers/flags.js' import { assetSelectorValue, formatDownloadedAssets, type DownloadedAssetRow } from '../../formatters/assets.js' import { @@ -87,7 +88,7 @@ export default class AssetsDownload extends AuthCommand { } if (flags.force && flags['skip-existing']) { - throw new Error('--force and --skip-existing are mutually exclusive.') + throw new InvalidInputError('--force and --skip-existing are mutually exclusive.') } try { diff --git a/packages/cli/src/commands/deploy.ts b/packages/cli/src/commands/deploy.ts index ce0e0e7e..314ac2ca 100644 --- a/packages/cli/src/commands/deploy.ts +++ b/packages/cli/src/commands/deploy.ts @@ -290,7 +290,7 @@ export default class Deploy extends AuthCommand { `${needsPlan} applies to a planned deploy only.`, `Re-run without --no-plan, or without ${needsPlan}.`, ) - this.exit(1) + this.exit(2) } } @@ -308,7 +308,7 @@ export default class Deploy extends AuthCommand { `--${refiningFlag} applies only when checks are scheduled after the deploy.`, `Add --schedule-on-deploy to schedule them, or drop --${refiningFlag}.`, ) - this.exit(1) + this.exit(2) } } diff --git a/packages/cli/src/commands/import/__tests__/cancel.spec.ts b/packages/cli/src/commands/import/__tests__/cancel.spec.ts index e0d09e23..8f20db2d 100644 --- a/packages/cli/src/commands/import/__tests__/cancel.spec.ts +++ b/packages/cli/src/commands/import/__tests__/cancel.spec.ts @@ -131,14 +131,14 @@ describe('import cancel command (flag validation)', () => { vi.mocked(api.projects.cancelImportPlan).mockResolvedValue({} as any) }) - it('rejects --all together with --plan-id (exit 1) instead of silently ignoring one', async () => { + it('rejects --all together with --plan-id (exit 2) instead of silently ignoring one', async () => { vi.mocked(detectCliMode).mockReturnValue('agent') vi.mocked(api.projects.findImportPlans).mockResolvedValue({ data: [plan('a'), plan('b')], } as any) const ctx = createCommandContext(cancelFlags({ 'all': true, 'plan-id': 'a' })) - await expect(ImportCancelCommand.prototype.run.call(ctx as any)).rejects.toThrow('EXIT_1') + await expect(ImportCancelCommand.prototype.run.call(ctx as any)).rejects.toThrow('EXIT_2') expect(ctx.style.fatal).toHaveBeenCalledWith(expect.stringContaining('cannot be used together')) expect(api.projects.cancelImportPlan).not.toHaveBeenCalled() }) @@ -148,7 +148,7 @@ describe('import cancel command (flag validation)', () => { vi.mocked(api.projects.findImportPlans).mockResolvedValue({ data: [] } as any) const ctx = createCommandContext(cancelFlags({ 'all': true, 'plan-id': 'a' })) - await expect(ImportCancelCommand.prototype.run.call(ctx as any)).rejects.toThrow('EXIT_1') + await expect(ImportCancelCommand.prototype.run.call(ctx as any)).rejects.toThrow('EXIT_2') expect(ctx.style.fatal).toHaveBeenCalledWith(expect.stringContaining('cannot be used together')) expect(api.projects.findImportPlans).not.toHaveBeenCalled() }) diff --git a/packages/cli/src/commands/import/plan.ts b/packages/cli/src/commands/import/plan.ts index 58ea9359..2706d295 100644 --- a/packages/cli/src/commands/import/plan.ts +++ b/packages/cli/src/commands/import/plan.ts @@ -12,6 +12,7 @@ import { execa } from 'execa' import { LOGICAL_ID_PATTERN } from '../../constants.js' import * as api from '../../rest/api.js' import { AuthCommand } from '../authCommand.js' +import { InvalidInputError } from '../../helpers/invalid-input-error.js' import commonMessages from '../../messages/common-messages.js' import { getRepoUrlFromGit, splitConfigFilePath } from '../../services/util.js' import { ChecklyConfig, ConfigNotFoundError, loadChecklyConfig } from '../../services/checkly-config-loader.js' @@ -1536,7 +1537,7 @@ function parseFilter (spec: string): ImportPlanFilter { const parseId = importables[type as keyof typeof importables] if (parseId === undefined) { - throw new Error(`Invalid resource specifier '${spec}': Unsupported resource type '${type}'`) + throw new InvalidInputError(`Invalid resource specifier '${spec}': Unsupported resource type '${type}'`) } try { @@ -1546,7 +1547,7 @@ function parseFilter (spec: string): ImportPlanFilter { } } catch (err) { if (err instanceof InvalidResourceIdentifierError) { - throw new Error(`Invalid resource specifier '${spec}': ${err.message}`, { cause: err }) + throw new InvalidInputError(`Invalid resource specifier '${spec}': ${err.message}`, { cause: err }) } throw err diff --git a/packages/cli/src/helpers/__tests__/api-fields.spec.ts b/packages/cli/src/helpers/__tests__/api-fields.spec.ts index 58b57d0a..ca9ec2ec 100644 --- a/packages/cli/src/helpers/__tests__/api-fields.spec.ts +++ b/packages/cli/src/helpers/__tests__/api-fields.spec.ts @@ -1,5 +1,6 @@ import { describe, it, expect } from 'vitest' import { parseFields } from '../api-fields.js' +import { InvalidInputError } from '../invalid-input-error.js' describe('parseFields', () => { it('parses string fields with =', () => { @@ -33,6 +34,7 @@ describe('parseFields', () => { it('throws on field without = or :=', () => { expect(() => parseFields(['broken'])).toThrow('Invalid field format: "broken"') + expect(() => parseFields(['broken'])).toThrow(InvalidInputError) }) it('throws on invalid JSON in := field', () => { diff --git a/packages/cli/src/helpers/__tests__/flags.spec.ts b/packages/cli/src/helpers/__tests__/flags.spec.ts index 20b391e6..0f0e5218 100644 --- a/packages/cli/src/helpers/__tests__/flags.spec.ts +++ b/packages/cli/src/helpers/__tests__/flags.spec.ts @@ -1,5 +1,6 @@ import { describe, expect, it } from 'vitest' import { normalizeFlagAliases, parseAutoOrWholeNumber } from '../flags.js' +import { InvalidInputError } from '../invalid-input-error.js' describe('normalizeFlagAliases', () => { const aliases = [ @@ -62,5 +63,6 @@ describe('parseAutoOrWholeNumber', () => { it.each(['-1', '1.5', 'abc', '', '1e3'])('rejects %j', input => { expect(() => parseAutoOrWholeNumber(input)).toThrow('Expected a non-negative whole number or "auto"') + expect(() => parseAutoOrWholeNumber(input)).toThrow(InvalidInputError) }) }) diff --git a/packages/cli/src/helpers/__tests__/import-plan-selection.spec.ts b/packages/cli/src/helpers/__tests__/import-plan-selection.spec.ts index bf6bf52c..3275521e 100644 --- a/packages/cli/src/helpers/__tests__/import-plan-selection.spec.ts +++ b/packages/cli/src/helpers/__tests__/import-plan-selection.spec.ts @@ -10,6 +10,7 @@ import { PlanSelectionError, reportNoCandidatePlans, resolvePlanNonInteractively, + selectPlansOrExit, validatePlanFlagsOrExit, } from '../import-plan-selection.js' @@ -97,10 +98,10 @@ describe('resolvePlanNonInteractively', () => { }) describe('validatePlanFlagsOrExit', () => { - it('exits 1 when --all and --plan-id are combined', () => { + it('exits 2, the usage-error code, when --all and --plan-id are combined', () => { const command = createCommand() - expect(() => validatePlanFlagsOrExit(command, { all: true, planId: 'a' })).toThrow('EXIT_1') + expect(() => validatePlanFlagsOrExit(command, { all: true, planId: 'a' })).toThrow('EXIT_2') expect(command.style.fatal).toHaveBeenCalledWith(expect.stringContaining('cannot be used together')) }) @@ -116,6 +117,18 @@ describe('validatePlanFlagsOrExit', () => { }) }) +describe('selectPlansOrExit', () => { + it('exits 2 when --all and --plan-id are combined, without selecting', async () => { + const command = createCommand() + const interactiveFallback = vi.fn() + + await expect(selectPlansOrExit(command, [plan('a')], { all: true, planId: 'a' }, 'cancel', interactiveFallback)) + .rejects.toThrow('EXIT_2') + expect(command.style.fatal).toHaveBeenCalledWith(expect.stringContaining('cannot be used together')) + expect(interactiveFallback).not.toHaveBeenCalled() + }) +}) + describe('reportNoCandidatePlans', () => { const options = { action: 'commit', nothingToDo: 'Nothing to commit.' } diff --git a/packages/cli/src/helpers/__tests__/invalid-input-error.spec.ts b/packages/cli/src/helpers/__tests__/invalid-input-error.spec.ts new file mode 100644 index 00000000..9a55f6e2 --- /dev/null +++ b/packages/cli/src/helpers/__tests__/invalid-input-error.spec.ts @@ -0,0 +1,18 @@ +import { describe, expect, it } from 'vitest' + +import { exitCodeFor, InvalidInputError } from '../invalid-input-error.js' + +describe('InvalidInputError', () => { + it('carries the usage-error exit code for oclif', () => { + const error = new InvalidInputError('--limit must be an integer between 1 and 100.') + + expect(error).toBeInstanceOf(Error) + expect(error.oclif).toEqual({ exit: 2 }) + }) + + it('exitCodeFor maps invalid input to 2 and anything else to 1', () => { + expect(exitCodeFor(new InvalidInputError('bad'))).toBe(2) + expect(exitCodeFor(new Error('boom'))).toBe(1) + expect(exitCodeFor('boom')).toBe(1) + }) +}) diff --git a/packages/cli/src/helpers/__tests__/number.spec.ts b/packages/cli/src/helpers/__tests__/number.spec.ts new file mode 100644 index 00000000..8f30a773 --- /dev/null +++ b/packages/cli/src/helpers/__tests__/number.spec.ts @@ -0,0 +1,27 @@ +import { describe, expect, it } from 'vitest' + +import { InvalidInputError } from '../invalid-input-error.js' +import { parseNonNegativeInteger, parsePositiveInteger, validateIntegerRange } from '../number.js' + +describe('number helpers', () => { + it('parsePositiveInteger accepts positive integers from strings and numbers', () => { + expect(parsePositiveInteger('12', 'ID')).toBe(12) + expect(parsePositiveInteger(3, 'ID')).toBe(3) + }) + + it.each(['abc', '0', '-1', '1.5'])('parsePositiveInteger rejects %j as invalid input', value => { + expect(() => parsePositiveInteger(value, 'ID')).toThrow('ID must be a positive integer.') + expect(() => parsePositiveInteger(value, 'ID')).toThrow(InvalidInputError) + }) + + it('parseNonNegativeInteger rejects a negative value as invalid input', () => { + expect(parseNonNegativeInteger(0, '--from')).toBe(0) + expect(() => parseNonNegativeInteger(-1, '--from')).toThrow(InvalidInputError) + }) + + it('validateIntegerRange rejects a value out of range as invalid input', () => { + expect(validateIntegerRange(5, '--limit', 1, 100)).toBe(5) + expect(() => validateIntegerRange(101, '--limit', 1, 100)).toThrow('--limit must be an integer between 1 and 100.') + expect(() => validateIntegerRange(101, '--limit', 1, 100)).toThrow(InvalidInputError) + }) +}) diff --git a/packages/cli/src/helpers/__tests__/test-helper.spec.ts b/packages/cli/src/helpers/__tests__/test-helper.spec.ts index b6f920ff..0e1b006c 100644 --- a/packages/cli/src/helpers/__tests__/test-helper.spec.ts +++ b/packages/cli/src/helpers/__tests__/test-helper.spec.ts @@ -1,6 +1,7 @@ import { describe, it, expect, vi, afterEach } from 'vitest' import { prepareMaxFailures, prepareReportersTypes, validateDetachReporterTypes } from '../test-helper.js' import type { CliMode } from '../cli-mode.js' +import { InvalidInputError } from '../invalid-input-error.js' const cliMode = vi.hoisted(() => ({ value: 'interactive' as CliMode })) vi.mock('../cli-mode.js', () => ({ detectCliMode: () => cliMode.value })) @@ -63,6 +64,7 @@ describe('validateDetachReporterTypes', () => { it('rejects json reporter in detach mode', () => { expect(() => validateDetachReporterTypes(['json'])).toThrow('--detach does not support --reporter json') + expect(() => validateDetachReporterTypes(['json'])).toThrow(InvalidInputError) }) it('rejects github reporter in detach mode', () => { diff --git a/packages/cli/src/helpers/__tests__/usage.spec.ts b/packages/cli/src/helpers/__tests__/usage.spec.ts index bcad78e0..becc4461 100644 --- a/packages/cli/src/helpers/__tests__/usage.spec.ts +++ b/packages/cli/src/helpers/__tests__/usage.spec.ts @@ -5,6 +5,7 @@ import { describeUsageError, projectionsParams, usageRangeParams, usageSeriesFlags, usageSeriesParams, usageTermsParams, validateUsageTermId, } from '../usage.js' +import { InvalidInputError } from '../invalid-input-error.js' const id = '550e8400-e29b-41d4-a716-446655440000' const account = 'a1111111-1111-1111-1111-111111111111' @@ -20,6 +21,7 @@ describe('usage input validation', () => { it.each(['2026-2-1', '01-02-2026', '2026-02-30', '2026-13-01', 'yesterday', ''])('rejects date %s', value => { expect(() => usageTermsParams({ to: value, limit: 100 })) .toThrow('--to must be a valid calendar date in YYYY-MM-DD format.') + expect(() => usageTermsParams({ to: value, limit: 100 })).toThrow(InvalidInputError) }) it('keeps omitted terms bounds unset', () => { diff --git a/packages/cli/src/helpers/api-fields.ts b/packages/cli/src/helpers/api-fields.ts index 3f94cbad..6d240318 100644 --- a/packages/cli/src/helpers/api-fields.ts +++ b/packages/cli/src/helpers/api-fields.ts @@ -1,3 +1,5 @@ +import { InvalidInputError } from './invalid-input-error.js' + export interface ParsedFields { [key: string]: unknown } @@ -13,14 +15,14 @@ export function parseFields (fields: string[]): ParsedFields { try { result[key] = JSON.parse(raw) } catch { - throw new Error(`Invalid JSON in field "${key}": ${raw}`) + throw new InvalidInputError(`Invalid JSON in field "${key}": ${raw}`) } continue } const eqIndex = entry.indexOf('=') if (eqIndex === -1) { - throw new Error(`Invalid field format: "${entry}". Expected key=value or key:=json.`) + throw new InvalidInputError(`Invalid field format: "${entry}". Expected key=value or key:=json.`) } result[entry.slice(0, eqIndex)] = entry.slice(eqIndex + 1) } diff --git a/packages/cli/src/helpers/flags.ts b/packages/cli/src/helpers/flags.ts index 4f277cce..e20896dc 100644 --- a/packages/cli/src/helpers/flags.ts +++ b/packages/cli/src/helpers/flags.ts @@ -1,4 +1,5 @@ import { Flags } from '@oclif/core' +import { InvalidInputError } from './invalid-input-error.js' export interface FlagAlias { alias: string @@ -72,7 +73,7 @@ export function parseAutoOrWholeNumber (input: string): number | 'auto' { return 'auto' } if (!/^\d+$/.test(input)) { - throw new Error(`Expected a non-negative whole number or "auto", got "${input}".`) + throw new InvalidInputError(`Expected a non-negative whole number or "auto", got "${input}".`) } return Number(input) } diff --git a/packages/cli/src/helpers/import-plan-selection.ts b/packages/cli/src/helpers/import-plan-selection.ts index 4188c8a3..80441e23 100644 --- a/packages/cli/src/helpers/import-plan-selection.ts +++ b/packages/cli/src/helpers/import-plan-selection.ts @@ -24,28 +24,15 @@ function describePlan (plan: ImportPlan): string { return ` ${plan.id} (${details.join(', ')})` } -/** - * Returns the reason a flag combination is contradictory, or `undefined` when - * the combination is valid. Contradictions never depend on remote state. - */ -function planFlagsConflict ({ all, planId }: PlanFlags): string | undefined { - if (all === true && planId !== undefined) { - return '--all and --plan-id cannot be used together.' - } - - return undefined -} - /** * Rejects contradictory flag combinations before any plans are fetched, so that * an impossible command line fails identically whether or not plans happen to * exist remotely. */ -export function validatePlanFlagsOrExit (command: BaseCommand, flags: PlanFlags): void { - const conflict = planFlagsConflict(flags) - if (conflict !== undefined) { - command.style.fatal(conflict) - command.exit(1) +export function validatePlanFlagsOrExit (command: BaseCommand, { all, planId }: PlanFlags): void { + if (all === true && planId !== undefined) { + command.style.fatal('--all and --plan-id cannot be used together.') + command.exit(2) } } @@ -173,12 +160,8 @@ export async function selectPlansOrExit ( action: string, interactiveFallback: () => Promise, ): Promise { + validatePlanFlagsOrExit(command, { all, planId }) try { - const conflict = planFlagsConflict({ all, planId }) - if (conflict !== undefined) { - throw new PlanSelectionError(conflict) - } - if (all) { return plans } diff --git a/packages/cli/src/helpers/invalid-input-error.ts b/packages/cli/src/helpers/invalid-input-error.ts new file mode 100644 index 00000000..4e28fa96 --- /dev/null +++ b/packages/cli/src/helpers/invalid-input-error.ts @@ -0,0 +1,17 @@ +/** + * Input that is invalid on its own, whatever the account holds: a malformed or + * out-of-range flag or argument value, or flags that cannot be combined. The + * command exits 2, the usage-error code, instead of 1. + */ +export class InvalidInputError extends Error { + // oclif's Command.catch and error handler take the exit code from here. + oclif = { exit: 2 } +} + +/** + * The exit code of a failure a command reports itself (e.g. as a JSON error + * document) instead of letting it propagate to oclif. + */ +export function exitCodeFor (err: unknown): number { + return err instanceof InvalidInputError ? 2 : 1 +} diff --git a/packages/cli/src/helpers/number.ts b/packages/cli/src/helpers/number.ts index 2a6983fa..e4404119 100644 --- a/packages/cli/src/helpers/number.ts +++ b/packages/cli/src/helpers/number.ts @@ -1,21 +1,23 @@ +import { InvalidInputError } from './invalid-input-error.js' + export function parsePositiveInteger (value: string | number, label: string): number { const parsed = typeof value === 'number' ? value : Number(value) if (!Number.isInteger(parsed) || parsed < 1) { - throw new Error(`${label} must be a positive integer.`) + throw new InvalidInputError(`${label} must be a positive integer.`) } return parsed } export function parseNonNegativeInteger (value: number, label: string): number { if (!Number.isInteger(value) || value < 0) { - throw new Error(`${label} must be a non-negative integer.`) + throw new InvalidInputError(`${label} must be a non-negative integer.`) } return value } export function validateIntegerRange (value: number, label: string, min: number, max: number): number { if (!Number.isInteger(value) || value < min || value > max) { - throw new Error(`${label} must be an integer between ${min} and ${max}.`) + throw new InvalidInputError(`${label} must be an integer between ${min} and ${max}.`) } return value } diff --git a/packages/cli/src/helpers/result-assets.ts b/packages/cli/src/helpers/result-assets.ts index aeaf4292..5ac947b5 100644 --- a/packages/cli/src/helpers/result-assets.ts +++ b/packages/cli/src/helpers/result-assets.ts @@ -15,6 +15,7 @@ import type { AssetManifestEntry, } from '../rest/asset-manifests.js' import { assetSelectorValue } from '../formatters/assets.js' +import { InvalidInputError } from './invalid-input-error.js' const debug = Debug('checkly:cli:helpers:result-assets') @@ -59,18 +60,18 @@ export type AssetSource = export function resolveAssetSource (flags: AssetSourceFlags): AssetSource { if (!flags['result-id']) { - throw new Error('--result-id is required.') + throw new InvalidInputError('--result-id is required.') } const hasCheckId = Boolean(flags['check-id']) const hasTestSessionId = Boolean(flags['test-session-id']) if (hasCheckId && hasTestSessionId) { - throw new Error('Use exactly one of --check-id or --test-session-id, not both.') + throw new InvalidInputError('Use exactly one of --check-id or --test-session-id, not both.') } if (!hasCheckId && !hasTestSessionId) { - throw new Error('Use exactly one of --check-id or --test-session-id.') + throw new InvalidInputError('Use exactly one of --check-id or --test-session-id.') } if (flags['check-id']) { diff --git a/packages/cli/src/helpers/test-helper.ts b/packages/cli/src/helpers/test-helper.ts index 6b22a2a8..a099b003 100644 --- a/packages/cli/src/helpers/test-helper.ts +++ b/packages/cli/src/helpers/test-helper.ts @@ -4,6 +4,7 @@ import type { Region } from '../index.js' import { ReporterType } from '../reporters/reporter.js' import { detectCliMode } from './cli-mode.js' import { DEFAULT_REGION } from './constants.js' +import { InvalidInputError } from './invalid-input-error.js' const unsupportedDetachedReporterTypes: ReporterType[] = ['json', 'github'] @@ -69,7 +70,7 @@ export function validateDetachReporterTypes (reporterTypes: ReporterType[]): voi const unsupported = [...new Set(reporterTypes.filter(type => unsupportedDetachedReporterTypes.includes(type)))] if (unsupported.length) { - throw new Error( + throw new InvalidInputError( `--detach does not support ${unsupported.map(type => `--reporter ${type}`).join(', ')}. ` + 'Detached runs exit after scheduling, before result reports can be written. ' + 'Omit --detach to write a report, or use `checkly test-sessions get --output json` after scheduling.', diff --git a/packages/cli/src/helpers/usage.ts b/packages/cli/src/helpers/usage.ts index ceb5b2ca..e9166929 100644 --- a/packages/cli/src/helpers/usage.ts +++ b/packages/cli/src/helpers/usage.ts @@ -16,6 +16,7 @@ import { import * as api from '../rest/api.js' import type { UsageNames } from '../formatters/usage.js' import { validateIntegerRange } from './number.js' +import { InvalidInputError } from './invalid-input-error.js' const DATE_ONLY_PATTERN = /^\d{4}-\d{2}-\d{2}$/ const ACCOUNT_ID_PATTERN = /^[\da-f]{8}-[\da-f]{4}-[\da-f]{4}-[\da-f]{4}-[\da-f]{12}$/i @@ -79,7 +80,7 @@ export function usageSeriesFlags () { function parseDateOnly (value: string | undefined, label: string): string | undefined { if (value === undefined) return undefined const trimmed = value.trim() - const error = new Error(`${label} must be a valid calendar date in YYYY-MM-DD format.`) + const error = new InvalidInputError(`${label} must be a valid calendar date in YYYY-MM-DD format.`) if (!DATE_ONLY_PATTERN.test(trimmed)) throw error const [year, month, day] = trimmed.split('-').map(Number) const parsed = new Date(0) @@ -108,13 +109,13 @@ interface UsageSeriesFlagValues extends UsageRangeFlagValues, UsagePaginationFla function rangeParams (flags: UsageDateFlags): UsageDateFlags { const from = parseDateOnly(flags.from, '--from') const to = parseDateOnly(flags.to, '--to') - if (from && to && from > to) throw new Error('--from must be on or before --to.') + if (from && to && from > to) throw new InvalidInputError('--from must be on or before --to.') return { from, to } } function cursor (value: string | undefined): string | undefined { if (value !== undefined && (value.length === 0 || value.length > 4096)) { - throw new Error('--cursor must contain between 1 and 4096 characters.') + throw new InvalidInputError('--cursor must contain between 1 and 4096 characters.') } return value } @@ -123,15 +124,15 @@ function parseList (values: string[] | undefined, label: string): string[] | und if (values === undefined) return undefined const entries = values.flatMap(value => value.split(',').map(part => part.trim())) if (entries.length === 0 || entries.some(entry => entry.length === 0)) { - throw new Error(`${label} must not contain empty values.`) + throw new InvalidInputError(`${label} must not contain empty values.`) } - if (entries.length > 100) throw new Error(`${label} accepts at most 100 values.`) + if (entries.length > 100) throw new InvalidInputError(`${label} accepts at most 100 values.`) return [...new Set(entries)] } function parseEnum (value: string, options: readonly T[], label: string): T { const match = options.find(option => option === value) - if (match === undefined) throw new Error(`${label} must be one of: ${options.join(', ')}.`) + if (match === undefined) throw new InvalidInputError(`${label} must be one of: ${options.join(', ')}.`) return match } @@ -143,13 +144,13 @@ function enumList ( export function validateUsageTermId (value: string, label = 'Usage term ID'): string { const trimmed = value.trim() - if (!isUuid(trimmed)) throw new Error(`${label} must be a valid UUID.`) + if (!isUuid(trimmed)) throw new InvalidInputError(`${label} must be a valid UUID.`) return trimmed } function rejectLegacyCheckType (flags: { 'check-type'?: string[] }): void { if (flags['check-type'] !== undefined) { - throw new Error('Usage v2 does not support --check-type filtering. ' + throw new InvalidInputError('Usage v2 does not support --check-type filtering. ' + 'Use --pricing-rule to select charges or --group-by checkType on series for a check-type breakdown.') } } @@ -165,7 +166,7 @@ export function usageTermsParams (flags: UsageTermsFlagValues): UsageTermsParams export function usageRangeParams (flags: UsageRangeFlagValues): UsageRangeParams { rejectLegacyCheckType(flags) const accountIds = parseList(flags['account-id'], '--account-id')?.map(value => { - if (!ACCOUNT_ID_PATTERN.test(value)) throw new Error('--account-id must contain valid GUIDs.') + if (!ACCOUNT_ID_PATTERN.test(value)) throw new InvalidInputError('--account-id must contain valid GUIDs.') return value }) return { @@ -194,7 +195,7 @@ export function usageSeriesParams (flags: UsageSeriesFlagValues): UsageSeriesPar export function projectionsParams (flags: { 'as-of'?: string }): UsageProjectionsParams { const asOf = parseDateOnly(flags['as-of'], '--as-of') if (asOf !== undefined && asOf >= new Date().toISOString().slice(0, 10)) { - throw new Error('--as-of must be a completed UTC date, before today. Defaults to yesterday UTC.') + throw new InvalidInputError('--as-of must be a completed UTC date, before today. Defaults to yesterday UTC.') } return asOf === undefined ? {} : { asOf } }