Skip to content

feat(cli): give failures a taxonomy exit code instead of a blanket 1#1058

Open
joshrotenberg wants to merge 1 commit into
mainfrom
fix/1045-exit-code-taxonomy
Open

feat(cli): give failures a taxonomy exit code instead of a blanket 1#1058
joshrotenberg wants to merge 1 commit into
mainfrom
fix/1045-exit-code-taxonomy

Conversation

@joshrotenberg

Copy link
Copy Markdown
Collaborator

Part of #1045, item 3 (GAP-AUTOMATION-02).

Problem

Every runtime failure exited 1, from a blanket process::exit(1). A script could tell that redisctl failed but not why, so branching on failure type meant matching on message text.

Change

crates/redisctl/src/error.rs gains an exit_code module holding the taxonomy the issue names, and RedisCtlError::exit_code() mapping each variant to one:

Code Name Variants
1 GENERIC Other, OutputError
2 USAGE UnsupportedDeploymentType, FileError
3 CONFIG Configuration, ProfileNotFound, ProfileTypeMismatch, NoProfileConfigured, MissingCredentials
4 AUTH AuthenticationFailed
5 NOT_FOUND ApiError (404)
6 VALIDATION InvalidInput
7 CONFLICT ApiError (409)
8 UPSTREAM ApiError (other)
9 RATE_LIMITED ApiError (429)
10 NETWORK ConnectionError
11 TIMEOUT Timeout
12 CANCELLED Cancelled

The match is exhaustive, so a new variant has to declare its code, the same way code() already works.

main.rs exits with e.exit_code() rather than 1. The two pre-dispatch exits in the prefix-inference layer do not have a RedisCtlError in hand, so they name their code directly: the ConfigError arms exit CONFIG, and the "cannot determine platform" arm exits USAGE.

The -o json/-o yaml envelope now carries exit_code alongside code. That is the field #1048 explicitly deferred to this change.

Choices worth flagging

  • USAGE is 2 because clap already exits 2 when argument parsing fails. Both mean the invocation was wrong, so a script sees one code for that class rather than two.
  • GENERIC stays 1, so anything not yet classified keeps its current behavior. Other is the anyhow catch-all and OutputError covers serialization and IO.
  • ApiError sniffs the status out of its message. It is the only variant that carries a server status, so without this, not-found, conflict and rate-limited would be unreachable codes. suggestions() already does the same for 404. The real fix is a status field on the variant, which belongs with the Config/Configuration merge in item 2 of Automation contract: tracing corrupts -o json, duplicate error line, exit codes, error envelope #1045.

Gap found while testing

--profile <nonexistent> still exits 1, not 3. It never reaches RedisCtlError::ProfileNotFound: connection.rs:174 and connection.rs:389 report it through anyhow::Context::with_context, which lands in Other. This is the "354 error paths collapse into a generic variant" problem described in item 2 of #1045, and fixing it means changing connection.rs, which is outside this change. Worth doing next, since a nonexistent profile is the most common config failure there is.

Behavior

Exit codes only. No message, format or control-flow changes. Success still exits 0.

$ redisctl enterprise database list; echo $?
error: Configuration error: No enterprise profiles configured
3

Checks

cargo fmt --all -- --check, cargo clippy --all-targets -- -D warnings and cargo test all pass locally (full suite green). cargo doc --no-deps --all-features produces only the pre-existing warnings in cli/enterprise.rs and redisctl-mcp, unchanged by this diff.

Tests added: four unit tests in error.rs covering the per-variant mapping, the ApiError status split, the envelope carrying the code, and an assertion that no classified variant falls back to 1. The existing -o json envelope test now also asserts the process exit code matches the envelope, plus one integration test that a config failure exits 3.

Every runtime failure exited 1, so a script could tell that redisctl
failed but not why, and had to match on message text to branch.

Adds an error::exit_code module with the taxonomy from #1045 (usage,
config, auth, not-found, validation, conflict, upstream, rate-limited,
network, timeout, cancelled) and a RedisCtlError::exit_code() that maps
each variant to one, exhaustively, so a new variant has to declare its
code. main exits with it instead of 1, and the two pre-dispatch exits in
the prefix-inference layer use CONFIG and USAGE.

USAGE is 2 to match clap, which already exits 2 when argument parsing
fails. GENERIC stays 1 for Other and OutputError, which are not
classified yet. ApiError is the only variant carrying a server status,
so it recovers 404/409/429 from the message the way suggestions()
already does for 404.

  $ redisctl enterprise database list; echo $?
  3

The -o json/yaml envelope now carries exit_code alongside code, the
field #1048 deferred to this change.

Part of #1045 (GAP-AUTOMATION-02).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant