From 72a66d1223af373d6aa543e38589bb26b67e9b36 Mon Sep 17 00:00:00 2001 From: dylan Date: Mon, 30 Oct 2023 15:05:45 -0700 Subject: [PATCH] chore: unify error displaying --- asconf/utils.go | 2 +- cmd/convert.go | 28 ++++++++++------------------ cmd/convert_test.go | 11 ++--------- cmd/diff.go | 8 ++++---- cmd/root.go | 19 +++++++++++++++++++ cmd/utils.go | 6 +++++- 6 files changed, 41 insertions(+), 33 deletions(-) diff --git a/asconf/utils.go b/asconf/utils.go index 2b7ef35..9946558 100644 --- a/asconf/utils.go +++ b/asconf/utils.go @@ -265,7 +265,7 @@ func ParseFmtString(in string) (f Format, err error) { f = AsConfig default: f = Invalid - err = fmt.Errorf("%w: %s", ErrInvalidFormat, f) + err = fmt.Errorf("%w: %s", ErrInvalidFormat, in) } return diff --git a/cmd/convert.go b/cmd/convert.go index c8c908e..df868bb 100644 --- a/cmd/convert.go +++ b/cmd/convert.go @@ -20,6 +20,7 @@ const ( var ( errTooManyArguments = fmt.Errorf("expected a maximum of %d arguments", convertArgMax) errFileNotExist = fmt.Errorf("file does not exist") + errMissingAerospikeVersion = fmt.Errorf("missing required flag '--aerospike-version'") errInvalidAerospikeVersion = fmt.Errorf("aerospike version must be in the form ..") errUnsupportedAerospikeVersion = fmt.Errorf("aerospike version unsupported") errInvalidOutput = fmt.Errorf("invalid output flag") @@ -114,8 +115,6 @@ func newConvertCmd() *cobra.Command { if !force { err = conf.Validate() if err != nil { - // validation errors are not user errors, so don't print usage - cmd.SilenceUsage = true return err } } @@ -167,30 +166,26 @@ func newConvertCmd() *cobra.Command { }, PreRunE: func(cmd *cobra.Command, args []string) error { - var multiErr error - if len(args) > convertArgMax { - logger.Errorf("Expected no more than %d argument(s)", convertArgMax) - multiErr = errors.Join(multiErr, errTooManyArguments) + return errTooManyArguments } if len(args) > 0 { source := args[0] if _, err := os.Stat(source); errors.Is(err, os.ErrNotExist) { - logger.Errorf("Source file does not exist %s", source) - multiErr = errors.Join(multiErr, errFileNotExist, err) + return errors.Join(errFileNotExist, err) } } // validate flags _, err := cmd.Flags().GetString("output") if err != nil { - multiErr = errors.Join(multiErr, err) + return err } force, err := cmd.Flags().GetBool("force") if err != nil { - multiErr = errors.Join(multiErr, err) + return err } if !force { @@ -199,30 +194,27 @@ func newConvertCmd() *cobra.Command { av, err := cmd.Flags().GetString("aerospike-version") if err != nil { - multiErr = errors.Join(multiErr, errInvalidAerospikeVersion, err) + return err } if !force { if av == "" { - logger.Error("missing required flag '--aerospike-version'") - multiErr = errors.Join(multiErr, errInvalidAerospikeVersion, err) + return errors.Join(errMissingAerospikeVersion, err) } supported, err := asconfig.IsSupportedVersion(av) if err != nil { - logger.Errorf("Failed to check aerospike version %s for compatibility", av) - multiErr = errors.Join(multiErr, errInvalidAerospikeVersion, err) + return errors.Join(errInvalidAerospikeVersion, err) } // TODO include valid versions in the error message // asconfig lib needs a getSupportedVersions func if !supported { - logger.Errorf("Unsupported aerospike server version: %s", av) - multiErr = errors.Join(multiErr, errUnsupportedAerospikeVersion) + return errUnsupportedAerospikeVersion } } - return multiErr + return nil }, } diff --git a/cmd/convert_test.go b/cmd/convert_test.go index 9db7982..94e7eaf 100644 --- a/cmd/convert_test.go +++ b/cmd/convert_test.go @@ -25,27 +25,20 @@ var preTestsConvert = []preTestConvert{ arguments: []string{"./bad_file.yaml", "too_many"}, expectedErrors: []error{ errTooManyArguments, - errFileNotExist, - errInvalidAerospikeVersion, - errUnsupportedAerospikeVersion, }, }, { flags: []string{}, - arguments: []string{"./bad_file.yaml", "too_many"}, + arguments: []string{"./bad_file.yaml"}, expectedErrors: []error{ - errTooManyArguments, errFileNotExist, - errInvalidAerospikeVersion, - errUnsupportedAerospikeVersion, }, }, { flags: []string{}, arguments: []string{"./convert_test.go"}, expectedErrors: []error{ - errInvalidAerospikeVersion, - errUnsupportedAerospikeVersion, + errMissingAerospikeVersion, }, }, { diff --git a/cmd/diff.go b/cmd/diff.go index 3b5f439..7536aea 100644 --- a/cmd/diff.go +++ b/cmd/diff.go @@ -1,13 +1,15 @@ package cmd import ( + "errors" "fmt" - "github.com/aerospike/asconfig/asconf" "os" "reflect" "sort" "strings" + "github.com/aerospike/asconfig/asconf" + "github.com/spf13/cobra" ) @@ -17,7 +19,7 @@ const ( ) var ( - errDiffConfigsDiffer = fmt.Errorf("configuration files are not equal") + errDiffConfigsDiffer = errors.Join(fmt.Errorf("configuration files are not equal"), SilentError) errDiffTooFewArgs = fmt.Errorf("diff requires atleast %d file paths as arguments", diffArgMin) errDiffTooManyArgs = fmt.Errorf("diff requires no more than %d file paths as arguments", diffArgMax) ) @@ -120,8 +122,6 @@ func newDiffCmd() *cobra.Command { if len(diffs) > 0 { fmt.Printf("Differences shown from %s to %s, '<' are from file1, '>' are from file2.\n", path1, path2) fmt.Println(strings.Join(diffs, "")) - cmd.SilenceUsage = true - cmd.SilenceErrors = true return errDiffConfigsDiffer } diff --git a/cmd/root.go b/cmd/root.go index b039b83..2c566b6 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -1,8 +1,10 @@ package cmd import ( + "errors" "fmt" "os" + "strings" "github.com/aerospike/asconfig/log" @@ -72,6 +74,15 @@ func newRootCmd() *cobra.Command { res.PersistentFlags().BoolP("version", "V", false, "Version for asconfig.") res.PersistentFlags().StringP("format", "F", "conf", "The format of the source file(s). Valid options are: yaml, yml, and conf.") + res.SilenceErrors = true + res.SilenceUsage = true + + res.SetFlagErrorFunc(func(cmd *cobra.Command, err error) error { + logger.Error(err) + cmd.Println(cmd.UsageString()) + return errors.Join(err, SilentError) + }) + return res } @@ -80,6 +91,14 @@ func newRootCmd() *cobra.Command { func Execute() { err := rootCmd.Execute() if err != nil { + if !errors.Is(err, SilentError) { + // handle wrapped errors + errs := strings.Split(err.Error(), "\n") + + for _, err := range errs { + logger.Error(err) + } + } os.Exit(1) } } diff --git a/cmd/utils.go b/cmd/utils.go index 0fb1319..3c67cd1 100644 --- a/cmd/utils.go +++ b/cmd/utils.go @@ -1,10 +1,12 @@ package cmd import ( - "github.com/aerospike/asconfig/asconf" + "errors" "path/filepath" "strings" + "github.com/aerospike/asconfig/asconf" + "github.com/spf13/cobra" ) @@ -35,3 +37,5 @@ func getConfFileFormat(path string, cmd *cobra.Command) (asconf.Format, error) { return fmt, nil } + +var SilentError = errors.New("SILENT")