Return exit code 1 even if stop on error is enabled #142
Conversation
| SilenceUsage: true, | ||
| SilenceErrors: true, |
There was a problem hiding this comment.
This makes it so when returning an error from a command, we don't:
- Show the usage string
- We tell the command we'll handle ourselves the errors coming from child commands
| @@ -6,6 +6,7 @@ import ( | |||
| "github.com/coveooss/credentials-sync/credentials" | |||
| "github.com/coveooss/credentials-sync/logger" | |||
| "github.com/coveooss/credentials-sync/targets" | |||
| "github.com/hashicorp/go-multierror" | |||
There was a problem hiding this comment.
Gives us a struct multierror that makes it possible to aggregate errors and that pretty print them for us.
e.g.
// printing
print(aMultiErrorWith4Errors)
// output
4 errors occurred:
* lol une erreur ici
* lol une erreur ici
* lol une erreur ici
* lol une erreur ici
pballandras
requested review from
wtrep,
dblanchette,
dlevesque-coveo and
dotboris
sync/config.go
Outdated
| @@ -50,12 +51,15 @@ func (config *Configuration) Sync() error { | |||
| for _, target := range allTargets { | |||
| go config.initTarget(target, creds, initChannel) | |||
| } | |||
| // This will later become a multierror object or stay nil, this is intended | |||
| var error_accumulator error | |||
There was a problem hiding this comment.
I'm pretty sure that this code base uses camelCase for variables. At least from the code around this.
There was a problem hiding this comment.
yup facepalms with a python
sync/config.go
Outdated
| // This will later become a multierror object or stay nil, this is intended | ||
| var error_accumulator error |
There was a problem hiding this comment.
What is intended here? The error type? If that's the case, I'd clarify this in the comment and explain why.
When people are going to be reading this code, I think that they'll be asking "Why is it this way?" and not "Did you intend for that to be this way?".
There was a problem hiding this comment.
I changed the comment 🚀
Currently
When the option stop on error is set to true in the config, the first error encountered is returned and we exit with code 1.
When the option stop on error is set to false in the config, the errors are logged as errors, but since they don't bubble up after the execution, we exit with 0.
Proposed change
Nothing changes when the option stop on error is set to true in the config.
When the option stop on error is set to false in the config, we record errors as they occur, and return all of them at once to the command that initiated them. Since we are returning an error, the program will exit with code 1.