Skip to content

Commit bfca0bf

Browse files
josecelanojosecelano
committed
refactor: [#83] unify error kind classification with generic ErrorKind enum
Replace command-specific error kinds (ProvisionErrorKind, ConfigureErrorKind) with a single generic ErrorKind enum that provides high-level error categorization across all commands. This refactor simplifies error handling by making errors self-describing through the Traceable trait's error_kind() method, eliminating the need for manual error classification in command code. Changes: - Add generic ErrorKind enum with 7 variants (TemplateRendering, InfrastructureOperation, NetworkConnectivity, CommandExecution, Timeout, FileSystem, Configuration) - Extend Traceable trait with error_kind() method - Implement error_kind() for all error types (9 main + 3 test types) - Update ProvisionFailureContext and ConfigureFailureContext to use ErrorKind - Update commands to call error.error_kind() instead of manual pattern matching - Remove old ProvisionErrorKind and ConfigureErrorKind enums - Update all tests and trace writers to use new error kind system Benefits: - Single source of truth for error categorization - Errors are self-describing via error_kind() method - No manual error classification needed in commands - Consistent error kinds across all commands - Easier to extend with new error types All tests passing (758), all linters passing.
1 parent 87248a6 commit bfca0bf

File tree

21 files changed

+285
-105
lines changed

21 files changed

+285
-105
lines changed

‎docs/refactors/error-kind-classification-strategy.md‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -636,24 +636,24 @@ impl Traceable for ProvisionCommandError {
636636

637637
### Phase Completion Tracker
638638

639-
| Phase | Status | Commit | Tests | Notes |
640-
| -------------------------------- | -------------- | ------ | ----- | ----- |
641-
| Phase 1: Generic ErrorKind Enum | ⏳ Not Started | - | - | - |
642-
| Phase 2: Extend Traceable Trait | ⏳ Not Started | - | - | - |
643-
| Phase 3: Implement error_kind() | ⏳ Not Started | - | - | - |
644-
| Phase 4: Update Failure Contexts | ⏳ Not Started | - | - | - |
645-
| Phase 5: Update Commands | ⏳ Not Started | - | - | - |
646-
| Phase 6: Update Trace Writers | ⏳ Not Started | - | - | - |
647-
| Phase 7: Update Tests | ⏳ Not Started | - | - | - |
648-
| Phase 8: Remove Old Enums | ⏳ Not Started | - | - | - |
639+
| Phase | Status | Commit | Tests | Notes |
640+
| -------------------------------- | ----------- | ------ | ----- | --------------------------------------------------------------------- |
641+
| Phase 1: Generic ErrorKind Enum | ✅ Complete | - | 758 | Created src/shared/error/kind.rs with 7 variants, 5 tests |
642+
| Phase 2: Extend Traceable Trait | ✅ Complete | - | 758 | Added error_kind() method to Traceable trait |
643+
| Phase 3: Implement error_kind() | ✅ Complete | - | 758 | All 9 error types + 3 test errors now have error_kind() |
644+
| Phase 4: Update Failure Contexts | ✅ Complete | - | 758 | Replaced ProvisionErrorKind/ConfigureErrorKind with generic ErrorKind |
645+
| Phase 5: Update Commands | ✅ Complete | - | 758 | Commands now call error.error_kind() instead of manual classification |
646+
| Phase 6-8: Cleanup | ✅ Complete | - | 758 | Updated all tests, trace writers, removed old enum exports |
649647

650648
**Legend**: ⏳ Not Started | 🚧 In Progress | ✅ Complete
651649

650+
**Note**: Phases 6-8 were combined as they were all part of updating tests and removing references to old enums. All work completed in single pass.
651+
652652
### Test Count Tracking
653653

654654
- **Baseline**: 753 tests passing (before refactor)
655-
- **Current**: TBD
656-
- **Target**: All tests passing with updated error kinds
655+
- **Current**: 758 tests passing (Phases 1-5 complete)
656+
- **Target**: All tests passing with updated error kinds ✅
657657

658658
---
659659

‎src/application/commands/configure.rs‎

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,15 @@ use tracing::{info, instrument, warn};
55
use crate::application::steps::{InstallDockerComposeStep, InstallDockerStep};
66
use crate::domain::environment::repository::EnvironmentRepository;
77
use crate::domain::environment::state::{
8-
BaseFailureContext, ConfigureErrorKind, ConfigureFailureContext, ConfigureStep,
8+
BaseFailureContext, ConfigureFailureContext, ConfigureStep,
99
};
1010
use crate::domain::environment::{
1111
ConfigureFailed, Configured, Configuring, Environment, Provisioned, TraceId,
1212
};
1313
use crate::infrastructure::external_tools::ansible::adapter::AnsibleClient;
1414
use crate::infrastructure::trace::ConfigureTraceWriter;
1515
use crate::shared::command::CommandError;
16+
use crate::shared::error::Traceable;
1617
use crate::shared::SystemClock;
1718

1819
/// Comprehensive error type for the `ConfigureCommand`
@@ -36,6 +37,12 @@ impl crate::shared::Traceable for ConfigureCommandError {
3637
Self::Command(e) => Some(e),
3738
}
3839
}
40+
41+
fn error_kind(&self) -> crate::shared::ErrorKind {
42+
match self {
43+
Self::Command(_) => crate::shared::ErrorKind::CommandExecution,
44+
}
45+
}
3946
}
4047

4148
/// `ConfigureCommand` orchestrates the complete infrastructure configuration workflow
@@ -250,10 +257,8 @@ impl ConfigureCommand {
250257
// Step that failed is directly provided - no reverse engineering needed
251258
let failed_step = current_step;
252259

253-
// Classify the error kind based on error type
254-
let error_kind = match error {
255-
ConfigureCommandError::Command(_) => ConfigureErrorKind::InstallationFailed,
256-
};
260+
// Get error kind from the error itself (errors are self-describing)
261+
let error_kind = error.error_kind();
257262

258263
let now = self.clock.now();
259264
let trace_id = TraceId::new();
@@ -397,7 +402,10 @@ mod tests {
397402
let current_step = ConfigureStep::InstallDocker;
398403
let context = command.build_failure_context(&environment, &error, current_step, started_at);
399404
assert_eq!(context.failed_step, ConfigureStep::InstallDocker);
400-
assert_eq!(context.error_kind, ConfigureErrorKind::InstallationFailed);
405+
assert_eq!(
406+
context.error_kind,
407+
crate::shared::ErrorKind::CommandExecution
408+
);
401409
assert_eq!(context.base.execution_started_at, started_at);
402410
}
403411
}

‎src/application/commands/provision.rs‎

Lines changed: 28 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ use crate::application::steps::{
2525
};
2626
use crate::domain::environment::repository::EnvironmentRepository;
2727
use crate::domain::environment::state::{
28-
BaseFailureContext, ProvisionErrorKind, ProvisionFailureContext, ProvisionStep,
28+
BaseFailureContext, ProvisionFailureContext, ProvisionStep,
2929
};
3030
use crate::domain::environment::{
3131
Created, Environment, ProvisionFailed, Provisioned, Provisioning, TraceId,
@@ -37,6 +37,7 @@ use crate::infrastructure::external_tools::ansible::AnsibleTemplateRenderer;
3737
use crate::infrastructure::external_tools::tofu::adapter::client::{InstanceInfo, OpenTofuError};
3838
use crate::infrastructure::external_tools::tofu::{ProvisionTemplateError, TofuTemplateRenderer};
3939
use crate::shared::command::CommandError;
40+
use crate::shared::error::Traceable;
4041
use crate::shared::ssh::{SshConnection, SshCredentials, SshError};
4142
use crate::shared::SystemClock;
4243

@@ -89,6 +90,17 @@ impl crate::shared::Traceable for ProvisionCommandError {
8990
Self::SshConnectivity(e) => Some(e),
9091
}
9192
}
93+
94+
fn error_kind(&self) -> crate::shared::ErrorKind {
95+
match self {
96+
Self::OpenTofuTemplateRendering(_) | Self::AnsibleTemplateRendering(_) => {
97+
crate::shared::ErrorKind::TemplateRendering
98+
}
99+
Self::OpenTofu(_) => crate::shared::ErrorKind::InfrastructureOperation,
100+
Self::SshConnectivity(_) => crate::shared::ErrorKind::NetworkConnectivity,
101+
Self::Command(_) => crate::shared::ErrorKind::CommandExecution,
102+
}
103+
}
92104
}
93105

94106
/// `ProvisionCommand` orchestrates the complete infrastructure provisioning workflow
@@ -449,16 +461,8 @@ impl ProvisionCommand {
449461
// Step that failed is directly provided - no reverse engineering needed
450462
let failed_step = current_step;
451463

452-
// Classify the error kind based on error type
453-
let error_kind = match error {
454-
ProvisionCommandError::OpenTofuTemplateRendering(_)
455-
| ProvisionCommandError::AnsibleTemplateRendering(_) => {
456-
ProvisionErrorKind::TemplateRendering
457-
}
458-
ProvisionCommandError::OpenTofu(_) => ProvisionErrorKind::InfrastructureProvisioning,
459-
ProvisionCommandError::SshConnectivity(_) => ProvisionErrorKind::NetworkConnectivity,
460-
ProvisionCommandError::Command(_) => ProvisionErrorKind::ConfigurationTimeout,
461-
};
464+
// Get error kind from the error itself (errors are self-describing)
465+
let error_kind = error.error_kind();
462466

463467
let now = self.clock.now();
464468
let trace_id = TraceId::new();
@@ -706,7 +710,10 @@ mod tests {
706710
let current_step = ProvisionStep::RenderOpenTofuTemplates;
707711
let context = command.build_failure_context(&environment, &error, current_step, started_at);
708712
assert_eq!(context.failed_step, ProvisionStep::RenderOpenTofuTemplates);
709-
assert_eq!(context.error_kind, ProvisionErrorKind::TemplateRendering);
713+
assert_eq!(
714+
context.error_kind,
715+
crate::shared::ErrorKind::TemplateRendering
716+
);
710717
assert_eq!(context.base.execution_started_at, started_at);
711718
}
712719

@@ -751,7 +758,10 @@ mod tests {
751758
let current_step = ProvisionStep::WaitSshConnectivity;
752759
let context = command.build_failure_context(&environment, &error, current_step, started_at);
753760
assert_eq!(context.failed_step, ProvisionStep::WaitSshConnectivity);
754-
assert_eq!(context.error_kind, ProvisionErrorKind::NetworkConnectivity);
761+
assert_eq!(
762+
context.error_kind,
763+
crate::shared::ErrorKind::NetworkConnectivity
764+
);
755765
assert_eq!(context.base.execution_started_at, started_at);
756766
}
757767

@@ -792,7 +802,10 @@ mod tests {
792802
let current_step = ProvisionStep::CloudInitWait;
793803
let context = command.build_failure_context(&environment, &error, current_step, started_at);
794804
assert_eq!(context.failed_step, ProvisionStep::CloudInitWait);
795-
assert_eq!(context.error_kind, ProvisionErrorKind::ConfigurationTimeout);
805+
assert_eq!(
806+
context.error_kind,
807+
crate::shared::ErrorKind::CommandExecution
808+
);
796809
assert_eq!(context.base.execution_started_at, started_at);
797810
}
798811

@@ -837,7 +850,7 @@ mod tests {
837850
assert_eq!(context.failed_step, ProvisionStep::OpenTofuInit);
838851
assert_eq!(
839852
context.error_kind,
840-
ProvisionErrorKind::InfrastructureProvisioning
853+
crate::shared::ErrorKind::InfrastructureOperation
841854
);
842855
assert_eq!(context.base.execution_started_at, started_at);
843856
}

‎src/application/steps/rendering/ansible_templates.rs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,10 @@ impl crate::shared::Traceable for RenderAnsibleTemplatesError {
7474
// None of the source errors implement Traceable
7575
None
7676
}
77+
78+
fn error_kind(&self) -> crate::shared::ErrorKind {
79+
crate::shared::ErrorKind::TemplateRendering
80+
}
7781
}
7882

7983
/// Simple step that renders `Ansible` templates to the build directory with runtime variables

‎src/domain/environment/state/configure_failed.rs‎

Lines changed: 9 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ use serde::{Deserialize, Serialize};
1515

1616
use crate::domain::environment::state::{AnyEnvironmentState, BaseFailureContext, StateTypeError};
1717
use crate::domain::environment::Environment;
18+
use crate::shared::ErrorKind;
1819

1920
// ============================================================================
2021
// Configure Command Error Context
@@ -30,7 +31,7 @@ pub struct ConfigureFailureContext {
3031
pub failed_step: ConfigureStep,
3132

3233
/// Error category for type-safe handling
33-
pub error_kind: ConfigureErrorKind,
34+
pub error_kind: ErrorKind,
3435

3536
/// Base failure context with common fields
3637
#[serde(flatten)]
@@ -46,15 +47,6 @@ pub enum ConfigureStep {
4647
InstallDockerCompose,
4748
}
4849

49-
/// Error categories for configure failures
50-
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
51-
pub enum ConfigureErrorKind {
52-
/// Software installation failed
53-
InstallationFailed,
54-
/// Command execution failed
55-
CommandExecutionFailed,
56-
}
57-
5850
/// Error state - Application configuration failed
5951
///
6052
/// The configuration command failed during execution. The `context` field
@@ -108,7 +100,7 @@ mod tests {
108100
fn create_test_context() -> ConfigureFailureContext {
109101
ConfigureFailureContext {
110102
failed_step: ConfigureStep::InstallDocker,
111-
error_kind: ConfigureErrorKind::InstallationFailed,
103+
error_kind: ErrorKind::CommandExecution,
112104
base: BaseFailureContext {
113105
error_summary: "Docker installation failed".to_string(),
114106
failed_at: Utc::now(),
@@ -127,10 +119,7 @@ mod tests {
127119
context: context.clone(),
128120
};
129121
assert_eq!(state.context.failed_step, ConfigureStep::InstallDocker);
130-
assert_eq!(
131-
state.context.error_kind,
132-
ConfigureErrorKind::InstallationFailed
133-
);
122+
assert_eq!(state.context.error_kind, ErrorKind::CommandExecution);
134123
}
135124

136125
#[test]
@@ -140,7 +129,7 @@ mod tests {
140129
};
141130
let json = serde_json::to_string(&state).unwrap();
142131
assert!(json.contains("InstallDocker"));
143-
assert!(json.contains("InstallationFailed"));
132+
assert!(json.contains("CommandExecution"));
144133
}
145134

146135
#[test]
@@ -205,7 +194,7 @@ mod tests {
205194
fn it_should_serialize_configure_failure_context() {
206195
let context = ConfigureFailureContext {
207196
failed_step: ConfigureStep::InstallDocker,
208-
error_kind: ConfigureErrorKind::InstallationFailed,
197+
error_kind: ErrorKind::CommandExecution,
209198
base: BaseFailureContext {
210199
error_summary: "Docker installation failed".to_string(),
211200
failed_at: Utc::now(),
@@ -218,7 +207,7 @@ mod tests {
218207

219208
let json = serde_json::to_string(&context).unwrap();
220209
assert!(json.contains("InstallDocker"));
221-
assert!(json.contains("InstallationFailed"));
210+
assert!(json.contains("CommandExecution"));
222211
}
223212

224213
#[test]
@@ -227,7 +216,7 @@ mod tests {
227216
let json = format!(
228217
r#"{{
229218
"failed_step": "InstallDockerCompose",
230-
"error_kind": "CommandExecutionFailed",
219+
"error_kind": "CommandExecution",
231220
"error_summary": "Command execution failed",
232221
"failed_at": "2025-10-06T10:00:00Z",
233222
"execution_started_at": "2025-10-06T09:59:30Z",
@@ -239,10 +228,7 @@ mod tests {
239228

240229
let context: ConfigureFailureContext = serde_json::from_str(&json).unwrap();
241230
assert_eq!(context.failed_step, ConfigureStep::InstallDockerCompose);
242-
assert_eq!(
243-
context.error_kind,
244-
ConfigureErrorKind::CommandExecutionFailed
245-
);
231+
assert_eq!(context.error_kind, ErrorKind::CommandExecution);
246232
}
247233
}
248234
}

‎src/domain/environment/state/configuring.rs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -163,16 +163,17 @@ mod tests {
163163
#[test]
164164
fn it_should_transition_from_configuring_to_configure_failed() {
165165
use crate::domain::environment::state::{
166-
BaseFailureContext, ConfigureErrorKind, ConfigureFailureContext, ConfigureStep,
166+
BaseFailureContext, ConfigureFailureContext, ConfigureStep,
167167
};
168168
use crate::domain::environment::TraceId;
169+
use crate::shared::ErrorKind;
169170
use chrono::Utc;
170171
use std::time::Duration;
171172

172173
let env = create_test_environment();
173174
let context = ConfigureFailureContext {
174175
failed_step: ConfigureStep::InstallDocker,
175-
error_kind: ConfigureErrorKind::InstallationFailed,
176+
error_kind: ErrorKind::CommandExecution,
176177
base: BaseFailureContext {
177178
error_summary: "ansible_playbook_error".to_string(),
178179
failed_at: Utc::now(),

‎src/domain/environment/state/mod.rs‎

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -61,16 +61,12 @@ mod running;
6161

6262
// Re-export state types
6363
pub use common::BaseFailureContext;
64-
pub use configure_failed::{
65-
ConfigureErrorKind, ConfigureFailed, ConfigureFailureContext, ConfigureStep,
66-
};
64+
pub use configure_failed::{ConfigureFailed, ConfigureFailureContext, ConfigureStep};
6765
pub use configured::Configured;
6866
pub use configuring::Configuring;
6967
pub use created::Created;
7068
pub use destroyed::Destroyed;
71-
pub use provision_failed::{
72-
ProvisionErrorKind, ProvisionFailed, ProvisionFailureContext, ProvisionStep,
73-
};
69+
pub use provision_failed::{ProvisionFailed, ProvisionFailureContext, ProvisionStep};
7470
pub use provisioned::Provisioned;
7571
pub use provisioning::Provisioning;
7672
pub use release_failed::ReleaseFailed;
@@ -390,12 +386,13 @@ mod tests {
390386
/// Helper to create a test `ProvisionFailureContext` with custom error message
391387
fn create_test_provision_context(error_message: &str) -> ProvisionFailureContext {
392388
use crate::domain::environment::TraceId;
389+
use crate::shared::ErrorKind;
393390
use chrono::Utc;
394391
use std::time::Duration;
395392

396393
ProvisionFailureContext {
397394
failed_step: ProvisionStep::OpenTofuApply,
398-
error_kind: ProvisionErrorKind::InfrastructureProvisioning,
395+
error_kind: ErrorKind::InfrastructureOperation,
399396
base: BaseFailureContext {
400397
error_summary: error_message.to_string(),
401398
failed_at: Utc::now(),
@@ -410,12 +407,13 @@ mod tests {
410407
/// Helper to create a test `ConfigureFailureContext` with custom error message
411408
fn create_test_configure_context(error_message: &str) -> ConfigureFailureContext {
412409
use crate::domain::environment::TraceId;
410+
use crate::shared::ErrorKind;
413411
use chrono::Utc;
414412
use std::time::Duration;
415413

416414
ConfigureFailureContext {
417415
failed_step: ConfigureStep::InstallDocker,
418-
error_kind: ConfigureErrorKind::InstallationFailed,
416+
error_kind: ErrorKind::CommandExecution,
419417
base: BaseFailureContext {
420418
error_summary: error_message.to_string(),
421419
failed_at: Utc::now(),
@@ -553,7 +551,7 @@ mod tests {
553551
};
554552
let json = serde_json::to_string(&state).unwrap();
555553
assert!(json.contains("InstallDocker"));
556-
assert!(json.contains("InstallationFailed"));
554+
assert!(json.contains("CommandExecution"));
557555
let deserialized: ConfigureFailed = serde_json::from_str(&json).unwrap();
558556
assert_eq!(deserialized.context.base.error_summary, "ansible_playbook");
559557
}

0 commit comments

Comments
 (0)