diff --git a/src/api/client.rs b/src/api/client.rs index bc05303..ac4d4c8 100644 --- a/src/api/client.rs +++ b/src/api/client.rs @@ -1,10 +1,52 @@ +use std::fmt::Debug; use std::time::Duration; use anyhow::{Context, Result}; use reqwest::header::{HeaderMap, AUTHORIZATION}; -use serde::Deserialize; - -use progenitor::progenitor_client::ResponseValue; +use serde::{Deserialize, Serialize}; + +use progenitor::progenitor_client::{Error as ProgenitorError, ResponseValue}; + +/// Convert a progenitor client error into a concise anyhow error. +/// +/// progenitor's own `Display` for `ErrorResponse` dumps headers and the typed +/// body via `Debug`, which is the verbose output the CLI is trying to avoid. +/// This collapses HTTP error responses to ` : ` so +/// the chain stays actionable (e.g. "401 Unauthorized") without leaking +/// internal struct shape. +#[allow( + clippy::needless_pass_by_value, + reason = "shape matches map_err's FnOnce(E) -> F" +)] +fn api_error(e: ProgenitorError) -> anyhow::Error { + if let ProgenitorError::ErrorResponse(rv) = &e { + let status = rv.status(); + let head = format!( + "API error: {} {}", + status.as_u16(), + status.canonical_reason().unwrap_or("HTTP error"), + ); + let msg = serde_json::to_value(rv.as_ref()) + .ok() + .as_ref() + .and_then(|v| v.get("message")) + .and_then(serde_json::Value::as_str) + .filter(|m| !m.is_empty()) + .map(str::to_owned); + return msg.map_or_else( + || anyhow::anyhow!("{head}"), + |m| anyhow::anyhow!("{head}: {m}"), + ); + } + if let Some(status) = e.status() { + return anyhow::anyhow!( + "API error: {} {}", + status.as_u16(), + status.canonical_reason().unwrap_or("HTTP error"), + ); + } + anyhow::anyhow!("API error: {e}") +} use super::generated::types::CreateRuleBody; use super::types::{ @@ -52,7 +94,7 @@ impl ApiClient { .get_public_user() .await .map(ResponseValue::into_inner) - .map_err(|e| anyhow::anyhow!("API error: {e}")) + .map_err(api_error) } pub async fn list_bugs( @@ -75,7 +117,7 @@ impl ApiClient { ) .await .map(ResponseValue::into_inner) - .map_err(|e| anyhow::anyhow!("API error: {e}")) + .map_err(api_error) } pub async fn get_bug(&self, bug_id: &BugId) -> Result { @@ -83,7 +125,7 @@ impl ApiClient { .get_public_bug(bug_id) .await .map(ResponseValue::into_inner) - .map_err(|e| anyhow::anyhow!("API error: {e}")) + .map_err(api_error) } pub async fn update_bug_close( @@ -103,7 +145,7 @@ impl ApiClient { .create_public_bug_review(bug_id, &body) .await .map(ResponseValue::into_inner) - .map_err(|e| anyhow::anyhow!("API error: {e}")) + .map_err(api_error) } pub async fn list_scans( @@ -118,7 +160,7 @@ impl ApiClient { .list_public_scans(NonZeroU64::new(limit.into()), Some(offset.into()), repo_id) .await .map(ResponseValue::into_inner) - .map_err(|e| anyhow::anyhow!("API error: {e}")) + .map_err(api_error) } pub async fn list_repos(&self, limit: u32, offset: u32) -> Result { @@ -128,7 +170,7 @@ impl ApiClient { .list_public_repos(NonZeroU64::new(limit.into()), Some(offset.into())) .await .map(ResponseValue::into_inner) - .map_err(|e| anyhow::anyhow!("API error: {e}")) + .map_err(api_error) } pub async fn create_rule( @@ -144,7 +186,7 @@ impl ApiClient { .create_rule(&body) .await .map(ResponseValue::into_inner) - .map_err(|e| anyhow::anyhow!("API error: {e}")) + .map_err(api_error) } pub async fn list_rules(&self, repo_id: &RepoId) -> Result { @@ -152,7 +194,7 @@ impl ApiClient { .list_rules(repo_id) .await .map(ResponseValue::into_inner) - .map_err(|e| anyhow::anyhow!("API error: {e}")) + .map_err(api_error) } pub async fn get_rule(&self, rule_id: &RuleId) -> Result { @@ -160,7 +202,7 @@ impl ApiClient { .get_rule(rule_id) .await .map(ResponseValue::into_inner) - .map_err(|e| anyhow::anyhow!("API error: {e}")) + .map_err(api_error) } pub async fn get_rule_request( @@ -171,7 +213,7 @@ impl ApiClient { .get_rule_request(rcr_id) .await .map(ResponseValue::into_inner) - .map_err(|e| anyhow::anyhow!("API error: {e}")) + .map_err(api_error) } pub async fn list_rule_requests(&self, repo_id: &RepoId) -> Result { @@ -179,7 +221,7 @@ impl ApiClient { .list_rule_requests(repo_id) .await .map(ResponseValue::into_inner) - .map_err(|e| anyhow::anyhow!("API error: {e}")) + .map_err(api_error) } } diff --git a/src/main.rs b/src/main.rs index b1e4bf1..2f46502 100644 --- a/src/main.rs +++ b/src/main.rs @@ -10,7 +10,7 @@ async fn main() -> ExitCode { match detail_cli::Cli::parse().run().await { Ok(()) => ExitCode::SUCCESS, Err(err) => { - let _ = Term::stderr().write_line(&format!("Error: {err}")); + let _ = Term::stderr().write_line(&format!("Error: {err:#}")); ExitCode::FAILURE } } diff --git a/tests/integration.rs b/tests/integration.rs index 5e8fffc..8f331c1 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -398,6 +398,26 @@ fn bugs_list_rejects_invalid_repo_format() { assert!(!out.success, "bugs list should reject a/b/c format"); } +#[test] +fn error_includes_status_code_for_auth_failures() { + // The token is well-formed (passes the local `dtl_` prefix check) so the + // CLI actually sends it to the API; we need a 401 from a real server to + // exercise the error-chain mapping. Gate on DETAIL_API_KEY like other + // live-API tests so this is skipped offline. + let _ = require_api_key!(); + + let env = Env::new("error_status_code"); + env.write_config(r#"api_token = "dtl_live_invalid_token""#); + + let out = env.run(&["repos", "list"]); + assert!(!out.success); + assert!( + out.stderr.contains("401") && out.stderr.contains("Unauthorized"), + "Error should include both the 401 status code and \"Unauthorized\":\nstderr: {}", + out.stderr, + ); +} + #[test] fn config_api_url_is_used() { let key = require_api_key!();