4xx errors leak the raw /api/v1 URL and every failure exits 1, so scripts cannot branch on failure kind #123
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
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 code1, 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) gives401a friendly, actionable hint and leaves every other non-2xx as a raw dump:ApiError'sDisplay(src/client/error.rs:6-13) isHTTP {status} from {url}: {message}, whereurlis the raw REST endpoint. So a missing repo surfaces the API plumbing:A user typed
rasterstate/zzz...; the error answers withhttps://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.rsmaps anyErrfromcli::runtoExitCode::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 all1.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).
ghsayscould not resolve to a Repository with the name 'x/y';fjsaysHTTP 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 writeif ! 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)
.context(...)treatment 401 already gets inensure_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 rawApiError(with the URL) as thecaused by:source line thatmain.rs:56-60already prints, so--debug-level detail survives without leading with/api/v1/.run_cli(src/main.rs:54-62): downcast theanyhow::ErrortoApiErrorand return, say,3for not-found,4for auth/forbidden,5for rate-limit/transient, keeping1as 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).owner/name,#number) over the resolved URL in the headline message.Converted to backlog item rasterstate/fj#134 (p1, size M).
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.