fix(errors): surface HTTP status in CLI error chain#290
Conversation
`{err}` formatting in main.rs dropped the entire anyhow chain, so auth
failures surfaced as a bare "Failed to fetch repositories" with no
indication that the underlying cause was a 401 — leaving users to guess
whether to re-auth, check permissions, or fix the repo name.
Two changes together fix this without becoming verbose:
- src/api/client.rs: add `api_error()` mapper that collapses
`progenitor::Error::ErrorResponse` to `<status> <reason>: <message>`
using the API's `ApiError.message` field. This avoids progenitor's
default Display, which dumps headers and the typed body via Debug.
- src/main.rs: switch to `{err:#}` so the (now-concise) chain is shown.
User now sees: "Error: Failed to fetch repositories: API error: 401
Unauthorized: Invalid API key format" — actionable, no struct/header
dumps. Adds the integration test from the bug report.
Fixes #271
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
sachiniyer
temporarily deployed
to
integration-tests
GitHub Actions
Inactive
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 3 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="tests/integration.rs">
<violation number="1" location="tests/integration.rs:408">
P3: Tighten this assertion so it actually requires the HTTP status code; the current `||` lets the test pass even when `401` is missing.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5170137d72
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let env = Env::new("error_status_code"); | ||
| env.write_config(r#"api_token = "dtl_live_invalid_token""#); | ||
|
|
||
| let out = env.run(&["repos", "list"]); |
There was a problem hiding this comment.
This new test is the only live API test that does not call require_api_key!() before executing a command that reaches api.detail.dev. In the documented/default case where DETAIL_API_KEY is absent, running cargo test --test integration in an offline or network-restricted environment now fails with a connection error instead of being silently skipped, so the assertion for 401/Unauthorized never holds. Please gate this test the same way as the other live API tests or use a local mock for the 401 response.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fair — the invalid token here is well-formed enough to be sent to the API (it passes the local dtl_ prefix check), so the test really does make a network call and would fail with a connection error in an offline environment. Gated on DETAIL_API_KEY like the other live-API tests in d5def86. Skipped a local mock for now since this file has no mocking infra; happy to add one if it's expected.
- Require both "401" and "Unauthorized" in stderr; the previous OR let the test pass even if the status code itself was missing, which is exactly the regression the test should catch. - Gate on DETAIL_API_KEY like other live-API tests. The token used here is well-formed enough to be sent to the API, so the test needs a real 401 response — without the key it would fail with a connection error in offline environments instead of being skipped. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
sachiniyer
temporarily deployed
to
integration-tests
GitHub Actions
Inactive
|
Pushed d5def86 addressing both inline reviews:
|
1 participant
Summary
main.rswas formatting CLI errors with{err}(Display), which only prints the top-level context and drops the rest of the anyhow chain. Auth failures showed as a bare "Failed to fetch repositories" with no hint of the underlying 401, so users couldn't tell whether to re-auth, check permissions, or fix the repo name.src/api/client.rs: addapi_error()mapper that collapsesprogenitor::Error::ErrorResponseto<status> <reason>: <message>using the API'sApiError.message— sidesteps progenitor's default Display, which dumps headers and the typed body via Debug.src/main.rs: switch to{err:#}so the (now-concise) anyhow chain is rendered end-to-end.error_includes_status_code_for_auth_failures).User-facing change:
repos listwith an invalid token now printsinstead of just
Error: Failed to fetch repositories.Fixes #271
Test plan
cargo buildcargo test --lib(290 passed)cargo test --test integration error_includes_status_code_for_auth_failures(passes)XDG_CONFIG_HOME=… detail repos listwith an invalid token; output is the concise chain shown above — no header/struct dumps.cargo fmt --checkcargo clippy --all-targetsintroduces no new lints in changed files (existing lints in the integration harness andsrc/lib.rsare pre-existing onmain).🤖 Generated with Claude Code