4xx errors leak the raw /api/v1 URL and every failure exits 1, so scripts cannot branch on failure kind #123

Closed
opened 2026-06-11 00:47:52 +00:00 by stephen · 2 comments
Owner

Observation

fj's failure reporting is inconsistent in two ways that hurt scripted use: 4xx errors leak the internal /api/v1/... URL into the user-facing message, and every command failure exits with the same code 1, so a script cannot tell "not found" from "auth expired" from "network down" without parsing English.

The error path forks on exactly one status. ensure_success (src/client/mod.rs:558-579) gives 401 a friendly, actionable hint and leaves every other non-2xx as a raw dump:

if status == StatusCode::UNAUTHORIZED {
    Err(anyhow::Error::new(err)
        .context("authentication failed (HTTP 401). Token may be invalid or revoked"))
} else {
    Err(anyhow::Error::new(err))   // ApiError Display = "HTTP {status} from {url}: {message}"
}

ApiError's Display (src/client/error.rs:6-13) is HTTP {status} from {url}: {message}, where url is the raw REST endpoint. So a missing repo surfaces the API plumbing:

$ fj repo view rasterstate/zzz-nonexistent-repo-xyz ; echo "EXIT=$?"
error: HTTP 404 Not Found from https://rasterhub.com/api/v1/repos/rasterstate/zzz-nonexistent-repo-xyz: The target couldn't be found.
EXIT=1

A user typed rasterstate/zzz...; the error answers with https://rasterhub.com/api/v1/repos/.... The /api/v1/ path is an implementation detail the CLI otherwise hides, and it reads as "fj broke" rather than "that repo does not exist."

The exit code is uniform. main.rs maps any Err from cli::run to ExitCode::from(1) (src/main.rs:54-62); clap usage errors use clap's own code (src/main.rs:90). There is no distinct code for not-found (404), forbidden (403), auth (401), rate-limit (429), or transport failure. They are all 1.

Why it matters

Two different audiences, same root cause:

  • Humans get a leaky, alarming message for the most common mistake (a typo'd repo/issue/PR). gh says could not resolve to a Repository with the name 'x/y'; fj says HTTP 404 Not Found from https://.../api/v1/.... The 401 branch proves the maintainers already value a friendly hint over a raw dump; 404/403/429 just never got the same treatment, so the quality is inconsistent across status codes.

  • Scripts/agents cannot branch on failure kind. A common agent pattern is "view the PR; if it doesn't exist yet, create it." With a single exit code that becomes "run the command, capture stderr, and string-match 404/couldn't be found," which is brittle across locales and server message wording. Distinct exit codes (e.g. not-found vs auth vs transient) let a script write if ! fj pr view 7 >/dev/null 2>&1; then ... and, better, distinguish "PR 7 absent" from "token expired, stop and re-auth" from "429, back off and retry." Today all three look identical.

These compound: the leaky message is also the only signal a script has, so the inconsistency in messages and the absence of exit codes are the same gap seen from two sides.

Possible directions (sketches)

  • (sketch) Give 403/404/429 the same .context(...) treatment 401 already gets in ensure_success (src/client/mod.rs:573): a short human sentence ("not found: /", "forbidden: token lacks scope / no access", "rate limited, retry after N s"), keeping the raw ApiError (with the URL) as the caused by: source line that main.rs:56-60 already prints, so --debug-level detail survives without leading with /api/v1/.
  • (sketch) Map error classes to stable exit codes in run_cli (src/main.rs:54-62): downcast the anyhow::Error to ApiError and return, say, 3 for not-found, 4 for auth/forbidden, 5 for rate-limit/transient, keeping 1 as the generic fallback. Document them once so scripts can rely on them (this is the gh-style "exit non-zero with a meaningful code" contract).
  • (sketch) When the failing request is a user-addressable resource, prefer echoing what the user typed (owner/name, #number) over the resolved URL in the headline message.
## Observation `fj`'s failure reporting is inconsistent in two ways that hurt scripted use: 4xx errors leak the internal `/api/v1/...` URL into the user-facing message, and every command failure exits with the same code `1`, so a script cannot tell "not found" from "auth expired" from "network down" without parsing English. The error path forks on exactly one status. `ensure_success` (`src/client/mod.rs:558-579`) gives `401` a friendly, actionable hint and leaves every other non-2xx as a raw dump: ```rust if status == StatusCode::UNAUTHORIZED { Err(anyhow::Error::new(err) .context("authentication failed (HTTP 401). Token may be invalid or revoked")) } else { Err(anyhow::Error::new(err)) // ApiError Display = "HTTP {status} from {url}: {message}" } ``` `ApiError`'s `Display` (`src/client/error.rs:6-13`) is `HTTP {status} from {url}: {message}`, where `url` is the raw REST endpoint. So a missing repo surfaces the API plumbing: ``` $ fj repo view rasterstate/zzz-nonexistent-repo-xyz ; echo "EXIT=$?" error: HTTP 404 Not Found from https://rasterhub.com/api/v1/repos/rasterstate/zzz-nonexistent-repo-xyz: The target couldn't be found. EXIT=1 ``` A user typed `rasterstate/zzz...`; the error answers with `https://rasterhub.com/api/v1/repos/...`. The `/api/v1/` path is an implementation detail the CLI otherwise hides, and it reads as "fj broke" rather than "that repo does not exist." The exit code is uniform. `main.rs` maps *any* `Err` from `cli::run` to `ExitCode::from(1)` (`src/main.rs:54-62`); clap usage errors use clap's own code (`src/main.rs:90`). There is no distinct code for not-found (404), forbidden (403), auth (401), rate-limit (429), or transport failure. They are all `1`. ## Why it matters Two different audiences, same root cause: - **Humans** get a leaky, alarming message for the most common mistake (a typo'd repo/issue/PR). `gh` says `could not resolve to a Repository with the name 'x/y'`; `fj` says `HTTP 404 Not Found from https://.../api/v1/...`. The 401 branch proves the maintainers already value a friendly hint over a raw dump; 404/403/429 just never got the same treatment, so the quality is inconsistent across status codes. - **Scripts/agents** cannot branch on failure kind. A common agent pattern is "view the PR; if it doesn't exist yet, create it." With a single exit code that becomes "run the command, capture stderr, and string-match `404`/`couldn't be found`," which is brittle across locales and server message wording. Distinct exit codes (e.g. not-found vs auth vs transient) let a script write `if ! fj pr view 7 >/dev/null 2>&1; then ...` and, better, distinguish "PR 7 absent" from "token expired, stop and re-auth" from "429, back off and retry." Today all three look identical. These compound: the leaky message is also the only signal a script has, so the inconsistency in messages and the absence of exit codes are the same gap seen from two sides. ## Possible directions (sketches) - *(sketch)* Give 403/404/429 the same `.context(...)` treatment 401 already gets in `ensure_success` (`src/client/mod.rs:573`): a short human sentence ("not found: <owner>/<name>", "forbidden: token lacks scope / no access", "rate limited, retry after N s"), keeping the raw `ApiError` (with the URL) as the `caused by:` source line that `main.rs:56-60` already prints, so `--debug`-level detail survives without leading with `/api/v1/`. - *(sketch)* Map error classes to stable exit codes in `run_cli` (`src/main.rs:54-62`): downcast the `anyhow::Error` to `ApiError` and return, say, `3` for not-found, `4` for auth/forbidden, `5` for rate-limit/transient, keeping `1` as the generic fallback. Document them once so scripts can rely on them (this is the gh-style "exit non-zero with a meaningful code" contract). - *(sketch)* When the failing request is a user-addressable resource, prefer echoing what the *user* typed (`owner/name`, `#number`) over the resolved URL in the headline message.
Author
Owner

Converted to backlog item rasterstate/fj#134 (p1, size M).

Converted to backlog item rasterstate/fj#134 (p1, size M).
Author
Owner

Resolved. The core fix (hide the /api/v1 URL behind the caused-by source for 4xx, plus stable exit codes 3/4/5) landed in #142. #163 extends it to the two remaining client-error classes that still led with the raw URL: 409 (conflict) and 422 (validation), surfacing the server's own message in the headline. Closing as fixed by #142 and #163.

Resolved. The core fix (hide the /api/v1 URL behind the caused-by source for 4xx, plus stable exit codes 3/4/5) landed in #142. #163 extends it to the two remaining client-error classes that still led with the raw URL: 409 (conflict) and 422 (validation), surfacing the server's own message in the headline. Closing as fixed by #142 and #163.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
rasterstate/fj#123
No description provided.