Remove loggedError #94
Conversation
Errors are duplicated in the logs because the application exists with log.Fatal and before that the error is logged by loggedError. Signed-off-by: Alexandre Vicenzi <[email protected]>
|
This becomes a bigger problem due to #93 now that all errors are logged to the console and a file. |
There was a problem hiding this comment.
Tbh, I doubt we need the SuseConnectError at all given the changes in #93. There you have effectively removed all usage of the ErrorCode field, so at this point, it's just a fancy wrapper around fmt.Errorf. Do you see any reason why we shouldn't just use fmt.Errorf instead?
| @@ -36,10 +40,22 @@ const ( | |||
| // SuseConnectError is a custom error type allowing us to distinguish between | |||
| // different error kinds via the `ErrorCode` field | |||
There was a problem hiding this comment.
| // different error kinds via the `ErrorCode` field | |
| // different error kinds via the `Code` field |
|
@dcermak yes, you are right. It seems this is not used anywhere else anymore once #93 is merged. Removing Instead of custom errors, we could have log levels and log a bit more, such as Info, Error, and Debug messages to help us in the future once a new bugzilla arises. This is a feature I could see being useful in a future PR. |
| @@ -33,7 +33,7 @@ func actionWrapper(action func(*cli.Context) error) func(*cli.Context) error { | |||
| if err := action(ctx); err != nil { | |||
| switch err.(type) { | |||
| case *cs.SuseConnectError: | |||
| if err.(*cs.SuseConnectError).ErrorCode == cs.GetCredentialsError { | |||
| if err.(*cs.SuseConnectError).Code == cs.GetCredentialsError { | |||
There was a problem hiding this comment.
If the plan is to use fmt.Errorf you can use errors.Is (or errors.As) if you use %w.
There was a problem hiding this comment.
This code is going away with #93, this PR needs a rebase once that is merged.
Errors are duplicated in the logs because the application exists with log.Fatal and before that the error is logged by loggedError.