diff --git a/Cargo.lock b/Cargo.lock index e3ba859..de27033 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1512,9 +1512,8 @@ checksum = "69cdb34c158ceb288df11e18b4bd39de994f6657d83847bdffdbd7f346754b0f" [[package]] name = "rabbitmq_http_client" -version = "0.65.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "17f03705ab15935d1ca97605ecdfbb4fed6938df94e64dafcb08e741265308d1" +version = "0.66.0" +source = "git+https://github.com/mkuratczyk/rabbitmq-http-api-rs?branch=parse-error-reason#3bff32654442012135f3406f1e0edb60a74fcab0" dependencies = [ "backtrace", "log", diff --git a/Cargo.toml b/Cargo.toml index 2af76b0..e565008 100644 --- a/Cargo.toml +++ b/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", ] } diff --git a/src/commands.rs b/src/commands.rs index 2608688..877d5f5 100644 --- a/src/commands.rs +++ b/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> { - 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> { - 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::().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::().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::>(); 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(¶ms) @@ -1852,7 +1852,7 @@ pub fn delete_policy_definition_keys_in( let str_keys: Vec<&str> = keys.iter().map(AsRef::as_ref).collect::>(); 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(¶ms)? @@ -1875,7 +1875,7 @@ pub fn delete_operator_policy_definition_keys( let str_keys: Vec<&str> = keys.iter().map(AsRef::as_ref).collect::>(); 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(¶ms) @@ -1895,7 +1895,7 @@ pub fn delete_operator_policy_definition_keys_in( let str_keys: Vec<&str> = keys.iter().map(AsRef::as_ref).collect::>(); 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(¶ms)? diff --git a/src/errors.rs b/src/errors.rs index 70b4ec1..265cb6a 100644 --- a/src/errors.rs +++ b/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, body: Option, + error_details: Option, headers: Option, }, - #[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, body: Option, + error_details: Option, headers: Option, }, #[error("Health check failed")] @@ -115,11 +117,11 @@ impl From 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 for CommandRunError { } } } + +fn format_client_error( + status_code: &StatusCode, + error_details: &Option, +) -> 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, +) -> 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 + ) +} diff --git a/src/tables.rs b/src/tables.rs index 7baa57e..e6013d0 100644 --- a/src/tables.rs +++ b/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, body: &Option, + error_details: &Option, ) -> 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) } diff --git a/tests/policies_tests.rs b/tests/policies_tests.rs index 6fbfec1..75cbc24 100644 --- a/tests/policies_tests.rs +++ b/tests/policies_tests.rs @@ -676,3 +676,32 @@ fn test_policies_declare_blanket() -> Result<(), Box> { Ok(()) } + +#[test] +fn test_policy_validation_error() -> Result<(), Box> { + 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(()) +}