Fix duplicate error output and improve error handling#16
Merged
Conversation
- Remove duplicate error wrapping in GetClusters function - Remove redundant fmt.Println(err) in Execute function - Let Cobra handle error output to avoid duplication - Update parseServiceFlag to return error instead of log.Fatal
- Convert all command handlers from Run to RunE - Replace log.Fatal calls with fmt.Errorf and error returns - Update all commands: restart, deploy, prune, revisions, run, completion - Ensure consistent error handling across the codebase - Update parseServiceFlag callers to handle returned errors
|
Caution Review failedThe pull request is closed. WalkthroughCommands in cmd/* switch from Run to RunE, converting handlers to return errors and replacing log.Fatalf with error propagation. parseServiceFlag now returns (cluster, service, error). Several commands add signal-aware contexts. main.go updates Execute handling. internal/ecs/list.go simplifies error return in GetClusters. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant Command as Cobra Command (RunE)
participant AWS as AWS/ECS Client
User->>CLI: run/prune/restart/revisions/deploy/completion
CLI->>Command: Execute()
Command->>Command: parseServiceFlag() → (cluster, service, err)
alt err != nil
Command-->>CLI: return err
CLI-->>User: exit code 1
else
Command->>AWS: init client / perform operation
alt operation error
Command-->>CLI: return err (wrapped)
CLI-->>User: exit code 1
else
Command-->>CLI: return nil
CLI-->>User: success output
end
end
sequenceDiagram
participant User
participant Command as Command RunE
participant Signal as OS Signals
participant Ctx as Context (signal.NotifyContext)
participant AWS as AWS/ECS Ops
Command->>Ctx: create cancelable context
Signal-->>Ctx: SIGINT/SIGTERM → cancel()
Command->>AWS: call ops with ctx
alt ctx canceled or error
AWS-->>Command: error
Command-->>User: return error (propagated)
else
AWS-->>Command: result
Command-->>User: print status, return nil
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.2.2)Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/product/migration-guide for migration instructions 📜 Recent review detailsConfiguration used: CodeRabbit UI 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (8)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
log.Fatalcalls with proper error returns for consistent error handlingChanges
Error Output Fix
GetClustersfunction that was causing "failed to list clusters" to appear twicefmt.Println(err)inExecute()function - Cobra already handles error outputError Handling Improvements
RuntoRunEto support error returnslog.Fatal*calls withfmt.Errorfand proper error returnsparseServiceFlagto return errors instead of callinglog.FatalparseServiceFlagin all command handlersTest Plan
go fmtandgo vet- no issueslog.Fatalcalls in cmd packageSummary by CodeRabbit