feat(cli): classify failures with distinct exit codes and a JSON error envelope#62
Open
dark-sorceror wants to merge 7 commits into
Open
feat(cli): classify failures with distinct exit codes and a JSON error envelope#62dark-sorceror wants to merge 7 commits into
dark-sorceror wants to merge 7 commits into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #55
Every failure mode used to look identical from the outside: exit code
1and anerror: <prose>line on stderr, even under--json. A script drivingtraceroothad no way to tell "retry later" (network) from "re-authenticate" (auth) from "give up" (not-found) from "I built the command wrong" (usage) without parsing English error strings.CliErroralready carried anexitCodefield that every one of its ~43 call sites left at the default1. This PR wires it up: each failure is now classified into one of five exit codes, and under--jsonevery failure emits a single structured error line instead of prose.How it works
code01internal2usage3auth4not_found5networkHuman-mode stderr is unchanged in shape (
error: <message>); only the exit code differs. Under--json, a failure instead writes exactly one line to stderr and leaves stdout empty, so ajqpipeline is never corrupted by an error:{"error":{"code":"not_found","message":"Trace not found"}}HTTP statuses are mapped to classes centrally in
src/api/client.ts(401/403 → auth, 404 → not_found, everything else non-2xx → internal); network errors and timeouts map tonetwork; a malformed--hostmaps tousage.Commander's own failures (unknown option, unknown command, missing option argument) used to bypass
CliErrorentirely and callprocess.exitdirectly.program.exitOverride()now routes them through the same central handler, sotraceroot traces list --bogusflagandtraceroot boguscmdexit2with the standarderror:line (or JSON envelope under--json), reported exactly once — commander's own printing is suppressed to avoid a double message.--helpand--versionare unaffected: they still print to stdout and exit0.process.exitwas also being called immediately after the stderr write, which could truncate the message on a piped/slow stderr (notably on Windows). The process now setsprocess.exitCodeand writes an empty chunk to stderr, exiting only once that write's callback confirms the pipe has drained — closing the gap without reintroducing the truncation risk, and without leaving a pending socket (e.g. an undici connect-timeout) holding the event loop open for seconds after the error was already reported.What's included
Exit-code machinery
src/output.ts—ExitCodeenum (usage/auth/notFound/network/internal),exitCodeToString,CliError'sexitCodeparam, andreportErrorgrowing a{ json }option that switches between the prose line and the JSON envelopeHTTP/network classification
src/api/client.ts—exitCodeForStatusmaps 401/403 → auth, 404 → not_found, else → internal; network errors andAbortSignal.timeout→ network; malformed--host→ usageCLI entry point
src/cli.ts—process.exitCodeinstead of an immediateprocess.exit;exitAfterStderrDrainhelper that exits only after stderr has flushed;program.exitOverride()plus aCommanderErrorbranch inrun()that routes commander-native usage errors throughreportError(exit2, single message, JSON envelope support), while--help/--versionkeep exiting0through their normal pathCall-site classification (each
CliErrornow passes an explicitExitCode)src/agents/index.ts,src/agents/select.ts— unknown/missing--agent→ usagesrc/skills/registry.ts,src/skills/select.ts— unknown/missing skill → usagesrc/commands/shared.ts— missing API key / host → authsrc/commands/login.ts— missing API key in non-interactive login → authsrc/commands/traces/list.ts,src/commands/detectors/list.ts,src/commands/findings/list.ts,src/commands/findings/get.ts— bad--limit/--from/--to/stray arguments → usagesrc/commands/instrument.ts— missing--outputin non-interactive mode → usagesrc/context.ts— invalid--timeout→ usagesrc/util/index.ts— invalid--sinceduration → usageDocs
README.md— new "Exit codes" section documenting the table and the--jsonenvelope shapesrc/cli.ts— the same table appended totraceroot --helpoutputTests
tests/output.test.ts—reportError's JSON-envelope branch,exitCodeToStringmapping, unchanged human-mode behaviortests/api/client.test.ts— HTTP status → exit-code class mapping, network/timeout → network, bad host → usagetests/output.contract.test.ts— spawns the real binary: per-class exit codes, envelope shape and stdout purity under--json, commander-native cases (unknown option/command, missing option argument,--help/--versionstill exit 0), all run against an isolated temp cwd (mkdtempSync) so a developer's stray repo.envcan't leak credentials into the test and flip a missing-credentials case into a network onePreserved behavior (deliberate, not overlooked)
requestOptional's 404 →null(used byfindFindingByTrace) is unaffected — that path never throws, so it never goes through the exit-code contracttracerootstill prints help to stderr and exits1internal(1) — not a usage/auth/not-found/network failure by this taxonomyTest plan
vitest— 615/615 tests pass, including strengthened contract tests that spawn the real binary and assert per-class exit codes, JSON envelope shape, commander-native cases, and the isolated temp cwdtraceroot traces get <bad-id> --jsonagainstapp.traceroot.ai→ exit4,{"error":{"code":"not_found",...}}traceroot status --api-key <invalid> --json→ exit3,code: "auth"traceroot statuswith no credentials configured → exit3traceroot traces list --host http://dead-host --json→ exit5,code: "network"traceroot traces list --host <connect-timeout-host>→ exits5in ~880ms (previously lingered ~10s before the process actually ended)traceroot traces list --limit banana→ exit2; same fortraceroot --bogusflagandtraceroot boguscmd, including the single-line usage envelope with empty stdout under--jsontraceroot traces get <valid-id>,--help,--version→ exit0;--helpoutput includes the exit-code tableSummary by cubic
Adds distinct exit codes for CLI failure classes and a single-line JSON error envelope under
--json, so scripts can branch without parsing prose. Also routescommanderusage errors through the same handler and waits for stderr to flush before exiting.New Features
--json, failures write exactly one line to stderr:{"error":{"code","message"}}; stdout stays empty.--host→ usage.commandernative errors (unknown command/option, missing option arg) now exit 2 and respect--json;--help/--versionstill exit 0. Help includes an exit-code table.Migration
--json, read errors from stderr (one JSON line). Keep stdout for data pipelines.Written for commit f09fead. Summary will update on new commits.