diff --git a/cli-engine/src/error.rs b/cli-engine/src/error.rs index 49bb13f..4ce32b0 100644 --- a/cli-engine/src/error.rs +++ b/cli-engine/src/error.rs @@ -2,6 +2,8 @@ use std::borrow::Cow; use thiserror::Error; +use crate::NextAction; + /// Crate-wide result type. pub type Result = std::result::Result; @@ -23,6 +25,10 @@ pub trait DetailedError: std::error::Error { fn error_fix(&self) -> Option> { None } + /// Structured follow-up actions for the envelope's `next_actions` (defaults to empty). + fn error_next_actions(&self) -> Vec { + Vec::new() + } } /// Framework error type. @@ -76,6 +82,11 @@ pub enum CliCoreError { system: String, /// Optional request id. request_id: String, + /// Structured follow-up actions captured from the source's + /// [`DetailedError::error_next_actions`] at wrap time — the source is + /// erased to `Box` immediately below, so this can't be + /// recovered later by downcasting. + next_actions: Vec, /// Source error. #[source] source: Box, @@ -188,12 +199,14 @@ impl CliCoreError { .error_request_id() .map_or_else(String::new, Cow::into_owned); let fix = source.error_fix().map_or_else(String::new, Cow::into_owned); + let next_actions = source.error_next_actions(); Self::with_fix( fix, Self::Detailed { code, system, request_id, + next_actions, source: Box::new(source), }, ) @@ -357,6 +370,45 @@ mod tests { assert_eq!(err.system(), Some("auth")); } + #[test] + fn with_detailed_error_captures_next_actions_before_erasure() { + #[derive(Debug, thiserror::Error)] + #[error("'/businesses' matches 2 operations")] + struct Ambiguous; + + impl DetailedError for Ambiguous { + fn error_code(&self) -> Cow<'static, str> { + Cow::Borrowed("AMBIGUOUS_MATCH") + } + + fn error_system(&self) -> Option> { + None + } + + fn error_request_id(&self) -> Option> { + None + } + + fn error_next_actions(&self) -> Vec { + vec![NextAction::new( + "api operation get /businesses --method GET", + "Get all businesses", + )] + } + } + + let err = CliCoreError::with_detailed_error(Ambiguous); + assert!(matches!(err, CliCoreError::Detailed { .. })); + let CliCoreError::Detailed { next_actions, .. } = &err else { + unreachable!("just asserted this is Detailed"); + }; + assert_eq!(next_actions.len(), 1); + assert_eq!( + next_actions[0].command, + "api operation get /businesses --method GET" + ); + } + #[test] fn empty_with_fix_does_not_wrap() { let inner = CliCoreError::message_for_system("auth", "not logged in"); diff --git a/cli-engine/src/output/envelope.rs b/cli-engine/src/output/envelope.rs index 2b3442a..3bd0d7d 100644 --- a/cli-engine/src/output/envelope.rs +++ b/cli-engine/src/output/envelope.rs @@ -383,7 +383,7 @@ impl Metadata { #[must_use] pub fn build_error_envelope(err: &(dyn std::error::Error + 'static), system: &str) -> Envelope { let fix = find_error_fix(err); - if let Some((code, mut sys, request_id)) = find_detailed_error(err) { + if let Some((code, mut sys, request_id, next_actions)) = find_detailed_error(err) { if sys.is_empty() { sys = system.to_owned(); } @@ -405,7 +405,7 @@ pub fn build_error_envelope(err: &(dyn std::error::Error + 'static), system: &st request_id, }), warnings: Vec::new(), - next_actions: Vec::new(), + next_actions, fix, serialization_error: None, }; @@ -429,7 +429,7 @@ fn find_error_fix(err: &(dyn std::error::Error + 'static)) -> Option { fn find_detailed_error( err: &(dyn std::error::Error + 'static), -) -> Option<(String, String, String)> { +) -> Option<(String, String, String, Vec)> { let mut current = Some(err); let mut fallback_system = None::; while let Some(error) = current { @@ -440,7 +440,7 @@ fn find_detailed_error( .. }) = error.downcast_ref::() { - return Some((code.clone(), system.clone(), request_id.clone())); + return Some((code.clone(), system.clone(), request_id.clone(), Vec::new())); } if let Some(crate::CliCoreError::System { system, .. }) = error.downcast_ref::() @@ -453,6 +453,7 @@ fn find_detailed_error( code, system, request_id, + next_actions, .. }) = error.downcast_ref::() { @@ -463,6 +464,7 @@ fn find_detailed_error( .filter(|_| system.is_empty()) .unwrap_or_else(|| system.clone()), request_id.clone(), + next_actions.clone(), )); } let detailed_transport = error.downcast_ref::().or_else(|| { @@ -498,11 +500,12 @@ fn find_detailed_error( detailed .error_request_id() .map_or_else(String::new, std::borrow::Cow::into_owned), + detailed.error_next_actions(), )); } current = error.source(); } - fallback_system.map(|system| ("ERROR".to_owned(), system, String::new())) + fallback_system.map(|system| ("ERROR".to_owned(), system, String::new(), Vec::new())) } /// Builds an error envelope from a [`DetailedError`]. @@ -533,7 +536,7 @@ pub fn build_detailed_error_envelope(err: &dyn DetailedError, system: &str) -> E request_id, }), warnings: Vec::new(), - next_actions: Vec::new(), + next_actions: err.error_next_actions(), fix: err .error_fix() .map(std::borrow::Cow::into_owned) @@ -697,6 +700,95 @@ mod tests { ); } + #[test] + fn build_error_envelope_surfaces_next_actions_from_a_detailed_error() { + use crate::error::DetailedError; + + #[derive(Debug, thiserror::Error)] + #[error("'/businesses' matches 2 operations")] + struct Ambiguous; + + impl DetailedError for Ambiguous { + fn error_code(&self) -> std::borrow::Cow<'static, str> { + std::borrow::Cow::Borrowed("AMBIGUOUS_MATCH") + } + + fn error_system(&self) -> Option> { + None + } + + fn error_request_id(&self) -> Option> { + None + } + + fn error_next_actions(&self) -> Vec { + vec![ + NextAction::new( + "api operation get /businesses --method GET", + "Get all businesses", + ), + NextAction::new( + "api operation get /businesses --method POST", + "Create a new business", + ), + ] + } + } + + // Mirrors the real path: a handler converts its `DetailedError` into a + // `CliCoreError` (type-erasing it), then the middleware renders that + // through `build_error_envelope` — never `build_detailed_error_envelope`. + let err = crate::CliCoreError::with_detailed_error(Ambiguous); + let envelope = build_error_envelope(&err, "api"); + + assert_eq!( + envelope.error.as_ref().map(|e| e.code.as_str()), + Some("AMBIGUOUS_MATCH") + ); + assert_eq!(envelope.next_actions.len(), 2); + assert_eq!( + envelope.next_actions[0].command, + "api operation get /businesses --method GET" + ); + assert_eq!( + envelope.next_actions[1].command, + "api operation get /businesses --method POST" + ); + } + + #[test] + fn build_detailed_error_envelope_surfaces_next_actions() { + use crate::error::DetailedError; + + #[derive(Debug, thiserror::Error)] + #[error("not found")] + struct NotFound; + + impl DetailedError for NotFound { + fn error_code(&self) -> std::borrow::Cow<'static, str> { + std::borrow::Cow::Borrowed("NOT_FOUND") + } + + fn error_system(&self) -> Option> { + None + } + + fn error_request_id(&self) -> Option> { + None + } + + fn error_next_actions(&self) -> Vec { + vec![NextAction::new("app list", "List applications")] + } + } + + let err = NotFound; + let envelope = build_detailed_error_envelope(&err, "applications"); + + assert_eq!(envelope.next_actions.len(), 1); + assert_eq!(envelope.next_actions[0].command, "app list"); + } + #[test] fn success_envelope_ignores_with_fix() { let envelope = Envelope::success(json!({"ok": true}), "api").with_fix("should not stick");