diff --git a/docs/contributing/error-handling.md b/docs/contributing/error-handling.md index d762b17c..41eba38f 100644 --- a/docs/contributing/error-handling.md +++ b/docs/contributing/error-handling.md @@ -574,6 +574,169 @@ match FileLock::acquire(&path, timeout) { - ✅ Platform-aware guidance included - ✅ Users control verbosity level +## 📐 Error Structure Template + +When defining new error types, use this template to ensure consistency: + +```rust +use thiserror::Error; +use std::path::PathBuf; + +#[derive(Debug, Error)] +pub enum YourCommandError { + // ===== File/Configuration Errors ===== + + /// Brief description of when this error occurs + /// + /// More detailed explanation if needed. + /// Use `.help()` for detailed troubleshooting steps. + #[error("Clear error message with context: {path} +Tip: Brief actionable hint - command example if applicable")] + ConfigFileNotFound { + /// Path to the missing file + path: PathBuf, + }, + + /// Brief description + #[error("Error message with multiple context values: '{path}' as {format}: {source} +Tip: Validate format with: command --check {path}")] + ParsingFailed { + /// Path to the file + path: PathBuf, + /// Expected format + format: String, + /// Original parsing error + #[source] + source: SomeError, + }, + + // ===== Operation Errors ===== + + /// Brief description + /// + /// Explanation of when this occurs. + #[error("Operation '{operation}' failed for '{name}': {source} +Tip: Check logs with: --verbose or --log-output file-and-stderr")] + OperationFailed { + /// Name of the resource + name: String, + /// Operation being performed + operation: String, + /// Underlying error + #[source] + source: Box, + }, + + // Add more error variants as needed, grouped by category +} + +impl YourCommandError { + /// Get detailed troubleshooting guidance for this error + /// + /// This method provides comprehensive troubleshooting steps that can be + /// displayed to users when they need more help resolving the error. + /// + /// # Example + /// + /// ```rust + /// if let Err(e) = some_operation() { + /// eprintln!("Error: {e}"); + /// if verbose { + /// eprintln!("\nTroubleshooting:\n{}", e.help()); + /// } + /// } + /// ``` + #[must_use] + pub fn help(&self) -> &'static str { + match self { + Self::ConfigFileNotFound { .. } => { + "Configuration File Not Found - Detailed Troubleshooting: + +1. Check file path is correct + - Verify path spelling: ls -la + - Use absolute or relative paths correctly + +2. Verify file permissions + - Check read permissions: ls -l + - Fix if needed: chmod 644 + +3. Common solutions + - Create the file if missing + - Check current directory: pwd + - Provide correct path in arguments + +For more information, see the documentation." + } + + Self::ParsingFailed { .. } => { + "Parsing Failed - Detailed Troubleshooting: + +1. Validate syntax + - Use appropriate validator tool + - Check for common syntax errors + +2. Verify format matches expectation + - Check file extension + - Validate structure + +For more information, see format documentation." + } + + Self::OperationFailed { .. } => { + "Operation Failed - Detailed Troubleshooting: + +1. Check system state + - Verify resources are available + - Check permissions + +2. Review logs for details + - Run with --verbose + - Check log output + +For persistent issues, contact support." + } + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn it_should_have_help_for_all_variants() { + // Create instances of all error variants + let errors: Vec = vec![ + // ... create test instances + ]; + + for error in errors { + let help = error.help(); + assert!(!help.is_empty(), "Help text should not be empty"); + assert!( + help.contains("Troubleshooting") || help.len() > 50, + "Help should contain actionable guidance" + ); + } + } + + #[test] + fn it_should_display_context_in_errors() { + // Test that context fields appear in error messages + } +} +``` + +### Template Guidelines + +1. **Group related errors** with section comments (e.g., `// ===== File Errors =====`) +2. **Always include context fields** (paths, names, IDs) relevant to the error +3. **Use `#[source]`** for all wrapped errors to preserve error chains +4. **Add brief tips** in error messages using `\nTip:` format +5. **Document each variant** with rustdoc comments explaining when it occurs +6. **Implement `.help()`** with detailed troubleshooting for each variant +7. **Write comprehensive tests** covering all variants and help text + ## 📋 Error Review Checklist When reviewing error handling code, verify: @@ -589,6 +752,7 @@ When reviewing error handling code, verify: - [ ] **Pattern Matching**: Can callers handle different error cases appropriately? - [ ] **Unwrap/Expect**: Is `unwrap()` avoided in favor of `expect()` with descriptive messages? - [ ] **Consistency**: Does the error follow project conventions? +- [ ] **Error Grouping**: Are related errors grouped with section comments? ## 🔗 Related Documentation diff --git a/src/presentation/commands/create/config_loader.rs b/src/presentation/commands/create/config_loader.rs index cd8965fa..46e931a1 100644 --- a/src/presentation/commands/create/config_loader.rs +++ b/src/presentation/commands/create/config_loader.rs @@ -91,7 +91,7 @@ impl ConfigLoader { config .clone() .to_environment_params() - .map_err(CreateSubcommandError::ConfigValidationFailed)?; + .map_err(|source| CreateSubcommandError::ConfigValidationFailed { source })?; Ok(config) } @@ -217,7 +217,7 @@ mod tests { assert!(result.is_err()); match result.unwrap_err() { - CreateSubcommandError::ConfigValidationFailed(_) => { + CreateSubcommandError::ConfigValidationFailed { .. } => { // Expected - validation should catch invalid environment name } other => panic!("Expected ConfigValidationFailed, got: {other:?}"), @@ -246,7 +246,7 @@ mod tests { assert!(result.is_err()); match result.unwrap_err() { - CreateSubcommandError::ConfigValidationFailed(_) => { + CreateSubcommandError::ConfigValidationFailed { .. } => { // Expected - validation should catch missing SSH keys } other => panic!("Expected ConfigValidationFailed, got: {other:?}"), diff --git a/src/presentation/commands/create/errors.rs b/src/presentation/commands/create/errors.rs index 76d6318d..1ce73713 100644 --- a/src/presentation/commands/create/errors.rs +++ b/src/presentation/commands/create/errors.rs @@ -31,15 +31,28 @@ impl std::fmt::Display for ConfigFormat { /// troubleshooting and user feedback. #[derive(Debug, Error)] pub enum CreateSubcommandError { + // ===== Configuration File Errors ===== /// Configuration file not found - #[error("Configuration file not found: {path}")] + /// + /// The specified configuration file does not exist or is not accessible. + /// Use `.help()` for detailed troubleshooting steps. + #[error( + "Configuration file not found: {path} +Tip: Check that the file path is correct: ls -la {path}" + )] ConfigFileNotFound { /// Path to the missing configuration file path: PathBuf, }, /// Failed to parse configuration file - #[error("Failed to parse configuration file '{path}' as {format}")] + /// + /// The configuration file exists but could not be parsed in the expected format. + /// Use `.help()` for detailed troubleshooting steps. + #[error( + "Failed to parse configuration file '{path}' as {format}: {source} +Tip: Validate {format} syntax with: jq . < {path}" + )] ConfigParsingFailed { /// Path to the configuration file path: PathBuf, @@ -51,28 +64,48 @@ pub enum CreateSubcommandError { }, /// Configuration validation failed - #[error("Configuration validation failed")] - ConfigValidationFailed( + /// + /// The configuration file was parsed successfully but contains invalid values. + /// Use `.help()` for detailed troubleshooting steps. + #[error( + "Configuration validation failed: {source} +Tip: Review the validation error and fix the configuration file" + )] + ConfigValidationFailed { /// Underlying validation error from domain layer #[source] - crate::application::command_handlers::create::config::CreateConfigError, - ), + source: crate::application::command_handlers::create::config::CreateConfigError, + }, + // ===== Command Execution Errors ===== /// Command execution failed - #[error("Create command execution failed")] - CommandFailed( + /// + /// The create operation failed during execution after validation passed. + /// Use `.help()` for detailed troubleshooting steps. + #[error( + "Create command execution failed: {source} +Tip: Check logs with --log-output file-and-stderr for detailed error information" + )] + CommandFailed { /// Underlying command handler error #[source] - CreateCommandHandlerError, - ), + source: CreateCommandHandlerError, + }, + // ===== Template Generation Errors ===== /// Template generation failed - #[error("Template generation failed")] - TemplateGenerationFailed( + /// + /// Failed to generate template configuration or files. + /// Use `.help()` for detailed troubleshooting steps. + #[error( + "Template generation failed: {source} +Tip: Check that you have write permissions in the target directory" + )] + TemplateGenerationFailed { /// Underlying template generation error from domain layer #[source] - crate::application::command_handlers::create::config::CreateConfigError, - ), + source: crate::application::command_handlers::create::config::CreateConfigError, + }, } impl CreateSubcommandError { @@ -150,10 +183,10 @@ Example valid configuration: For more information, see the configuration documentation." } }, - Self::ConfigValidationFailed(inner) | Self::TemplateGenerationFailed(inner) => { - inner.help() + Self::ConfigValidationFailed { source } | Self::TemplateGenerationFailed { source } => { + source.help() } - Self::CommandFailed(inner) => inner.help(), + Self::CommandFailed { source } => source.help(), } } } @@ -228,13 +261,15 @@ mod tests { format: ConfigFormat::Json, source: Box::new(std::io::Error::new(std::io::ErrorKind::InvalidData, "test")), }, - CreateSubcommandError::ConfigValidationFailed( - CreateConfigError::InvalidEnvironmentName(EnvironmentNameError::InvalidFormat { - attempted_name: "test".to_string(), - reason: "invalid".to_string(), - valid_examples: vec!["dev".to_string()], - }), - ), + CreateSubcommandError::ConfigValidationFailed { + source: CreateConfigError::InvalidEnvironmentName( + EnvironmentNameError::InvalidFormat { + attempted_name: "test".to_string(), + reason: "invalid".to_string(), + valid_examples: vec!["dev".to_string()], + }, + ), + }, ]; for error in errors { diff --git a/src/presentation/commands/create/subcommands/environment.rs b/src/presentation/commands/create/subcommands/environment.rs index 3436e49d..1c69dd7b 100644 --- a/src/presentation/commands/create/subcommands/environment.rs +++ b/src/presentation/commands/create/subcommands/environment.rs @@ -139,8 +139,8 @@ fn execute_create_command( output.progress("Validating configuration and creating environment..."); #[allow(clippy::manual_inspect)] - command_handler.execute(config).map_err(|err| { - let error = CreateSubcommandError::CommandFailed(err); + command_handler.execute(config).map_err(|source| { + let error = CreateSubcommandError::CommandFailed { source }; report_error(output, &error); error }) @@ -298,7 +298,7 @@ mod tests { assert!(result2.is_err(), "Second create should fail"); match result2.unwrap_err() { - CreateSubcommandError::CommandFailed(_) => { + CreateSubcommandError::CommandFailed { .. } => { // Expected - environment already exists } other => panic!("Expected CommandFailed, got: {other:?}"), @@ -503,7 +503,7 @@ mod tests { assert!(result2.is_err(), "Second execution should fail"); match result2.unwrap_err() { - CreateSubcommandError::CommandFailed(_) => { + CreateSubcommandError::CommandFailed { .. } => { // Expected } other => panic!("Expected CommandFailed, got: {other:?}"), diff --git a/src/presentation/commands/create/subcommands/template.rs b/src/presentation/commands/create/subcommands/template.rs index e6f7f0c9..14d98c19 100644 --- a/src/presentation/commands/create/subcommands/template.rs +++ b/src/presentation/commands/create/subcommands/template.rs @@ -46,7 +46,7 @@ pub fn handle_template_generation(output_path: &Path) -> Result<(), CreateSubcom .block_on(async { EnvironmentCreationConfig::generate_template_file(output_path) .await - .map_err(CreateSubcommandError::TemplateGenerationFailed) + .map_err(|source| CreateSubcommandError::TemplateGenerationFailed { source }) })?; output.success(&format!( diff --git a/src/presentation/commands/create/tests/integration.rs b/src/presentation/commands/create/tests/integration.rs index 15898f2d..ec146354 100644 --- a/src/presentation/commands/create/tests/integration.rs +++ b/src/presentation/commands/create/tests/integration.rs @@ -87,7 +87,7 @@ fn it_should_reject_invalid_environment_name() { assert!(result.is_err(), "Should fail for invalid environment name"); match result.unwrap_err() { - create::CreateSubcommandError::ConfigValidationFailed(_) => { + create::CreateSubcommandError::ConfigValidationFailed { .. } => { // Expected } other => panic!("Expected ConfigValidationFailed, got: {other:?}"), @@ -103,7 +103,7 @@ fn it_should_reject_missing_ssh_keys() { assert!(result.is_err(), "Should fail for missing SSH keys"); match result.unwrap_err() { - create::CreateSubcommandError::ConfigValidationFailed(_) => { + create::CreateSubcommandError::ConfigValidationFailed { .. } => { // Expected } other => panic!("Expected ConfigValidationFailed, got: {other:?}"), @@ -124,7 +124,7 @@ fn it_should_reject_duplicate_environment() { assert!(result2.is_err(), "Second create should fail"); match result2.unwrap_err() { - create::CreateSubcommandError::CommandFailed(_) => { + create::CreateSubcommandError::CommandFailed { .. } => { // Expected - environment already exists } other => panic!("Expected CommandFailed, got: {other:?}"), diff --git a/src/presentation/commands/destroy/errors.rs b/src/presentation/commands/destroy/errors.rs index c69d6e76..38872fc8 100644 --- a/src/presentation/commands/destroy/errors.rs +++ b/src/presentation/commands/destroy/errors.rs @@ -16,6 +16,7 @@ use crate::domain::environment::name::EnvironmentNameError; /// Each variant includes relevant context and actionable error messages. #[derive(Debug, Error)] pub enum DestroySubcommandError { + // ===== Environment Validation Errors ===== /// Environment name validation failed /// /// The provided environment name doesn't meet the validation requirements. @@ -38,6 +39,18 @@ Tip: Check if environment exists: ls -la {data_dir}/" )] EnvironmentNotAccessible { name: String, data_dir: String }, + // ===== Repository Access Errors ===== + /// Repository operation failed + /// + /// Failed to create or access the environment repository. + /// Use `.help()` for detailed troubleshooting steps. + #[error( + "Failed to access environment repository at '{data_dir}': {reason} +Tip: Check directory permissions and disk space" + )] + RepositoryAccessFailed { data_dir: String, reason: String }, + + // ===== Destroy Operation Errors ===== /// Destroy operation failed /// /// The destruction process encountered an error during execution. @@ -51,16 +64,6 @@ Tip: Check logs and try running with --log-output file-and-stderr for more detai #[source] source: DestroyCommandHandlerError, }, - - /// Repository operation failed - /// - /// Failed to create or access the environment repository. - /// Use `.help()` for detailed troubleshooting steps. - #[error( - "Failed to access environment repository at '{data_dir}': {reason} -Tip: Check directory permissions and disk space" - )] - RepositoryAccessFailed { data_dir: String, reason: String }, } impl DestroySubcommandError {