From 7b26ff5bece22ee954860e68f69f5d1b94407626 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Thu, 15 Jun 2023 16:45:29 +0200 Subject: [PATCH] chore(cosmovisor): remove cosmovisor error package (#16574) --- tools/cosmovisor/args.go | 21 +-- tools/cosmovisor/args_test.go | 94 +----------- tools/cosmovisor/cmd/cosmovisor/init.go | 3 +- tools/cosmovisor/cmd/cosmovisor/main.go | 11 +- tools/cosmovisor/errors/multi.go | 85 ----------- tools/cosmovisor/errors/multi_test.go | 190 ------------------------ 6 files changed, 15 insertions(+), 389 deletions(-) delete mode 100644 tools/cosmovisor/errors/multi.go delete mode 100644 tools/cosmovisor/errors/multi_test.go diff --git a/tools/cosmovisor/args.go b/tools/cosmovisor/args.go index 6f380b42d9..198924e016 100644 --- a/tools/cosmovisor/args.go +++ b/tools/cosmovisor/args.go @@ -2,6 +2,7 @@ package cosmovisor import ( "encoding/json" + "errors" "fmt" "net/url" "os" @@ -10,8 +11,6 @@ import ( "strings" "time" - "cosmossdk.io/log" - cverrors "cosmossdk.io/tools/cosmovisor/errors" "cosmossdk.io/x/upgrade/plan" upgradetypes "cosmossdk.io/x/upgrade/types" ) @@ -197,10 +196,10 @@ func GetConfigFromEnv() (*Config, error) { } errs = append(errs, cfg.validate()...) - if len(errs) > 0 { - return nil, cverrors.FlattenErrors(errs...) + return nil, errors.Join(errs...) } + return cfg, nil } @@ -217,20 +216,6 @@ func parseEnvDuration(input string) (time.Duration, error) { return duration, nil } -// LogConfigOrError logs either the config details or the error. -func LogConfigOrError(logger log.Logger, cfg *Config, err error) { - if cfg == nil && err == nil { - return - } - logger.Info("configuration:") - switch { - case err != nil: - cverrors.LogErrors(logger, "configuration errors found", err) - case cfg != nil: - logger.Info(cfg.DetailString()) - } -} - // validate returns an error if this config is invalid. // it enforces Home/cosmovisor is a valid directory and exists, // and that Name is set diff --git a/tools/cosmovisor/args_test.go b/tools/cosmovisor/args_test.go index f1b9e967fd..3edcdaf2b2 100644 --- a/tools/cosmovisor/args_test.go +++ b/tools/cosmovisor/args_test.go @@ -1,21 +1,16 @@ package cosmovisor import ( - "bytes" "fmt" - "io" "os" "path/filepath" "testing" "time" - "github.com/rs/zerolog" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" - "cosmossdk.io/log" - "cosmossdk.io/tools/cosmovisor/errors" "cosmossdk.io/x/upgrade/plan" ) @@ -632,8 +627,8 @@ func (s *argsTestSuite) TestGetConfigFromEnv() { assert.NoError(t, err) } else if assert.Error(t, err) { errCount := 1 - if multi, isMulti := err.(*errors.MultiError); isMulti { - errCount = multi.Len() + if errMulti, ok := err.(interface{ Unwrap() []error }); ok { + errCount = len(errMulti.Unwrap()) } assert.Equal(t, tc.expectedErrCount, errCount, "error count") } @@ -642,91 +637,6 @@ func (s *argsTestSuite) TestGetConfigFromEnv() { } } -func (s *argsTestSuite) TestLogConfigOrError() { - cfg := &Config{ - Home: "/no/place/like/it", - Name: "cosmotestvisor", - AllowDownloadBinaries: true, - RestartAfterUpgrade: true, - PollInterval: 999, - UnsafeSkipBackup: false, - DataBackupPath: "/no/place/like/it", - PreupgradeMaxRetries: 20, - } - errNormal := fmt.Errorf("this is a single error") - errs := []error{ - fmt.Errorf("multi-error error 1"), - fmt.Errorf("multi-error error 2"), - fmt.Errorf("multi-error error 3"), - } - errMulti := errors.FlattenErrors(errs...) - - makeTestLogger := func(testName string, out io.Writer) log.Logger { - output := zerolog.ConsoleWriter{Out: out, TimeFormat: time.Kitchen, NoColor: true} - logger := zerolog.New(output).With().Str("test", testName).Timestamp().Logger() - return log.NewCustomLogger(logger) - } - - tests := []struct { - name string - cfg *Config - err error - contains []string - notcontains []string - }{ - { - name: "normal error", - cfg: nil, - err: errNormal, - contains: []string{"configuration error", errNormal.Error()}, // TODO: Fix this. - notcontains: nil, - }, - { - name: "multi error", - cfg: nil, - err: errMulti, - contains: []string{"configuration errors found", errs[0].Error(), errs[1].Error(), errs[2].Error()}, - notcontains: nil, - }, - { - name: "config", - cfg: cfg, - err: nil, - contains: []string{"Configurable Values", cfg.DetailString()}, - notcontains: nil, - }, - { - name: "error and config - no config details", - cfg: cfg, - err: errNormal, - contains: []string{"error"}, - notcontains: []string{"Configuration is valid", EnvName, cfg.Home}, // Just some spot checks. - }, - { - name: "nil nil - no output", - cfg: nil, - err: nil, - contains: nil, - notcontains: []string{" "}, - }, - } - - for _, tc := range tests { - s.T().Run(tc.name, func(t *testing.T) { - var b bytes.Buffer - logger := makeTestLogger(tc.name, &b) - LogConfigOrError(logger, tc.cfg, tc.err) - output := b.String() - for _, expected := range tc.contains { - assert.Contains(t, output, expected) - } - for _, unexpected := range tc.notcontains { - assert.NotContains(t, output, unexpected) - } - }) - } -} - var sink interface{} func BenchmarkDetailString(b *testing.B) { diff --git a/tools/cosmovisor/cmd/cosmovisor/init.go b/tools/cosmovisor/cmd/cosmovisor/init.go index da5da92ec0..f085fe5154 100644 --- a/tools/cosmovisor/cmd/cosmovisor/init.go +++ b/tools/cosmovisor/cmd/cosmovisor/init.go @@ -11,7 +11,6 @@ import ( "cosmossdk.io/log" "cosmossdk.io/tools/cosmovisor" - cverrors "cosmossdk.io/tools/cosmovisor/errors" "cosmossdk.io/x/upgrade/plan" ) @@ -106,7 +105,7 @@ func getConfigForInitCmd() (*cosmovisor.Config, error) { errs = append(errs, fmt.Errorf("%s must be an absolute path", cosmovisor.EnvHome)) } if len(errs) > 0 { - return nil, cverrors.FlattenErrors(errs...) + return nil, errors.Join(errs...) } return cfg, nil } diff --git a/tools/cosmovisor/cmd/cosmovisor/main.go b/tools/cosmovisor/cmd/cosmovisor/main.go index 206802ac19..1b13832f49 100644 --- a/tools/cosmovisor/cmd/cosmovisor/main.go +++ b/tools/cosmovisor/cmd/cosmovisor/main.go @@ -5,7 +5,6 @@ import ( "os" "cosmossdk.io/log" - cverrors "cosmossdk.io/tools/cosmovisor/errors" ) func main() { @@ -13,7 +12,15 @@ func main() { ctx := context.WithValue(context.Background(), log.ContextKey, logger) if err := NewRootCmd().ExecuteContext(ctx); err != nil { - cverrors.LogErrors(logger, "", err) + if errMulti, ok := err.(interface{ Unwrap() []error }); ok { + err := errMulti.Unwrap() + for _, e := range err { + logger.Error("", "error", e) + } + } else { + logger.Error("", "error", err) + } + os.Exit(1) } } diff --git a/tools/cosmovisor/errors/multi.go b/tools/cosmovisor/errors/multi.go deleted file mode 100644 index efa3d5a9c2..0000000000 --- a/tools/cosmovisor/errors/multi.go +++ /dev/null @@ -1,85 +0,0 @@ -package errors - -import ( - "fmt" - "strings" - - "cosmossdk.io/log" -) - -// MultiError is an error combining multiple other errors. -// It will never have 0 or 1 errors. It will always have two or more. -type MultiError struct { - errs []error -} - -// FlattenErrors possibly creates a MultiError. -// Nil entries are ignored. -// If all provided errors are nil (or nothing is provided), nil is returned. -// If only one non-nil error is provided, it is returned unchanged. -// If two or more non-nil errors are provided, the returned error will be of type *MultiError -// and it will contain each non-nil error. -func FlattenErrors(errs ...error) error { - rv := MultiError{} - for _, err := range errs { - if err != nil { - if merr, isMerr := err.(*MultiError); isMerr { - rv.errs = append(rv.errs, merr.errs...) - } else { - rv.errs = append(rv.errs, err) - } - } - } - switch rv.Len() { - case 0: - return nil - case 1: - return rv.errs[0] - } - return &rv -} - -// GetErrors gets all the errors that make up this MultiError. -func (e MultiError) GetErrors() []error { - // Return a copy of the errs slice to prevent alteration of the original slice. - rv := make([]error, e.Len()) - copy(rv, e.errs) - return rv -} - -// Len gets the number of errors in this MultiError. -func (e MultiError) Len() int { - return len(e.errs) -} - -// Error implements the error interface for a MultiError. -func (e *MultiError) Error() string { - var sb strings.Builder - fmt.Fprintf(&sb, "%d errors: ", len(e.errs)) - for i, err := range e.errs { - if i != 0 { - sb.WriteString(", ") - } - fmt.Fprintf(&sb, "%d: %v", i+1, err) - } - return sb.String() -} - -// String implements the string interface for a MultiError. -func (e MultiError) String() string { - return e.Error() -} - -func LogErrors(logger log.Logger, msg string, err error) { - switch err := err.(type) { - case *MultiError: - if msg != "" { - logger.Error(msg) - } - for i, e := range err.GetErrors() { - logger.Error(fmt.Sprintf(" %d:", i+1), "error", e) - } - default: - logger.Error(msg, "error", err) - } -} diff --git a/tools/cosmovisor/errors/multi_test.go b/tools/cosmovisor/errors/multi_test.go deleted file mode 100644 index 83454b2cfb..0000000000 --- a/tools/cosmovisor/errors/multi_test.go +++ /dev/null @@ -1,190 +0,0 @@ -package errors - -import ( - "errors" - "fmt" - "testing" - - "github.com/stretchr/testify/require" - "github.com/stretchr/testify/suite" -) - -type MultiErrorTestSuite struct { - suite.Suite - - err1 error - err2 error - err3 error - err4 error -} - -func TestMultiErrorTestSuite(t *testing.T) { - suite.Run(t, new(MultiErrorTestSuite)) -} - -func (s *MultiErrorTestSuite) SetupTest() { - s.err1 = errors.New("expected error one") - s.err2 = errors.New("expected error two") - s.err3 = errors.New("expected error three") - s.err3 = errors.New("expected error four") -} - -func (s *MultiErrorTestSuite) TestFlattenErrors() { - tests := []struct { - name string - input []error - expected error - }{ - { - name: "none in nil out", - input: []error{}, - expected: nil, - }, - { - name: "nil in nil out", - input: []error{nil}, - expected: nil, - }, - { - name: "nils in nil out", - input: []error{nil, nil, nil}, - expected: nil, - }, - { - name: "one in same out", - input: []error{s.err1}, - expected: s.err1, - }, - { - name: "nils and one in that one out", - input: []error{nil, s.err2, nil}, - expected: s.err2, - }, - { - name: "two in multi out with both", - input: []error{s.err1, s.err2}, - expected: &MultiError{errs: []error{s.err1, s.err2}}, - }, - { - name: "two and nils in multi out with both", - input: []error{nil, s.err1, nil, s.err2, nil}, - expected: &MultiError{errs: []error{s.err1, s.err2}}, - }, - { - name: "lots in multi out", - input: []error{s.err1, s.err2, s.err3, s.err2, s.err1}, - expected: &MultiError{errs: []error{s.err1, s.err2, s.err3, s.err2, s.err1}}, - }, - { - name: "multi and non in one multi out with all", - input: []error{&MultiError{errs: []error{s.err1, s.err2}}, s.err3}, - expected: &MultiError{errs: []error{s.err1, s.err2, s.err3}}, - }, - { - name: "non and multi in one multi out with all", - input: []error{s.err1, &MultiError{errs: []error{s.err2, s.err3}}}, - expected: &MultiError{errs: []error{s.err1, s.err2, s.err3}}, - }, - { - name: "two multi in one multi out with all", - input: []error{&MultiError{errs: []error{s.err1, s.err2}}, &MultiError{errs: []error{s.err3, s.err4}}}, - expected: &MultiError{errs: []error{s.err1, s.err2, s.err3, s.err4}}, - }, - } - - for _, tc := range tests { - s.T().Run(tc.name, func(t *testing.T) { - actual := FlattenErrors(tc.input...) - require.Equal(t, tc.expected, actual) - }) - } -} - -func (s *MultiErrorTestSuite) TestGetErrors() { - tests := []struct { - name string - multi MultiError - expected []error - }{ - { - name: "two", - multi: MultiError{errs: []error{s.err3, s.err1}}, - expected: []error{s.err3, s.err1}, - }, - { - name: "three", - multi: MultiError{errs: []error{s.err3, s.err1, s.err2}}, - expected: []error{s.err3, s.err1, s.err2}, - }, - } - - for _, tc := range tests { - s.T().Run(tc.name, func(t *testing.T) { - // Make sure it's getting what's expected. - actual1 := tc.multi.GetErrors() - require.NotSame(t, tc.expected, actual1) - require.Equal(t, tc.expected, actual1) - // Make sure that changing what was given back doesn't alter the original. - actual1[0] = errors.New("unexpected error") - actual2 := tc.multi.GetErrors() - require.NotEqual(t, actual1, actual2) - require.Equal(t, tc.expected, actual2) - }) - } -} - -func (s *MultiErrorTestSuite) TestLen() { - tests := []struct { - name string - multi MultiError - expected int - }{ - { - name: "two", - multi: MultiError{errs: []error{s.err3, s.err1}}, - expected: 2, - }, - { - name: "three", - multi: MultiError{errs: []error{s.err3, s.err1, s.err2}}, - expected: 3, - }, - } - - for _, tc := range tests { - s.T().Run(tc.name, func(t *testing.T) { - actual := tc.multi.Len() - require.Equal(t, tc.expected, actual) - }) - } -} - -func (s *MultiErrorTestSuite) TestErrorAndString() { - tests := []struct { - name string - multi MultiError - expected string - }{ - { - name: "two", - multi: MultiError{errs: []error{s.err1, s.err2}}, - expected: fmt.Sprintf("2 errors: 1: %s, 2: %s", s.err1, s.err2), - }, - { - name: "three", - multi: MultiError{errs: []error{s.err1, s.err2, s.err3}}, - expected: fmt.Sprintf("3 errors: 1: %s, 2: %s, 3: %s", s.err1, s.err2, s.err3), - }, - } - - for _, tc := range tests { - s.T().Run(tc.name+" Error", func(t *testing.T) { - actual := tc.multi.Error() - require.Equal(t, tc.expected, actual) - }) - s.T().Run(tc.name+" String", func(t *testing.T) { - actual := tc.multi.String() - require.Equal(t, tc.expected, actual) - }) - } -}