Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
164 changes: 164 additions & 0 deletions docs/contributing/error-handling.md
Original file line number Diff line number Diff line change
Expand Up @@ -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<dyn std::error::Error + Send + Sync>,
},

// 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 <path>
- Use absolute or relative paths correctly

2. Verify file permissions
- Check read permissions: ls -l <path>
- Fix if needed: chmod 644 <path>

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<YourCommandError> = 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:
Expand All @@ -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

Expand Down
6 changes: 3 additions & 3 deletions src/presentation/commands/create/config_loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ impl ConfigLoader {
config
.clone()
.to_environment_params()
.map_err(CreateSubcommandError::ConfigValidationFailed)?;
.map_err(|source| CreateSubcommandError::ConfigValidationFailed { source })?;

Ok(config)
}
Expand Down Expand Up @@ -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:?}"),
Expand Down Expand Up @@ -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:?}"),
Expand Down
83 changes: 59 additions & 24 deletions src/presentation/commands/create/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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 {
Expand Down Expand Up @@ -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(),
}
}
}
Expand Down Expand Up @@ -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 {
Expand Down
8 changes: 4 additions & 4 deletions src/presentation/commands/create/subcommands/environment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
})
Expand Down Expand Up @@ -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:?}"),
Expand Down Expand Up @@ -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:?}"),
Expand Down
2 changes: 1 addition & 1 deletion src/presentation/commands/create/subcommands/template.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!(
Expand Down
6 changes: 3 additions & 3 deletions src/presentation/commands/create/tests/integration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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:?}"),
Expand All @@ -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:?}"),
Expand All @@ -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:?}"),
Expand Down
Loading