Print descriptive errors on failures

Cargo.toml

@@ -17,7 +17,7 @@ reqwest = { version = "0.12", features = [
     "__rustls",
     "rustls-tls-native-roots",
 ] }
-rabbitmq_http_client = { version = "0.65.0", features = [
+rabbitmq_http_client = { git = "https://github.com/mkuratczyk/rabbitmq-http-api-rs", branch = "parse-error-reason", features = [
     "blocking",
     "tabled",
 ] }

src/commands.rs

@@ -129,7 +129,7 @@ pub fn list_policies_in_and_applying_to(
     let policies = client.list_policies_in(vhost)?;
     Ok(policies
         .into_iter()
-        .filter(|pol| apply_to.does_apply_to(pol.apply_to.clone()))
+        .filter(|pol| apply_to.does_apply_to(pol.apply_to))
         .collect())
 }
 
@@ -139,10 +139,10 @@ pub fn list_matching_policies_in(
     name: &str,
     typ: PolicyTarget,
 ) -> ClientResult<Vec<responses::Policy>> {
-    let candidates = list_policies_in_and_applying_to(client, vhost, typ.clone())?;
+    let candidates = list_policies_in_and_applying_to(client, vhost, typ)?;
     Ok(candidates
         .into_iter()
-        .filter(|pol| pol.does_match_name(vhost, name, typ.clone()))
+        .filter(|pol| pol.does_match_name(vhost, name, typ))
         .collect())
 }
 
@@ -165,7 +165,7 @@ pub fn list_operator_policies_in_and_applying_to(
     let policies = client.list_operator_policies_in(vhost)?;
     Ok(policies
         .into_iter()
-        .filter(|pol| apply_to.does_apply_to(pol.apply_to.clone()))
+        .filter(|pol| apply_to.does_apply_to(pol.apply_to))
         .collect())
 }
 
@@ -175,10 +175,10 @@ pub fn list_matching_operator_policies_in(
     name: &str,
     typ: PolicyTarget,
 ) -> ClientResult<Vec<responses::Policy>> {
-    let candidates = list_operator_policies_in_and_applying_to(client, vhost, typ.clone())?;
+    let candidates = list_operator_policies_in_and_applying_to(client, vhost, typ)?;
     Ok(candidates
         .into_iter()
-        .filter(|pol| pol.does_match_name(vhost, name, typ.clone()))
+        .filter(|pol| pol.does_match_name(vhost, name, typ))
         .collect())
 }
 
@@ -1578,7 +1578,7 @@ pub fn declare_policy(
         vhost,
         name,
         pattern,
-        apply_to: apply_to.clone(),
+        apply_to,
         priority: priority.parse::<i32>().unwrap(),
         definition: parsed_definition,
     };
@@ -1606,7 +1606,7 @@ pub fn declare_operator_policy(
         vhost,
         name: &name,
         pattern: &pattern,
-        apply_to: apply_to.clone(),
+        apply_to,
         priority: priority.parse::<i32>().unwrap(),
         definition: parsed_definition,
     };
@@ -1832,7 +1832,7 @@ pub fn delete_policy_definition_keys(
     let str_keys: Vec<&str> = keys.iter().map(AsRef::as_ref).collect::<Vec<_>>();
 
     let pol = client.get_policy(vhost, &name)?;
-    let updated_pol = pol.without_keys(str_keys);
+    let updated_pol = pol.without_keys(&str_keys);
 
     let params = PolicyParams::from(&updated_pol);
     client.declare_policy(&params)
@@ -1852,7 +1852,7 @@ pub fn delete_policy_definition_keys_in(
     let str_keys: Vec<&str> = keys.iter().map(AsRef::as_ref).collect::<Vec<_>>();
 
     for pol in pols {
-        let updated_pol = pol.without_keys(str_keys.clone());
+        let updated_pol = pol.without_keys(&str_keys);
 
         let params = PolicyParams::from(&updated_pol);
         client.declare_policy(&params)?
@@ -1875,7 +1875,7 @@ pub fn delete_operator_policy_definition_keys(
     let str_keys: Vec<&str> = keys.iter().map(AsRef::as_ref).collect::<Vec<_>>();
 
     let pol = client.get_operator_policy(vhost, &name)?;
-    let updated_pol = pol.without_keys(str_keys);
+    let updated_pol = pol.without_keys(&str_keys);
 
     let params = PolicyParams::from(&updated_pol);
     client.declare_operator_policy(&params)
@@ -1895,7 +1895,7 @@ pub fn delete_operator_policy_definition_keys_in(
     let str_keys: Vec<&str> = keys.iter().map(AsRef::as_ref).collect::<Vec<_>>();
 
     for pol in pols {
-        let updated_pol = pol.without_keys(str_keys.clone());
+        let updated_pol = pol.without_keys(&str_keys);
 
         let params = PolicyParams::from(&updated_pol);
         client.declare_operator_policy(&params)?

src/errors.rs

@@ -60,18 +60,20 @@ pub enum CommandRunError {
     PrivateKeyFileUnsupported { local_path: String },
     #[error("TLS certificate and private key files do not match")]
     CertificateKeyMismatch { cert_path: String, key_path: String },
-    #[error("API responded with a client error: status code of {status_code}")]
+    #[error("{}", format_client_error(.status_code, .error_details))]
     ClientError {
         status_code: StatusCode,
         url: Option<Url>,
         body: Option<String>,
+        error_details: Option<rabbitmq_http_client::error::ErrorDetails>,
         headers: Option<HeaderMap>,
     },
-    #[error("API responded with a client error: status code of {status_code}")]
+    #[error("{}", format_server_error(.status_code, .error_details))]
     ServerError {
         status_code: StatusCode,
         url: Option<Url>,
         body: Option<String>,
+        error_details: Option<rabbitmq_http_client::error::ErrorDetails>,
         headers: Option<HeaderMap>,
     },
     #[error("Health check failed")]
@@ -115,11 +117,11 @@ impl From<HttpClientError> for CommandRunError {
         use ApiClientError::*;
         match value {
             UnsupportedArgumentValue { property } => Self::UnsupportedArgumentValue { property },
-            ClientErrorResponse { status_code, url, body, headers, .. } => {
-                Self::ClientError { status_code, url, body, headers }
+            ClientErrorResponse { status_code, url, body, error_details, headers, .. } => {
+                Self::ClientError { status_code, url, body, error_details, headers }
             }
-            ServerErrorResponse { status_code, url, body, headers, .. } => {
-                Self::ServerError { status_code, url, body, headers }
+            ServerErrorResponse { status_code, url, body, error_details, headers, .. } => {
+                Self::ServerError { status_code, url, body, error_details, headers }
             }
             HealthCheckFailed { path, details, status_code } => {
                 Self::HealthCheckFailed { health_check_path: path, details, status_code }
@@ -137,3 +139,36 @@ impl From<HttpClientError> for CommandRunError {
         }
     }
 }
+
+fn format_client_error(
+    status_code: &StatusCode,
+    error_details: &Option<rabbitmq_http_client::error::ErrorDetails>,
+) -> String {
+    if let Some(details) = error_details
+        && let Some(reason) = details.reason()
+    {
+        return reason.to_string();
+    }
+    format!(
+        "API responded with a client error: status code of {}",
+        status_code
+    )
+}
+
+fn format_server_error(
+    status_code: &StatusCode,
+    error_details: &Option<rabbitmq_http_client::error::ErrorDetails>,
+) -> String {
+    if let Some(details) = error_details
+        && let Some(reason) = details.reason()
+    {
+        return format!(
+            "API responded with a server error: status code of {}\n\n{}",
+            status_code, reason
+        );
+    }
+    format!(
+        "API responded with a server error: status code of {}",
+        status_code
+    )
+}

src/tables.rs

@@ -314,14 +314,16 @@ pub fn failure_details(error: &HttpClientError) -> Table {
             status_code,
             url,
             body,
+            error_details,
             ..
-        } => generic_failed_request_details(status_code, url, body),
+        } => generic_failed_request_details(status_code, url, body, error_details),
         HttpClientError::ServerErrorResponse {
             status_code,
             url,
             body,
+            error_details,
             ..
-        } => generic_failed_request_details(status_code, url, body),
+        } => generic_failed_request_details(status_code, url, body, error_details),
         HttpClientError::HealthCheckFailed {
             status_code,
             path,
@@ -496,12 +498,13 @@ fn generic_failed_request_details(
     status_code: &StatusCode,
     url: &Option<Url>,
     body: &Option<String>,
+    error_details: &Option<rabbitmq_http_client::error::ErrorDetails>,
 ) -> Table {
     let status_code_s = status_code.to_string();
     let url_s = url.clone().unwrap().to_string();
     let body_s = body.clone().unwrap_or("N/A".to_string());
 
-    let data = vec![
+    let mut data = vec![
         RowOfTwo {
             key: "result",
             value: "request failed",
@@ -520,6 +523,15 @@ fn generic_failed_request_details(
         },
     ];
 
+    if let Some(details) = error_details
+        && let Some(reason) = details.reason()
+    {
+        data.push(RowOfTwo {
+            key: "error",
+            value: reason,
+        });
+    }
+
     build_simple_table(data)
 }
 

tests/policies_tests.rs

@@ -676,3 +676,32 @@ fn test_policies_declare_blanket() -> Result<(), Box<dyn Error>> {
 
     Ok(())
 }
+
+#[test]
+fn test_policy_validation_error() -> Result<(), Box<dyn Error>> {
+    let policy_name = "test_policy_validation_error";
+
+    // Attempt to declare a policy with invalid/unknown settings in the definition
+    // This should fail with a descriptive validation error message
+    run_fails([
+        "declare",
+        "policy",
+        "--name",
+        policy_name,
+        "--pattern",
+        "^qq$",
+        "--apply-to",
+        "queues",
+        "--priority",
+        "1",
+        "--definition",
+        r#"{"foo": "bar", "invalid-setting": 12345}"#,
+    ])
+    .stderr(output_includes("Validation failed"))
+    .stderr(output_includes("not recognised").or(output_includes("not recognized")));
+
+    // Verify the policy was not created
+    run_succeeds(["list", "policies"]).stdout(output_includes(policy_name).not());
+
+    Ok(())
+}