Skip to content

Commit ffd627e

Browse files
committed
fix: implement layer-based error wrapping strategy
Remove redundant error wrapping to prevent message duplication while maintaining error chains for proper error inspection. Changes: - Entry point layer (cmd/): Pass through all errors - Orchestration layer (deploy/delete/describe/validate): Pass through errors from lower layers - Business logic layer (resolve/): Pass through config and file system errors that already have full context - Update tests to verify actual error messages from boundary layers Before: failed to resolve stack dependencies: failed to get stack vpc: failed to resolve stack 'vpc' for context 'dev': stack 'vpc' not found in configuration After: failed to resolve stack 'vpc' for context 'dev': stack 'vpc' not found in configuration This implements ADR 0023 layer-based error wrapping strategy.
1 parent 4f46a6e commit ffd627e

13 files changed

Lines changed: 27 additions & 30 deletions

File tree

cmd/describe.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ func describeSingleStack(ctx context.Context, stackName, contextName, configFile
7272
// Resolve the target stack configuration
7373
stack, err := resolver.ResolveStack(ctx, contextName, stackName)
7474
if err != nil {
75-
return fmt.Errorf("failed to resolve stack %s: %w", stackName, err)
75+
return err
7676
}
7777

7878
// Get describer instance
@@ -81,7 +81,7 @@ func describeSingleStack(ctx context.Context, stackName, contextName, configFile
8181
// Retrieve stack information from AWS
8282
stackDesc, err := d.DescribeStack(ctx, stack)
8383
if err != nil {
84-
return fmt.Errorf("failed to describe stack %s: %w", stackName, err)
84+
return err
8585
}
8686

8787
// Format and display the information

cmd/describe_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -234,7 +234,7 @@ stacks:
234234

235235
// Verify error handling
236236
assert.Error(t, err, "describe command should return error when describer fails")
237-
assert.Contains(t, err.Error(), "failed to describe stack nonexistent-stack")
237+
assert.Contains(t, err.Error(), "stack not found")
238238

239239
// Verify mock expectations
240240
mockDescriber.AssertExpectations(t)
@@ -257,7 +257,7 @@ func TestDescribeCommand_HandlesResolverError(t *testing.T) {
257257

258258
// Verify error handling
259259
assert.Error(t, err, "describe command should return error when resolver fails")
260-
assert.Contains(t, err.Error(), "failed to resolve stack")
260+
assert.Contains(t, err.Error(), "failed to read config file")
261261

262262
// Verify no describer calls were made since resolver failed
263263
mockDescriber.AssertNotCalled(t, "DescribeStack")

cmd/diff.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ func diffSingleStack(ctx context.Context, stackName, contextName, configFile str
7474
// Resolve the target stack
7575
targetStack, err := resolver.ResolveStack(ctx, contextName, stackName)
7676
if err != nil {
77-
return fmt.Errorf("failed to resolve stack %s: %w", stackName, err)
77+
return err
7878
}
7979

8080
// Create diff options based on command flags
@@ -90,7 +90,7 @@ func diffSingleStack(ctx context.Context, stackName, contextName, configFile str
9090
// Perform the diff
9191
result, err := d.DiffStack(ctx, targetStack, options)
9292
if err != nil {
93-
return fmt.Errorf("failed to diff stack %s: %w", stackName, err)
93+
return err
9494
}
9595

9696
// Output the results using plain text

internal/delete/deleter.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ func (d *StackDeleter) DeleteSingleStack(ctx context.Context, stackName, context
123123
// Resolve single stack
124124
stack, err := d.resolver.ResolveStack(ctx, contextName, stackName)
125125
if err != nil {
126-
return fmt.Errorf("failed to resolve stack dependencies: %w", err)
126+
return err
127127
}
128128

129129
return d.deleteStackWithFeedback(ctx, stack, contextName)
@@ -134,7 +134,7 @@ func (d *StackDeleter) DeleteAllStacks(ctx context.Context, contextName string)
134134
// Get list of stacks to delete
135135
stackNames, err := d.configProvider.ListStacks(contextName)
136136
if err != nil {
137-
return fmt.Errorf("failed to get stacks for context %s: %w", contextName, err)
137+
return err
138138
}
139139
if len(stackNames) == 0 {
140140
fmt.Printf("No stacks found in context %s\n", contextName)
@@ -144,7 +144,7 @@ func (d *StackDeleter) DeleteAllStacks(ctx context.Context, contextName string)
144144
// Get dependency order without resolving stacks
145145
deploymentOrder, err := d.resolver.GetDependencyOrder(contextName, stackNames)
146146
if err != nil {
147-
return fmt.Errorf("failed to calculate dependency order: %w", err)
147+
return err
148148
}
149149

150150
// Reverse the deployment order for safe deletion
@@ -159,7 +159,7 @@ func (d *StackDeleter) DeleteAllStacks(ctx context.Context, contextName string)
159159
// Resolve this specific stack
160160
stack, err := d.resolver.ResolveStack(ctx, contextName, stackName)
161161
if err != nil {
162-
return fmt.Errorf("failed to resolve stack %s: %w", stackName, err)
162+
return err
163163
}
164164

165165
err = d.deleteStackWithFeedback(ctx, stack, contextName)

internal/delete/deleter_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -276,7 +276,7 @@ func TestDeleteSingleStack_ResolverFailure(t *testing.T) {
276276

277277
// Assertions
278278
assert.Error(t, err)
279-
assert.Contains(t, err.Error(), "failed to resolve stack dependencies")
279+
assert.Contains(t, err.Error(), "stack not found")
280280
mockResolver.AssertExpectations(t)
281281
}
282282

@@ -408,7 +408,7 @@ func TestDeleteAllStacks_ListStacksFailure(t *testing.T) {
408408

409409
// Assertions
410410
assert.Error(t, err)
411-
assert.Contains(t, err.Error(), "failed to get stacks for context dev")
411+
assert.Contains(t, err.Error(), "context not found")
412412
mockConfigProvider.AssertExpectations(t)
413413
}
414414

@@ -431,7 +431,7 @@ func TestDeleteAllStacks_GetDependencyOrderFailure(t *testing.T) {
431431

432432
// Assertions
433433
assert.Error(t, err)
434-
assert.Contains(t, err.Error(), "failed to calculate dependency order")
434+
assert.Contains(t, err.Error(), "circular dependency")
435435
mockConfigProvider.AssertExpectations(t)
436436
mockResolver.AssertExpectations(t)
437437
}
@@ -458,7 +458,7 @@ func TestDeleteAllStacks_ResolveStackFailure(t *testing.T) {
458458

459459
// Assertions
460460
assert.Error(t, err)
461-
assert.Contains(t, err.Error(), "failed to resolve stack vpc")
461+
assert.Contains(t, err.Error(), "stack resolution failed")
462462
mockConfigProvider.AssertExpectations(t)
463463
mockResolver.AssertExpectations(t)
464464
}

internal/deploy/deployer.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -341,7 +341,7 @@ func (d *StackDeployer) DeploySingleStack(ctx context.Context, stackName, contex
341341
// Resolve single stack
342342
stack, err := d.resolver.ResolveStack(ctx, contextName, stackName)
343343
if err != nil {
344-
return fmt.Errorf("failed to resolve stack dependencies: %w", err)
344+
return err
345345
}
346346

347347
return d.deployStackWithFeedback(ctx, stack, contextName)
@@ -370,7 +370,7 @@ func (d *StackDeployer) DeployAllStacks(ctx context.Context, contextName string)
370370
// Resolve this specific stack to get fresh parameter values
371371
stack, err := d.resolver.ResolveStack(ctx, contextName, stackName)
372372
if err != nil {
373-
return fmt.Errorf("failed to resolve stack %s: %w", stackName, err)
373+
return err
374374
}
375375

376376
err = d.deployStackWithFeedback(ctx, stack, contextName)

internal/deploy/deployer_test.go

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,6 @@ func TestDeploySingleStack_ResolverError(t *testing.T) {
164164

165165
// Verify error is propagated correctly
166166
assert.Error(t, err)
167-
assert.Contains(t, err.Error(), "failed to resolve stack dependencies")
168167
assert.Contains(t, err.Error(), "config load failed")
169168

170169
mockProvider.AssertExpectations(t)
@@ -201,7 +200,6 @@ func TestDeployAllStacks_ConfigLoadError(t *testing.T) {
201200

202201
// Should fail during config loading for individual stack resolution
203202
assert.Error(t, err)
204-
assert.Contains(t, err.Error(), "failed to resolve stack")
205203
assert.Contains(t, err.Error(), "config resolution failed")
206204

207205
mockProvider.AssertExpectations(t)

internal/describe/describer.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ func (d *StackDescriber) DescribeStack(ctx context.Context, stack *model.Stack)
3636
// Use existing AWS operations to get stack information
3737
stackInfo, err := cfOps.DescribeStack(ctx, stack.Name)
3838
if err != nil {
39-
return nil, fmt.Errorf("failed to describe stack %s: %w", stack.Name, err)
39+
return nil, err
4040
}
4141

4242
// Convert AWS StackInfo to our StackDescription format

internal/describe/describer_test.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,6 @@ func TestStackDescriber_DescribeStack_AWSError(t *testing.T) {
127127
// Verify
128128
assert.Error(t, err, "DescribeStack should return error when AWS operations fail")
129129
assert.Nil(t, result, "Result should be nil on error")
130-
assert.Contains(t, err.Error(), "failed to describe stack failing-stack")
131130
assert.Contains(t, err.Error(), "AWS CloudFormation error: stack not found")
132131

133132
mockCFOps.AssertExpectations(t)

internal/resolve/stack_resolver.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -54,19 +54,19 @@ func (r *StackResolver) ResolveStack(ctx context.Context, context string, stackN
5454
// Load configuration
5555
cfg, err := r.configProvider.LoadConfig(ctx, context)
5656
if err != nil {
57-
return nil, fmt.Errorf("failed to load config: %w", err)
57+
return nil, err
5858
}
5959

6060
// Get stack configuration
6161
stackConfig, err := r.configProvider.GetStack(stackName, context)
6262
if err != nil {
63-
return nil, fmt.Errorf("failed to get stack %s: %w", stackName, err)
63+
return nil, err
6464
}
6565

6666
// Read raw template content
6767
rawTemplate, err := r.fileSystemResolver.Resolve(stackConfig.Template)
6868
if err != nil {
69-
return nil, fmt.Errorf("failed to read template: %w", err)
69+
return nil, err
7070
}
7171

7272
// Process template with variables (parameters and context)

0 commit comments

Comments
 (0)