CyberCoveLLC/threshold
1
0 Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Refactor host_repo -> domain errors (#786)

Browse Source 
* Refactor domain errors: New -> E, create helper New and Wrap
* Update host repo to use domain errors
This commit is contained in:
Louis Ruch authored and GitHub committed 2020-12-09 10:11:38 -08:00
1 parent aa4157639c
commit ec0020de90
33 files changed
+996 -374
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

internal/servers/controller/handlers/errors.go
+3 -2
View File
@@ -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)
internal/servers/controller/handlers/errors_test.go
+94 -4
View File
@@ -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) {
internal/servers/controller/handlers/host_catalogs/host_catalog_service.go
+21
View File
@@ -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
internal/servers/controller/handlers/host_sets/host_set_service.go
+43 -6
View File
@@ -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 {
internal/servers/controller/handlers/hosts/host_service.go
+19 -3
View File
@@ -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