From ec0020de90d2075da9fb18a881925673bb1ee280 Mon Sep 17 00:00:00 2001 From: Louis Ruch Date: Wed, 9 Dec 2020 10:11:38 -0800 Subject: [PATCH] Refactor host_repo -> domain errors (#786) * Refactor domain errors: New -> E, create helper New and Wrap * Update host repo to use domain errors --- internal/db/id.go | 5 +- internal/errors/code.go | 10 +- internal/errors/code_test.go | 20 ++ internal/errors/error.go | 108 ++++++-- internal/errors/error_test.go | 247 ++++++++++++++++-- internal/errors/info.go | 22 +- internal/errors/is.go | 17 ++ internal/errors/is_test.go | 69 ++++- internal/errors/match_test.go | 46 ++-- internal/errors/option.go | 9 + internal/errors/option_test.go | 13 + internal/errors/sentinels.go | 18 +- internal/host/static/errors.go | 9 - internal/host/static/host.go | 4 +- internal/host/static/host_catalog.go | 4 +- internal/host/static/host_set.go | 4 +- internal/host/static/host_set_member.go | 7 +- internal/host/static/public_ids.go | 9 +- internal/host/static/repository.go | 9 +- internal/host/static/repository_host.go | 90 ++++--- .../host/static/repository_host_catalog.go | 73 +++--- .../static/repository_host_catalog_test.go | 46 ++-- internal/host/static/repository_host_set.go | 73 +++--- .../host/static/repository_host_set_member.go | 70 ++--- .../static/repository_host_set_member_test.go | 40 +-- .../host/static/repository_host_set_test.go | 58 ++-- internal/host/static/repository_host_test.go | 81 +++--- internal/host/static/repository_test.go | 14 +- .../servers/controller/handlers/errors.go | 5 +- .../controller/handlers/errors_test.go | 98 ++++++- .../host_catalogs/host_catalog_service.go | 21 ++ .../handlers/host_sets/host_set_service.go | 49 +++- .../controller/handlers/hosts/host_service.go | 22 +- 33 files changed, 996 insertions(+), 374 deletions(-) delete mode 100644 internal/host/static/errors.go diff --git a/internal/db/id.go b/internal/db/id.go index d506ffa6c..33f6e078b 100644 --- a/internal/db/id.go +++ b/internal/db/id.go @@ -17,12 +17,13 @@ func NewPublicId(prefix string) (string, error) { } func newId(prefix string) (string, error) { + const op = "db.newId" if prefix == "" { - return "", fmt.Errorf("missing prefix %w", errors.ErrInvalidParameter) + return "", errors.New(errors.InvalidParameter, op, "missing prefix") } publicId, err := base62.Random(10) if err != nil { - return "", fmt.Errorf("unable to generate id: %w", err) + return "", errors.Wrap(err, op, errors.WithMsg("unable to generate id")) } return fmt.Sprintf("%s_%s", prefix, publicId), nil } diff --git a/internal/errors/code.go b/internal/errors/code.go index 03de4b61f..04fa4a27e 100644 --- a/internal/errors/code.go +++ b/internal/errors/code.go @@ -21,13 +21,17 @@ const ( Unknown Code = 0 // Unknown will be equal to a zero value for Codes // General function errors are reserved Codes 100-999 - InvalidParameter Code = 100 // InvalidParameter represents and invalid parameter for an operation. + InvalidParameter Code = 100 // InvalidParameter represents an invalid parameter for an operation. + InvalidAddress Code = 101 // InvalidAddress represents an invalid host address for an operation + InvalidPublicId Code = 102 // InvalidPublicId represents an invalid public Id for an operation + InvalidFieldMask Code = 103 // InvalidFieldMask represents an invalid field mast for an operation + EmptyFieldMask Code = 104 // EmptyFieldMask represents an empty field mask for an operation - // DB errors are resevered Codes from 1000-1999 + // DB errors are reserved Codes from 1000-1999 CheckConstraint Code = 1000 // CheckConstraint represents a check constraint error NotNull Code = 1001 // NotNull represents a value must not be null error NotUnique Code = 1002 // NotUnique represents a value must be unique error - NotSpecificIntegrity Code = 1003 // NotSpecificIntegrity represents an integrity error that has no specificy domain error code + NotSpecificIntegrity Code = 1003 // NotSpecificIntegrity represents an integrity error that has no specific domain error code MissingTable Code = 1004 // Missing table represents an undefined table error RecordNotFound Code = 1100 // RecordNotFound represents that a record/row was not found matching the criteria MultipleRecords Code = 1101 // MultipleRecords represents that multiple records/rows were found matching the criteria diff --git a/internal/errors/code_test.go b/internal/errors/code_test.go index 4977b5a2c..2a61d6865 100644 --- a/internal/errors/code_test.go +++ b/internal/errors/code_test.go @@ -32,6 +32,26 @@ func TestCode_Both_String_Info(t *testing.T) { c: InvalidParameter, want: InvalidParameter, }, + { + name: "InvalidAddress", + c: InvalidAddress, + want: InvalidAddress, + }, + { + name: "InvalidPublicId", + c: InvalidPublicId, + want: InvalidPublicId, + }, + { + name: "InvalidFieldMask", + c: InvalidFieldMask, + want: InvalidFieldMask, + }, + { + name: "EmptyFieldMask", + c: EmptyFieldMask, + want: EmptyFieldMask, + }, { name: "CheckConstraint", c: CheckConstraint, diff --git a/internal/errors/error.go b/internal/errors/error.go index 73ce921ca..baf7d1882 100644 --- a/internal/errors/error.go +++ b/internal/errors/error.go @@ -13,9 +13,8 @@ import ( type Op string // Err provides the ability to specify a Msg, Op, Code and Wrapped error. -// Errs must have a Code and all other fields are optional. We've chosen Err -// over Error for the identifier to support the easy embedding of Errs. Errs -// can be embedded without a conflict between the embedded Err and Err.Error(). +// We've chosen Err over Error for the identifier to support the easy embedding of Errs. +// Errs can be embedded without a conflict between the embedded Err and Err.Error(). type Err struct { // Code is the error's code, which can be used to get the error's // errorCodeInfo, which contains the error's Kind and Message @@ -32,22 +31,85 @@ type Err struct { Wrapped error } -// New creates a new Err and supports the options of: -// WithOp - allows you to specify an optional Op (operation) -// WithMsg() - allows you to specify an optional error msg, if the default +// E creates a new Err with provided code and supports the options of: +// +// * WithOp() - allows you to specify an optional Op (operation). +// +// * WithMsg() - allows you to specify an optional error msg, if the default // msg for the error Code is not sufficient. -// WithWrap() - allows you to specify -// an error to wrap -func New(c Code, opt ...Option) error { +// +// * WithWrap() - allows you to specify an error to wrap. +// If the wrapped error is a boundary domain error, the wrapped error code +// will be used as the returned error's code. +// +// * WithCode() - allows you to specify an optional Code, this code will be prioritized +// over a code used from WithWrap(). +func E(opt ...Option) error { opts := GetOpts(opt...) + var code Code + + // check if options includes a wrapped error to take code from + var err *Err + if As(opts.withErrWrapped, &err) { + code = err.Code + } + + // if options include withCode prioritize using that code + // even if one was set via wrapped error above + if opts.withCode != Unknown { + code = opts.withCode + } + return &Err{ - Code: c, + Code: code, Op: opts.withOp, Wrapped: opts.withErrWrapped, Msg: opts.withErrMsg, } } +// New creates a new Err with provided code, op and msg +// It supports the options of: +// +// * WithWrap() - allows you to specify an error to wrap +func New(c Code, op Op, msg string, opt ...Option) error { + if c != Unknown { + opt = append(opt, WithCode(c)) + } + if op != "" { + opt = append(opt, WithOp(op)) + } + if msg != "" { + opt = append(opt, WithMsg(msg)) + } + + return E(opt...) +} + +// Wrap creates a new Err from the provided err and op, +// preserving the code from the originating error. +// It supports the options of: +// +// * WithMsg() - allows you to specify an optional error msg, if the default +// msg for the error Code is not sufficient. +func Wrap(e error, op Op, opt ...Option) error { + if op != "" { + opt = append(opt, WithOp(op)) + } + if e != nil { + // TODO: once db package has been refactored to only return domain errors, + // this convert can be removed + err := Convert(e) + if err != nil { + // wrapped converted error + e = err + } + opt = append(opt, WithWrap(e)) + } + + return E(opt...) +} + // Convert will convert the error to a Boundary *Err (returning it as an error) // and attempt to add a helpful error msg as well. If that's not possible, it // will return nil @@ -55,7 +117,8 @@ func Convert(e error) *Err { if e == nil { return nil } - if err, ok := e.(*Err); ok { + var err *Err + if As(e, &err) { return err } var pqError *pq.Error @@ -63,19 +126,19 @@ func Convert(e error) *Err { if pqError.Code.Class() == "23" { // class of integrity constraint violations switch pqError.Code { case "23505": // unique_violation - return New(NotUnique, WithMsg(pqError.Detail), WithWrap(ErrNotUnique)).(*Err) + return E(WithMsg(pqError.Detail), WithWrap(ErrNotUnique)).(*Err) case "23502": // not_null_violation msg := fmt.Sprintf("%s must not be empty", pqError.Column) - return New(NotNull, WithMsg(msg), WithWrap(ErrNotNull)).(*Err) + return E(WithMsg(msg), WithWrap(ErrNotNull)).(*Err) case "23514": // check_violation msg := fmt.Sprintf("%s constraint failed", pqError.Constraint) - return New(CheckConstraint, WithMsg(msg), WithWrap(ErrCheckConstraint)).(*Err) + return E(WithMsg(msg), WithWrap(ErrCheckConstraint)).(*Err) default: - return New(NotSpecificIntegrity, WithMsg(pqError.Message)).(*Err) + return E(WithCode(NotSpecificIntegrity), WithMsg(pqError.Message)).(*Err) } } if pqError.Code == "42P01" { - return New(MissingTable, WithMsg(pqError.Message)).(*Err) + return E(WithCode(MissingTable), WithMsg(pqError.Message)).(*Err) } } // unfortunately, we can't help. @@ -104,18 +167,25 @@ func (e *Err) Error() string { join(&s, ": ", e.Msg) } - if info, ok := errorCodeInfo[e.Code]; ok { + var skipInfo bool + var wrapped *Err + if As(e.Wrapped, &wrapped) { + // if wrapped error code is the same as this error, don't print redundant info + skipInfo = wrapped.Code == e.Code + } + + if info, ok := errorCodeInfo[e.Code]; ok && !skipInfo { if e.Msg == "" { join(&s, ": ", info.Message) // provide a default. join(&s, ", ", info.Kind.String()) } else { join(&s, ": ", info.Kind.String()) } + join(&s, ": ", fmt.Sprintf("error #%d", e.Code)) } - join(&s, ": ", fmt.Sprintf("error #%d", e.Code)) if e.Wrapped != nil { - join(&s, ": \n", e.Wrapped.Error()) + join(&s, ": ", e.Wrapped.Error()) } return s.String() } diff --git a/internal/errors/error_test.go b/internal/errors/error_test.go index fcfe21b91..08ace9241 100644 --- a/internal/errors/error_test.go +++ b/internal/errors/error_test.go @@ -3,6 +3,7 @@ package errors_test import ( "context" stderrors "errors" + "fmt" "testing" "github.com/hashicorp/boundary/internal/db" @@ -12,18 +13,17 @@ import ( "github.com/stretchr/testify/require" ) -func Test_NewError(t *testing.T) { +func Test_ErrorE(t *testing.T) { t.Parallel() tests := []struct { name string - code errors.Code opt []errors.Option want error }{ { name: "all-options", - code: errors.InvalidParameter, opt: []errors.Option{ + errors.WithCode(errors.InvalidParameter), errors.WithOp("alice.Bob"), errors.WithWrap(errors.ErrRecordNotFound), errors.WithMsg("test msg"), @@ -42,11 +42,204 @@ func Test_NewError(t *testing.T) { Code: errors.Unknown, }, }, + { + name: "withCode", + opt: []errors.Option{ + errors.WithCode(errors.RecordNotFound), + }, + want: &errors.Err{ + Code: errors.RecordNotFound, + }, + }, + { + name: "uses-wrapped-code", + opt: []errors.Option{ + errors.WithWrap(errors.ErrRecordNotFound), + }, + want: &errors.Err{ + Code: errors.RecordNotFound, + Wrapped: errors.ErrRecordNotFound, + }, + }, + { + name: "conflicting-withCode-withWrap", + opt: []errors.Option{ + errors.WithCode(errors.InvalidFieldMask), + errors.WithWrap(errors.ErrRecordNotFound), + }, + want: &errors.Err{ + Code: errors.InvalidFieldMask, + Wrapped: errors.ErrRecordNotFound, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { assert, require := assert.New(t), require.New(t) - err := errors.New(tt.code, tt.opt...) + err := errors.E(tt.opt...) + require.Error(err) + assert.Equal(tt.want, err) + }) + } +} + +func Test_NewError(t *testing.T) { + t.Parallel() + tests := []struct { + name string + code errors.Code + op errors.Op + msg string + opt []errors.Option + want error + }{ + { + name: "all-options", + code: errors.InvalidParameter, + op: "alice.Bob", + msg: "test msg", + opt: []errors.Option{ + errors.WithWrap(errors.ErrRecordNotFound), + }, + want: &errors.Err{ + Op: "alice.Bob", + Wrapped: errors.ErrRecordNotFound, + Msg: "test msg", + Code: errors.InvalidParameter, + }, + }, + { + name: "no-options", + opt: nil, + want: &errors.Err{ + Code: errors.Unknown, + }, + }, + { + name: "conflicting-op", + op: "alice.Bob", + opt: []errors.Option{ + errors.WithOp("bab.Op"), + }, + want: &errors.Err{ + Op: "alice.Bob", + Code: errors.Unknown, + }, + }, + { + name: "conflicting-msg", + msg: "test msg", + opt: []errors.Option{ + errors.WithMsg("dont use this message"), + }, + want: &errors.Err{ + Msg: "test msg", + Code: errors.Unknown, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert, require := assert.New(t), require.New(t) + err := errors.New(tt.code, tt.op, tt.msg, tt.opt...) + require.Error(err) + assert.Equal(tt.want, err) + }) + } +} + +func Test_WrapError(t *testing.T) { + t.Parallel() + testErr := errors.E(errors.WithCode(errors.InvalidParameter), errors.WithOp("alice.Bob"), errors.WithMsg("test msg")) + tests := []struct { + name string + opt []errors.Option + err error + op errors.Op + want error + }{ + { + name: "boundary-error", + err: testErr, + op: "alice.Bob", + opt: []errors.Option{ + errors.WithMsg("test msg"), + }, + want: &errors.Err{ + Wrapped: testErr, + Op: "alice.Bob", + Msg: "test msg", + Code: errors.InvalidParameter, + }, + }, + { + name: "boundary-error-no-op", + err: testErr, + opt: []errors.Option{ + errors.WithMsg("test msg"), + }, + want: &errors.Err{ + Wrapped: testErr, + Msg: "test msg", + Code: errors.InvalidParameter, + }, + }, + { + name: "boundary-error-no-options", + err: testErr, + want: &errors.Err{ + Wrapped: testErr, + Code: errors.InvalidParameter, + }, + }, + { + name: "std-error", + err: fmt.Errorf("std error"), + want: &errors.Err{ + Wrapped: fmt.Errorf("std error"), + Code: errors.Unknown, + }, + }, + { + name: "conflicting-with-wrap", + err: testErr, + opt: []errors.Option{ + errors.WithWrap(fmt.Errorf("dont wrap this error")), + }, + want: &errors.Err{ + Wrapped: testErr, + Code: errors.InvalidParameter, + }, + }, + { + name: "conflicting-with-op", + err: testErr, + op: "alice.Bob", + opt: []errors.Option{ + errors.WithOp("bad.Op"), + }, + want: &errors.Err{ + Wrapped: testErr, + Op: "alice.Bob", + Code: errors.InvalidParameter, + }, + }, + { + name: "NotSpecificIntegrity", + err: &pq.Error{ + Code: pq.ErrorCode("23001"), + Message: "test msg", + }, + want: &errors.Err{ + Wrapped: errors.E(errors.WithCode(errors.NotSpecificIntegrity), errors.WithMsg("test msg")), + Code: errors.NotSpecificIntegrity, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert, require := assert.New(t), require.New(t) + err := errors.Wrap(tt.err, tt.op, tt.opt...) require.Error(err) assert.Equal(tt.want, err) }) @@ -67,12 +260,12 @@ func TestError_Info(t *testing.T) { }, { name: "Unknown", - err: errors.New(errors.Unknown).(*errors.Err), + err: errors.E().(*errors.Err), want: errors.Unknown, }, { name: "InvalidParameter", - err: errors.New(errors.InvalidParameter).(*errors.Err), + err: errors.E(errors.WithCode(errors.InvalidParameter)).(*errors.Err), want: errors.InvalidParameter, }, } @@ -93,24 +286,39 @@ func TestError_Error(t *testing.T) { }{ { name: "msg", - err: errors.New(errors.Unknown, errors.WithMsg("test msg")), + err: errors.E(errors.WithMsg("test msg")), want: "test msg: unknown: error #0", }, { name: "code", - err: errors.New(errors.CheckConstraint), + err: errors.E(errors.WithCode(errors.CheckConstraint)), want: "constraint check failed, integrity violation: error #1000", }, { name: "op-msg-and-code", - err: errors.New(errors.CheckConstraint, errors.WithOp("alice.bob"), errors.WithMsg("test msg")), + err: errors.E(errors.WithCode(errors.CheckConstraint), errors.WithOp("alice.bob"), errors.WithMsg("test msg")), want: "alice.bob: test msg: integrity violation: error #1000", }, { name: "unknown", - err: errors.New(errors.Unknown), + err: errors.E(), want: "unknown, unknown: error #0", }, + { + name: "wrapped-no-code", + err: errors.E(errors.WithWrap(errors.E(errors.WithCode(errors.InvalidParameter), errors.WithMsg("wrapped msg"))), errors.WithMsg("test msg")), + want: "test msg: wrapped msg: parameter violation: error #100", + }, + { + name: "wrapped-different-error-codes", + err: errors.E(errors.WithCode(errors.CheckConstraint), errors.WithWrap(errors.E(errors.WithCode(errors.InvalidParameter), errors.WithMsg("wrapped msg"))), errors.WithMsg("test msg")), + want: "test msg: integrity violation: error #1000: wrapped msg: parameter violation: error #100", + }, + { + name: "wrapped-same-error-codes", + err: errors.E(errors.WithCode(errors.CheckConstraint), errors.WithWrap(errors.E(errors.WithCode(errors.CheckConstraint), errors.WithMsg("wrapped msg"))), errors.WithMsg("test msg")), + want: "test msg: wrapped msg: integrity violation: error #1000", + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -129,7 +337,7 @@ func TestError_Error(t *testing.T) { func TestError_Unwrap(t *testing.T) { t.Parallel() - testErr := errors.New(errors.Unknown, errors.WithMsg("test error")) + testErr := errors.E(errors.WithMsg("test error")) tests := []struct { name string @@ -139,7 +347,7 @@ func TestError_Unwrap(t *testing.T) { }{ { name: "ErrInvalidParameter", - err: errors.New(errors.InvalidParameter, errors.WithWrap(errors.ErrInvalidParameter)), + err: errors.E(errors.WithWrap(errors.ErrInvalidParameter)), want: errors.ErrInvalidParameter, wantIsErr: errors.ErrInvalidParameter, }, @@ -170,6 +378,7 @@ func TestError_Unwrap(t *testing.T) { func TestConvertError(t *testing.T) { t.Parallel() + testErr := errors.E(errors.WithCode(errors.InvalidParameter), errors.WithOp("alice.Bob"), errors.WithMsg("test msg")) const ( createTable = ` create table if not exists test_table ( @@ -210,7 +419,12 @@ func TestConvertError(t *testing.T) { e: &pq.Error{ Code: pq.ErrorCode("23001"), }, - wantErr: errors.New(errors.NotSpecificIntegrity), + wantErr: errors.E(errors.WithCode(errors.NotSpecificIntegrity)), + }, + { + name: "convert-domain-error", + e: testErr, + wantErr: testErr, }, } for _, tt := range tests { @@ -237,7 +451,7 @@ func TestConvertError(t *testing.T) { e := errors.Convert(err) require.NotNil(e) assert.True(errors.Is(e, errors.ErrNotUnique)) - assert.Equal("Key (name)=(alice) already exists.: integrity violation: error #1002: \nunique constraint violation: integrity violation: error #1002", e.Error()) + assert.Equal("Key (name)=(alice) already exists.: unique constraint violation: integrity violation: error #1002", e.Error()) }) t.Run("ErrCodeNotNull", func(t *testing.T) { assert, require := assert.New(t), require.New(t) @@ -249,7 +463,7 @@ func TestConvertError(t *testing.T) { e := errors.Convert(err) require.NotNil(e) assert.True(errors.Is(e, errors.ErrNotNull)) - assert.Equal("description must not be empty: integrity violation: error #1001: \nnot null constraint violated: integrity violation: error #1001", e.Error()) + assert.Equal("description must not be empty: not null constraint violated: integrity violation: error #1001", e.Error()) }) t.Run("ErrCodeCheckConstraint", func(t *testing.T) { assert, require := assert.New(t), require.New(t) @@ -261,12 +475,13 @@ func TestConvertError(t *testing.T) { e := errors.Convert(err) require.NotNil(e) assert.True(errors.Is(e, errors.ErrCheckConstraint)) - assert.Equal("test_table_five_check constraint failed: integrity violation: error #1000: \ncheck constraint violated: integrity violation: error #1000", e.Error()) + assert.Equal("test_table_five_check constraint failed: check constraint violated: integrity violation: error #1000", e.Error()) }) t.Run("MissingTable", func(t *testing.T) { assert, require := assert.New(t), require.New(t) _, err := rw.Exec(ctx, missingTable, nil) require.Error(err) + e := errors.Convert(err) require.NotNil(e) assert.True(errors.Match(errors.T(errors.MissingTable), e)) diff --git a/internal/errors/info.go b/internal/errors/info.go index 47124ac9a..a7ce9dda4 100644 --- a/internal/errors/info.go +++ b/internal/errors/info.go @@ -1,7 +1,11 @@ package errors +// Info contains details of the specific error code type Info struct { - Kind Kind + // Kind specifies the kind of error (unknown, parameter, integrity, etc). + Kind Kind + + // Message provides a default message for the error code Message string } @@ -16,6 +20,22 @@ var errorCodeInfo = map[Code]Info{ Message: "invalid parameter", Kind: Parameter, }, + InvalidAddress: { + Message: "invalid address", + Kind: Parameter, + }, + InvalidPublicId: { + Message: "invalid public id", + Kind: Parameter, + }, + InvalidFieldMask: { + Message: "invalid field mask", + Kind: Parameter, + }, + EmptyFieldMask: { + Message: "empty field mask", + Kind: Parameter, + }, CheckConstraint: { Message: "constraint check failed", Kind: Integrity, diff --git a/internal/errors/is.go b/internal/errors/is.go index 6cf39bb40..3f31aa0d1 100644 --- a/internal/errors/is.go +++ b/internal/errors/is.go @@ -89,3 +89,20 @@ func IsMissingTableError(err error) bool { } return false } + +// IsNotFoundError returns a boolean indicating whether the error is known to +// report a not found violation. +func IsNotFoundError(err error) bool { + if err == nil { + return false + } + + var domainErr *Err + if errors.As(err, &domainErr) { + if domainErr.Code == RecordNotFound { + return true + } + } + + return false +} diff --git a/internal/errors/is_test.go b/internal/errors/is_test.go index af71a970a..4c4949193 100644 --- a/internal/errors/is_test.go +++ b/internal/errors/is_test.go @@ -2,6 +2,7 @@ package errors_test import ( "context" + "fmt" "testing" "github.com/hashicorp/boundary/internal/db" @@ -44,7 +45,7 @@ func TestError_IsUnique(t *testing.T) { }, { name: "wrapped-pq-is-unique", - in: errors.New(errors.NotUnique, + in: errors.E(errors.WithCode(errors.NotUnique), errors.WithWrap(&pq.Error{ Code: pq.ErrorCode("23505"), }), @@ -56,6 +57,11 @@ func TestError_IsUnique(t *testing.T) { in: errors.ErrRecordNotFound, want: false, }, + { + name: "conflicting-wrapped-code", + in: errors.E(errors.WithCode(errors.NotNull), errors.WithWrap(errors.E(errors.WithCode(errors.NotUnique)))), + want: false, + }, } for _, tt := range tests { tt := tt @@ -96,12 +102,12 @@ func TestError_IsCheckConstraint(t *testing.T) { }, { name: "ErrCodeCheckConstraint", - in: errors.New(errors.CheckConstraint), + in: errors.E(errors.WithCode(errors.CheckConstraint)), want: true, }, { name: "wrapped-pq-is-check-constraint", - in: errors.New(errors.CheckConstraint, + in: errors.E(errors.WithCode(errors.CheckConstraint), errors.WithWrap(&pq.Error{ Code: pq.ErrorCode("23514"), }), @@ -113,6 +119,11 @@ func TestError_IsCheckConstraint(t *testing.T) { in: errors.ErrRecordNotFound, want: false, }, + { + name: "conflicting-wrapped-code", + in: errors.E(errors.WithCode(errors.NotNull), errors.WithWrap(errors.E(errors.WithCode(errors.CheckConstraint)))), + want: false, + }, } for _, tt := range tests { tt := tt @@ -160,12 +171,12 @@ func TestError_IsNotNullError(t *testing.T) { }, { name: "ErrCodeNotNull", - in: errors.New(errors.NotNull), + in: errors.E(errors.WithCode(errors.NotNull)), want: true, }, { name: "wrapped-pq-is-not-null", - in: errors.New(errors.NotNull, + in: errors.E(errors.WithCode(errors.NotNull), errors.WithWrap(&pq.Error{ Code: pq.ErrorCode("23502"), }), @@ -177,6 +188,11 @@ func TestError_IsNotNullError(t *testing.T) { in: errors.ErrRecordNotFound, want: false, }, + { + name: "conflicting-wrapped-code", + in: errors.E(errors.WithCode(errors.CheckConstraint), errors.WithWrap(errors.E(errors.WithCode(errors.NotNull)))), + want: false, + }, } for _, tt := range tests { tt := tt @@ -240,3 +256,46 @@ func TestError_IsMissingTableError(t *testing.T) { assert.True(errors.IsMissingTableError(err)) }) } + +func TestError_IsNotFoundError(t *testing.T) { + var tests = []struct { + name string + in error + want bool + }{ + { + name: "nil-error", + in: nil, + want: false, + }, + { + name: "not-found-error", + in: errors.E(errors.WithCode(errors.RecordNotFound)), + want: true, + }, + { + name: "sentinel-not-found", + in: errors.ErrRecordNotFound, + want: true, + }, + { + name: "std-err", + in: fmt.Errorf("std error"), + want: false, + }, + { + name: "conflicting-wrapped-code", + in: errors.E(errors.WithCode(errors.NotNull), errors.WithWrap(errors.E(errors.WithCode(errors.RecordNotFound)))), + want: false, + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + assert := assert.New(t) + err := tt.in + got := errors.IsNotFoundError(err) + assert.Equal(tt.want, got) + }) + } +} diff --git a/internal/errors/match_test.go b/internal/errors/match_test.go index b902ecdc5..50dd222c0 100644 --- a/internal/errors/match_test.go +++ b/internal/errors/match_test.go @@ -149,7 +149,7 @@ func TestMatch(t *testing.T) { { name: "nil template", template: nil, - err: New(NotUnique, WithMsg("this thing was must be unique")), + err: E(WithCode(NotUnique), WithMsg("this thing was must be unique")), want: false, }, { @@ -161,8 +161,8 @@ func TestMatch(t *testing.T) { { name: "match on Kind only", template: T(Integrity), - err: New( - NotUnique, + err: E( + WithCode(NotUnique), WithMsg("this thing must be unique"), WithOp("alice.Bob"), WithWrap(ErrInvalidFieldMask), @@ -172,8 +172,8 @@ func TestMatch(t *testing.T) { { name: "no match on Kind only", template: T(Integrity), - err: New( - RecordNotFound, + err: E( + WithCode(RecordNotFound), WithMsg("this thing is missing"), WithOp("alice.Bob"), WithWrap(ErrInvalidFieldMask), @@ -183,8 +183,8 @@ func TestMatch(t *testing.T) { { name: "match on Code only", template: T(NotUnique), - err: New( - NotUnique, + err: E( + WithCode(NotUnique), WithMsg("this thing must be unique"), WithOp("alice.Bob"), WithWrap(ErrInvalidFieldMask), @@ -194,8 +194,8 @@ func TestMatch(t *testing.T) { { name: "no match on Code only", template: T(NotUnique), - err: New( - RecordNotFound, + err: E( + WithCode(RecordNotFound), WithMsg("this thing is missing"), WithOp("alice.Bob"), WithWrap(ErrInvalidFieldMask), @@ -205,8 +205,8 @@ func TestMatch(t *testing.T) { { name: "match on Op only", template: T(Op("alice.Bob")), - err: New( - NotUnique, + err: E( + WithCode(NotUnique), WithMsg("this thing must be unique"), WithOp("alice.Bob"), WithWrap(ErrInvalidFieldMask), @@ -216,8 +216,8 @@ func TestMatch(t *testing.T) { { name: "no match on Op only", template: T(Op("alice.Alice")), - err: New( - RecordNotFound, + err: E( + WithCode(RecordNotFound), WithMsg("this thing is missing"), WithOp("alice.Bob"), WithWrap(ErrInvalidFieldMask), @@ -233,8 +233,8 @@ func TestMatch(t *testing.T) { ErrInvalidFieldMask, Op("alice.Bob"), ), - err: New( - InvalidParameter, + err: E( + WithCode(InvalidParameter), WithMsg("this thing must be unique"), WithOp("alice.Bob"), WithWrap(ErrInvalidFieldMask), @@ -244,8 +244,8 @@ func TestMatch(t *testing.T) { { name: "match on Wrapped only", template: T(ErrInvalidFieldMask), - err: New( - NotUnique, + err: E( + WithCode(NotUnique), WithMsg("this thing must be unique"), WithOp("alice.Bob"), WithWrap(ErrInvalidFieldMask), @@ -255,8 +255,8 @@ func TestMatch(t *testing.T) { { name: "no match on Wrapped only", template: T(ErrNotUnique), - err: New( - RecordNotFound, + err: E( + WithCode(RecordNotFound), WithMsg("this thing is missing"), WithOp("alice.Bob"), WithWrap(ErrInvalidFieldMask), @@ -266,8 +266,8 @@ func TestMatch(t *testing.T) { { name: "match on Wrapped only stderror", template: T(stdErr), - err: New( - NotUnique, + err: E( + WithCode(NotUnique), WithMsg("this thing must be unique"), WithOp("alice.Bob"), WithWrap(stdErr), @@ -277,8 +277,8 @@ func TestMatch(t *testing.T) { { name: "no match on Wrapped only stderror", template: T(stderrors.New("no match")), - err: New( - RecordNotFound, + err: E( + WithCode(RecordNotFound), WithMsg("this thing is missing"), WithOp("alice.Bob"), WithWrap(stdErr), diff --git a/internal/errors/option.go b/internal/errors/option.go index 469939ece..4d3837a15 100644 --- a/internal/errors/option.go +++ b/internal/errors/option.go @@ -14,6 +14,7 @@ type Option func(*Options) // Options - how Options are represented. type Options struct { + withCode Code withErrWrapped error withErrMsg string withOp Op @@ -46,3 +47,11 @@ func WithOp(op Op) Option { o.withOp = op } } + +// WithCode provides an option to provide a code when creating a new +// error. +func WithCode(code Code) Option { + return func(o *Options) { + o.withCode = code + } +} diff --git a/internal/errors/option_test.go b/internal/errors/option_test.go index c9987b6b7..73f06117e 100644 --- a/internal/errors/option_test.go +++ b/internal/errors/option_test.go @@ -48,4 +48,17 @@ func Test_getOpts(t *testing.T) { testOpts.withOp = "alice.bob" assert.Equal(opts, testOpts) }) + t.Run("WithCode", func(t *testing.T) { + assert := assert.New(t) + // test default + opts := GetOpts() + testOpts := getDefaultOptions() + testOpts.withOp = "" + assert.Equal(opts, testOpts) + + // try setting it + opts = GetOpts(WithCode(NotUnique)) + testOpts.withCode = NotUnique + assert.Equal(opts, testOpts) + }) } diff --git a/internal/errors/sentinels.go b/internal/errors/sentinels.go index 30dc9bf79..ec2ab5c61 100644 --- a/internal/errors/sentinels.go +++ b/internal/errors/sentinels.go @@ -6,39 +6,39 @@ package errors // Matching function. var ( // ErrInvalidPublicId indicates an invalid PublicId. - ErrInvalidPublicId = New(InvalidParameter, WithMsg("invalid publicId")) + ErrInvalidPublicId = E(WithCode(InvalidParameter), WithMsg("invalid publicId")) // ErrInvalidParameter is returned by create and update methods if // an attribute on a struct contains illegal or invalid values. - ErrInvalidParameter = New(InvalidParameter, WithMsg("invalid parameter")) + ErrInvalidParameter = E(WithCode(InvalidParameter), WithMsg("invalid parameter")) // ErrInvalidFieldMask is returned by update methods if the field mask // contains unknown fields or fields that cannot be updated. - ErrInvalidFieldMask = New(InvalidParameter, WithMsg("invalid field mask")) + ErrInvalidFieldMask = E(WithCode(InvalidParameter), WithMsg("invalid field mask")) // ErrEmptyFieldMask is returned by update methods if the field mask is // empty. - ErrEmptyFieldMask = New(InvalidParameter, WithMsg("empty field mask")) + ErrEmptyFieldMask = E(WithCode(InvalidParameter), WithMsg("empty field mask")) // ErrNotUnique is returned by create and update methods when a write // to the repository resulted in a unique constraint violation. - ErrNotUnique = New(NotUnique, WithMsg("unique constraint violation")) + ErrNotUnique = E(WithCode(NotUnique), WithMsg("unique constraint violation")) // ErrNotNull is returned by methods when a write to the repository resulted // in a check constraint violation - ErrCheckConstraint = New(CheckConstraint, WithMsg("check constraint violated")) + ErrCheckConstraint = E(WithCode(CheckConstraint), WithMsg("check constraint violated")) // ErrNotNull is returned by methods when a write to the repository resulted // in a not null constraint violation - ErrNotNull = New(NotNull, WithMsg("not null constraint violated")) + ErrNotNull = E(WithCode(NotNull), WithMsg("not null constraint violated")) // ErrRecordNotFound returns a "record not found" error and it only occurs // when attempting to read from the database into struct. // When reading into a slice it won't return this error. - ErrRecordNotFound = New(RecordNotFound, WithMsg("record not found")) + ErrRecordNotFound = E(WithCode(RecordNotFound), WithMsg("record not found")) // ErrMultipleRecords is returned by update and delete methods when a // write to the repository would result in more than one record being // changed resulting in the transaction being rolled back. - ErrMultipleRecords = New(MultipleRecords, WithMsg("multiple records")) + ErrMultipleRecords = E(WithCode(MultipleRecords), WithMsg("multiple records")) ) diff --git a/internal/host/static/errors.go b/internal/host/static/errors.go deleted file mode 100644 index 3c1c6ca0d..000000000 --- a/internal/host/static/errors.go +++ /dev/null @@ -1,9 +0,0 @@ -package static - -import "errors" - -var ( - // ErrInvalidAddress results from attempting to perform an operation - // that sets an address on a host to an invalid value. - ErrInvalidAddress = errors.New("invalid address") -) diff --git a/internal/host/static/host.go b/internal/host/static/host.go index 8a7115b73..881f0d26e 100644 --- a/internal/host/static/host.go +++ b/internal/host/static/host.go @@ -1,8 +1,6 @@ package static import ( - "fmt" - "github.com/hashicorp/boundary/internal/errors" "github.com/hashicorp/boundary/internal/host/static/store" "github.com/hashicorp/boundary/internal/oplog" @@ -25,7 +23,7 @@ type Host struct { // ignored. func NewHost(catalogId string, opt ...Option) (*Host, error) { if catalogId == "" { - return nil, fmt.Errorf("new: static host: no catalog id: %w", errors.ErrInvalidParameter) + return nil, errors.New(errors.InvalidParameter, "static.NewHost", "no catalog id") } opts := getOpts(opt...) diff --git a/internal/host/static/host_catalog.go b/internal/host/static/host_catalog.go index 4c881955c..0651d72c6 100644 --- a/internal/host/static/host_catalog.go +++ b/internal/host/static/host_catalog.go @@ -1,8 +1,6 @@ package static import ( - "fmt" - "github.com/hashicorp/boundary/internal/errors" "github.com/hashicorp/boundary/internal/host/static/store" "github.com/hashicorp/boundary/internal/oplog" @@ -21,7 +19,7 @@ type HostCatalog struct { // ignored. func NewHostCatalog(scopeId string, opt ...Option) (*HostCatalog, error) { if scopeId == "" { - return nil, fmt.Errorf("new: static host catalog: no scope id: %w", errors.ErrInvalidParameter) + return nil, errors.New(errors.InvalidParameter, "static.NewHostCatalog", "no scope id") } opts := getOpts(opt...) diff --git a/internal/host/static/host_set.go b/internal/host/static/host_set.go index 39d40af34..27b7579ce 100644 --- a/internal/host/static/host_set.go +++ b/internal/host/static/host_set.go @@ -1,8 +1,6 @@ package static import ( - "fmt" - "github.com/hashicorp/boundary/internal/errors" "github.com/hashicorp/boundary/internal/host/static/store" "github.com/hashicorp/boundary/internal/oplog" @@ -20,7 +18,7 @@ type HostSet struct { // ignored. func NewHostSet(catalogId string, opt ...Option) (*HostSet, error) { if catalogId == "" { - return nil, fmt.Errorf("new: static host set: no catalog id: %w", errors.ErrInvalidParameter) + return nil, errors.New(errors.InvalidParameter, "static.NewHostSet", "no catalog id") } opts := getOpts(opt...) diff --git a/internal/host/static/host_set_member.go b/internal/host/static/host_set_member.go index 287520ef4..2a5af4c9f 100644 --- a/internal/host/static/host_set_member.go +++ b/internal/host/static/host_set_member.go @@ -1,8 +1,6 @@ package static import ( - "fmt" - "github.com/hashicorp/boundary/internal/errors" "github.com/hashicorp/boundary/internal/host/static/store" ) @@ -16,11 +14,12 @@ type HostSetMember struct { // NewHostSetMember creates a new in memory HostSetMember representing the // membership of hostId in hostSetId. func NewHostSetMember(hostSetId, hostId string, opt ...Option) (*HostSetMember, error) { + const op = "static.NewHostSetMember" if hostSetId == "" { - return nil, fmt.Errorf("new: static host set member: no host set id: %w", errors.ErrInvalidParameter) + return nil, errors.New(errors.InvalidParameter, op, "no host set id") } if hostId == "" { - return nil, fmt.Errorf("new: static host set member: no host id: %w", errors.ErrInvalidParameter) + return nil, errors.New(errors.InvalidParameter, op, "no host id") } member := &HostSetMember{ HostSetMember: &store.HostSetMember{ diff --git a/internal/host/static/public_ids.go b/internal/host/static/public_ids.go index e9ffa3418..498107ea2 100644 --- a/internal/host/static/public_ids.go +++ b/internal/host/static/public_ids.go @@ -1,9 +1,8 @@ package static import ( - "fmt" - "github.com/hashicorp/boundary/internal/db" + "github.com/hashicorp/boundary/internal/errors" ) // PublicId prefixes for the resources in the static package. @@ -16,7 +15,7 @@ const ( func newHostCatalogId() (string, error) { id, err := db.NewPublicId(HostCatalogPrefix) if err != nil { - return "", fmt.Errorf("new host catalog id: %w", err) + return "", errors.Wrap(err, "static.newHostCatalogId") } return id, err } @@ -24,7 +23,7 @@ func newHostCatalogId() (string, error) { func newHostId() (string, error) { id, err := db.NewPublicId(HostPrefix) if err != nil { - return "", fmt.Errorf("new host id: %w", err) + return "", errors.Wrap(err, "static.newHostId") } return id, err } @@ -32,7 +31,7 @@ func newHostId() (string, error) { func newHostSetId() (string, error) { id, err := db.NewPublicId(HostSetPrefix) if err != nil { - return "", fmt.Errorf("new host set id: %w", err) + return "", errors.Wrap(err, "static.newHostSetId") } return id, err } diff --git a/internal/host/static/repository.go b/internal/host/static/repository.go index e09e21937..12444b385 100644 --- a/internal/host/static/repository.go +++ b/internal/host/static/repository.go @@ -1,8 +1,6 @@ package static import ( - "fmt" - "github.com/hashicorp/boundary/internal/db" "github.com/hashicorp/boundary/internal/errors" "github.com/hashicorp/boundary/internal/kms" @@ -25,13 +23,14 @@ type Repository struct { // limit applied to all ListX methods. // routines to access it. func NewRepository(r db.Reader, w db.Writer, kms *kms.Kms, opt ...Option) (*Repository, error) { + const op = "static.NewRepository" switch { case r == nil: - return nil, fmt.Errorf("db.Reader: %w", errors.ErrInvalidParameter) + return nil, errors.New(errors.InvalidParameter, op, "db.Reader") case w == nil: - return nil, fmt.Errorf("db.Writer: %w", errors.ErrInvalidParameter) + return nil, errors.New(errors.InvalidParameter, op, "db.Writer") case kms == nil: - return nil, fmt.Errorf("kms: %w", errors.ErrInvalidParameter) + return nil, errors.New(errors.InvalidParameter, op, "kms") } opts := getOpts(opt...) diff --git a/internal/host/static/repository_host.go b/internal/host/static/repository_host.go index d5e0817b8..f39d2afde 100644 --- a/internal/host/static/repository_host.go +++ b/internal/host/static/repository_host.go @@ -22,24 +22,25 @@ import ( // Both h.Name and h.Description are optional. If h.Name is set, it must be // unique within h.CatalogId. func (r *Repository) CreateHost(ctx context.Context, scopeId string, h *Host, opt ...Option) (*Host, error) { + const op = "static.CreateHost" if h == nil { - return nil, fmt.Errorf("create: static host: %w", errors.ErrInvalidParameter) + return nil, errors.New(errors.InvalidParameter, op, "nil Host") } if h.Host == nil { - return nil, fmt.Errorf("create: static host: embedded Host: %w", errors.ErrInvalidParameter) + return nil, errors.New(errors.InvalidParameter, op, "nil embedded Host") } if h.CatalogId == "" { - return nil, fmt.Errorf("create: static host: no catalog id: %w", errors.ErrInvalidParameter) + return nil, errors.New(errors.InvalidParameter, op, "no catalog id") } if h.PublicId != "" { - return nil, fmt.Errorf("create: static host: public id not empty: %w", errors.ErrInvalidParameter) + return nil, errors.New(errors.InvalidParameter, op, "public id not empty") } if scopeId == "" { - return nil, fmt.Errorf("create: static host: no scopeId: %w", errors.ErrInvalidParameter) + return nil, errors.New(errors.InvalidParameter, op, "no scope id") } h.Address = strings.TrimSpace(h.Address) if len(h.Address) < MinHostAddressLength || len(h.Address) > MaxHostAddressLength { - return nil, fmt.Errorf("create: static host: bad address: %w", ErrInvalidAddress) + return nil, errors.New(errors.InvalidAddress, op, "invalid address") } h = h.clone() @@ -47,20 +48,24 @@ func (r *Repository) CreateHost(ctx context.Context, scopeId string, h *Host, op if opts.withPublicId != "" { if !strings.HasPrefix(opts.withPublicId, HostPrefix+"_") { - return nil, fmt.Errorf("create: static host: passed-in public ID %q has wrong prefix, should be %q: %w", opts.withPublicId, HostPrefix, errors.ErrInvalidPublicId) + return nil, errors.New( + errors.InvalidPublicId, + op, + fmt.Sprintf("passed-in public ID %q has wrong prefix, should be %q", opts.withPublicId, HostPrefix), + ) } h.PublicId = opts.withPublicId } else { id, err := newHostId() if err != nil { - return nil, fmt.Errorf("create: static host: %w", err) + return nil, errors.Wrap(err, op) } h.PublicId = id } oplogWrapper, err := r.kms.GetWrapper(ctx, scopeId, kms.KeyPurposeOplog) if err != nil { - return nil, fmt.Errorf("create: static host: unable to get oplog wrapper: %w", err) + return nil, errors.Wrap(err, op, errors.WithMsg("unable to get oplog wrapper")) } var newHost *Host @@ -73,14 +78,17 @@ func (r *Repository) CreateHost(ctx context.Context, scopeId string, h *Host, op if err != nil { if errors.IsUniqueError(err) { - return nil, fmt.Errorf("create: static host: in catalog: %s: name %s already exists: %w", - h.CatalogId, h.Name, errors.ErrNotUnique) + return nil, errors.Wrap(err, op, errors.WithMsg(fmt.Sprintf("in catalog: %s: name %s already exists", h.CatalogId, h.Name))) } if errors.IsCheckConstraintError(err) || errors.IsNotNullError(err) { - return nil, fmt.Errorf("create: static host: in catalog: %s: %q: %w", - h.CatalogId, h.Address, ErrInvalidAddress) + return nil, errors.New( + errors.InvalidAddress, + op, + fmt.Sprintf("in catalog: %s: %q", h.CatalogId, h.Address), + errors.WithWrap(err), + ) } - return nil, fmt.Errorf("create: static host: in catalog: %s: %w", h.CatalogId, err) + return nil, errors.Wrap(err, op, errors.WithMsg(fmt.Sprintf("in catalog: %s", h.CatalogId))) } return newHost, nil } @@ -98,20 +106,21 @@ func (r *Repository) CreateHost(ctx context.Context, scopeId string, h *Host, op // An attribute of h will be set to NULL in the database if the attribute // in h is the zero value and it is included in fieldMaskPaths. func (r *Repository) UpdateHost(ctx context.Context, scopeId string, h *Host, version uint32, fieldMaskPaths []string, opt ...Option) (*Host, int, error) { + const op = "static.UpdateHost" if h == nil { - return nil, db.NoRowsAffected, fmt.Errorf("update: static host: %w", errors.ErrInvalidParameter) + return nil, db.NoRowsAffected, errors.New(errors.InvalidParameter, op, "nil Host") } if h.Host == nil { - return nil, db.NoRowsAffected, fmt.Errorf("update: static host: embedded Host: %w", errors.ErrInvalidParameter) + return nil, db.NoRowsAffected, errors.New(errors.InvalidParameter, op, "nil embedded Host") } if h.PublicId == "" { - return nil, db.NoRowsAffected, fmt.Errorf("update: static host: missing public id: %w", errors.ErrInvalidParameter) + return nil, db.NoRowsAffected, errors.New(errors.InvalidParameter, op, "no public id") } if version == 0 { - return nil, db.NoRowsAffected, fmt.Errorf("update: static host: no version supplied: %w", errors.ErrInvalidParameter) + return nil, db.NoRowsAffected, errors.New(errors.InvalidParameter, op, "no version") } if scopeId == "" { - return nil, db.NoRowsAffected, fmt.Errorf("update: static host: no scopeId: %w", errors.ErrInvalidParameter) + return nil, db.NoRowsAffected, errors.New(errors.InvalidParameter, op, "no scope id") } for _, f := range fieldMaskPaths { @@ -121,10 +130,10 @@ func (r *Repository) UpdateHost(ctx context.Context, scopeId string, h *Host, ve case strings.EqualFold("Address", f): h.Address = strings.TrimSpace(h.Address) if len(h.Address) < MinHostAddressLength || len(h.Address) > MaxHostAddressLength { - return nil, db.NoRowsAffected, fmt.Errorf("update: static host: bad address: %w", ErrInvalidAddress) + return nil, db.NoRowsAffected, errors.New(errors.InvalidAddress, op, "invalid address") } default: - return nil, db.NoRowsAffected, fmt.Errorf("update: static host: field: %s: %w", f, errors.ErrInvalidFieldMask) + return nil, db.NoRowsAffected, errors.New(errors.InvalidFieldMask, op, fmt.Sprintf("field: %s", f)) } } var dbMask, nullFields []string @@ -138,12 +147,12 @@ func (r *Repository) UpdateHost(ctx context.Context, scopeId string, h *Host, ve nil, ) if len(dbMask) == 0 && len(nullFields) == 0 { - return nil, db.NoRowsAffected, fmt.Errorf("update: static host: %w", errors.ErrEmptyFieldMask) + return nil, db.NoRowsAffected, errors.New(errors.EmptyFieldMask, op, "empty field mask") } oplogWrapper, err := r.kms.GetWrapper(ctx, scopeId, kms.KeyPurposeOplog) if err != nil { - return nil, db.NoRowsAffected, fmt.Errorf("update: static host: unable to get oplog wrapper: %w", err) + return nil, db.NoRowsAffected, errors.Wrap(err, op, errors.WithMsg("unable to get oplog wrapper")) } var rowsUpdated int @@ -156,7 +165,7 @@ func (r *Repository) UpdateHost(ctx context.Context, scopeId string, h *Host, ve db.WithOplog(oplogWrapper, h.oplog(oplog.OpType_OP_TYPE_UPDATE)), db.WithVersion(&version)) if err == nil && rowsUpdated > 1 { - return errors.ErrMultipleRecords + return errors.E(errors.WithCode(errors.MultipleRecords)) } return err }, @@ -164,13 +173,17 @@ func (r *Repository) UpdateHost(ctx context.Context, scopeId string, h *Host, ve if err != nil { if errors.IsUniqueError(err) { - return nil, db.NoRowsAffected, fmt.Errorf("update: static host: %s: name %s already exists: %w", - h.PublicId, h.Name, errors.ErrNotUnique) + return nil, db.NoRowsAffected, errors.Wrap(err, op, errors.WithMsg(fmt.Sprintf("in %s: name %s already exists", h.PublicId, h.Name))) } if errors.IsCheckConstraintError(err) || errors.IsNotNullError(err) { - return nil, db.NoRowsAffected, fmt.Errorf("update: static host: %s: %q: %w", h.PublicId, h.Address, ErrInvalidAddress) + return nil, db.NoRowsAffected, errors.New( + errors.InvalidAddress, + op, + fmt.Sprintf("in %s: %q", h.PublicId, h.Address), + errors.WithWrap(err), + ) } - return nil, db.NoRowsAffected, fmt.Errorf("update: static host: %s: %w", h.PublicId, err) + return nil, db.NoRowsAffected, errors.Wrap(err, op, errors.WithMsg(fmt.Sprintf("in %s", h.PublicId))) } return returnedHost, rowsUpdated, nil @@ -179,16 +192,17 @@ func (r *Repository) UpdateHost(ctx context.Context, scopeId string, h *Host, ve // LookupHost will look up a host in the repository. If the host is not // found, it will return nil, nil. All options are ignored. func (r *Repository) LookupHost(ctx context.Context, publicId string, opt ...Option) (*Host, error) { + const op = "static.LookupHost" if publicId == "" { - return nil, fmt.Errorf("lookup: static host: missing public id %w", errors.ErrInvalidParameter) + return nil, errors.New(errors.InvalidParameter, op, "no public id") } h := allocHost() h.PublicId = publicId if err := r.reader.LookupByPublicId(ctx, h); err != nil { - if errors.Is(err, errors.ErrRecordNotFound) { + if errors.IsNotFoundError(err) { return nil, nil } - return nil, fmt.Errorf("lookup: static host: failed %w for %s", err, publicId) + return nil, errors.Wrap(err, op, errors.WithMsg(fmt.Sprintf("failed for %s", publicId))) } return h, nil } @@ -196,8 +210,9 @@ func (r *Repository) LookupHost(ctx context.Context, publicId string, opt ...Opt // ListHosts returns a slice of Hosts for the catalogId. // WithLimit is the only option supported. func (r *Repository) ListHosts(ctx context.Context, catalogId string, opt ...Option) ([]*Host, error) { + const op = "static.ListHosts" if catalogId == "" { - return nil, fmt.Errorf("list: static host: missing catalog id: %w", errors.ErrInvalidParameter) + return nil, errors.New(errors.InvalidParameter, op, "no catalog id") } opts := getOpts(opt...) limit := r.defaultLimit @@ -208,7 +223,7 @@ func (r *Repository) ListHosts(ctx context.Context, catalogId string, opt ...Opt var hosts []*Host err := r.reader.SearchWhere(ctx, &hosts, "catalog_id = ?", []interface{}{catalogId}, db.WithLimit(limit)) if err != nil { - return nil, fmt.Errorf("list: static host: %w", err) + return nil, errors.Wrap(err, op) } return hosts, nil } @@ -217,15 +232,16 @@ func (r *Repository) ListHosts(ctx context.Context, catalogId string, opt ...Opt // returning a count of the number of records deleted. All options are // ignored. func (r *Repository) DeleteHost(ctx context.Context, scopeId string, publicId string, opt ...Option) (int, error) { + const op = "static.DeleteHost" if publicId == "" { - return db.NoRowsAffected, fmt.Errorf("delete: static host: missing public id: %w", errors.ErrInvalidParameter) + return db.NoRowsAffected, errors.New(errors.InvalidParameter, op, "no public id") } h := allocHost() h.PublicId = publicId oplogWrapper, err := r.kms.GetWrapper(ctx, scopeId, kms.KeyPurposeOplog) if err != nil { - return db.NoRowsAffected, fmt.Errorf("delete: static host catalog: unable to get oplog wrapper: %w", err) + return db.NoRowsAffected, errors.Wrap(err, op, errors.WithMsg("unable to get oplog wrapper")) } var rowsDeleted int @@ -235,14 +251,14 @@ func (r *Repository) DeleteHost(ctx context.Context, scopeId string, publicId st dh := h.clone() rowsDeleted, err = w.Delete(ctx, dh, db.WithOplog(oplogWrapper, h.oplog(oplog.OpType_OP_TYPE_DELETE))) if err == nil && rowsDeleted > 1 { - return errors.ErrMultipleRecords + return errors.E(errors.WithCode(errors.MultipleRecords)) } return err }, ) if err != nil { - return db.NoRowsAffected, fmt.Errorf("delete: static host: %s: %w", publicId, err) + return db.NoRowsAffected, errors.Wrap(err, op, errors.WithMsg(fmt.Sprintf("delete failed for %s", publicId))) } return rowsDeleted, nil diff --git a/internal/host/static/repository_host_catalog.go b/internal/host/static/repository_host_catalog.go index 6ba8ed674..916d04a07 100644 --- a/internal/host/static/repository_host_catalog.go +++ b/internal/host/static/repository_host_catalog.go @@ -21,17 +21,18 @@ import ( // // Both c.CreateTime and c.UpdateTime are ignored. func (r *Repository) CreateCatalog(ctx context.Context, c *HostCatalog, opt ...Option) (*HostCatalog, error) { + const op = "static.CreateCatalog" if c == nil { - return nil, fmt.Errorf("create: static host catalog: %w", errors.ErrInvalidParameter) + return nil, errors.New(errors.InvalidParameter, op, "nil HostCatalog") } if c.HostCatalog == nil { - return nil, fmt.Errorf("create: static host catalog: embedded HostCatalog: %w", errors.ErrInvalidParameter) + return nil, errors.New(errors.InvalidParameter, op, "nil embedded HostCatalog") } if c.ScopeId == "" { - return nil, fmt.Errorf("create: static host catalog: no scope id: %w", errors.ErrInvalidParameter) + return nil, errors.New(errors.InvalidParameter, op, "no scope id") } if c.PublicId != "" { - return nil, fmt.Errorf("create: static host catalog: public id not empty: %w", errors.ErrInvalidParameter) + return nil, errors.New(errors.InvalidParameter, op, "public id not empty") } c = c.clone() @@ -39,20 +40,24 @@ func (r *Repository) CreateCatalog(ctx context.Context, c *HostCatalog, opt ...O if opts.withPublicId != "" { if !strings.HasPrefix(opts.withPublicId, HostCatalogPrefix+"_") { - return nil, fmt.Errorf("create: static host catalog: passed-in public ID %q has wrong prefix, should be %q: %w", opts.withPublicId, HostCatalogPrefix, errors.ErrInvalidPublicId) + return nil, errors.New( + errors.InvalidPublicId, + op, + fmt.Sprintf("passed-in public ID %q has wrong prefix, should be %q", opts.withPublicId, HostCatalogPrefix), + ) } c.PublicId = opts.withPublicId } else { id, err := newHostCatalogId() if err != nil { - return nil, fmt.Errorf("create: static host catalog: %w", err) + return nil, errors.Wrap(err, op) } c.PublicId = id } oplogWrapper, err := r.kms.GetWrapper(ctx, c.ScopeId, kms.KeyPurposeOplog) if err != nil { - return nil, fmt.Errorf("create: static host catalog: unable to get oplog wrapper: %w", err) + return nil, errors.Wrap(err, op, errors.WithMsg("unable to get oplog wrapper")) } metadata := newCatalogMetadata(c, oplog.OpType_OP_TYPE_CREATE) @@ -74,10 +79,9 @@ func (r *Repository) CreateCatalog(ctx context.Context, c *HostCatalog, opt ...O if err != nil { if errors.IsUniqueError(err) { - return nil, fmt.Errorf("create: static host catalog: in scope: %s: name %s already exists: %w", - c.ScopeId, c.Name, errors.ErrNotUnique) + return nil, errors.Wrap(err, op, errors.WithMsg(fmt.Sprintf("in scope: %s: name %s already exists", c.ScopeId, c.Name))) } - return nil, fmt.Errorf("create: static host catalog: in scope: %s: %w", c.ScopeId, err) + return nil, errors.Wrap(err, op, errors.WithMsg(fmt.Sprintf("in scope: %s", c.ScopeId))) } return newHostCatalog, nil } @@ -94,20 +98,21 @@ func (r *Repository) CreateCatalog(ctx context.Context, c *HostCatalog, opt ...O // An attribute of c will be set to NULL in the database if the attribute // in c is the zero value and it is included in fieldMask. func (r *Repository) UpdateCatalog(ctx context.Context, c *HostCatalog, version uint32, fieldMask []string, opt ...Option) (*HostCatalog, int, error) { + const op = "static.UpdateCatalog" if c == nil { - return nil, db.NoRowsAffected, fmt.Errorf("update: static host catalog: %w", errors.ErrInvalidParameter) + return nil, db.NoRowsAffected, errors.New(errors.InvalidParameter, op, "nil HostCatalog") } if c.HostCatalog == nil { - return nil, db.NoRowsAffected, fmt.Errorf("update: static host catalog: embedded HostCatalog: %w", errors.ErrInvalidParameter) + return nil, db.NoRowsAffected, errors.New(errors.InvalidParameter, op, "nil embedded HostCatalog") } if c.PublicId == "" { - return nil, db.NoRowsAffected, fmt.Errorf("update: static host catalog: missing public id: %w", errors.ErrInvalidParameter) + return nil, db.NoRowsAffected, errors.New(errors.InvalidParameter, op, "no public id") } if c.ScopeId == "" { - return nil, db.NoRowsAffected, fmt.Errorf("update: static host catalog: missing scope id: %w", errors.ErrInvalidParameter) + return nil, db.NoRowsAffected, errors.New(errors.InvalidParameter, op, "no scope id") } if len(fieldMask) == 0 { - return nil, db.NoRowsAffected, fmt.Errorf("update: static host catalog: %w", errors.ErrEmptyFieldMask) + return nil, db.NoRowsAffected, errors.New(errors.EmptyFieldMask, op, "empty field mask") } var dbMask, nullFields []string @@ -123,13 +128,13 @@ func (r *Repository) UpdateCatalog(ctx context.Context, c *HostCatalog, version dbMask = append(dbMask, "description") default: - return nil, db.NoRowsAffected, fmt.Errorf("update: static host catalog: field: %s: %w", f, errors.ErrInvalidFieldMask) + return nil, db.NoRowsAffected, errors.New(errors.InvalidFieldMask, op, fmt.Sprintf("field: %s", f)) } } oplogWrapper, err := r.kms.GetWrapper(ctx, c.ScopeId, kms.KeyPurposeOplog) if err != nil { - return nil, db.NoRowsAffected, fmt.Errorf("update: static host catalog: unable to get oplog wrapper: %w", err) + return nil, db.NoRowsAffected, errors.Wrap(err, op, errors.WithMsg("unable to get oplog wrapper")) } c = c.clone() @@ -154,7 +159,7 @@ func (r *Repository) UpdateCatalog(ctx context.Context, c *HostCatalog, version db.WithVersion(&version), ) if err == nil && rowsUpdated > 1 { - return errors.ErrMultipleRecords + return errors.E(errors.WithCode(errors.MultipleRecords)) } return err }, @@ -162,10 +167,9 @@ func (r *Repository) UpdateCatalog(ctx context.Context, c *HostCatalog, version if err != nil { if errors.IsUniqueError(err) { - return nil, db.NoRowsAffected, fmt.Errorf("update: static host catalog: %s: name %s already exists: %w", - c.PublicId, c.Name, errors.ErrNotUnique) + return nil, db.NoRowsAffected, errors.Wrap(err, op, errors.WithMsg(fmt.Sprintf("in %s: name %s already exists", c.PublicId, c.Name))) } - return nil, db.NoRowsAffected, fmt.Errorf("update: static host catalog: %s: %w", c.PublicId, err) + return nil, db.NoRowsAffected, errors.Wrap(err, op, errors.WithMsg(fmt.Sprintf("in %s", c.PublicId))) } return returnedCatalog, rowsUpdated, nil @@ -174,24 +178,26 @@ func (r *Repository) UpdateCatalog(ctx context.Context, c *HostCatalog, version // LookupCatalog returns the HostCatalog for id. Returns nil, nil if no // HostCatalog is found for id. func (r *Repository) LookupCatalog(ctx context.Context, id string, opt ...Option) (*HostCatalog, error) { + const op = "static.LookupCatalog" if id == "" { - return nil, fmt.Errorf("lookup: static host catalog: missing public id: %w", errors.ErrInvalidParameter) + return nil, errors.New(errors.InvalidParameter, op, "no public id") } c := allocCatalog() c.PublicId = id if err := r.reader.LookupByPublicId(ctx, c); err != nil { - if err == errors.ErrRecordNotFound { + if errors.IsNotFoundError(err) { return nil, nil } - return nil, fmt.Errorf("lookup: static host catalog: %s: %w", id, err) + return nil, errors.Wrap(err, op, errors.WithMsg(fmt.Sprintf("failed for: %s", id))) } return c, nil } // ListCatalogs returns a slice of HostCatalogs for the scopeId. WithLimit is the only option supported. func (r *Repository) ListCatalogs(ctx context.Context, scopeId string, opt ...Option) ([]*HostCatalog, error) { + const op = "static.ListCatalogs" if scopeId == "" { - return nil, fmt.Errorf("list: static host catalog: missing scope id: %w", errors.ErrInvalidParameter) + return nil, errors.New(errors.InvalidParameter, op, "no scope id") } opts := getOpts(opt...) limit := r.defaultLimit @@ -202,7 +208,7 @@ func (r *Repository) ListCatalogs(ctx context.Context, scopeId string, opt ...Op var hostCatalogs []*HostCatalog err := r.reader.SearchWhere(ctx, &hostCatalogs, "scope_id = ?", []interface{}{scopeId}, db.WithLimit(limit)) if err != nil { - return nil, fmt.Errorf("list: static host catalog: %w", err) + return nil, errors.Wrap(err, op) } return hostCatalogs, nil } @@ -210,24 +216,25 @@ func (r *Repository) ListCatalogs(ctx context.Context, scopeId string, opt ...Op // DeleteCatalog deletes id from the repository returning a count of the // number of records deleted. func (r *Repository) DeleteCatalog(ctx context.Context, id string, opt ...Option) (int, error) { + const op = "static.DeleteCatalog" if id == "" { - return db.NoRowsAffected, fmt.Errorf("delete: static host catalog: missing public id: %w", errors.ErrInvalidParameter) + return db.NoRowsAffected, errors.New(errors.InvalidParameter, op, "no public id") } c := allocCatalog() c.PublicId = id if err := r.reader.LookupByPublicId(ctx, c); err != nil { - if errors.Is(err, errors.ErrRecordNotFound) { + if errors.IsNotFoundError(err) { return db.NoRowsAffected, nil } - return db.NoRowsAffected, fmt.Errorf("delete: static host catalog: failed %w for %s", err, id) + return db.NoRowsAffected, errors.Wrap(err, op, errors.WithMsg(fmt.Sprintf("failed for %s", id))) } if c.ScopeId == "" { - return db.NoRowsAffected, fmt.Errorf("delete: static host catalog: missing scope id: %w", errors.ErrInvalidParameter) + return db.NoRowsAffected, errors.New(errors.InvalidParameter, op, "no scope id") } oplogWrapper, err := r.kms.GetWrapper(ctx, c.ScopeId, kms.KeyPurposeOplog) if err != nil { - return db.NoRowsAffected, fmt.Errorf("delete: static host catalog: unable to get oplog wrapper: %w", err) + return db.NoRowsAffected, errors.Wrap(err, op, errors.WithMsg("unable to get oplog wrapper")) } metadata := newCatalogMetadata(c, oplog.OpType_OP_TYPE_DELETE) @@ -247,14 +254,14 @@ func (r *Repository) DeleteCatalog(ctx context.Context, id string, opt ...Option db.WithOplog(oplogWrapper, metadata), ) if err == nil && rowsDeleted > 1 { - return errors.ErrMultipleRecords + return errors.E(errors.WithCode(errors.MultipleRecords)) } return err }, ) if err != nil { - return db.NoRowsAffected, fmt.Errorf("delete: static host catalog: %s: %w", c.PublicId, err) + return db.NoRowsAffected, errors.Wrap(err, op, errors.WithMsg(fmt.Sprintf("delete failed for %s", c.PublicId))) } return rowsDeleted, nil diff --git a/internal/host/static/repository_host_catalog_test.go b/internal/host/static/repository_host_catalog_test.go index 26bd26fa8..526d74611 100644 --- a/internal/host/static/repository_host_catalog_test.go +++ b/internal/host/static/repository_host_catalog_test.go @@ -25,16 +25,16 @@ func TestRepository_CreateCatalog(t *testing.T) { in *HostCatalog opts []Option want *HostCatalog - wantIsErr error + wantIsErr errors.Code }{ { name: "nil-catalog", - wantIsErr: errors.ErrInvalidParameter, + wantIsErr: errors.InvalidParameter, }, { name: "nil-embedded-catalog", in: &HostCatalog{}, - wantIsErr: errors.ErrInvalidParameter, + wantIsErr: errors.InvalidParameter, }, { name: "valid-no-options", @@ -87,8 +87,8 @@ func TestRepository_CreateCatalog(t *testing.T) { assert.Empty(tt.in.PublicId) } got, err := repo.CreateCatalog(context.Background(), tt.in, tt.opts...) - if tt.wantIsErr != nil { - assert.Truef(errors.Is(err, tt.wantIsErr), "want err: %q got: %q", tt.wantIsErr, err) + if tt.wantIsErr != 0 { + assert.Truef(errors.Match(errors.T(tt.wantIsErr), err), "want err: %q got: %q", tt.wantIsErr, err) assert.Nil(got) return } @@ -236,7 +236,7 @@ func TestRepository_UpdateCatalog(t *testing.T) { masks []string want *HostCatalog wantCount int - wantIsErr error + wantIsErr errors.Code }{ { name: "nil-catalog", @@ -245,7 +245,7 @@ func TestRepository_UpdateCatalog(t *testing.T) { }, chgFn: makeNil(), masks: []string{"Name", "Description"}, - wantIsErr: errors.ErrInvalidParameter, + wantIsErr: errors.InvalidParameter, }, { name: "nil-embedded-catalog", @@ -254,7 +254,7 @@ func TestRepository_UpdateCatalog(t *testing.T) { }, chgFn: makeEmbeddedNil(), masks: []string{"Name", "Description"}, - wantIsErr: errors.ErrInvalidParameter, + wantIsErr: errors.InvalidParameter, }, { name: "no-public-id", @@ -263,7 +263,7 @@ func TestRepository_UpdateCatalog(t *testing.T) { }, chgFn: deletePublicId(), masks: []string{"Name", "Description"}, - wantIsErr: errors.ErrInvalidParameter, + wantIsErr: errors.InvalidParameter, }, { name: "updating-non-existent-catalog", @@ -274,7 +274,7 @@ func TestRepository_UpdateCatalog(t *testing.T) { }, chgFn: combine(nonExistentPublicId(), changeName("test-update-name-repo")), masks: []string{"Name"}, - wantIsErr: errors.ErrRecordNotFound, + wantIsErr: errors.RecordNotFound, }, { name: "empty-field-mask", @@ -284,7 +284,7 @@ func TestRepository_UpdateCatalog(t *testing.T) { }, }, chgFn: changeName("test-update-name-repo"), - wantIsErr: errors.ErrEmptyFieldMask, + wantIsErr: errors.EmptyFieldMask, }, { name: "read-only-fields-in-field-mask", @@ -295,7 +295,7 @@ func TestRepository_UpdateCatalog(t *testing.T) { }, chgFn: changeName("test-update-name-repo"), masks: []string{"PublicId", "CreateTime", "UpdateTime", "ScopeId"}, - wantIsErr: errors.ErrInvalidFieldMask, + wantIsErr: errors.InvalidFieldMask, }, { name: "unknown-field-in-field-mask", @@ -306,7 +306,7 @@ func TestRepository_UpdateCatalog(t *testing.T) { }, chgFn: changeName("test-update-name-repo"), masks: []string{"Bilbo"}, - wantIsErr: errors.ErrInvalidFieldMask, + wantIsErr: errors.InvalidFieldMask, }, { name: "change-name", @@ -448,8 +448,8 @@ func TestRepository_UpdateCatalog(t *testing.T) { orig = tt.chgFn(orig) } got, gotCount, err := repo.UpdateCatalog(context.Background(), orig, 1, tt.masks) - if tt.wantIsErr != nil { - assert.Truef(errors.Is(err, tt.wantIsErr), "want err: %q got: %q", tt.wantIsErr, err) + if tt.wantIsErr != 0 { + assert.Truef(errors.Match(errors.T(tt.wantIsErr), err), "want err: %q got: %q", tt.wantIsErr, err) assert.Equal(tt.wantCount, gotCount, "row count") assert.Nil(got) return @@ -579,7 +579,7 @@ func TestRepository_LookupCatalog(t *testing.T) { name string id string want *HostCatalog - wantErr error + wantErr errors.Code }{ { name: "found", @@ -595,7 +595,7 @@ func TestRepository_LookupCatalog(t *testing.T) { name: "bad-public-id", id: "", want: nil, - wantErr: errors.ErrInvalidParameter, + wantErr: errors.InvalidParameter, }, } @@ -609,8 +609,8 @@ func TestRepository_LookupCatalog(t *testing.T) { assert.NotNil(repo) got, err := repo.LookupCatalog(context.Background(), tt.id) - if tt.wantErr != nil { - assert.Truef(errors.Is(err, tt.wantErr), "want err: %q got: %q", tt.wantErr, err) + if tt.wantErr != 0 { + assert.Truef(errors.Match(errors.T(tt.wantErr), err), "want err: %q got: %q", tt.wantErr, err) return } assert.NoError(err) @@ -641,7 +641,7 @@ func TestRepository_DeleteCatalog(t *testing.T) { name string id string want int - wantErr error + wantErr errors.Code }{ { name: "found", @@ -657,7 +657,7 @@ func TestRepository_DeleteCatalog(t *testing.T) { name: "bad-public-id", id: "", want: 0, - wantErr: errors.ErrInvalidParameter, + wantErr: errors.InvalidParameter, }, } @@ -671,8 +671,8 @@ func TestRepository_DeleteCatalog(t *testing.T) { assert.NotNil(repo) got, err := repo.DeleteCatalog(context.Background(), tt.id) - if tt.wantErr != nil { - assert.Truef(errors.Is(err, tt.wantErr), "want err: %q got: %q", tt.wantErr, err) + if tt.wantErr != 0 { + assert.Truef(errors.Match(errors.T(tt.wantErr), err), "want err: %q got: %q", tt.wantErr, err) return } assert.NoError(err) diff --git a/internal/host/static/repository_host_set.go b/internal/host/static/repository_host_set.go index 476c208bf..86551f3e5 100644 --- a/internal/host/static/repository_host_set.go +++ b/internal/host/static/repository_host_set.go @@ -20,20 +20,21 @@ import ( // Both s.Name and s.Description are optional. If s.Name is set, it must be // unique within s.CatalogId. func (r *Repository) CreateSet(ctx context.Context, scopeId string, s *HostSet, opt ...Option) (*HostSet, error) { + const op = "static.CreateSet" if s == nil { - return nil, fmt.Errorf("create: static host set: %w", errors.ErrInvalidParameter) + return nil, errors.New(errors.InvalidParameter, op, "nil HostSet") } if s.HostSet == nil { - return nil, fmt.Errorf("create: static host set: embedded HostSet: %w", errors.ErrInvalidParameter) + return nil, errors.New(errors.InvalidParameter, op, "nil embedded HostSet") } if s.CatalogId == "" { - return nil, fmt.Errorf("create: static host set: no catalog id: %w", errors.ErrInvalidParameter) + return nil, errors.New(errors.InvalidParameter, op, "no catalog id") } if s.PublicId != "" { - return nil, fmt.Errorf("create: static host set: public id not empty: %w", errors.ErrInvalidParameter) + return nil, errors.New(errors.InvalidParameter, op, "public id not empty") } if scopeId == "" { - return nil, fmt.Errorf("create: static host set: no scopeId: %w", errors.ErrInvalidParameter) + return nil, errors.New(errors.InvalidParameter, op, "no scope id") } s = s.clone() @@ -41,20 +42,24 @@ func (r *Repository) CreateSet(ctx context.Context, scopeId string, s *HostSet, if opts.withPublicId != "" { if !strings.HasPrefix(opts.withPublicId, HostSetPrefix+"_") { - return nil, fmt.Errorf("create: static host set: passed-in public ID %q has wrong prefix, should be %q: %w", opts.withPublicId, HostSetPrefix, errors.ErrInvalidPublicId) + return nil, errors.New( + errors.InvalidPublicId, + op, + fmt.Sprintf("passed-in public ID %q has wrong prefix, should be %q", opts.withPublicId, HostSetPrefix), + ) } s.PublicId = opts.withPublicId } else { id, err := newHostSetId() if err != nil { - return nil, fmt.Errorf("create: static host set: %w", err) + return nil, errors.Wrap(err, op) } s.PublicId = id } oplogWrapper, err := r.kms.GetWrapper(ctx, scopeId, kms.KeyPurposeOplog) if err != nil { - return nil, fmt.Errorf("create: static host set: unable to get oplog wrapper: %w", err) + return nil, errors.Wrap(err, op, errors.WithMsg("unable to get oplog wrapper")) } var newHostSet *HostSet @@ -67,10 +72,9 @@ func (r *Repository) CreateSet(ctx context.Context, scopeId string, s *HostSet, if err != nil { if errors.IsUniqueError(err) { - return nil, fmt.Errorf("create: static host set: in catalog: %s: name %s already exists: %w", - s.CatalogId, s.Name, errors.ErrNotUnique) + return nil, errors.Wrap(err, op, errors.WithMsg(fmt.Sprintf("in catalog: %s: name %s already exists", s.CatalogId, s.Name))) } - return nil, fmt.Errorf("create: static host set: in catalog: %s: %w", s.CatalogId, err) + return nil, errors.Wrap(err, op, errors.WithMsg(fmt.Sprintf("in catalog: %s", s.CatalogId))) } return newHostSet, nil } @@ -90,20 +94,21 @@ func (r *Repository) CreateSet(ctx context.Context, scopeId string, s *HostSet, // The WithLimit option can be used to limit the number of hosts returned. // All other options are ignored. func (r *Repository) UpdateSet(ctx context.Context, scopeId string, s *HostSet, version uint32, fieldMaskPaths []string, opt ...Option) (*HostSet, []*Host, int, error) { + const op = "static.UpdateSet" if s == nil { - return nil, nil, db.NoRowsAffected, fmt.Errorf("update: static host set: %w", errors.ErrInvalidParameter) + return nil, nil, db.NoRowsAffected, errors.New(errors.InvalidParameter, op, "nil HostSet") } if s.HostSet == nil { - return nil, nil, db.NoRowsAffected, fmt.Errorf("update: static host set: embedded HostSet: %w", errors.ErrInvalidParameter) + return nil, nil, db.NoRowsAffected, errors.New(errors.InvalidParameter, op, "nil embedded HostSet") } if s.PublicId == "" { - return nil, nil, db.NoRowsAffected, fmt.Errorf("update: static host set: missing public id: %w", errors.ErrInvalidParameter) + return nil, nil, db.NoRowsAffected, errors.New(errors.InvalidParameter, op, "no public id") } if version == 0 { - return nil, nil, db.NoRowsAffected, fmt.Errorf("update: static host set: no version supplied: %w", errors.ErrInvalidParameter) + return nil, nil, db.NoRowsAffected, errors.New(errors.InvalidParameter, op, "no version") } if scopeId == "" { - return nil, nil, db.NoRowsAffected, fmt.Errorf("update: static host set: no scopeId: %w", errors.ErrInvalidParameter) + return nil, nil, db.NoRowsAffected, errors.New(errors.InvalidParameter, op, "no scope id") } for _, f := range fieldMaskPaths { @@ -111,7 +116,7 @@ func (r *Repository) UpdateSet(ctx context.Context, scopeId string, s *HostSet, case strings.EqualFold("Name", f): case strings.EqualFold("Description", f): default: - return nil, nil, db.NoRowsAffected, fmt.Errorf("update: static host set: field: %s: %w", f, errors.ErrInvalidFieldMask) + return nil, nil, db.NoRowsAffected, errors.New(errors.InvalidFieldMask, op, fmt.Sprintf("field: %s", f)) } } var dbMask, nullFields []string @@ -124,7 +129,7 @@ func (r *Repository) UpdateSet(ctx context.Context, scopeId string, s *HostSet, nil, ) if len(dbMask) == 0 && len(nullFields) == 0 { - return nil, nil, db.NoRowsAffected, fmt.Errorf("update: static host set: %w", errors.ErrEmptyFieldMask) + return nil, nil, db.NoRowsAffected, errors.New(errors.EmptyFieldMask, op, "empty field mask") } opts := getOpts(opt...) @@ -136,7 +141,7 @@ func (r *Repository) UpdateSet(ctx context.Context, scopeId string, s *HostSet, oplogWrapper, err := r.kms.GetWrapper(ctx, scopeId, kms.KeyPurposeOplog) if err != nil { - return nil, nil, db.NoRowsAffected, fmt.Errorf("update: static host set: unable to get oplog wrapper: %w", err) + return nil, nil, db.NoRowsAffected, errors.Wrap(err, op, errors.WithMsg("unable to get oplog wrapper")) } var rowsUpdated int @@ -150,7 +155,7 @@ func (r *Repository) UpdateSet(ctx context.Context, scopeId string, s *HostSet, db.WithOplog(oplogWrapper, s.oplog(oplog.OpType_OP_TYPE_UPDATE)), db.WithVersion(&version)) if err == nil && rowsUpdated > 1 { - return errors.ErrMultipleRecords + return errors.E(errors.WithCode(errors.MultipleRecords)) } if err != nil { return err @@ -162,10 +167,9 @@ func (r *Repository) UpdateSet(ctx context.Context, scopeId string, s *HostSet, if err != nil { if errors.IsUniqueError(err) { - return nil, nil, db.NoRowsAffected, fmt.Errorf("update: static host set: %s: name %s already exists: %w", - s.PublicId, s.Name, errors.ErrNotUnique) + return nil, nil, db.NoRowsAffected, errors.Wrap(err, op, errors.WithMsg(fmt.Sprintf("in %s: name %s already exists", s.PublicId, s.Name))) } - return nil, nil, db.NoRowsAffected, fmt.Errorf("update: static host set: %s: %w", s.PublicId, err) + return nil, nil, db.NoRowsAffected, errors.Wrap(err, op, errors.WithMsg(fmt.Sprintf("in %s", s.PublicId))) } return returnedHostSet, hosts, rowsUpdated, nil @@ -176,8 +180,9 @@ func (r *Repository) UpdateSet(ctx context.Context, scopeId string, s *HostSet, // found, it will return nil, nil, nil. The WithLimit option can be used to // limit the number of hosts returned. All other options are ignored. func (r *Repository) LookupSet(ctx context.Context, publicId string, opt ...Option) (*HostSet, []*Host, error) { + const op = "static.LookupSet" if publicId == "" { - return nil, nil, fmt.Errorf("lookup: static host set: missing public id %w", errors.ErrInvalidParameter) + return nil, nil, errors.New(errors.InvalidParameter, op, "no public id") } opts := getOpts(opt...) limit := r.defaultLimit @@ -192,7 +197,7 @@ func (r *Repository) LookupSet(ctx context.Context, publicId string, opt ...Opti var hosts []*Host _, err := r.writer.DoTx(ctx, db.StdRetryCnt, db.ExpBackoff{}, func(reader db.Reader, _ db.Writer) error { if err := reader.LookupByPublicId(ctx, s); err != nil { - if errors.Is(err, errors.ErrRecordNotFound) { + if errors.IsNotFoundError(err) { s = nil return nil } @@ -204,7 +209,7 @@ func (r *Repository) LookupSet(ctx context.Context, publicId string, opt ...Opti }) if err != nil { - return nil, nil, fmt.Errorf("lookup: static host set: failed %w for %s", err, publicId) + return nil, nil, errors.Wrap(err, op, errors.WithMsg(fmt.Sprintf("in %s", s.PublicId))) } return s, hosts, nil @@ -213,8 +218,9 @@ func (r *Repository) LookupSet(ctx context.Context, publicId string, opt ...Opti // ListSets returns a slice of HostSets for the catalogId. WithLimit is the // only option supported. func (r *Repository) ListSets(ctx context.Context, catalogId string, opt ...Option) ([]*HostSet, error) { + const op = "static.ListSets" if catalogId == "" { - return nil, fmt.Errorf("list: static host set: missing catalog id: %w", errors.ErrInvalidParameter) + return nil, errors.New(errors.InvalidParameter, op, "no catalog id") } opts := getOpts(opt...) limit := r.defaultLimit @@ -225,7 +231,7 @@ func (r *Repository) ListSets(ctx context.Context, catalogId string, opt ...Opti var sets []*HostSet err := r.reader.SearchWhere(ctx, &sets, "catalog_id = ?", []interface{}{catalogId}, db.WithLimit(limit)) if err != nil { - return nil, fmt.Errorf("list: static host set: %w", err) + return nil, errors.Wrap(err, op) } return sets, nil } @@ -234,18 +240,19 @@ func (r *Repository) ListSets(ctx context.Context, catalogId string, opt ...Opti // returning a count of the number of records deleted. All options are // ignored. func (r *Repository) DeleteSet(ctx context.Context, scopeId string, publicId string, opt ...Option) (int, error) { + const op = "static.DeleteSet" if publicId == "" { - return db.NoRowsAffected, fmt.Errorf("delete: static host set: missing public id: %w", errors.ErrInvalidParameter) + return db.NoRowsAffected, errors.New(errors.InvalidParameter, op, "no public id") } if scopeId == "" { - return db.NoRowsAffected, fmt.Errorf("delete: static host set: no scopeId: %w", errors.ErrInvalidParameter) + return db.NoRowsAffected, errors.New(errors.InvalidParameter, op, "no scope id") } s := allocHostSet() s.PublicId = publicId oplogWrapper, err := r.kms.GetWrapper(ctx, scopeId, kms.KeyPurposeOplog) if err != nil { - return db.NoRowsAffected, fmt.Errorf("delete: static host set: unable to get oplog wrapper: %w", err) + return db.NoRowsAffected, errors.Wrap(err, op, errors.WithMsg("unable to get oplog wrapper")) } var rowsDeleted int @@ -255,14 +262,14 @@ func (r *Repository) DeleteSet(ctx context.Context, scopeId string, publicId str ds := s.clone() rowsDeleted, err = w.Delete(ctx, ds, db.WithOplog(oplogWrapper, s.oplog(oplog.OpType_OP_TYPE_DELETE))) if err == nil && rowsDeleted > 1 { - return errors.ErrMultipleRecords + return errors.E(errors.WithCode(errors.MultipleRecords)) } return err }, ) if err != nil { - return db.NoRowsAffected, fmt.Errorf("delete: static host set: %s: %w", publicId, err) + return db.NoRowsAffected, errors.Wrap(err, op, errors.WithMsg(fmt.Sprintf("delete failed for %s", s.PublicId))) } return rowsDeleted, nil diff --git a/internal/host/static/repository_host_set_member.go b/internal/host/static/repository_host_set_member.go index 9f394356c..00d773a6c 100644 --- a/internal/host/static/repository_host_set_member.go +++ b/internal/host/static/repository_host_set_member.go @@ -18,28 +18,29 @@ import ( // the set to be added. The version must match the current version of the // setId in the repository. func (r *Repository) AddSetMembers(ctx context.Context, scopeId string, setId string, version uint32, hostIds []string, opt ...Option) ([]*Host, error) { + const op = "static.AddSetMembers" if scopeId == "" { - return nil, fmt.Errorf("add: static host set members: missing scope id: %w", errors.ErrInvalidParameter) + return nil, errors.New(errors.InvalidParameter, op, "no scope id") } if setId == "" { - return nil, fmt.Errorf("add: static host set members: missing set id: %w", errors.ErrInvalidParameter) + return nil, errors.New(errors.InvalidParameter, op, "no set id") } if version == 0 { - return nil, fmt.Errorf("add: static host set members: version is zero: %w", errors.ErrInvalidParameter) + return nil, errors.New(errors.InvalidParameter, op, "no version") } if len(hostIds) == 0 { - return nil, fmt.Errorf("add: static host set members: empty hostIds: %w", errors.ErrInvalidParameter) + return nil, errors.New(errors.InvalidParameter, op, "no host ids") } // Create in-memory host set members members, err := r.newMembers(setId, hostIds) if err != nil { - return nil, fmt.Errorf("add: static host set members: %w", err) + return nil, errors.Wrap(err, op) } wrapper, err := r.kms.GetWrapper(ctx, scopeId, kms.KeyPurposeOplog) if err != nil { - return nil, fmt.Errorf("add: static host set members: unable to get oplog wrapper: %w", err) + return nil, errors.Wrap(err, op, errors.WithMsg("unable to get oplog wrapper")) } var hosts []*Host @@ -61,7 +62,7 @@ func (r *Repository) AddSetMembers(ctx context.Context, scopeId string, setId st return err }) if err != nil { - return nil, fmt.Errorf("add: static host set members: %w", err) + return nil, errors.Wrap(err, op) } return hosts, nil @@ -73,7 +74,7 @@ func (r *Repository) newMembers(setId string, hostIds []string) ([]interface{}, var m *HostSetMember m, err := NewHostSetMember(setId, id) if err != nil { - return nil, fmt.Errorf("new members: %w", err) + return nil, errors.Wrap(err, "static.newMembers") } members = append(members, m) } @@ -83,29 +84,30 @@ func (r *Repository) newMembers(setId string, hostIds []string) ([]interface{}, func createMembers(ctx context.Context, w db.Writer, members []interface{}) ([]*oplog.Message, error) { var msgs []*oplog.Message if err := w.CreateItems(ctx, members, db.NewOplogMsgs(&msgs)); err != nil { - return nil, fmt.Errorf("unable to create host set members: %w", err) + return nil, errors.Wrap(err, "static.createMembers") } return msgs, nil } func updateVersion(ctx context.Context, w db.Writer, wrapper wrapping.Wrapper, metadata oplog.Metadata, msgs []*oplog.Message, set *HostSet, version uint32) error { + const op = "static.updateVersion" setMsg := new(oplog.Message) rowsUpdated, err := w.Update(ctx, set, []string{"Version"}, nil, db.NewOplogMsg(setMsg), db.WithVersion(&version)) switch { case err != nil: - return fmt.Errorf("unable to update host set version: %w", err) + return errors.Wrap(err, op) case rowsUpdated > 1: - return fmt.Errorf("unable to update host set version: %w", errors.ErrMultipleRecords) + return errors.E(errors.WithCode(errors.MultipleRecords)) } msgs = append(msgs, setMsg) // Write oplog ticket, err := w.GetTicket(set) if err != nil { - return fmt.Errorf("unable to get ticket: %w", err) + return errors.Wrap(err, op, errors.WithMsg("unable to get ticket")) } if err := w.WriteOplogEntryWith(ctx, wrapper, ticket, metadata, msgs); err != nil { - return fmt.Errorf("unable to write oplog: %w", err) + return errors.Wrap(err, op, errors.WithMsg("unable to write oplog")) } return nil } @@ -142,7 +144,7 @@ func getHosts(ctx context.Context, reader db.Reader, setId string, limit int) ([ params, db.WithLimit(limit), ); err != nil { - return nil, fmt.Errorf("get hosts: %w", err) + return nil, errors.Wrap(err, "static.getHosts") } if len(hosts) == 0 { return nil, nil @@ -154,28 +156,29 @@ func getHosts(ctx context.Context, reader db.Reader, setId string, limit int) ([ // returns the number of hosts deleted from the set. The version must match // the current version of the setId in the repository. func (r *Repository) DeleteSetMembers(ctx context.Context, scopeId string, setId string, version uint32, hostIds []string, opt ...Option) (int, error) { + const op = "static.DeleteSetMembers" if scopeId == "" { - return db.NoRowsAffected, fmt.Errorf("delete: static host set members: missing scope id: %w", errors.ErrInvalidParameter) + return db.NoRowsAffected, errors.New(errors.InvalidParameter, op, "no scope id") } if setId == "" { - return db.NoRowsAffected, fmt.Errorf("delete: static host set members: missing set id: %w", errors.ErrInvalidParameter) + return db.NoRowsAffected, errors.New(errors.InvalidParameter, op, "no set id") } if version == 0 { - return db.NoRowsAffected, fmt.Errorf("delete: static host set members: version is zero: %w", errors.ErrInvalidParameter) + return db.NoRowsAffected, errors.New(errors.InvalidParameter, op, "no version") } if len(hostIds) == 0 { - return db.NoRowsAffected, fmt.Errorf("delete: static host set members: empty hostIds: %w", errors.ErrInvalidParameter) + return db.NoRowsAffected, errors.New(errors.InvalidParameter, op, "no host ids") } // Create in-memory host set members members, err := r.newMembers(setId, hostIds) if err != nil { - return db.NoRowsAffected, fmt.Errorf("delete: static host set members: %w", err) + return db.NoRowsAffected, errors.Wrap(err, op) } wrapper, err := r.kms.GetWrapper(ctx, scopeId, kms.KeyPurposeOplog) if err != nil { - return db.NoRowsAffected, fmt.Errorf("delete: static host set members: unable to get oplog wrapper: %w", err) + return db.NoRowsAffected, errors.Wrap(err, op, errors.WithMsg("unable to get oplog wrapper")) } _, err = r.writer.DoTx(ctx, db.StdRetryCnt, db.ExpBackoff{}, func(_ db.Reader, w db.Writer) error { @@ -193,19 +196,20 @@ func (r *Repository) DeleteSetMembers(ctx context.Context, scopeId string, setId }) if err != nil { - return db.NoRowsAffected, fmt.Errorf("delete: static host set members: %w", err) + return db.NoRowsAffected, errors.Wrap(err, op) } return len(hostIds), nil } func deleteMembers(ctx context.Context, w db.Writer, members []interface{}) ([]*oplog.Message, error) { + const op = "static.deleteMembers" var msgs []*oplog.Message rowsDeleted, err := w.DeleteItems(ctx, members, db.NewOplogMsgs(&msgs)) if err != nil { - return nil, fmt.Errorf("unable to delete host set members: %w", err) + return nil, errors.Wrap(err, op) } if rowsDeleted != len(members) { - return nil, fmt.Errorf("set members deleted %d did not match request for %d", rowsDeleted, len(members)) + return nil, errors.E(errors.WithMsg(fmt.Sprintf("set members deleted %d did not match request for %d", rowsDeleted, len(members)))) } return msgs, nil } @@ -216,14 +220,15 @@ func deleteMembers(ctx context.Context, w db.Writer, members []interface{}) ([]* // set to be added. The version must match the current version of the setId // in the repository. If hostIds is empty, all hosts will be removed setId. func (r *Repository) SetSetMembers(ctx context.Context, scopeId string, setId string, version uint32, hostIds []string, opt ...Option) ([]*Host, int, error) { + const op = "static.SetSetMembers" if scopeId == "" { - return nil, db.NoRowsAffected, fmt.Errorf("set: static host set members: missing scope id: %w", errors.ErrInvalidParameter) + return nil, db.NoRowsAffected, errors.New(errors.InvalidParameter, op, "no scope id") } if setId == "" { - return nil, db.NoRowsAffected, fmt.Errorf("set: static host set members: missing set id: %w", errors.ErrInvalidParameter) + return nil, db.NoRowsAffected, errors.New(errors.InvalidParameter, op, "no set id") } if version == 0 { - return nil, db.NoRowsAffected, fmt.Errorf("set: static host set members: version is zero: %w", errors.ErrInvalidParameter) + return nil, db.NoRowsAffected, errors.New(errors.InvalidParameter, op, "no version") } // TODO(mgaffney) 08/2020: Oplog does not currently support bulk @@ -241,13 +246,13 @@ func (r *Repository) SetSetMembers(ctx context.Context, scopeId string, setId st // this pattern. changes, err := r.changes(ctx, setId, hostIds) if err != nil { - return nil, db.NoRowsAffected, fmt.Errorf("set: static host set members: %w", err) + return nil, db.NoRowsAffected, errors.Wrap(err, op) } var deletions, additions []interface{} for _, c := range changes { m, err := NewHostSetMember(setId, c.HostId) if err != nil { - return nil, db.NoRowsAffected, fmt.Errorf("set: static host set members: %w", err) + return nil, db.NoRowsAffected, errors.Wrap(err, op) } switch c.Action { case "delete": @@ -261,7 +266,7 @@ func (r *Repository) SetSetMembers(ctx context.Context, scopeId string, setId st if len(changes) > 0 { wrapper, err := r.kms.GetWrapper(ctx, scopeId, kms.KeyPurposeOplog) if err != nil { - return nil, db.NoRowsAffected, fmt.Errorf("set: static host set members: unable to get oplog wrapper: %w", err) + return nil, db.NoRowsAffected, errors.Wrap(err, op, errors.WithMsg("unable to get oplog wrapper")) } _, err = r.writer.DoTx(ctx, db.StdRetryCnt, db.ExpBackoff{}, func(reader db.Reader, w db.Writer) error { @@ -299,7 +304,7 @@ func (r *Repository) SetSetMembers(ctx context.Context, scopeId string, setId st }) if err != nil { - return nil, db.NoRowsAffected, fmt.Errorf("set: static host set members: %w", err) + return nil, db.NoRowsAffected, errors.Wrap(err, op) } } return hosts, len(changes), nil @@ -311,6 +316,7 @@ type change struct { } func (r *Repository) changes(ctx context.Context, setId string, hostIds []string) ([]*change, error) { + const op = "static.changes" var inClauseSpots []string // starts at 2 because there is already a $1 in the query for i := 2; i < len(hostIds)+2; i++ { @@ -329,7 +335,7 @@ func (r *Repository) changes(ctx context.Context, setId string, hostIds []string } rows, err := r.reader.Query(ctx, query, params) if err != nil { - return nil, fmt.Errorf("changes: query failed: %w", err) + return nil, errors.Wrap(err, op, errors.WithMsg("query failed")) } defer rows.Close() @@ -337,7 +343,7 @@ func (r *Repository) changes(ctx context.Context, setId string, hostIds []string for rows.Next() { var chg change if err := r.reader.ScanRows(rows, &chg); err != nil { - return nil, fmt.Errorf("changes: scan row failed: %w", err) + return nil, errors.Wrap(err, op, errors.WithMsg("scan row failed")) } changes = append(changes, &chg) } diff --git a/internal/host/static/repository_host_set_member_test.go b/internal/host/static/repository_host_set_member_test.go index 1dc53430f..774334395 100644 --- a/internal/host/static/repository_host_set_member_test.go +++ b/internal/host/static/repository_host_set_member_test.go @@ -48,7 +48,7 @@ func TestRepository_AddSetMembers_Parameters(t *testing.T) { args args want []*Host wantErr bool - wantIsErr error + wantIsErr errors.Code }{ { name: "empty-scope-id", @@ -57,7 +57,7 @@ func TestRepository_AddSetMembers_Parameters(t *testing.T) { version: set.Version, hostIds: hostIds, }, - wantIsErr: errors.ErrInvalidParameter, + wantIsErr: errors.InvalidParameter, }, { name: "empty-set-id", @@ -66,7 +66,7 @@ func TestRepository_AddSetMembers_Parameters(t *testing.T) { version: set.Version, hostIds: hostIds, }, - wantIsErr: errors.ErrInvalidParameter, + wantIsErr: errors.InvalidParameter, }, { name: "zero-version", @@ -75,7 +75,7 @@ func TestRepository_AddSetMembers_Parameters(t *testing.T) { setId: set.PublicId, hostIds: hostIds, }, - wantIsErr: errors.ErrInvalidParameter, + wantIsErr: errors.InvalidParameter, }, { name: "empty-host-ids", @@ -84,7 +84,7 @@ func TestRepository_AddSetMembers_Parameters(t *testing.T) { setId: set.PublicId, version: set.Version, }, - wantIsErr: errors.ErrInvalidParameter, + wantIsErr: errors.InvalidParameter, }, { name: "invalid-version", @@ -116,8 +116,8 @@ func TestRepository_AddSetMembers_Parameters(t *testing.T) { require.NoError(err) require.NotNil(repo) got, err := repo.AddSetMembers(context.Background(), tt.args.scopeId, tt.args.setId, tt.args.version, tt.args.hostIds, tt.args.opt...) - if tt.wantIsErr != nil { - assert.Truef(errors.Is(err, tt.wantIsErr), "want err: %q got: %q", tt.wantIsErr, err) + if tt.wantIsErr != 0 { + assert.Truef(errors.Match(errors.T(tt.wantIsErr), err), "want err: %q got: %q", tt.wantIsErr, err) assert.Nil(got) assert.Error(db.TestVerifyOplog(t, rw, tt.args.setId, db.WithOperation(oplog.OpType_OP_TYPE_CREATE), db.WithCreateNotBefore(10*time.Second))) return @@ -239,7 +239,7 @@ func TestRepository_DeleteSetMembers_Parameters(t *testing.T) { args args want int wantErr bool - wantIsErr error + wantIsErr errors.Code }{ { name: "empty-scope-id", @@ -248,7 +248,7 @@ func TestRepository_DeleteSetMembers_Parameters(t *testing.T) { version: set.Version, hostIds: hostIds, }, - wantIsErr: errors.ErrInvalidParameter, + wantIsErr: errors.InvalidParameter, }, { name: "empty-set-id", @@ -257,7 +257,7 @@ func TestRepository_DeleteSetMembers_Parameters(t *testing.T) { version: set.Version, hostIds: hostIds, }, - wantIsErr: errors.ErrInvalidParameter, + wantIsErr: errors.InvalidParameter, }, { name: "zero-version", @@ -266,7 +266,7 @@ func TestRepository_DeleteSetMembers_Parameters(t *testing.T) { setId: set.PublicId, hostIds: hostIds, }, - wantIsErr: errors.ErrInvalidParameter, + wantIsErr: errors.InvalidParameter, }, { name: "empty-host-ids", @@ -275,7 +275,7 @@ func TestRepository_DeleteSetMembers_Parameters(t *testing.T) { setId: set.PublicId, version: set.Version, }, - wantIsErr: errors.ErrInvalidParameter, + wantIsErr: errors.InvalidParameter, }, { name: "invalid-version", @@ -308,8 +308,8 @@ func TestRepository_DeleteSetMembers_Parameters(t *testing.T) { require.NoError(err) require.NotNil(repo) got, err := repo.DeleteSetMembers(context.Background(), tt.args.scopeId, tt.args.setId, tt.args.version, tt.args.hostIds, tt.args.opt...) - if tt.wantIsErr != nil { - assert.Truef(errors.Is(err, tt.wantIsErr), "want err: %q got: %q", tt.wantIsErr, err) + if tt.wantIsErr != 0 { + assert.Truef(errors.Match(errors.T(tt.wantIsErr), err), "want err: %q got: %q", tt.wantIsErr, err) assert.Zero(got) assert.Error(db.TestVerifyOplog(t, rw, tt.args.setId, db.WithOperation(oplog.OpType_OP_TYPE_DELETE), db.WithCreateNotBefore(10*time.Second))) return @@ -437,7 +437,7 @@ func TestRepository_SetSetMembers_Parameters(t *testing.T) { want []*Host wantCount int wantErr bool - wantIsErr error + wantIsErr errors.Code }{ { name: "empty-scope-id", @@ -446,7 +446,7 @@ func TestRepository_SetSetMembers_Parameters(t *testing.T) { version: set.Version, hostIds: hostIds, }, - wantIsErr: errors.ErrInvalidParameter, + wantIsErr: errors.InvalidParameter, }, { name: "empty-set-id", @@ -455,7 +455,7 @@ func TestRepository_SetSetMembers_Parameters(t *testing.T) { version: set.Version, hostIds: hostIds, }, - wantIsErr: errors.ErrInvalidParameter, + wantIsErr: errors.InvalidParameter, }, { name: "zero-version", @@ -464,7 +464,7 @@ func TestRepository_SetSetMembers_Parameters(t *testing.T) { setId: set.PublicId, hostIds: hostIds, }, - wantIsErr: errors.ErrInvalidParameter, + wantIsErr: errors.InvalidParameter, }, { name: "invalid-version", @@ -497,8 +497,8 @@ func TestRepository_SetSetMembers_Parameters(t *testing.T) { require.NoError(err) require.NotNil(repo) got, gotCount, err := repo.SetSetMembers(context.Background(), tt.args.scopeId, tt.args.setId, tt.args.version, tt.args.hostIds, tt.args.opt...) - if tt.wantIsErr != nil { - assert.Truef(errors.Is(err, tt.wantIsErr), "want err: %q got: %q", tt.wantIsErr, err) + if tt.wantIsErr != 0 { + assert.Truef(errors.Match(errors.T(tt.wantIsErr), err), "want err: %q got: %q", tt.wantIsErr, err) assert.Nil(got) assert.Equal(tt.wantCount, gotCount) assert.Error(db.TestVerifyOplog(t, rw, tt.args.setId, db.WithOperation(oplog.OpType_OP_TYPE_UPDATE), db.WithCreateNotBefore(10*time.Second))) diff --git a/internal/host/static/repository_host_set_test.go b/internal/host/static/repository_host_set_test.go index baac89d7d..41ab06468 100644 --- a/internal/host/static/repository_host_set_test.go +++ b/internal/host/static/repository_host_set_test.go @@ -33,23 +33,23 @@ func TestRepository_CreateSet(t *testing.T) { in *HostSet opts []Option want *HostSet - wantIsErr error + wantIsErr errors.Code }{ { name: "nil-HostSet", - wantIsErr: errors.ErrInvalidParameter, + wantIsErr: errors.InvalidParameter, }, { name: "nil-embedded-HostSet", in: &HostSet{}, - wantIsErr: errors.ErrInvalidParameter, + wantIsErr: errors.InvalidParameter, }, { name: "invalid-no-catalog-id", in: &HostSet{ HostSet: &store.HostSet{}, }, - wantIsErr: errors.ErrInvalidParameter, + wantIsErr: errors.InvalidParameter, }, { name: "invalid-public-id-set", @@ -59,7 +59,7 @@ func TestRepository_CreateSet(t *testing.T) { PublicId: "abcd_OOOOOOOOOO", }, }, - wantIsErr: errors.ErrInvalidParameter, + wantIsErr: errors.InvalidParameter, }, { name: "valid-no-options", @@ -114,8 +114,8 @@ func TestRepository_CreateSet(t *testing.T) { require.NoError(err) require.NotNil(repo) got, err := repo.CreateSet(context.Background(), prj.GetPublicId(), tt.in, tt.opts...) - if tt.wantIsErr != nil { - assert.Truef(errors.Is(err, tt.wantIsErr), "want err: %q got: %q", tt.wantIsErr, err) + if tt.wantIsErr != 0 { + assert.Truef(errors.Match(errors.T(tt.wantIsErr), err), "want err: %q got: %q", tt.wantIsErr, err) assert.Nil(got) return } @@ -264,7 +264,7 @@ func TestRepository_UpdateSet(t *testing.T) { masks []string want *HostSet wantCount int - wantIsErr error + wantIsErr errors.Code }{ { name: "nil-host-set", @@ -273,7 +273,7 @@ func TestRepository_UpdateSet(t *testing.T) { }, chgFn: makeNil(), masks: []string{"Name", "Description"}, - wantIsErr: errors.ErrInvalidParameter, + wantIsErr: errors.InvalidParameter, }, { name: "nil-embedded-host-set", @@ -282,7 +282,7 @@ func TestRepository_UpdateSet(t *testing.T) { }, chgFn: makeEmbeddedNil(), masks: []string{"Name", "Description"}, - wantIsErr: errors.ErrInvalidParameter, + wantIsErr: errors.InvalidParameter, }, { name: "no-public-id", @@ -291,7 +291,7 @@ func TestRepository_UpdateSet(t *testing.T) { }, chgFn: deletePublicId(), masks: []string{"Name", "Description"}, - wantIsErr: errors.ErrInvalidParameter, + wantIsErr: errors.InvalidParameter, }, { name: "updating-non-existent-host-set", @@ -302,7 +302,7 @@ func TestRepository_UpdateSet(t *testing.T) { }, chgFn: combine(nonExistentPublicId(), changeName("test-update-name-repo")), masks: []string{"Name"}, - wantIsErr: errors.ErrRecordNotFound, + wantIsErr: errors.RecordNotFound, }, { name: "empty-field-mask", @@ -312,7 +312,7 @@ func TestRepository_UpdateSet(t *testing.T) { }, }, chgFn: changeName("test-update-name-repo"), - wantIsErr: errors.ErrEmptyFieldMask, + wantIsErr: errors.EmptyFieldMask, }, { name: "read-only-fields-in-field-mask", @@ -323,7 +323,7 @@ func TestRepository_UpdateSet(t *testing.T) { }, chgFn: changeName("test-update-name-repo"), masks: []string{"PublicId", "CreateTime", "UpdateTime", "CatalogId"}, - wantIsErr: errors.ErrInvalidFieldMask, + wantIsErr: errors.InvalidFieldMask, }, { name: "unknown-field-in-field-mask", @@ -334,7 +334,7 @@ func TestRepository_UpdateSet(t *testing.T) { }, chgFn: changeName("test-update-name-repo"), masks: []string{"Bilbo"}, - wantIsErr: errors.ErrInvalidFieldMask, + wantIsErr: errors.InvalidFieldMask, }, { name: "change-name", @@ -480,8 +480,8 @@ func TestRepository_UpdateSet(t *testing.T) { orig = tt.chgFn(orig) } got, gotHosts, gotCount, err := repo.UpdateSet(context.Background(), prj.GetPublicId(), orig, 1, tt.masks) - if tt.wantIsErr != nil { - assert.Truef(errors.Is(err, tt.wantIsErr), "want err: %q got: %q", tt.wantIsErr, err) + if tt.wantIsErr != 0 { + assert.Truef(errors.Match(errors.T(tt.wantIsErr), err), "want err: %q got: %q", tt.wantIsErr, err) assert.Equal(tt.wantCount, gotCount, "row count") assert.Nil(got) return @@ -724,11 +724,11 @@ func TestRepository_LookupSet(t *testing.T) { in string want *HostSet wantHosts []*Host - wantIsErr error + wantIsErr errors.Code }{ { name: "with-no-public-id", - wantIsErr: errors.ErrInvalidParameter, + wantIsErr: errors.InvalidParameter, }, { name: "with-non-existing-host-set-id", @@ -756,8 +756,8 @@ func TestRepository_LookupSet(t *testing.T) { assert.NoError(err) require.NotNil(repo) got, gotHosts, err := repo.LookupSet(context.Background(), tt.in) - if tt.wantIsErr != nil { - assert.Truef(errors.Is(err, tt.wantIsErr), "want err: %q got: %q", tt.wantIsErr, err) + if tt.wantIsErr != 0 { + assert.Truef(errors.Match(errors.T(tt.wantIsErr), err), "want err: %q got: %q", tt.wantIsErr, err) assert.Nil(got) return } @@ -864,11 +864,11 @@ func TestRepository_ListSets(t *testing.T) { in string opts []Option want []*HostSet - wantIsErr error + wantIsErr errors.Code }{ { name: "with-no-catalog-id", - wantIsErr: errors.ErrInvalidParameter, + wantIsErr: errors.InvalidParameter, }, { name: "Catalog-with-no-host-sets", @@ -890,8 +890,8 @@ func TestRepository_ListSets(t *testing.T) { assert.NoError(err) require.NotNil(repo) got, err := repo.ListSets(context.Background(), tt.in, tt.opts...) - if tt.wantIsErr != nil { - assert.Truef(errors.Is(err, tt.wantIsErr), "want err: %q got: %q", tt.wantIsErr, err) + if tt.wantIsErr != 0 { + assert.Truef(errors.Match(errors.T(tt.wantIsErr), err), "want err: %q got: %q", tt.wantIsErr, err) assert.Nil(got) return } @@ -992,11 +992,11 @@ func TestRepository_DeleteSet(t *testing.T) { name string in string want int - wantIsErr error + wantIsErr errors.Code }{ { name: "With no public id", - wantIsErr: errors.ErrInvalidParameter, + wantIsErr: errors.InvalidParameter, }, { name: "With non existing host set id", @@ -1018,8 +1018,8 @@ func TestRepository_DeleteSet(t *testing.T) { assert.NoError(err) require.NotNil(repo) got, err := repo.DeleteSet(context.Background(), prj.PublicId, tt.in) - if tt.wantIsErr != nil { - assert.Truef(errors.Is(err, tt.wantIsErr), "want err: %q got: %q", tt.wantIsErr, err) + if tt.wantIsErr != 0 { + assert.Truef(errors.Match(errors.T(tt.wantIsErr), err), "want err: %q got: %q", tt.wantIsErr, err) assert.Zero(got) return } diff --git a/internal/host/static/repository_host_test.go b/internal/host/static/repository_host_test.go index 06fc342d4..0511ed887 100644 --- a/internal/host/static/repository_host_test.go +++ b/internal/host/static/repository_host_test.go @@ -33,23 +33,23 @@ func TestRepository_CreateHost(t *testing.T) { in *Host opts []Option want *Host - wantIsErr error + wantIsErr errors.Code }{ { name: "nil-Host", - wantIsErr: errors.ErrInvalidParameter, + wantIsErr: errors.InvalidParameter, }, { name: "nil-embedded-Host", in: &Host{}, - wantIsErr: errors.ErrInvalidParameter, + wantIsErr: errors.InvalidParameter, }, { name: "invalid-no-catalog-id", in: &Host{ Host: &store.Host{}, }, - wantIsErr: errors.ErrInvalidParameter, + wantIsErr: errors.InvalidParameter, }, { name: "invalid-public-id-set", @@ -60,7 +60,7 @@ func TestRepository_CreateHost(t *testing.T) { Address: "127.0.0.1", }, }, - wantIsErr: errors.ErrInvalidParameter, + wantIsErr: errors.InvalidParameter, }, { name: "valid-no-options", @@ -118,7 +118,7 @@ func TestRepository_CreateHost(t *testing.T) { CatalogId: catalog.PublicId, }, }, - wantIsErr: ErrInvalidAddress, + wantIsErr: errors.InvalidAddress, }, { name: "invalid-address-to-short", @@ -128,7 +128,7 @@ func TestRepository_CreateHost(t *testing.T) { Address: "12", }, }, - wantIsErr: ErrInvalidAddress, + wantIsErr: errors.InvalidAddress, }, { name: "invalid-empty-address", @@ -138,7 +138,18 @@ func TestRepository_CreateHost(t *testing.T) { Address: " ", }, }, - wantIsErr: ErrInvalidAddress, + wantIsErr: errors.InvalidAddress, + }, + { + name: "invalid_public_id", + in: &Host{ + Host: &store.Host{ + CatalogId: catalog.PublicId, + Address: "127.0.0.1", + }, + }, + opts: []Option{WithPublicId("bad_prefix")}, + wantIsErr: errors.InvalidPublicId, }, } @@ -150,8 +161,8 @@ func TestRepository_CreateHost(t *testing.T) { require.NoError(err) require.NotNil(repo) got, err := repo.CreateHost(context.Background(), prj.GetPublicId(), tt.in, tt.opts...) - if tt.wantIsErr != nil { - assert.Truef(errors.Is(err, tt.wantIsErr), "want err: %q got: %q", tt.wantIsErr, err) + if tt.wantIsErr != 0 { + assert.Truef(errors.Match(errors.T(tt.wantIsErr), err), "want err: %q got: %q", tt.wantIsErr, err) assert.Nil(got) return } @@ -309,7 +320,7 @@ func TestRepository_UpdateHost(t *testing.T) { masks []string want *Host wantCount int - wantIsErr error + wantIsErr errors.Code }{ { name: "nil-host", @@ -320,7 +331,7 @@ func TestRepository_UpdateHost(t *testing.T) { }, chgFn: makeNil(), masks: []string{"Name", "Description"}, - wantIsErr: errors.ErrInvalidParameter, + wantIsErr: errors.InvalidParameter, }, { name: "nil-embedded-host", @@ -331,7 +342,7 @@ func TestRepository_UpdateHost(t *testing.T) { }, chgFn: makeEmbeddedNil(), masks: []string{"Name", "Description"}, - wantIsErr: errors.ErrInvalidParameter, + wantIsErr: errors.InvalidParameter, }, { name: "no-public-id", @@ -342,7 +353,7 @@ func TestRepository_UpdateHost(t *testing.T) { }, chgFn: deletePublicId(), masks: []string{"Name", "Description"}, - wantIsErr: errors.ErrInvalidParameter, + wantIsErr: errors.InvalidParameter, }, { name: "updating-non-existent-host", @@ -354,7 +365,7 @@ func TestRepository_UpdateHost(t *testing.T) { }, chgFn: combine(nonExistentPublicId(), changeName("test-update-name-repo")), masks: []string{"Name"}, - wantIsErr: errors.ErrRecordNotFound, + wantIsErr: errors.RecordNotFound, }, { name: "empty-field-mask", @@ -365,7 +376,7 @@ func TestRepository_UpdateHost(t *testing.T) { }, }, chgFn: changeName("test-update-name-repo"), - wantIsErr: errors.ErrEmptyFieldMask, + wantIsErr: errors.EmptyFieldMask, }, { name: "read-only-fields-in-field-mask", @@ -377,7 +388,7 @@ func TestRepository_UpdateHost(t *testing.T) { }, chgFn: changeName("test-update-name-repo"), masks: []string{"PublicId", "CreateTime", "UpdateTime", "CatalogId"}, - wantIsErr: errors.ErrInvalidFieldMask, + wantIsErr: errors.InvalidFieldMask, }, { name: "unknown-field-in-field-mask", @@ -389,7 +400,7 @@ func TestRepository_UpdateHost(t *testing.T) { }, chgFn: changeName("test-update-name-repo"), masks: []string{"Bilbo"}, - wantIsErr: errors.ErrInvalidFieldMask, + wantIsErr: errors.InvalidFieldMask, }, { name: "change-name", @@ -550,7 +561,7 @@ func TestRepository_UpdateHost(t *testing.T) { }, chgFn: changeAddress("11"), masks: []string{"Address"}, - wantIsErr: ErrInvalidAddress, + wantIsErr: errors.InvalidAddress, }, { name: "delete-address", @@ -561,7 +572,7 @@ func TestRepository_UpdateHost(t *testing.T) { }, chgFn: changeAddress(""), masks: []string{"Address"}, - wantIsErr: ErrInvalidAddress, + wantIsErr: errors.InvalidAddress, }, { name: "change-empty-address", @@ -572,7 +583,7 @@ func TestRepository_UpdateHost(t *testing.T) { }, chgFn: changeAddress(" "), masks: []string{"Address"}, - wantIsErr: ErrInvalidAddress, + wantIsErr: errors.InvalidAddress, }, } @@ -596,8 +607,8 @@ func TestRepository_UpdateHost(t *testing.T) { orig = tt.chgFn(orig) } got, gotCount, err := repo.UpdateHost(context.Background(), prj.GetPublicId(), orig, 1, tt.masks) - if tt.wantIsErr != nil { - assert.Truef(errors.Is(err, tt.wantIsErr), "want err: %q got: %q", tt.wantIsErr, err) + if tt.wantIsErr != 0 { + assert.Truef(errors.Match(errors.T(tt.wantIsErr), err), "want err: %q got: %q", tt.wantIsErr, err) assert.Equal(tt.wantCount, gotCount, "row count") assert.Nil(got) return @@ -748,11 +759,11 @@ func TestRepository_LookupHost(t *testing.T) { name string in string want *Host - wantIsErr error + wantIsErr errors.Code }{ { name: "with-no-public-id", - wantIsErr: errors.ErrInvalidParameter, + wantIsErr: errors.InvalidParameter, }, { name: "with-non-existing-host-id", @@ -773,8 +784,8 @@ func TestRepository_LookupHost(t *testing.T) { assert.NoError(err) require.NotNil(repo) got, err := repo.LookupHost(context.Background(), tt.in) - if tt.wantIsErr != nil { - assert.Truef(errors.Is(err, tt.wantIsErr), "want err: %q got: %q", tt.wantIsErr, err) + if tt.wantIsErr != 0 { + assert.Truef(errors.Match(errors.T(tt.wantIsErr), err), "want err: %q got: %q", tt.wantIsErr, err) assert.Nil(got) return } @@ -802,11 +813,11 @@ func TestRepository_ListHosts(t *testing.T) { in string opts []Option want []*Host - wantIsErr error + wantIsErr errors.Code }{ { name: "with-no-catalog-id", - wantIsErr: errors.ErrInvalidParameter, + wantIsErr: errors.InvalidParameter, }, { name: "Catalog-with-no-hosts", @@ -828,8 +839,8 @@ func TestRepository_ListHosts(t *testing.T) { assert.NoError(err) require.NotNil(repo) got, err := repo.ListHosts(context.Background(), tt.in, tt.opts...) - if tt.wantIsErr != nil { - assert.Truef(errors.Is(err, tt.wantIsErr), "want err: %q got: %q", tt.wantIsErr, err) + if tt.wantIsErr != 0 { + assert.Truef(errors.Match(errors.T(tt.wantIsErr), err), "want err: %q got: %q", tt.wantIsErr, err) assert.Nil(got) return } @@ -930,11 +941,11 @@ func TestRepository_DeleteHost(t *testing.T) { name string in string want int - wantIsErr error + wantIsErr errors.Code }{ { name: "With no public id", - wantIsErr: errors.ErrInvalidParameter, + wantIsErr: errors.InvalidParameter, }, { name: "With non existing host id", @@ -956,8 +967,8 @@ func TestRepository_DeleteHost(t *testing.T) { assert.NoError(err) require.NotNil(repo) got, err := repo.DeleteHost(context.Background(), catalog.ScopeId, tt.in) - if tt.wantIsErr != nil { - assert.Truef(errors.Is(err, tt.wantIsErr), "want err: %q got: %q", tt.wantIsErr, err) + if tt.wantIsErr != 0 { + assert.Truef(errors.Match(errors.T(tt.wantIsErr), err), "want err: %q got: %q", tt.wantIsErr, err) assert.Zero(got) return } diff --git a/internal/host/static/repository_test.go b/internal/host/static/repository_test.go index 0e4f59b36..f0018c5c0 100644 --- a/internal/host/static/repository_test.go +++ b/internal/host/static/repository_test.go @@ -28,7 +28,7 @@ func TestRepository_New(t *testing.T) { name string args args want *Repository - wantIsErr error + wantIsErr errors.Code }{ { name: "valid", @@ -67,7 +67,7 @@ func TestRepository_New(t *testing.T) { kms: kmsCache, }, want: nil, - wantIsErr: errors.ErrInvalidParameter, + wantIsErr: errors.InvalidParameter, }, { name: "nil-writer", @@ -77,7 +77,7 @@ func TestRepository_New(t *testing.T) { kms: kmsCache, }, want: nil, - wantIsErr: errors.ErrInvalidParameter, + wantIsErr: errors.InvalidParameter, }, { name: "nil-kms", @@ -87,7 +87,7 @@ func TestRepository_New(t *testing.T) { kms: nil, }, want: nil, - wantIsErr: errors.ErrInvalidParameter, + wantIsErr: errors.InvalidParameter, }, { name: "all-nils", @@ -97,7 +97,7 @@ func TestRepository_New(t *testing.T) { kms: nil, }, want: nil, - wantIsErr: errors.ErrInvalidParameter, + wantIsErr: errors.InvalidParameter, }, } for _, tt := range tests { @@ -105,8 +105,8 @@ func TestRepository_New(t *testing.T) { t.Run(tt.name, func(t *testing.T) { assert, require := assert.New(t), require.New(t) got, err := NewRepository(tt.args.r, tt.args.w, tt.args.kms, tt.args.opts...) - if tt.wantIsErr != nil { - assert.Truef(errors.Is(err, tt.wantIsErr), "want err: %q got: %q", tt.wantIsErr, err) + if tt.wantIsErr != 0 { + assert.Truef(errors.Match(errors.T(tt.wantIsErr), err), "want err: %q got: %q", tt.wantIsErr, err) assert.Nil(got) return } diff --git a/internal/servers/controller/handlers/errors.go b/internal/servers/controller/handlers/errors.go index 2336f835e..2ed22873c 100644 --- a/internal/servers/controller/handlers/errors.go +++ b/internal/servers/controller/handlers/errors.go @@ -145,9 +145,10 @@ func backendErrorToApiError(inErr error) error { Kind: codes.Unimplemented.String(), Message: stErr.Message(), }} - case errors.Is(inErr, errors.ErrRecordNotFound): + case errors.Is(inErr, errors.ErrRecordNotFound), errors.Match(errors.T(errors.RecordNotFound), inErr): return NotFoundErrorf(genericNotFoundMsg) - case errors.Is(inErr, errors.ErrInvalidFieldMask), errors.Is(inErr, errors.ErrEmptyFieldMask): + case errors.Is(inErr, errors.ErrInvalidFieldMask), errors.Is(inErr, errors.ErrEmptyFieldMask), + errors.Match(errors.T(errors.InvalidFieldMask), inErr), errors.Match(errors.T(errors.EmptyFieldMask), inErr): return InvalidArgumentErrorf("Error in provided request", map[string]string{"update_mask": "Invalid update mask provided."}) case errors.IsUniqueError(inErr), errors.Is(inErr, errors.ErrNotUnique): return InvalidArgumentErrorf(genericUniquenessMsg, nil) diff --git a/internal/servers/controller/handlers/errors_test.go b/internal/servers/controller/handlers/errors_test.go index 6c0ecdac2..0fc2177bd 100644 --- a/internal/servers/controller/handlers/errors_test.go +++ b/internal/servers/controller/handlers/errors_test.go @@ -100,7 +100,7 @@ func TestApiErrorHandler(t *testing.T) { expected: apiError{ status: http.StatusInternalServerError, inner: &pb.Error{ - Kind: "Internal", + Kind: "Internal", Message: "Some random error", }, }, @@ -111,22 +111,44 @@ func TestApiErrorHandler(t *testing.T) { expected: apiError{ status: http.StatusInternalServerError, inner: &pb.Error{ - Kind: "Internal", + Kind: "Internal", Message: fmt.Sprintf("test error: %s", errors.ErrInvalidPublicId), }, }, }, + { + name: "Domain error Db invalid parameter", + err: errors.E(errors.WithCode(errors.InvalidPublicId)), + expected: apiError{ + status: http.StatusInternalServerError, + inner: &pb.Error{ + Kind: "Internal", + Message: "invalid public id, parameter violation: error #102", + }, + }, + }, { name: "Db invalid parameter", err: fmt.Errorf("test error: %w", errors.ErrInvalidParameter), expected: apiError{ status: http.StatusInternalServerError, inner: &pb.Error{ - Kind: "Internal", + Kind: "Internal", Message: fmt.Sprintf("test error: %s", errors.ErrInvalidParameter), }, }, }, + { + name: "Domain error Db invalid parameter", + err: errors.E(errors.WithCode(errors.InvalidParameter)), + expected: apiError{ + status: http.StatusInternalServerError, + inner: &pb.Error{ + Kind: "Internal", + Message: "invalid parameter, parameter violation: error #100", + }, + }, + }, { name: "Db invalid field mask", err: fmt.Errorf("test error: %w", errors.ErrInvalidFieldMask), @@ -139,6 +161,18 @@ func TestApiErrorHandler(t *testing.T) { }, }, }, + { + name: "Domain error Db invalid field mask", + err: errors.E(errors.WithCode(errors.InvalidFieldMask)), + expected: apiError{ + status: http.StatusBadRequest, + inner: &pb.Error{ + Kind: "InvalidArgument", + Message: "Error in provided request", + Details: &pb.ErrorDetails{RequestFields: []*pb.FieldError{{Name: "update_mask", Description: "Invalid update mask provided."}}}, + }, + }, + }, { name: "Db empty field mask", err: fmt.Errorf("test error: %w", errors.ErrEmptyFieldMask), @@ -151,6 +185,18 @@ func TestApiErrorHandler(t *testing.T) { }, }, }, + { + name: "Domain error Db empty field mask", + err: errors.E(errors.WithCode(errors.EmptyFieldMask)), + expected: apiError{ + status: http.StatusBadRequest, + inner: &pb.Error{ + Kind: "InvalidArgument", + Message: "Error in provided request", + Details: &pb.ErrorDetails{RequestFields: []*pb.FieldError{{Name: "update_mask", Description: "Invalid update mask provided."}}}, + }, + }, + }, { name: "Db not unique", err: fmt.Errorf("test error: %w", errors.ErrNotUnique), @@ -162,6 +208,17 @@ func TestApiErrorHandler(t *testing.T) { }, }, }, + { + name: "Domain error Db not unqiue", + err: errors.E(errors.WithCode(errors.NotUnique)), + expected: apiError{ + status: http.StatusBadRequest, + inner: &pb.Error{ + Kind: "InvalidArgument", + Message: genericUniquenessMsg, + }, + }, + }, { name: "Db record not found", err: fmt.Errorf("test error: %w", errors.ErrRecordNotFound), @@ -173,17 +230,50 @@ func TestApiErrorHandler(t *testing.T) { }, }, }, + { + name: "Domain error Db record not found", + err: errors.E(errors.WithCode(errors.RecordNotFound)), + expected: apiError{ + status: http.StatusNotFound, + inner: &pb.Error{ + Kind: "NotFound", + Message: genericNotFoundMsg, + }, + }, + }, { name: "Db multiple records", err: fmt.Errorf("test error: %w", errors.ErrMultipleRecords), expected: apiError{ status: http.StatusInternalServerError, inner: &pb.Error{ - Kind: "Internal", + Kind: "Internal", Message: fmt.Sprintf("test error: %s", errors.ErrMultipleRecords), }, }, }, + { + name: "Domain error Db multiple records", + err: errors.E(errors.WithCode(errors.MultipleRecords)), + expected: apiError{ + status: http.StatusInternalServerError, + inner: &pb.Error{ + Kind: "Internal", + Message: "multiple records, search issue: error #1101", + }, + }, + }, + { + name: "Wrapped domain error", + err: errors.E(errors.WithCode(errors.InvalidAddress), errors.WithMsg("test msg"), errors.WithWrap(errors.E(errors.WithCode(errors.NotNull), errors.WithMsg("inner msg")))), + expected: apiError{ + status: http.StatusInternalServerError, + inner: &pb.Error{ + Kind: "Internal", + Message: "test msg: parameter violation: error #101: inner msg: integrity violation: error #1001", + }, + }, + }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { diff --git a/internal/servers/controller/handlers/host_catalogs/host_catalog_service.go b/internal/servers/controller/handlers/host_catalogs/host_catalog_service.go index da779f389..f23aadb35 100644 --- a/internal/servers/controller/handlers/host_catalogs/host_catalog_service.go +++ b/internal/servers/controller/handlers/host_catalogs/host_catalog_service.go @@ -5,6 +5,7 @@ import ( "fmt" "github.com/hashicorp/boundary/internal/auth" + "github.com/hashicorp/boundary/internal/errors" pb "github.com/hashicorp/boundary/internal/gen/controller/api/resources/hostcatalogs" pbs "github.com/hashicorp/boundary/internal/gen/controller/api/services" "github.com/hashicorp/boundary/internal/host" @@ -180,6 +181,10 @@ func (s Service) createInRepo(ctx context.Context, projId string, item *pb.HostC } h, err := static.NewHostCatalog(projId, opts...) if err != nil { + if e := errors.Convert(err); e != nil { + // This is a domain error, push this error through so the error interceptor can interpret it correctly. + return nil, e + } return nil, handlers.ApiErrorWithCodeAndMessage(codes.Internal, "Unable to build host catalog for creation: %v.", err) } repo, err := s.staticRepoFn() @@ -188,6 +193,10 @@ func (s Service) createInRepo(ctx context.Context, projId string, item *pb.HostC } out, err := repo.CreateCatalog(ctx, h) if err != nil { + if e := errors.Convert(err); e != nil { + // This is a domain error, push this error through so the error interceptor can interpret it correctly. + return nil, e + } return nil, fmt.Errorf("unable to create host catalog: %w", err) } if out == nil { @@ -207,6 +216,10 @@ func (s Service) updateInRepo(ctx context.Context, projId, id string, mask []str version := item.GetVersion() h, err := static.NewHostCatalog(projId, opts...) if err != nil { + if e := errors.Convert(err); e != nil { + // This is a domain error, push this error through so the error interceptor can interpret it correctly. + return nil, e + } return nil, fmt.Errorf("unable to build host catalog for update: %w", err) } h.PublicId = id @@ -220,6 +233,10 @@ func (s Service) updateInRepo(ctx context.Context, projId, id string, mask []str } out, rowsUpdated, err := repo.UpdateCatalog(ctx, h, version, dbMask) if err != nil { + if e := errors.Convert(err); e != nil { + // This is a domain error, push this error through so the error interceptor can interpret it correctly. + return nil, e + } return nil, fmt.Errorf("unable to update host catalog: %w", err) } if rowsUpdated == 0 { @@ -235,6 +252,10 @@ func (s Service) deleteFromRepo(ctx context.Context, id string) (bool, error) { } rows, err := repo.DeleteCatalog(ctx, id) if err != nil { + if e := errors.Convert(err); e != nil { + // This is a domain error, push this error through so the error interceptor can interpret it correctly. + return false, e + } return false, fmt.Errorf("unable to delete host: %w", err) } return rows > 0, nil diff --git a/internal/servers/controller/handlers/host_sets/host_set_service.go b/internal/servers/controller/handlers/host_sets/host_set_service.go index 9cf1d1af9..3ea503b52 100644 --- a/internal/servers/controller/handlers/host_sets/host_set_service.go +++ b/internal/servers/controller/handlers/host_sets/host_set_service.go @@ -212,6 +212,10 @@ func (s Service) createInRepo(ctx context.Context, scopeId, catalogId string, it } h, err := static.NewHostSet(catalogId, opts...) if err != nil { + if e := errors.Convert(err); e != nil { + // This is a domain error, push this error through so the error interceptor can interpret it correctly. + return nil, e + } return nil, handlers.ApiErrorWithCodeAndMessage(codes.Internal, "Unable to build host set for creation: %v.", err) } repo, err := s.staticRepoFn() @@ -220,9 +224,9 @@ func (s Service) createInRepo(ctx context.Context, scopeId, catalogId string, it } out, err := repo.CreateSet(ctx, scopeId, h) if err != nil { - if errors.IsUniqueError(err) || errors.Is(err, errors.ErrNotUnique) { - // Push this error through so the error interceptor can interpret it correctly. - return nil, err + if e := errors.Convert(err); e != nil { + // This is a domain error, push this error through so the error interceptor can interpret it correctly. + return nil, e } return nil, fmt.Errorf("unable to create host set: %w", err) } @@ -242,6 +246,10 @@ func (s Service) updateInRepo(ctx context.Context, scopeId, catalogId, id string } h, err := static.NewHostSet(catalogId, opts...) if err != nil { + if e := errors.Convert(err); e != nil { + // This is a domain error, push this error through so the error interceptor can interpret it correctly. + return nil, e + } return nil, handlers.ApiErrorWithCodeAndMessage(codes.Internal, "Unable to build host set for update: %v.", err) } h.PublicId = id @@ -255,6 +263,10 @@ func (s Service) updateInRepo(ctx context.Context, scopeId, catalogId, id string } out, m, rowsUpdated, err := repo.UpdateSet(ctx, scopeId, h, item.GetVersion(), dbMask) if err != nil { + if e := errors.Convert(err); e != nil { + // This is a domain error, push this error through so the error interceptor can interpret it correctly. + return nil, e + } return nil, fmt.Errorf("unable to update host set: %w.", err) } if rowsUpdated == 0 { @@ -270,6 +282,10 @@ func (s Service) deleteFromRepo(ctx context.Context, scopeId, id string) (bool, } rows, err := repo.DeleteSet(ctx, scopeId, id) if err != nil { + if e := errors.Convert(err); e != nil { + // This is a domain error, push this error through so the error interceptor can interpret it correctly. + return false, e + } return false, fmt.Errorf("unable to delete host: %w", err) } return rows > 0, nil @@ -298,11 +314,18 @@ func (s Service) addInRepo(ctx context.Context, scopeId, setId string, hostIds [ } _, err = repo.AddSetMembers(ctx, scopeId, setId, version, strutil.RemoveDuplicates(hostIds, false)) if err != nil { - // TODO: Figure out a way to surface more helpful error info beyond the Internal error. + if e := errors.Convert(err); e != nil { + // This is a domain error, push this error through so the error interceptor can interpret it correctly. + return nil, e + } return nil, handlers.ApiErrorWithCodeAndMessage(codes.Internal, "Unable to add hosts to host set: %v.", err) } out, m, err := repo.LookupSet(ctx, setId) if err != nil { + if e := errors.Convert(err); e != nil { + // This is a domain error, push this error through so the error interceptor can interpret it correctly. + return nil, e + } return nil, fmt.Errorf("unable to look up host set after adding hosts: %w", err) } if out == nil { @@ -318,12 +341,19 @@ func (s Service) setInRepo(ctx context.Context, scopeId, setId string, hostIds [ } _, _, err = repo.SetSetMembers(ctx, scopeId, setId, version, strutil.RemoveDuplicates(hostIds, false)) if err != nil { - // TODO: Figure out a way to surface more helpful error info beyond the Internal error. + if e := errors.Convert(err); e != nil { + // This is a domain error, push this error through so the error interceptor can interpret it correctly. + return nil, e + } return nil, handlers.ApiErrorWithCodeAndMessage(codes.Internal, "Unable to set hosts in host set: %v.", err) } out, m, err := repo.LookupSet(ctx, setId) if err != nil { + if e := errors.Convert(err); e != nil { + // This is a domain error, push this error through so the error interceptor can interpret it correctly. + return nil, e + } return nil, fmt.Errorf("unable to look up host set after setting hosts: %w", err) } if out == nil { @@ -339,11 +369,18 @@ func (s Service) removeInRepo(ctx context.Context, scopeId, setId string, hostId } _, err = repo.DeleteSetMembers(ctx, scopeId, setId, version, strutil.RemoveDuplicates(hostIds, false)) if err != nil { - // TODO: Figure out a way to surface more helpful error info beyond the Internal error. + if e := errors.Convert(err); e != nil { + // This is a domain error, push this error through so the error interceptor can interpret it correctly. + return nil, e + } return nil, handlers.ApiErrorWithCodeAndMessage(codes.Internal, "Unable to remove hosts from host set: %v.", err) } out, m, err := repo.LookupSet(ctx, setId) if err != nil { + if e := errors.Convert(err); e != nil { + // This is a domain error, push this error through so the error interceptor can interpret it correctly. + return nil, e + } return nil, fmt.Errorf("unable to look up host set: %w", err) } if out == nil { diff --git a/internal/servers/controller/handlers/hosts/host_service.go b/internal/servers/controller/handlers/hosts/host_service.go index 244d4772c..1178994d7 100644 --- a/internal/servers/controller/handlers/hosts/host_service.go +++ b/internal/servers/controller/handlers/hosts/host_service.go @@ -169,6 +169,10 @@ func (s Service) createInRepo(ctx context.Context, scopeId, catalogId string, it } h, err := static.NewHost(catalogId, opts...) if err != nil { + if e := errors.Convert(err); e != nil { + // This is a domain error, push this error through so the error interceptor can interpret it correctly. + return nil, e + } return nil, handlers.ApiErrorWithCodeAndMessage(codes.Internal, "Unable to build host for creation: %v.", err) } @@ -178,9 +182,9 @@ func (s Service) createInRepo(ctx context.Context, scopeId, catalogId string, it } out, err := repo.CreateHost(ctx, scopeId, h) if err != nil { - if errors.IsUniqueError(err) || errors.Is(err, errors.ErrNotUnique) { - // Push this error through so the error interceptor can interpret it correctly. - return nil, err + if e := errors.Convert(err); e != nil { + // This is a domain error, push this error through so the error interceptor can interpret it correctly. + return nil, e } return nil, handlers.ApiErrorWithCodeAndMessage(codes.Internal, "Unable to create host: %v.", err) } @@ -207,6 +211,10 @@ func (s Service) updateInRepo(ctx context.Context, scopeId, catalogId, id string } h, err := static.NewHost(catalogId, opts...) if err != nil { + if e := errors.Convert(err); e != nil { + // This is a domain error, push this error through so the error interceptor can interpret it correctly. + return nil, e + } return nil, handlers.ApiErrorWithCodeAndMessage(codes.Internal, "Unable to build host for update: %v.", err) } h.PublicId = id @@ -220,6 +228,10 @@ func (s Service) updateInRepo(ctx context.Context, scopeId, catalogId, id string } out, rowsUpdated, err := repo.UpdateHost(ctx, scopeId, h, item.GetVersion(), dbMask) if err != nil { + if e := errors.Convert(err); e != nil { + // This is a domain error, push this error through so the error interceptor can interpret it correctly. + return nil, e + } return nil, fmt.Errorf("unable to update host: %w", err) } if rowsUpdated == 0 { @@ -235,6 +247,10 @@ func (s Service) deleteFromRepo(ctx context.Context, scopeId, id string) (bool, } rows, err := repo.DeleteHost(ctx, scopeId, id) if err != nil { + if e := errors.Convert(err); e != nil { + // This is a domain error, push this error through so the error interceptor can interpret it correctly. + return false, e + } return false, fmt.Errorf("unable to delete host: %w", err) } return rows > 0, nil