From 0d19892e038bc142fc31d8df7c0f2d0f55f1449d Mon Sep 17 00:00:00 2001 From: Jim Date: Sun, 17 Sep 2023 19:35:43 -0400 Subject: [PATCH] fix (events): convert multierror to stdlib error (#3743) * fix (events): convert multierror to stdlib error multierror errors will be converted to a stdlib error (because multierror doesn't support json marshaling). * refact (errors): convert from multierror to stdlib errors.Join(...) * refact: convert from pkg/errors to stdlib error wrapping --- go.mod | 2 +- internal/auth/oidc/auth_method.go | 18 +++++++-------- .../oidc/repository_auth_method_update.go | 22 +++++++++---------- internal/bsr/internal/checksum/checksum.go | 17 +++++++------- .../bsr/internal/checksum/checksum_test.go | 19 +++------------- internal/bsr/internal/sign/sign.go | 17 +++++++------- internal/bsr/internal/sign/sign_test.go | 13 ++--------- internal/cmd/base/base.go | 4 ++-- internal/cmd/base/dev.go | 12 +++++----- internal/cmd/base/format.go | 2 +- internal/cmd/base/keyring.go | 2 +- internal/cmd/base/servers.go | 7 +++--- internal/cmd/commands/server/server.go | 11 +++++----- internal/cmd/ops/server.go | 11 +++++----- .../handlers/targets/target_service.go | 5 ++--- internal/daemon/controller/listeners.go | 18 ++++++++------- internal/daemon/worker/listeners.go | 5 +++-- internal/db/read_writer.go | 8 +++---- .../db/schema/internal/postgres/postgres.go | 3 +-- internal/errors/match_test.go | 5 ++--- .../event/cloudevents_formatter_node_test.go | 6 +++-- internal/event/context.go | 4 +++- internal/event/event_error.go | 8 +++++++ internal/event/event_error_test.go | 22 +++++++++++++++++++ internal/event/eventer.go | 7 +++--- internal/event/eventer_retry.go | 10 ++++----- internal/event/eventer_retry_test.go | 22 +++---------------- .../session/service_worker_status_report.go | 14 ++++++------ 28 files changed, 142 insertions(+), 152 deletions(-) diff --git a/go.mod b/go.mod index 845a99a488..11ba5bddf8 100644 --- a/go.mod +++ b/go.mod @@ -63,7 +63,7 @@ require ( github.com/oligot/go-mod-upgrade v0.9.1 github.com/ory/dockertest/v3 v3.9.1 github.com/pires/go-proxyproto v0.7.0 - github.com/pkg/errors v0.9.1 + github.com/pkg/errors v0.9.1 // indirect github.com/posener/complete v1.2.3 github.com/prometheus/client_golang v1.12.1 github.com/ryanuber/go-glob v1.0.0 diff --git a/internal/auth/oidc/auth_method.go b/internal/auth/oidc/auth_method.go index 133d8ae964..416132efa7 100644 --- a/internal/auth/oidc/auth_method.go +++ b/internal/auth/oidc/auth_method.go @@ -5,6 +5,7 @@ package oidc import ( "context" + stderrors "errors" "fmt" "net/url" "sort" @@ -16,7 +17,6 @@ import ( "github.com/hashicorp/boundary/internal/oplog" wrapping "github.com/hashicorp/go-kms-wrapping/v2" "github.com/hashicorp/go-kms-wrapping/v2/extras/structwrapping" - "github.com/hashicorp/go-multierror" kvbuilder "github.com/hashicorp/go-secure-stdlib/kv-builder" "google.golang.org/protobuf/proto" ) @@ -254,26 +254,26 @@ func (a *AuthMethod) hmacClientSecret(ctx context.Context, cipher wrapping.Wrapp // components of a complete/valid oidc auth method. func (am *AuthMethod) isComplete(ctx context.Context) error { const op = "oidc.(AuthMethod).isComplete" - var result *multierror.Error + var result error if err := am.validate(ctx, op); err != nil { - result = multierror.Append(result, errors.Wrap(ctx, err, op)) + result = stderrors.Join(result, errors.Wrap(ctx, err, op)) } if am.Issuer == "" { - result = multierror.Append(result, errors.New(ctx, errors.InvalidParameter, op, "missing issuer")) + result = stderrors.Join(result, errors.New(ctx, errors.InvalidParameter, op, "missing issuer")) } if am.ApiUrl == "" { - result = multierror.Append(result, errors.New(ctx, errors.InvalidParameter, op, "missing api url")) + result = stderrors.Join(result, errors.New(ctx, errors.InvalidParameter, op, "missing api url")) } if am.ClientId == "" { - result = multierror.Append(result, errors.New(ctx, errors.InvalidParameter, op, "missing client id")) + result = stderrors.Join(result, errors.New(ctx, errors.InvalidParameter, op, "missing client id")) } if am.ClientSecret == "" { - result = multierror.Append(result, errors.New(ctx, errors.InvalidParameter, op, "missing client secret")) + result = stderrors.Join(result, errors.New(ctx, errors.InvalidParameter, op, "missing client secret")) } if len(am.SigningAlgs) == 0 { - result = multierror.Append(result, errors.New(ctx, errors.InvalidParameter, op, "missing signing algorithms")) + result = stderrors.Join(result, errors.New(ctx, errors.InvalidParameter, op, "missing signing algorithms")) } - return result.ErrorOrNil() + return result } type convertedValues struct { diff --git a/internal/auth/oidc/repository_auth_method_update.go b/internal/auth/oidc/repository_auth_method_update.go index 4c8f46a6a9..c24d184dc7 100644 --- a/internal/auth/oidc/repository_auth_method_update.go +++ b/internal/auth/oidc/repository_auth_method_update.go @@ -5,6 +5,7 @@ package oidc import ( "context" + stderrors "errors" "fmt" "net/http" "strings" @@ -14,7 +15,6 @@ import ( "github.com/hashicorp/boundary/internal/kms" "github.com/hashicorp/boundary/internal/oplog" "github.com/hashicorp/go-dbw" - "github.com/hashicorp/go-multierror" "github.com/hashicorp/go-secure-stdlib/strutil" ) @@ -699,21 +699,21 @@ func (r *Repository) ValidateDiscoveryInfo(ctx context.Context, opt ...Option) e return errors.Wrap(ctx, err, op) } - var result *multierror.Error + var result error if info.Issuer != am.Issuer { - result = multierror.Append(result, errors.New(ctx, errors.InvalidParameter, op, + result = stderrors.Join(result, errors.New(ctx, errors.InvalidParameter, op, fmt.Sprintf("auth method issuer doesn't match discovery issuer: expected %s and got %s", am.Issuer, info.Issuer))) } for _, a := range am.SigningAlgs { if !strutil.StrListContains(info.IdTokenSigningAlgsSupported, a) { - result = multierror.Append(result, errors.New(ctx, errors.InvalidParameter, op, + result = stderrors.Join(result, errors.New(ctx, errors.InvalidParameter, op, fmt.Sprintf("auth method signing alg is not in discovered supported algs: expected %s and got %s", a, info.IdTokenSigningAlgsSupported))) } } providerClient, err := provider.HTTPClient() if err != nil { - result = multierror.Append(result, errors.New(ctx, errors.Unknown, op, "unable to get oidc http client", errors.WithWrap(err))) - return result.ErrorOrNil() + result = stderrors.Join(result, errors.New(ctx, errors.Unknown, op, "unable to get oidc http client", errors.WithWrap(err))) + return result } // we need to prevent redirects during these tests... we don't want to have @@ -725,25 +725,25 @@ func (r *Repository) ValidateDiscoveryInfo(ctx context.Context, opt ...Option) e // test JWKs URL statusCode, err := pingEndpoint(ctx, providerClient, "JWKs", "GET", info.JWKSURL) if err != nil { - result = multierror.Append(result, errors.New(ctx, errors.Unknown, op, fmt.Sprintf("unable to verify JWKs endpoint: %s", info.JWKSURL), errors.WithWrap(err))) + result = stderrors.Join(result, errors.New(ctx, errors.Unknown, op, fmt.Sprintf("unable to verify JWKs endpoint: %s", info.JWKSURL), errors.WithWrap(err))) } if statusCode != 200 { - result = multierror.Append(result, errors.New(ctx, errors.Unknown, op, fmt.Sprintf("non-200 status (%d) from JWKs endpoint: %s", statusCode, info.JWKSURL), errors.WithWrap(err))) + result = stderrors.Join(result, errors.New(ctx, errors.Unknown, op, fmt.Sprintf("non-200 status (%d) from JWKs endpoint: %s", statusCode, info.JWKSURL), errors.WithWrap(err))) } // test Auth URL if _, err := pingEndpoint(ctx, providerClient, "AuthURL", "GET", info.AuthURL); err != nil { - result = multierror.Append(result, errors.New(ctx, errors.Unknown, op, fmt.Sprintf("unable to verify authorize endpoint: %s", info.AuthURL), errors.WithWrap(err))) + result = stderrors.Join(result, errors.New(ctx, errors.Unknown, op, fmt.Sprintf("unable to verify authorize endpoint: %s", info.AuthURL), errors.WithWrap(err))) } // test Token URL if _, err := pingEndpoint(ctx, providerClient, "TokenURL", "POST", info.TokenURL); err != nil { - result = multierror.Append(result, errors.New(ctx, errors.Unknown, op, fmt.Sprintf("unable to verify token endpoint: %s", info.TokenURL), errors.WithWrap(err))) + result = stderrors.Join(result, errors.New(ctx, errors.Unknown, op, fmt.Sprintf("unable to verify token endpoint: %s", info.TokenURL), errors.WithWrap(err))) } // we're not verifying the UserInfo URL, since it's not a required dependency. - return result.ErrorOrNil() + return result } type HTTPClient interface { diff --git a/internal/bsr/internal/checksum/checksum.go b/internal/bsr/internal/checksum/checksum.go index f4a3b3adf3..62c7f0c352 100644 --- a/internal/bsr/internal/checksum/checksum.go +++ b/internal/bsr/internal/checksum/checksum.go @@ -15,7 +15,6 @@ import ( "github.com/hashicorp/boundary/internal/bsr/internal/is" "github.com/hashicorp/go-kms-wrapping/v2/extras/crypto" - "github.com/hashicorp/go-multierror" ) const ( @@ -88,30 +87,30 @@ func (f *File) Read(b []byte) (int, error) { func (f *File) Close() error { const op = "checksum.(File).Close" - var closeErrors *multierror.Error + var closeErrors error // Call stat before closure; calling it after results in an err s, err := f.Stat() if err != nil { - closeErrors = multierror.Append(closeErrors, fmt.Errorf("%s: %w", op, err)) - return closeErrors.ErrorOrNil() + closeErrors = errors.Join(closeErrors, fmt.Errorf("%s: %w", op, err)) + return closeErrors } // f.Sha256SumWriter will close f.underlying if err := f.Sha256SumWriter.Close(); err != nil { - closeErrors = multierror.Append(closeErrors, fmt.Errorf("%s: %w", op, err)) + closeErrors = errors.Join(closeErrors, fmt.Errorf("%s: %w", op, err)) } sum, err := f.Sha256SumWriter.Sum(f.ctx, crypto.WithHexEncoding(true)) if err != nil { - closeErrors = multierror.Append(closeErrors, fmt.Errorf("%s: %w", op, err)) - return closeErrors.ErrorOrNil() + closeErrors = errors.Join(closeErrors, fmt.Errorf("%s: %w", op, err)) + return closeErrors } if _, err := f.checksumWriter.Write([]byte(fmt.Sprintf(checksumLine, sum, s.Name()))); err != nil { - closeErrors = multierror.Append(closeErrors, fmt.Errorf("%s: %w", op, err)) + closeErrors = errors.Join(closeErrors, fmt.Errorf("%s: %w", op, err)) } - return closeErrors.ErrorOrNil() + return closeErrors } var _ writerFile = (*File)(nil) diff --git a/internal/bsr/internal/checksum/checksum_test.go b/internal/bsr/internal/checksum/checksum_test.go index 3336451bf0..225c33ae3b 100644 --- a/internal/bsr/internal/checksum/checksum_test.go +++ b/internal/bsr/internal/checksum/checksum_test.go @@ -13,7 +13,6 @@ import ( "github.com/hashicorp/boundary/internal/bsr/internal/checksum" "github.com/hashicorp/boundary/internal/bsr/internal/fstest" - "github.com/hashicorp/go-multierror" "github.com/stretchr/testify/require" ) @@ -88,11 +87,7 @@ func TestFile(t *testing.T) { testString, testSum + " test\n", nil, - func() error { - var wantErrors *multierror.Error - wantErrors = multierror.Append(wantErrors, errors.New("checksum.(File).Close: crypto.(Sha256SumWriter).Close: close error")) - return wantErrors.ErrorOrNil() - }(), + errors.New("checksum.(File).Close: crypto.(Sha256SumWriter).Close: close error"), true, }, { @@ -102,11 +97,7 @@ func TestFile(t *testing.T) { testString, testSum + " test\n", nil, - func() error { - var wantErrors *multierror.Error - wantErrors = multierror.Append(wantErrors, errors.New("checksum.(File).Close: stat error")) - return wantErrors.ErrorOrNil() - }(), + errors.New("checksum.(File).Close: stat error"), false, }, { @@ -116,11 +107,7 @@ func TestFile(t *testing.T) { testString, testSum + " test\n", nil, - func() error { - var wantErrors *multierror.Error - wantErrors = multierror.Append(wantErrors, errors.New("checksum.(File).Close: write failed")) - return wantErrors.ErrorOrNil() - }(), + errors.New("checksum.(File).Close: write failed"), false, }, } diff --git a/internal/bsr/internal/sign/sign.go b/internal/bsr/internal/sign/sign.go index 015d0f64ac..8adfec2770 100644 --- a/internal/bsr/internal/sign/sign.go +++ b/internal/bsr/internal/sign/sign.go @@ -16,7 +16,6 @@ import ( "github.com/hashicorp/boundary/internal/bsr/internal/is" "github.com/hashicorp/boundary/internal/bsr/kms" wrapping "github.com/hashicorp/go-kms-wrapping/v2" - "github.com/hashicorp/go-multierror" "google.golang.org/protobuf/proto" ) @@ -142,28 +141,28 @@ func NewFile(ctx context.Context, f writerFile, w io.Writer, keys *kms.Keys) (*F func (f *File) Close() error { const op = "sign.(File).Close" - var closeErrors *multierror.Error + var closeErrors error if err := f.Writer.Close(); err != nil { - closeErrors = multierror.Append(closeErrors, fmt.Errorf("%s: %w", op, err)) + closeErrors = errors.Join(closeErrors, fmt.Errorf("%s: %w", op, err)) } sig, err := f.Writer.Sign(f.ctx) if err != nil { - closeErrors = multierror.Append(closeErrors, fmt.Errorf("%s: %w", op, err)) - return closeErrors.ErrorOrNil() + closeErrors = errors.Join(closeErrors, fmt.Errorf("%s: %w", op, err)) + return closeErrors } b, err := proto.Marshal(sig) if err != nil { - closeErrors = multierror.Append(closeErrors, fmt.Errorf("%s: %w", op, err)) - return closeErrors.ErrorOrNil() + closeErrors = errors.Join(closeErrors, fmt.Errorf("%s: %w", op, err)) + return closeErrors } if _, err := f.signatureWriter.Write(b); err != nil { - closeErrors = multierror.Append(closeErrors, fmt.Errorf("%s: %w", op, err)) + closeErrors = errors.Join(closeErrors, fmt.Errorf("%s: %w", op, err)) } - return closeErrors.ErrorOrNil() + return closeErrors } var _ writerFile = (*File)(nil) diff --git a/internal/bsr/internal/sign/sign_test.go b/internal/bsr/internal/sign/sign_test.go index 6a1240010a..7e4add26c8 100644 --- a/internal/bsr/internal/sign/sign_test.go +++ b/internal/bsr/internal/sign/sign_test.go @@ -16,7 +16,6 @@ import ( "github.com/hashicorp/boundary/internal/bsr/internal/sign" "github.com/hashicorp/boundary/internal/bsr/kms" wrapping "github.com/hashicorp/go-kms-wrapping/v2" - "github.com/hashicorp/go-multierror" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "google.golang.org/protobuf/proto" @@ -111,11 +110,7 @@ func TestFile(t *testing.T) { testString, testSig, nil, - func() error { - var wantErrors *multierror.Error - wantErrors = multierror.Append(wantErrors, errors.New("sign.(File).Close: sign.(Writer).Close: close error")) - return wantErrors.ErrorOrNil() - }(), + errors.New("sign.(File).Close: sign.(Writer).Close: close error"), true, }, { @@ -126,11 +121,7 @@ func TestFile(t *testing.T) { testString, testSig, nil, - func() error { - var wantErrors *multierror.Error - wantErrors = multierror.Append(wantErrors, errors.New("sign.(File).Close: write failed")) - return wantErrors.ErrorOrNil() - }(), + errors.New("sign.(File).Close: write failed"), false, }, } diff --git a/internal/cmd/base/base.go b/internal/cmd/base/base.go index cbd9e8218d..2a9321ad5b 100644 --- a/internal/cmd/base/base.go +++ b/internal/cmd/base/base.go @@ -6,6 +6,7 @@ package base import ( "bytes" "context" + "errors" "flag" "fmt" "io" @@ -27,7 +28,6 @@ import ( "github.com/hashicorp/go-secure-stdlib/parseutil" "github.com/hashicorp/go-secure-stdlib/pluginutil/v2" "github.com/mitchellh/cli" - "github.com/pkg/errors" "github.com/posener/complete" ) @@ -246,7 +246,7 @@ func (c *Command) Client(opt ...Option) (*api.Client, error) { if modifiedTLS { // Setup TLS config if err := c.client.SetTLSConfig(tlsConfig); err != nil { - return nil, errors.Wrap(err, "failed to setup TLS config") + return nil, fmt.Errorf("failed to setup TLS config: %w", err) } } diff --git a/internal/cmd/base/dev.go b/internal/cmd/base/dev.go index 9f3f04473e..51071f246d 100644 --- a/internal/cmd/base/dev.go +++ b/internal/cmd/base/dev.go @@ -6,6 +6,7 @@ package base import ( "context" "crypto/ed25519" + "errors" "fmt" "net" "net/url" @@ -24,7 +25,6 @@ import ( "github.com/hashicorp/boundary/internal/types/scope" "github.com/hashicorp/boundary/testing/dbtest" capoidc "github.com/hashicorp/cap/oidc" - "github.com/hashicorp/go-multierror" "github.com/jimlambrt/gldap" "github.com/jimlambrt/gldap/testdirectory" ) @@ -75,7 +75,7 @@ func (b *Server) CreateDevDatabase(ctx context.Context, opt ...Option) error { if err != nil { err = fmt.Errorf("unable to initialize dev database with dialect %s: %w", dialect, err) if c != nil { - err = multierror.Append(err, c()) + err = errors.Join(err, c()) } return err } @@ -87,7 +87,7 @@ func (b *Server) CreateDevDatabase(ctx context.Context, opt ...Option) error { if _, err := schema.MigrateStore(ctx, schema.Dialect(dialect), b.DatabaseUrl); err != nil { err = fmt.Errorf("error initializing store: %w", err) if c != nil { - err = multierror.Append(err, c()) + err = errors.Join(err, c()) } return err } @@ -102,21 +102,21 @@ func (b *Server) CreateDevDatabase(ctx context.Context, opt ...Option) error { if err := b.OpenAndSetServerDatabase(ctx, dialect); err != nil { if c != nil { - err = multierror.Append(err, c()) + err = errors.Join(err, c()) } return err } if err := b.CreateGlobalKmsKeys(ctx); err != nil { if c != nil { - err = multierror.Append(err, c()) + err = errors.Join(err, c()) } return err } if _, err := b.CreateInitialLoginRole(ctx); err != nil { if c != nil { - err = multierror.Append(err, c()) + err = errors.Join(err, c()) } return err } diff --git a/internal/cmd/base/format.go b/internal/cmd/base/format.go index 3b5e9b210a..5028a8aec8 100644 --- a/internal/cmd/base/format.go +++ b/internal/cmd/base/format.go @@ -5,6 +5,7 @@ package base import ( "encoding/json" + "errors" "fmt" "os" "sort" @@ -18,7 +19,6 @@ import ( "github.com/hashicorp/boundary/version" "github.com/mitchellh/cli" "github.com/mitchellh/go-wordwrap" - "github.com/pkg/errors" ) // This is adapted from the code in the strings package for TrimSpace diff --git a/internal/cmd/base/keyring.go b/internal/cmd/base/keyring.go index 53c4703d41..42d8c765cb 100644 --- a/internal/cmd/base/keyring.go +++ b/internal/cmd/base/keyring.go @@ -6,6 +6,7 @@ package base import ( "encoding/base64" "encoding/json" + "errors" "fmt" "os" "runtime" @@ -13,7 +14,6 @@ import ( "github.com/hashicorp/boundary/api/authtokens" nkeyring "github.com/jefferai/keyring" - "github.com/pkg/errors" zkeyring "github.com/zalando/go-keyring" ) diff --git a/internal/cmd/base/servers.go b/internal/cmd/base/servers.go index c632d06b62..994b99cebc 100644 --- a/internal/cmd/base/servers.go +++ b/internal/cmd/base/servers.go @@ -38,7 +38,6 @@ import ( "github.com/hashicorp/go-hclog" wrapping "github.com/hashicorp/go-kms-wrapping/v2" "github.com/hashicorp/go-kms-wrapping/v2/extras/multi" - "github.com/hashicorp/go-multierror" configutil "github.com/hashicorp/go-secure-stdlib/configutil/v2" "github.com/hashicorp/go-secure-stdlib/gatedwriter" "github.com/hashicorp/go-secure-stdlib/listenerutil" @@ -722,13 +721,13 @@ func (b *Server) SetupKMSes(ctx context.Context, ui cli.Ui, config *config.Confi } func (b *Server) RunShutdownFuncs() error { - var mErr *multierror.Error + var mErr error for _, f := range b.ShutdownFuncs { if err := f(); err != nil { - mErr = multierror.Append(mErr, err) + mErr = errors.Join(mErr, err) } } - return mErr.ErrorOrNil() + return mErr } // OpenAndSetServerDatabase calls OpenDatabase and sets its result *db.DB to the Server's diff --git a/internal/cmd/commands/server/server.go b/internal/cmd/commands/server/server.go index a675970961..8a0702a1ae 100644 --- a/internal/cmd/commands/server/server.go +++ b/internal/cmd/commands/server/server.go @@ -27,7 +27,6 @@ import ( "github.com/hashicorp/boundary/internal/event" "github.com/hashicorp/boundary/internal/kms" "github.com/hashicorp/go-hclog" - "github.com/hashicorp/go-multierror" "github.com/hashicorp/go-secure-stdlib/mlock" "github.com/hashicorp/go-secure-stdlib/parseutil" "github.com/hashicorp/go-uuid" @@ -807,19 +806,19 @@ func (c *Command) Reload(newConf *config.Config) error { c.ReloadFuncsLock.RLock() defer c.ReloadFuncsLock.RUnlock() - var reloadErrors *multierror.Error + var reloadErrors error for _, relFunc := range c.ReloadFuncs["listeners"] { if relFunc != nil { if err := relFunc(); err != nil { - reloadErrors = multierror.Append(reloadErrors, fmt.Errorf("error encountered reloading listener: %w", err)) + reloadErrors = stderrors.Join(reloadErrors, fmt.Errorf("error encountered reloading listener: %w", err)) } } } err := c.reloadControllerDatabase(newConf) if err != nil { - reloadErrors = multierror.Append(reloadErrors, fmt.Errorf("failed to reload controller database: %w", err)) + reloadErrors = stderrors.Join(reloadErrors, fmt.Errorf("failed to reload controller database: %w", err)) } if newConf != nil && c.worker != nil { @@ -837,7 +836,7 @@ func (c *Command) Reload(newConf *config.Config) error { return nil }() if workerReloadErr != nil { - reloadErrors = multierror.Append(reloadErrors, fmt.Errorf("error encountered reloading worker initial upstreams: %w", workerReloadErr)) + reloadErrors = stderrors.Join(reloadErrors, fmt.Errorf("error encountered reloading worker initial upstreams: %w", workerReloadErr)) } } @@ -850,7 +849,7 @@ func (c *Command) Reload(newConf *config.Config) error { } } - return reloadErrors.ErrorOrNil() + return reloadErrors } func verifyKmsSetup(dbase *db.DB) error { diff --git a/internal/cmd/ops/server.go b/internal/cmd/ops/server.go index c003e1c1e6..75a69e0448 100644 --- a/internal/cmd/ops/server.go +++ b/internal/cmd/ops/server.go @@ -7,6 +7,7 @@ package ops import ( "context" + "errors" "fmt" "net" "net/http" @@ -18,10 +19,8 @@ import ( "github.com/hashicorp/boundary/internal/daemon/worker" "github.com/hashicorp/go-cleanhttp" "github.com/hashicorp/go-hclog" - "github.com/hashicorp/go-multierror" "github.com/hashicorp/go-secure-stdlib/listenerutil" "github.com/mitchellh/cli" - "github.com/pkg/errors" "github.com/prometheus/client_golang/prometheus/promhttp" ) @@ -89,7 +88,7 @@ func (s *Server) Start() { func (s *Server) Shutdown() error { const op = "ops.(Server).Shutdown" - var closeErrors *multierror.Error + var closeErrors error for _, b := range s.bundles { if b == nil || b.ln == nil || b.ln.Config == nil || b.ln.OpsListener == nil || b.ln.HTTPServer == nil { return fmt.Errorf("%s: missing bundle, listener or its fields", op) @@ -100,17 +99,17 @@ func (s *Server) Shutdown() error { err := b.ln.HTTPServer.Shutdown(ctx) if err != nil { - multierror.Append(closeErrors, fmt.Errorf("%s: failed to shutdown http server: %w", op, err)) + errors.Join(closeErrors, fmt.Errorf("%s: failed to shutdown http server: %w", op, err)) } err = b.ln.OpsListener.Close() err = listenerCloseErrorCheck(b.ln.Config.Type, err) if err != nil { - multierror.Append(closeErrors, fmt.Errorf("%s: failed to close listener mux: %w", op, err)) + errors.Join(closeErrors, fmt.Errorf("%s: failed to close listener mux: %w", op, err)) } } - return closeErrors.ErrorOrNil() + return closeErrors } // WaitIfHealthExists waits for a configurable period of time `d` if the health endpoint has been diff --git a/internal/daemon/controller/handlers/targets/target_service.go b/internal/daemon/controller/handlers/targets/target_service.go index 5163b69ac3..692f3338c3 100644 --- a/internal/daemon/controller/handlers/targets/target_service.go +++ b/internal/daemon/controller/handlers/targets/target_service.go @@ -42,7 +42,6 @@ import ( pb "github.com/hashicorp/boundary/sdk/pbs/controller/api/resources/targets" fm "github.com/hashicorp/boundary/version" "github.com/hashicorp/go-bexpr" - "github.com/hashicorp/go-multierror" "github.com/hashicorp/go-secure-stdlib/strutil" "github.com/mr-tron/base58" "google.golang.org/grpc/codes" @@ -965,7 +964,7 @@ func (s Service) AuthorizeSession(ctx context.Context, req *pbs.AuthorizeSession deleteCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second) defer cancel() _, err := sessionRepo.DeleteSession(deleteCtx, sess.PublicId) - retErr = multierror.Append(retErr, err) + retErr = stderrors.Join(retErr, err) } }() @@ -996,7 +995,7 @@ func (s Service) AuthorizeSession(ctx context.Context, req *pbs.AuthorizeSession deleteCtx, cancel := context.WithTimeout(context.Background(), time.Minute) defer cancel() err := credRepo.Revoke(deleteCtx, sess.PublicId) - retErr = multierror.Append(retErr, err) + retErr = stderrors.Join(retErr, err) // This leaves the credential in a state which will allow it to be cleaned up // by the periodic credential cleanup job. } diff --git a/internal/daemon/controller/listeners.go b/internal/daemon/controller/listeners.go index 46d3008bf0..b5d6966657 100644 --- a/internal/daemon/controller/listeners.go +++ b/internal/daemon/controller/listeners.go @@ -6,6 +6,7 @@ package controller import ( "context" "crypto/tls" + stderrors "errors" "fmt" "math" "net" @@ -295,13 +296,14 @@ func (c *Controller) stopServersAndListeners() error { mg.Go(c.stopApiGrpcServerAndListener) stopErrors := mg.Wait() + convertedStopErrors := stopErrors.ErrorOrNil() err := c.stopAnyListeners() if err != nil { - stopErrors = multierror.Append(stopErrors, err) + convertedStopErrors = stderrors.Join(convertedStopErrors, err) } - return stopErrors.ErrorOrNil() + return convertedStopErrors } func (c *Controller) stopClusterGrpcServerAndListener() error { @@ -321,7 +323,7 @@ func (c *Controller) stopClusterGrpcServerAndListener() error { } func (c *Controller) stopHttpServersAndListeners() error { - var closeErrors *multierror.Error + var closeErrors error for i := range c.apiListeners { ln := c.apiListeners[i] if ln.HTTPServer == nil { @@ -335,11 +337,11 @@ func (c *Controller) stopHttpServersAndListeners() error { err := ln.ApiListener.Close() // The HTTP Shutdown call should close this, but just in case. err = listenerCloseErrorCheck(ln.Config.Type, err) if err != nil { - multierror.Append(closeErrors, err) + closeErrors = stderrors.Join(closeErrors, err) } } - return closeErrors.ErrorOrNil() + return closeErrors } func (c *Controller) stopApiGrpcServerAndListener() error { @@ -357,7 +359,7 @@ func (c *Controller) stopApiGrpcServerAndListener() error { // listeners to make sure we didn't miss any; // expected to run at the end of stopServersAndListeners. func (c *Controller) stopAnyListeners() error { - var closeErrors *multierror.Error + var closeErrors error for i := range c.apiListeners { ln := c.apiListeners[i] if ln == nil || ln.ApiListener == nil { @@ -367,11 +369,11 @@ func (c *Controller) stopAnyListeners() error { err := ln.ApiListener.Close() err = listenerCloseErrorCheck(ln.Config.Type, err) if err != nil { - multierror.Append(closeErrors, err) + closeErrors = stderrors.Join(closeErrors, err) } } - return closeErrors.ErrorOrNil() + return closeErrors } // listenerCloseErrorCheck does some validation on an error returned diff --git a/internal/daemon/worker/listeners.go b/internal/daemon/worker/listeners.go index f53cc19240..303ca7dfbb 100644 --- a/internal/daemon/worker/listeners.go +++ b/internal/daemon/worker/listeners.go @@ -309,13 +309,14 @@ func (w *Worker) stopServersAndListeners() error { mg.Go(w.stopClusterGrpcServer) stopErrors := mg.Wait() + convertedStopErrors := stopErrors.ErrorOrNil() err := w.stopAnyListeners() if err != nil { - stopErrors = multierror.Append(stopErrors, err) + convertedStopErrors = stderrors.Join(convertedStopErrors, err) } - return stopErrors.ErrorOrNil() + return convertedStopErrors } func (w *Worker) stopHttpServer() error { diff --git a/internal/db/read_writer.go b/internal/db/read_writer.go index fa14db8a86..adde78dc4b 100644 --- a/internal/db/read_writer.go +++ b/internal/db/read_writer.go @@ -6,6 +6,7 @@ package db import ( "context" "database/sql" + stderrors "errors" "fmt" "reflect" "sync/atomic" @@ -16,7 +17,6 @@ import ( "github.com/hashicorp/boundary/internal/oplog/store" "github.com/hashicorp/go-dbw" wrapping "github.com/hashicorp/go-kms-wrapping/v2" - "github.com/hashicorp/go-multierror" ) const ( @@ -442,14 +442,14 @@ func (rw *Db) DoTx(ctx context.Context, retries uint, backOff Backoff, handler T return info, errors.Wrap(ctx, err, op, errors.WithoutEvent()) } - var txnErr *multierror.Error + var txnErr error if commitErr := beginTx.Commit(ctx); commitErr != nil { - txnErr = multierror.Append(txnErr, errors.Wrap(ctx, commitErr, op, errors.WithMsg("commit error"))) + txnErr = stderrors.Join(txnErr, errors.Wrap(ctx, commitErr, op, errors.WithMsg("commit error"))) // unsure if rolling back is required or possible, but including // this attempt to rollback on a commit error just in case it's // possible. if err := beginTx.Rollback(ctx); err != nil { - return info, multierror.Append(txnErr, errors.Wrap(ctx, err, op, errors.WithMsg("rollback error"))) + return info, stderrors.Join(txnErr, errors.Wrap(ctx, err, op, errors.WithMsg("rollback error"))) } return info, txnErr } diff --git a/internal/db/schema/internal/postgres/postgres.go b/internal/db/schema/internal/postgres/postgres.go index 01ee143cb2..e375c4e51b 100644 --- a/internal/db/schema/internal/postgres/postgres.go +++ b/internal/db/schema/internal/postgres/postgres.go @@ -40,7 +40,6 @@ import ( "github.com/hashicorp/boundary/internal/db/schema/migration" "github.com/hashicorp/boundary/internal/db/schema/migrations" "github.com/hashicorp/boundary/internal/errors" - "github.com/hashicorp/go-multierror" "github.com/jackc/pgx/v5/pgconn" ) @@ -197,7 +196,7 @@ func (p *Postgres) CommitRun(ctx context.Context) error { } if err := p.tx.Commit(); err != nil { if errRollback := p.tx.Rollback(); errRollback != nil && errRollback != sql.ErrTxDone { - err = multierror.Append(err, errRollback) + err = stderrors.Join(err, errRollback) } return errors.Wrap(ctx, err, op) } diff --git a/internal/errors/match_test.go b/internal/errors/match_test.go index 58a4848476..18d9b109d3 100644 --- a/internal/errors/match_test.go +++ b/internal/errors/match_test.go @@ -8,7 +8,6 @@ import ( stderrors "errors" "testing" - "github.com/hashicorp/go-multierror" "github.com/stretchr/testify/assert" ) @@ -303,13 +302,13 @@ func TestMatch(t *testing.T) { { name: "match on hashicorp multi error", template: T(errInvalidFieldMask), - err: multierror.Append(stdErr, errInvalidFieldMask), + err: stderrors.Join(stdErr, errInvalidFieldMask), want: true, }, { name: "match on hashicorp multi error for specific code", template: T(InvalidFieldMask), - err: multierror.Append(stdErr, errInvalidFieldMask), + err: stderrors.Join(stdErr, errInvalidFieldMask), want: true, }, { diff --git a/internal/event/cloudevents_formatter_node_test.go b/internal/event/cloudevents_formatter_node_test.go index 9f0d91fee9..c438006714 100644 --- a/internal/event/cloudevents_formatter_node_test.go +++ b/internal/event/cloudevents_formatter_node_test.go @@ -312,10 +312,12 @@ func TestNode_Process(t *testing.T) { tt.n.Predicate = tt.predicate gotEvent, err := tt.n.Process(ctx, tt.e) - if tt.wantIsError != nil { + if tt.wantIsError != nil || tt.wantErrContains != "" { require.Error(err) assert.Nil(gotEvent) - assert.ErrorIs(err, tt.wantIsError) + if tt.wantIsError != nil { + assert.ErrorIs(err, tt.wantIsError) + } if tt.wantErrContains != "" { assert.Contains(err.Error(), tt.wantErrContains) } diff --git a/internal/event/context.go b/internal/event/context.go index 4dab459959..0c1835a0eb 100644 --- a/internal/event/context.go +++ b/internal/event/context.go @@ -130,7 +130,9 @@ func WriteObservation(ctx context.Context, caller Op, opt ...Option) error { // WriteError will write an error event. It will first check the // ctx for an eventer, then try event.SysEventer() and if no eventer can be -// found an hclog.Logger will be created and used. +// found an hclog.Logger will be created and used. Note: any multierror errors +// will be converted to a stdlib error (because multierror doesn't support json +// marshaling). // // The options WithInfoMsg, WithInfo, WithId and WithRequestInfo are supported // and all other options are ignored. diff --git a/internal/event/event_error.go b/internal/event/event_error.go index 1bd6be5dbe..6c6533122a 100644 --- a/internal/event/event_error.go +++ b/internal/event/event_error.go @@ -4,7 +4,9 @@ package event import ( + "errors" "fmt" + "reflect" ) // errorVersion defines the version of error events @@ -36,6 +38,12 @@ func newError(fromOperation Op, e error, opt ...Option) (*err, error) { return nil, fmt.Errorf("%s: %w", op, err) } } + // multierror doesn't support json Marshaling which will cause a problem + // when the event is formatted, so we're going to convert it here to a + // stdlib error + if reflect.TypeOf(e).String() == "*multierror.Error" { + e = errors.New(e.Error()) + } newErr := &err{ Id: Id(opts.withId), Op: fromOperation, diff --git a/internal/event/event_error_test.go b/internal/event/event_error_test.go index e941c2a040..37a38d3f1c 100644 --- a/internal/event/event_error_test.go +++ b/internal/event/event_error_test.go @@ -7,6 +7,7 @@ import ( "fmt" "testing" + "github.com/hashicorp/go-multierror" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -65,6 +66,27 @@ func Test_newError(t *testing.T) { Info: map[string]any{"msg": "hello"}, }, }, + { + name: "multierror-conversion", + fromOp: Op("multierror"), + e: func() error { + return multierror.Append(fmt.Errorf("%s: multierror all opts: %w", "multierror", ErrInvalidParameter)) + }(), + opts: []Option{ + WithId("multierror"), + WithRequestInfo(TestRequestInfo(t)), + WithInfo("msg", "hello"), + }, + want: &err{ + ErrorFields: fmt.Errorf("1 error occurred:\n\t* multierror: multierror all opts: invalid parameter\n\n"), + Error: "1 error occurred:\n\t* multierror: multierror all opts: invalid parameter\n\n", + Version: errorVersion, + Op: Op("multierror"), + Id: "multierror", + RequestInfo: TestRequestInfo(t), + Info: map[string]any{"msg": "hello"}, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/internal/event/eventer.go b/internal/event/eventer.go index ae7b444948..224377b2fe 100644 --- a/internal/event/eventer.go +++ b/internal/event/eventer.go @@ -25,7 +25,6 @@ import ( "github.com/hashicorp/eventlogger/sinks/writer" "github.com/hashicorp/go-hclog" wrapping "github.com/hashicorp/go-kms-wrapping/v2" - "github.com/hashicorp/go-multierror" "go.uber.org/atomic" "google.golang.org/protobuf/types/known/fieldmaskpb" ) @@ -784,7 +783,7 @@ func (e *Eventer) ReleaseGate() error { return nil } - var totalErrs *multierror.Error + var totalErrs error for _, qe := range e.gatedQueue { var writeErr error if qe == nil { @@ -804,11 +803,11 @@ func (e *Eventer) ReleaseGate() error { writeErr = fmt.Errorf("unknown event type %T", t) } if writeErr != nil { - totalErrs = multierror.Append(fmt.Errorf("in %s, error sending queued %s event: %w", op, queuedOp, writeErr)) + totalErrs = errors.Join(fmt.Errorf("in %s, error sending queued %s event: %w", op, queuedOp, writeErr)) } } e.gatedQueue = nil - return totalErrs.ErrorOrNil() + return totalErrs } // StandardLogger will create *log.Logger that will emit events through this diff --git a/internal/event/eventer_retry.go b/internal/event/eventer_retry.go index 8a6cfa5ccc..2358994cd0 100644 --- a/internal/event/eventer_retry.go +++ b/internal/event/eventer_retry.go @@ -5,13 +5,13 @@ package event import ( "context" + stderrors "errors" "fmt" "math" "math/rand" "time" "github.com/hashicorp/eventlogger" - "github.com/hashicorp/go-multierror" ) const ( @@ -55,7 +55,7 @@ func (e *Eventer) retrySend(ctx context.Context, retries uint, backOff backoff, ATTEMPTS: for attempts := uint(1); ; attempts++ { if attempts > retries+1 { - retryErrors = multierror.Append(retryErrors, fmt.Errorf("%s: reached max of %d: %w", op, retries, ErrMaxRetries)) + retryErrors = stderrors.Join(retryErrors, fmt.Errorf("%s: reached max of %d: %w", op, retries, ErrMaxRetries)) return retryErrors } var err error @@ -63,18 +63,18 @@ ATTEMPTS: if len(attemptStatus.Warnings) > 0 { var retryWarnings error for _, w := range attemptStatus.Warnings { - retryWarnings = multierror.Append(retryWarnings, w) + retryWarnings = stderrors.Join(retryWarnings, w) } e.logger.Error("unable to send event", "operation", op, "warning", retryWarnings) } if err != nil { - retryErrors = multierror.Append(retryErrors, fmt.Errorf("%s: %w", op, err)) + retryErrors = stderrors.Join(retryErrors, fmt.Errorf("%s: %w", op, err)) d := backOff.duration(attempts) info.retries++ info.backoff = info.backoff + d select { case <-ctx.Done(): - retryErrors = multierror.Append(retryErrors, ctx.Err()) + retryErrors = stderrors.Join(retryErrors, ctx.Err()) break ATTEMPTS case <-time.After(d): continue diff --git a/internal/event/eventer_retry_test.go b/internal/event/eventer_retry_test.go index 17c4950e0d..e9b6153459 100644 --- a/internal/event/eventer_retry_test.go +++ b/internal/event/eventer_retry_test.go @@ -11,7 +11,6 @@ import ( "testing" "github.com/hashicorp/eventlogger" - "github.com/hashicorp/go-multierror" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -113,24 +112,9 @@ func TestEventer_retrySend(t *testing.T) { err := eventer.retrySend(tt.ctx, tt.retries, tt.backOff, tt.handler) if tt.wantErrIs != nil { require.Error(err) - multi, isMultiError := err.(*multierror.Error) - switch isMultiError { - case true: - matched := false - for _, e := range multi.WrappedErrors() { - if assert.ErrorIs(e, tt.wantErrIs) { - if tt.wantErrContain != "" { - assert.Contains(err.Error(), tt.wantErrContain) - } - matched = true - } - } - assert.True(matched) - default: - assert.ErrorIs(err, tt.wantErrIs) - if tt.wantErrContain != "" { - assert.Contains(err.Error(), tt.wantErrContain) - } + assert.ErrorIs(err, tt.wantErrIs) + if tt.wantErrContain != "" { + assert.Contains(err.Error(), tt.wantErrContain) } return } diff --git a/internal/session/service_worker_status_report.go b/internal/session/service_worker_status_report.go index 2b635025c7..6a3d4a6127 100644 --- a/internal/session/service_worker_status_report.go +++ b/internal/session/service_worker_status_report.go @@ -5,11 +5,11 @@ package session import ( "context" + stderrors "errors" "fmt" "github.com/hashicorp/boundary/internal/errors" "github.com/hashicorp/boundary/internal/event" - "github.com/hashicorp/go-multierror" ) // StateReport is used to report on the state of a Session. @@ -44,27 +44,27 @@ func WorkerStatusReport(ctx context.Context, repo *Repository, connRepo *Connect } } - merr := new(multierror.Error) + var merr error err := connRepo.updateBytesUpBytesDown(ctx, reportedConnections...) if err != nil { - merr = multierror.Append(merr, errors.New(ctx, errors.Internal, op, fmt.Sprintf("failed to update bytes up and down for worker reported connections: %v", err))) + merr = stderrors.Join(merr, errors.New(ctx, errors.Internal, op, fmt.Sprintf("failed to update bytes up and down for worker reported connections: %v", err))) } notActive, err := repo.CheckIfNotActive(ctx, reportedSessions) if err != nil { - merr = multierror.Append(merr, errors.New(ctx, errors.Internal, op, fmt.Sprintf("Error checking session state for worker %s: %v", workerId, err))) + merr = stderrors.Join(merr, errors.New(ctx, errors.Internal, op, fmt.Sprintf("Error checking session state for worker %s: %v", workerId, err))) } closed, err := connRepo.closeOrphanedConnections(ctx, workerId, reportedConnectionIds) if err != nil { - merr = multierror.Append(merr, errors.New(ctx, errors.Internal, op, fmt.Sprintf("Error closing orphaned connections for worker %s: %v", workerId, err))) + merr = stderrors.Join(merr, errors.New(ctx, errors.Internal, op, fmt.Sprintf("Error closing orphaned connections for worker %s: %v", workerId, err))) } if len(closed) > 0 { event.WriteSysEvent(ctx, op, "marked unclaimed connections as closed", "controller_id", workerId, "count", len(closed)) } - if merr.ErrorOrNil() != nil { - return nil, merr.ErrorOrNil() + if merr != nil { + return nil, merr } return notActive, nil }