From 5c5b07c04a1ba8e195bbfc3f9707d6e36594ad0b Mon Sep 17 00:00:00 2001 From: s-christoff Date: Thu, 8 Apr 2021 19:20:36 -0500 Subject: [PATCH] Revert "Refactor internal/servers/controller/handlers errors (#1070)" (#1092) This reverts commit 21ac3cdddd8dde48515d40854c7946e57f7c469e. --- .../handlers/accounts/account_service.go | 18 ++--- .../authmethods/authmethod_service.go | 23 +++--- .../handlers/authtokens/authtoken_service.go | 22 +++--- .../handlers/groups/group_service.go | 48 +++++------ .../host_catalogs/host_catalog_service.go | 44 +++++++---- .../handlers/host_sets/host_set_service.go | 79 ++++++++++++++----- .../controller/handlers/hosts/host_service.go | 38 ++++++--- .../controller/handlers/mask_manager.go | 11 +-- .../controller/handlers/roles/role_service.go | 30 +++---- .../handlers/scopes/scope_service.go | 13 +-- .../handlers/sessions/session_service.go | 8 +- .../handlers/targets/target_service.go | 29 +++---- .../controller/handlers/users/user_service.go | 21 ++--- 13 files changed, 202 insertions(+), 182 deletions(-) diff --git a/internal/servers/controller/handlers/accounts/account_service.go b/internal/servers/controller/handlers/accounts/account_service.go index 9503f71e4..df26f100d 100644 --- a/internal/servers/controller/handlers/accounts/account_service.go +++ b/internal/servers/controller/handlers/accounts/account_service.go @@ -56,9 +56,8 @@ type Service struct { // NewService returns a user service which handles user related requests to boundary. func NewService(repo common.PasswordAuthRepoFactory) (Service, error) { - const op = "accounts.NewService" if repo == nil { - return Service{}, errors.New(errors.InvalidParameter, op, "missing password repository") + return Service{}, fmt.Errorf("nil password repository provided") } return Service{repoFn: repo}, nil } @@ -226,7 +225,6 @@ func (s Service) getFromRepo(ctx context.Context, id string) (*pb.Account, error } func (s Service) createInRepo(ctx context.Context, authMethodId, scopeId string, item *pb.Account) (*pb.Account, error) { - const op = "accounts.(Service).createInRepo" pwAttrs := &pb.PasswordAccountAttributes{} if err := handlers.StructToProto(item.GetAttributes(), pwAttrs); err != nil { return nil, handlers.InvalidArgumentErrorf("Error in provided request.", @@ -254,7 +252,7 @@ func (s Service) createInRepo(ctx context.Context, authMethodId, scopeId string, } out, err := repo.CreateAccount(ctx, scopeId, a, createOpts...) if err != nil { - return nil, errors.Wrap(err, op) + return nil, fmt.Errorf("unable to create user: %w", err) } if out == nil { return nil, handlers.ApiErrorWithCodeAndMessage(codes.Internal, "Unable to create user but no error returned from repository.") @@ -263,7 +261,6 @@ func (s Service) createInRepo(ctx context.Context, authMethodId, scopeId string, } func (s Service) updateInRepo(ctx context.Context, scopeId, authMethId, id string, mask []string, item *pb.Account) (*pb.Account, error) { - const op = "accounts.(Service).updateInRepo" var opts []password.Option if desc := item.GetDescription(); desc != nil { opts = append(opts, password.WithDescription(desc.GetValue())) @@ -301,7 +298,7 @@ func (s Service) updateInRepo(ctx context.Context, scopeId, authMethId, id strin return nil, handlers.InvalidArgumentErrorf("Error in provided request.", map[string]string{"attributes.login_name": "Length too short."}) } - return nil, errors.Wrap(err, op) + return nil, fmt.Errorf("unable to update auth method: %w", err) } if rowsUpdated == 0 { return nil, handlers.NotFoundErrorf("Account %q doesn't exist or incorrect version provided.", id) @@ -310,7 +307,6 @@ func (s Service) updateInRepo(ctx context.Context, scopeId, authMethId, id strin } func (s Service) deleteFromRepo(ctx context.Context, scopeId, id string) (bool, error) { - const op = "accounts.(Service).deleteFromRepo" repo, err := s.repoFn() if err != nil { return false, err @@ -320,7 +316,7 @@ func (s Service) deleteFromRepo(ctx context.Context, scopeId, id string) (bool, if errors.IsNotFoundError(err) { return false, nil } - return false, errors.Wrap(err, op) + return false, fmt.Errorf("unable to delete account: %w", err) } return rows > 0, nil } @@ -346,7 +342,6 @@ func (s Service) listFromRepo(ctx context.Context, authMethodId string) ([]*pb.A } func (s Service) changePasswordInRepo(ctx context.Context, scopeId, id string, version uint32, currentPassword, newPassword string) (*pb.Account, error) { - const op = "account.(Service).changePasswordInRepo" repo, err := s.repoFn() if err != nil { return nil, err @@ -363,7 +358,7 @@ func (s Service) changePasswordInRepo(ctx context.Context, scopeId, id string, v return nil, handlers.InvalidArgumentErrorf("Error in provided request.", map[string]string{"new_password": "New password equal to current password."}) } - return nil, errors.Wrap(err, op) + return nil, fmt.Errorf("unable to change password: %w", err) } if out == nil { return nil, handlers.ApiErrorWithCodeAndMessage(codes.PermissionDenied, "Failed to change password.") @@ -372,7 +367,6 @@ func (s Service) changePasswordInRepo(ctx context.Context, scopeId, id string, v } func (s Service) setPasswordInRepo(ctx context.Context, scopeId, id string, version uint32, pw string) (*pb.Account, error) { - const op = "accounts.(Service).setPasswordInRepo" repo, err := s.repoFn() if err != nil { return nil, err @@ -386,7 +380,7 @@ func (s Service) setPasswordInRepo(ctx context.Context, scopeId, id string, vers return nil, handlers.InvalidArgumentErrorf("Error in provided request.", map[string]string{"password": "Password is too short."}) } - return nil, errors.Wrap(err, op) + return nil, fmt.Errorf("unable to set password: %w", err) } return toProto(out) } diff --git a/internal/servers/controller/handlers/authmethods/authmethod_service.go b/internal/servers/controller/handlers/authmethods/authmethod_service.go index a6459088d..f39ffb698 100644 --- a/internal/servers/controller/handlers/authmethods/authmethod_service.go +++ b/internal/servers/controller/handlers/authmethods/authmethod_service.go @@ -2,6 +2,7 @@ package authmethods import ( "context" + stderrors "errors" "fmt" "strings" @@ -106,15 +107,14 @@ type Service struct { // NewService returns a auth method service which handles auth method related requests to boundary. func NewService(kms *kms.Kms, pwRepoFn common.PasswordAuthRepoFactory, iamRepoFn common.IamRepoFactory, atRepoFn common.AuthTokenRepoFactory) (Service, error) { - const op = "authmethods.NewService" if kms == nil { - return Service{}, errors.New(errors.InvalidParameter, op, "missing kms") + return Service{}, stderrors.New("nil kms provided") } if pwRepoFn == nil { - return Service{}, errors.New(errors.InvalidParameter, op, "missing password repository") + return Service{}, fmt.Errorf("nil password repository provided") } if iamRepoFn == nil { - return Service{}, errors.New(errors.InvalidParameter, op, "missing iam repository") + return Service{}, fmt.Errorf("nil iam repository provided") } return Service{kms: kms, pwRepoFn: pwRepoFn, iamRepoFn: iamRepoFn, atRepoFn: atRepoFn}, nil } @@ -340,7 +340,6 @@ func (s Service) listFromRepo(ctx context.Context, scopeIds []string) ([]*pb.Aut } func (s Service) createInRepo(ctx context.Context, scopeId string, item *pb.AuthMethod) (*pb.AuthMethod, error) { - const op = "authmethods.(Service).createInRepo" var opts []password.Option if item.GetName() != nil { opts = append(opts, password.WithName(item.GetName().GetValue())) @@ -354,11 +353,11 @@ func (s Service) createInRepo(ctx context.Context, scopeId string, item *pb.Auth } repo, err := s.pwRepoFn() if err != nil { - return nil, errors.Wrap(err, op) + return nil, err } out, err := repo.CreateAuthMethod(ctx, u) if err != nil { - return nil, errors.Wrap(err, op, errors.WithMsg("unable to create auth method")) + return nil, fmt.Errorf("unable to create auth method: %w", err) } if out == nil { return nil, handlers.ApiErrorWithCodeAndMessage(codes.Internal, "Unable to create auth method but no error returned from repository.") @@ -367,7 +366,6 @@ func (s Service) createInRepo(ctx context.Context, scopeId string, item *pb.Auth } func (s Service) updateInRepo(ctx context.Context, scopeId, id string, mask []string, item *pb.AuthMethod) (*pb.AuthMethod, error) { - const op = "authmethods.(Service).updateInRepo" var opts []password.Option if desc := item.GetDescription(); desc != nil { opts = append(opts, password.WithDescription(desc.GetValue())) @@ -400,11 +398,11 @@ func (s Service) updateInRepo(ctx context.Context, scopeId, id string, mask []st } repo, err := s.pwRepoFn() if err != nil { - return nil, errors.Wrap(err, op) + return nil, err } out, rowsUpdated, err := repo.UpdateAuthMethod(ctx, u, version, dbMask) if err != nil { - return nil, errors.Wrap(err, op, errors.WithMsg("unable to update auth method")) + return nil, fmt.Errorf("unable to update auth method: %w", err) } if rowsUpdated == 0 { return nil, handlers.NotFoundErrorf("AuthMethod %q doesn't exist or incorrect version provided.", id) @@ -413,17 +411,16 @@ func (s Service) updateInRepo(ctx context.Context, scopeId, id string, mask []st } func (s Service) deleteFromRepo(ctx context.Context, scopeId, id string) (bool, error) { - const op = "authmethods.(Service).deleteFromRepo" repo, err := s.pwRepoFn() if err != nil { - return false, errors.Wrap(err, op) + return false, err } rows, err := repo.DeleteAuthMethod(ctx, scopeId, id) if err != nil { if errors.IsNotFoundError(err) { return false, nil } - return false, errors.Wrap(err, op) + return false, fmt.Errorf("unable to delete auth method: %w", err) } return rows > 0, nil } diff --git a/internal/servers/controller/handlers/authtokens/authtoken_service.go b/internal/servers/controller/handlers/authtokens/authtoken_service.go index 194583cd5..671b82b29 100644 --- a/internal/servers/controller/handlers/authtokens/authtoken_service.go +++ b/internal/servers/controller/handlers/authtokens/authtoken_service.go @@ -43,12 +43,11 @@ type Service struct { // NewService returns a user service which handles user related requests to boundary. func NewService(repo common.AuthTokenRepoFactory, iamRepoFn common.IamRepoFactory) (Service, error) { - const op = "authtoken.NewService" if repo == nil { - return Service{}, errors.New(errors.InvalidParameter, op, "missing auth token repository") + return Service{}, fmt.Errorf("nil auth token repository provided") } if iamRepoFn == nil { - return Service{}, errors.New(errors.InvalidParameter, op, "missing iam repository") + return Service{}, fmt.Errorf("nil iam repository provided") } return Service{repoFn: repo, iamRepoFn: iamRepoFn}, nil } @@ -144,33 +143,34 @@ func (s Service) DeleteAuthToken(ctx context.Context, req *pbs.DeleteAuthTokenRe } func (s Service) getFromRepo(ctx context.Context, id string) (*pb.AuthToken, error) { - const op = "authtokens.(Service).getFromRepo" repo, err := s.repoFn() if err != nil { - return nil, errors.Wrap(err, op) + return nil, err } u, err := repo.LookupAuthToken(ctx, id) - if err != nil && !errors.IsNotFoundError(err) { - return nil, errors.Wrap(err, op) + if err != nil { + if errors.IsNotFoundError(err) { + return nil, handlers.NotFoundErrorf("AuthToken %q doesn't exist.", id) + } + return nil, fmt.Errorf("unable to lookup auth token: %w", err) } if u == nil { - return nil, errors.New(errors.InvalidParameter, op, fmt.Sprintf("AuthToken %q not found", id)) + return nil, handlers.NotFoundErrorf("AuthToken %q doesn't exist.", id) } return toProto(u), nil } func (s Service) deleteFromRepo(ctx context.Context, id string) (bool, error) { - const op = "authtokens.(Service).deleteFromRepo" repo, err := s.repoFn() if err != nil { - return false, errors.Wrap(err, op) + return false, err } rows, err := repo.DeleteAuthToken(ctx, id) if err != nil { if errors.IsNotFoundError(err) { return false, nil } - return false, errors.Wrap(err, op, errors.WithMsg("unable to delete user")) + return false, fmt.Errorf("unable to delete user: %w", err) } return rows > 0, nil } diff --git a/internal/servers/controller/handlers/groups/group_service.go b/internal/servers/controller/handlers/groups/group_service.go index dcf241a84..7298fead4 100644 --- a/internal/servers/controller/handlers/groups/group_service.go +++ b/internal/servers/controller/handlers/groups/group_service.go @@ -60,9 +60,8 @@ type Service struct { // NewService returns a group service which handles group related requests to boundary. func NewService(repo common.IamRepoFactory) (Service, error) { - const op = "groups.NewService" if repo == nil { - return Service{}, errors.New(errors.InvalidParameter, op, "missing iam repository") + return Service{}, fmt.Errorf("nil iam repository provided") } return Service{repoFn: repo}, nil } @@ -248,23 +247,24 @@ func (s Service) RemoveGroupMembers(ctx context.Context, req *pbs.RemoveGroupMem } func (s Service) getFromRepo(ctx context.Context, id string) (*pb.Group, error) { - const op = "groups.(Service).getFromRepo" repo, err := s.repoFn() if err != nil { - return nil, errors.Wrap(err, op) + return nil, err } g, m, err := repo.LookupGroup(ctx, id) - if err != nil && !errors.IsNotFoundError(err) { - return nil, errors.Wrap(err, op) + if err != nil { + if errors.IsNotFoundError(err) { + return nil, handlers.NotFoundErrorf("Group %q doesn't exist.", id) + } + return nil, fmt.Errorf("unable to get group: %w", err) } if g == nil { - return nil, errors.New(errors.InvalidParameter, op, fmt.Sprintf("group %q not found", id)) + return nil, handlers.NotFoundErrorf("Group %q doesn't exist.", id) } return toProto(g, m), nil } func (s Service) createInRepo(ctx context.Context, scopeId string, item *pb.Group) (*pb.Group, error) { - const op = "groups.(Service).createInRepo" var opts []iam.Option if item.GetName() != nil { opts = append(opts, iam.WithName(item.GetName().GetValue())) @@ -278,11 +278,11 @@ func (s Service) createInRepo(ctx context.Context, scopeId string, item *pb.Grou } repo, err := s.repoFn() if err != nil { - return nil, errors.Wrap(err, op) + return nil, err } out, err := repo.CreateGroup(ctx, u) if err != nil { - return nil, errors.Wrap(err, op) + return nil, fmt.Errorf("unable to create group: %w", err) } if out == nil { return nil, handlers.ApiErrorWithCodeAndMessage(codes.Internal, "Unable to create group but no error returned from repository.") @@ -291,7 +291,6 @@ func (s Service) createInRepo(ctx context.Context, scopeId string, item *pb.Grou } func (s Service) updateInRepo(ctx context.Context, scopeId, id string, mask []string, item *pb.Group) (*pb.Group, error) { - const op = "groups.(Service).updateInRepo" var opts []iam.Option if desc := item.GetDescription(); desc != nil { opts = append(opts, iam.WithDescription(desc.GetValue())) @@ -311,11 +310,11 @@ func (s Service) updateInRepo(ctx context.Context, scopeId, id string, mask []st } repo, err := s.repoFn() if err != nil { - return nil, errors.Wrap(err, op) + return nil, err } out, m, rowsUpdated, err := repo.UpdateGroup(ctx, g, version, dbMask) if err != nil { - return nil, errors.Wrap(err, op) + return nil, fmt.Errorf("unable to update group: %w", err) } if rowsUpdated == 0 { return nil, handlers.NotFoundErrorf("Group %q doesn't exist or incorrect version provided.", id) @@ -324,7 +323,6 @@ func (s Service) updateInRepo(ctx context.Context, scopeId, id string, mask []st } func (s Service) deleteFromRepo(ctx context.Context, id string) (bool, error) { - const op = "groups.(Service).deleteFromRepo" repo, err := s.repoFn() if err != nil { return false, err @@ -334,20 +332,19 @@ func (s Service) deleteFromRepo(ctx context.Context, id string) (bool, error) { if errors.IsNotFoundError(err) { return false, nil } - return false, errors.Wrap(err, op, errors.WithMsg("unable to delete group")) + return false, fmt.Errorf("unable to delete group: %w", err) } return rows > 0, nil } func (s Service) listFromRepo(ctx context.Context, scopeIds []string) ([]*pb.Group, error) { - const op = "groups.(Service).listFromRepo" repo, err := s.repoFn() if err != nil { - return nil, errors.Wrap(err, op) + return nil, err } gl, err := repo.ListGroups(ctx, scopeIds) if err != nil { - return nil, errors.Wrap(err, op) + return nil, fmt.Errorf("unable to list groups: %w", err) } var outGl []*pb.Group for _, g := range gl { @@ -357,10 +354,9 @@ func (s Service) listFromRepo(ctx context.Context, scopeIds []string) ([]*pb.Gro } func (s Service) addMembersInRepo(ctx context.Context, groupId string, userIds []string, version uint32) (*pb.Group, error) { - const op = "groups.(Service).addMembersInRepo" repo, err := s.repoFn() if err != nil { - return nil, errors.Wrap(err, op) + return nil, err } _, err = repo.AddGroupMembers(ctx, groupId, version, strutil.RemoveDuplicates(userIds, false)) if err != nil { @@ -369,7 +365,7 @@ func (s Service) addMembersInRepo(ctx context.Context, groupId string, userIds [ } out, m, err := repo.LookupGroup(ctx, groupId) if err != nil { - return nil, errors.Wrap(err, op, errors.WithMsg("unable to look up group after adding memebers")) + return nil, fmt.Errorf("unable to look up group after adding members: %w", err) } if out == nil { return nil, handlers.ApiErrorWithCodeAndMessage(codes.Internal, "Unable to lookup group after adding member to it.") @@ -378,10 +374,9 @@ func (s Service) addMembersInRepo(ctx context.Context, groupId string, userIds [ } func (s Service) setMembersInRepo(ctx context.Context, groupId string, userIds []string, version uint32) (*pb.Group, error) { - const op = "groups.(Service).setMembersInRepo" repo, err := s.repoFn() if err != nil { - return nil, errors.Wrap(err, op) + return nil, err } _, _, err = repo.SetGroupMembers(ctx, groupId, version, strutil.RemoveDuplicates(userIds, false)) if err != nil { @@ -390,7 +385,7 @@ func (s Service) setMembersInRepo(ctx context.Context, groupId string, userIds [ } out, m, err := repo.LookupGroup(ctx, groupId) if err != nil { - return nil, errors.Wrap(err, op, errors.WithMsg("unable to look up group after setting members")) + return nil, fmt.Errorf("unable to look up group after setting members: %w", err) } if out == nil { return nil, handlers.ApiErrorWithCodeAndMessage(codes.Internal, "Unable to lookup group after setting members for it.") @@ -399,10 +394,9 @@ func (s Service) setMembersInRepo(ctx context.Context, groupId string, userIds [ } func (s Service) removeMembersInRepo(ctx context.Context, groupId string, userIds []string, version uint32) (*pb.Group, error) { - const op = "groups.(Service).removeMembersInRepo" repo, err := s.repoFn() if err != nil { - return nil, errors.Wrap(err, op) + return nil, err } _, err = repo.DeleteGroupMembers(ctx, groupId, version, strutil.RemoveDuplicates(userIds, false)) if err != nil { @@ -411,7 +405,7 @@ func (s Service) removeMembersInRepo(ctx context.Context, groupId string, userId } out, m, err := repo.LookupGroup(ctx, groupId) if err != nil { - return nil, errors.Wrap(err, op, errors.WithMsg("unable to look up group after removing members")) + return nil, fmt.Errorf("unable to look up group after removing members: %w", err) } if out == nil { return nil, handlers.ApiErrorWithCodeAndMessage(codes.Internal, "Unable to lookup group after removing members from it.") 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 74bafce19..0acd93c35 100644 --- a/internal/servers/controller/handlers/host_catalogs/host_catalog_service.go +++ b/internal/servers/controller/handlers/host_catalogs/host_catalog_service.go @@ -96,12 +96,11 @@ func populateCollectionAuthorizedActions(ctx context.Context, // NewService returns a host catalog Service which handles host catalog related requests to boundary and uses the provided // repositories for storage and retrieval. func NewService(repoFn common.StaticRepoFactory, iamRepoFn common.IamRepoFactory) (Service, error) { - const op = "host_catalogs.NewService" if repoFn == nil { - return Service{}, errors.New(errors.InvalidParameter, op, "missing static repository") + return Service{}, fmt.Errorf("nil static repository provided") } if iamRepoFn == nil { - return Service{}, errors.New(errors.InvalidParameter, op, "missing iam repository") + return Service{}, fmt.Errorf("nil iam repository provided") } return Service{staticRepoFn: repoFn, iamRepoFn: iamRepoFn}, nil } @@ -277,7 +276,6 @@ func (s Service) listFromRepo(ctx context.Context, scopeIds []string) ([]*pb.Hos } func (s Service) createInRepo(ctx context.Context, projId string, item *pb.HostCatalog) (*pb.HostCatalog, error) { - const op = "host_catalogs.(Servivce).createInRepo" var opts []static.Option if item.GetName() != nil { opts = append(opts, static.WithName(item.GetName().GetValue())) @@ -287,15 +285,23 @@ func (s Service) createInRepo(ctx context.Context, projId string, item *pb.HostC } h, err := static.NewHostCatalog(projId, opts...) if err != nil { - return nil, errors.Wrap(err, op, errors.WithMsg("unable to build host catalog for creation")) + 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() if err != nil { - return nil, errors.Wrap(err, op) + return nil, err } out, err := repo.CreateCatalog(ctx, h) if err != nil { - return nil, errors.Wrap(err, op, errors.WithMsg("unable to create host catalog")) + 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 { return nil, handlers.ApiErrorWithCodeAndMessage(codes.Internal, "Unable to create host catalog but no error returned from repository.") @@ -304,7 +310,6 @@ func (s Service) createInRepo(ctx context.Context, projId string, item *pb.HostC } func (s Service) updateInRepo(ctx context.Context, projId, id string, mask []string, item *pb.HostCatalog) (*pb.HostCatalog, error) { - const op = "service.(Service).updateInRepo" var opts []static.Option if desc := item.GetDescription(); desc != nil { opts = append(opts, static.WithDescription(desc.GetValue())) @@ -315,7 +320,11 @@ func (s Service) updateInRepo(ctx context.Context, projId, id string, mask []str version := item.GetVersion() h, err := static.NewHostCatalog(projId, opts...) if err != nil { - return nil, errors.Wrap(err, op, errors.WithMsg("unable to build host catalog for update")) + 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 dbMask := maskManager.Translate(mask) @@ -324,11 +333,15 @@ func (s Service) updateInRepo(ctx context.Context, projId, id string, mask []str } repo, err := s.staticRepoFn() if err != nil { - return nil, errors.Wrap(err, op) + return nil, err } out, rowsUpdated, err := repo.UpdateCatalog(ctx, h, version, dbMask) if err != nil { - return nil, errors.Wrap(err, op, errors.WithMsg("unable to update host catalog")) + 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 { return nil, handlers.NotFoundErrorf("Host Catalog %q doesn't exist or incorrect version provided.", id) @@ -337,14 +350,17 @@ func (s Service) updateInRepo(ctx context.Context, projId, id string, mask []str } func (s Service) deleteFromRepo(ctx context.Context, id string) (bool, error) { - const op = "service.(Service).deleteFromRepo" repo, err := s.staticRepoFn() if err != nil { - return false, errors.Wrap(err, op) + return false, err } rows, err := repo.DeleteCatalog(ctx, id) if err != nil { - return false, errors.Wrap(err, op, errors.WithMsg("unable to delete host")) + 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 01855f40d..1eece1bb2 100644 --- a/internal/servers/controller/handlers/host_sets/host_set_service.go +++ b/internal/servers/controller/handlers/host_sets/host_set_service.go @@ -61,9 +61,8 @@ var _ pbs.HostSetServiceServer = Service{} // NewService returns a host set Service which handles host set related requests to boundary and uses the provided // repositories for storage and retrieval. func NewService(repoFn common.StaticRepoFactory) (Service, error) { - const op = "host_sets.NewService" if repoFn == nil { - return Service{}, errors.New(errors.InvalidParameter, op, "missing static repository") + return Service{}, fmt.Errorf("nil static repository provided") } return Service{staticRepoFn: repoFn}, nil } @@ -246,7 +245,6 @@ func (s Service) getFromRepo(ctx context.Context, id string) (*pb.HostSet, error } func (s Service) createInRepo(ctx context.Context, scopeId, catalogId string, item *pb.HostSet) (*pb.HostSet, error) { - const op = "host_sets.(Service).createInRepo" var opts []static.Option if item.GetName() != nil { opts = append(opts, static.WithName(item.GetName().GetValue())) @@ -256,7 +254,11 @@ func (s Service) createInRepo(ctx context.Context, scopeId, catalogId string, it } h, err := static.NewHostSet(catalogId, opts...) if err != nil { - return nil, errors.Wrap(err, op, errors.WithMsg("Unable to build host set for creation")) + 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() if err != nil { @@ -264,7 +266,11 @@ func (s Service) createInRepo(ctx context.Context, scopeId, catalogId string, it } out, err := repo.CreateSet(ctx, scopeId, h) if err != nil { - return nil, errors.Wrap(err, op, errors.WithMsg("unable to create host set")) + 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) } if out == nil { return nil, handlers.ApiErrorWithCodeAndMessage(codes.Internal, "Unable to create host set but no error returned from repository.") @@ -273,7 +279,6 @@ func (s Service) createInRepo(ctx context.Context, scopeId, catalogId string, it } func (s Service) updateInRepo(ctx context.Context, scopeId, catalogId, id string, mask []string, item *pb.HostSet) (*pb.HostSet, error) { - const op = "host_sets.(Service).updateInRepo" var opts []static.Option if desc := item.GetDescription(); desc != nil { opts = append(opts, static.WithDescription(desc.GetValue())) @@ -283,7 +288,11 @@ func (s Service) updateInRepo(ctx context.Context, scopeId, catalogId, id string } h, err := static.NewHostSet(catalogId, opts...) if err != nil { - return nil, errors.Wrap(err, op, errors.WithMsg("Unable to build host set for update")) + 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 dbMask := maskManager.Translate(mask) @@ -296,7 +305,11 @@ 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 { - return nil, errors.Wrap(err, op, errors.WithMsg("unable to update host set")) + 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 { return nil, handlers.NotFoundErrorf("Host Set %q doesn't exist or incorrect version provided.", id) @@ -305,14 +318,17 @@ func (s Service) updateInRepo(ctx context.Context, scopeId, catalogId, id string } func (s Service) deleteFromRepo(ctx context.Context, scopeId, id string) (bool, error) { - const op = "host_sets.(Service).deleteFromRepo" repo, err := s.staticRepoFn() if err != nil { return false, err } rows, err := repo.DeleteSet(ctx, scopeId, id) if err != nil { - return false, errors.Wrap(err, op, errors.WithMsg("unable to delete host")) + 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 } @@ -334,18 +350,25 @@ func (s Service) listFromRepo(ctx context.Context, catalogId string) ([]*pb.Host } func (s Service) addInRepo(ctx context.Context, scopeId, setId string, hostIds []string, version uint32) (*pb.HostSet, error) { - const op = "host_sets.(Service).addInRepo" repo, err := s.staticRepoFn() if err != nil { return nil, err } _, err = repo.AddSetMembers(ctx, scopeId, setId, version, strutil.RemoveDuplicates(hostIds, false)) if err != nil { - return nil, errors.Wrap(err, op, errors.WithMsg("Unable to add hosts to host set")) + 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 { - return nil, errors.Wrap(err, op, errors.WithMsg("unable to look up host set after adding hosts")) + 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 { return nil, handlers.ApiErrorWithCodeAndMessage(codes.Internal, "Unable to lookup host set after adding hosts to it.") @@ -354,19 +377,26 @@ func (s Service) addInRepo(ctx context.Context, scopeId, setId string, hostIds [ } func (s Service) setInRepo(ctx context.Context, scopeId, setId string, hostIds []string, version uint32) (*pb.HostSet, error) { - const op = "host_sets.(Service).setInRepo" repo, err := s.staticRepoFn() if err != nil { - return nil, errors.Wrap(err, op) + return nil, err } _, _, err = repo.SetSetMembers(ctx, scopeId, setId, version, strutil.RemoveDuplicates(hostIds, false)) if err != nil { - return nil, errors.Wrap(err, op, errors.WithMsg("Unable to set hosts in host set")) + 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 { - return nil, errors.Wrap(err, op, errors.WithMsg("unable to look up host set after setting hosts")) + 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 { return nil, handlers.ApiErrorWithCodeAndMessage(codes.Internal, "Unable to lookup host set after setting hosts for it.") @@ -375,18 +405,25 @@ func (s Service) setInRepo(ctx context.Context, scopeId, setId string, hostIds [ } func (s Service) removeInRepo(ctx context.Context, scopeId, setId string, hostIds []string, version uint32) (*pb.HostSet, error) { - const op = "host_sets.(Service).removeInRepo" repo, err := s.staticRepoFn() if err != nil { - return nil, errors.Wrap(err, op) + return nil, err } _, err = repo.DeleteSetMembers(ctx, scopeId, setId, version, strutil.RemoveDuplicates(hostIds, false)) if err != nil { - return nil, errors.Wrap(err, op, errors.WithMsg("Unable to remove hosts from host set")) + 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 { - return nil, errors.Wrap(err, op, errors.WithMsg("unable to look up host set")) + 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 { return nil, handlers.ApiErrorWithCodeAndMessage(codes.Internal, "Unable to lookup host set after removing hosts from it.") diff --git a/internal/servers/controller/handlers/hosts/host_service.go b/internal/servers/controller/handlers/hosts/host_service.go index a53c963eb..910490b65 100644 --- a/internal/servers/controller/handlers/hosts/host_service.go +++ b/internal/servers/controller/handlers/hosts/host_service.go @@ -59,9 +59,8 @@ var _ pbs.HostServiceServer = Service{} // NewService returns a host Service which handles host related requests to boundary and uses the provided // repositories for storage and retrieval. func NewService(repoFn common.StaticRepoFactory) (Service, error) { - const op = "hosts.NewService" if repoFn == nil { - return Service{}, errors.New(errors.InvalidParameter, op, "missing static repository") + return Service{}, fmt.Errorf("nil static repository provided") } return Service{staticRepoFn: repoFn}, nil } @@ -190,7 +189,6 @@ func (s Service) getFromRepo(ctx context.Context, id string) (*pb.Host, error) { } func (s Service) createInRepo(ctx context.Context, scopeId, catalogId string, item *pb.Host) (*pb.Host, error) { - const op = "hosts.(Service).createInRepo" ha := &pb.StaticHostAttributes{} if err := handlers.StructToProto(item.GetAttributes(), ha); err != nil { return nil, handlers.ApiErrorWithCodeAndMessage(codes.Internal, "Failed converting attributes to subtype proto: %s", err) @@ -207,16 +205,24 @@ func (s Service) createInRepo(ctx context.Context, scopeId, catalogId string, it } h, err := static.NewHost(catalogId, opts...) if err != nil { - return nil, errors.Wrap(err, op, errors.WithMsg("Unable to build host for creation")) + 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) } repo, err := s.staticRepoFn() if err != nil { - return nil, errors.Wrap(err, op) + return nil, err } out, err := repo.CreateHost(ctx, scopeId, h) if err != nil { - return nil, errors.Wrap(err, op, errors.WithMsg("Unable to create host")) + 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) } if out == nil { return nil, handlers.ApiErrorWithCodeAndMessage(codes.Internal, "Unable to create host but no error returned from repository.") @@ -225,7 +231,6 @@ func (s Service) createInRepo(ctx context.Context, scopeId, catalogId string, it } func (s Service) updateInRepo(ctx context.Context, scopeId, catalogId, id string, mask []string, item *pb.Host) (*pb.Host, error) { - const op = "hosts.(Service).updateInRepo" ha := &pb.StaticHostAttributes{} if err := handlers.StructToProto(item.GetAttributes(), ha); err != nil { return nil, handlers.ApiErrorWithCodeAndMessage(codes.Internal, "Failed converting attributes to subtype proto: %s", err) @@ -242,7 +247,11 @@ func (s Service) updateInRepo(ctx context.Context, scopeId, catalogId, id string } h, err := static.NewHost(catalogId, opts...) if err != nil { - return nil, errors.Wrap(err, op, errors.WithMsg("Unable to build host for update")) + 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 dbMask := maskManager.Translate(mask) @@ -255,7 +264,11 @@ 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 { - return nil, errors.Wrap(err, op, errors.WithMsg("unable to update host")) + 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 { return nil, handlers.NotFoundErrorf("Host %q doesn't exist or incorrect version provided.", id) @@ -264,14 +277,17 @@ func (s Service) updateInRepo(ctx context.Context, scopeId, catalogId, id string } func (s Service) deleteFromRepo(ctx context.Context, scopeId, id string) (bool, error) { - const op = "hosts.(Service).deleteFromRepo" repo, err := s.staticRepoFn() if err != nil { return false, err } rows, err := repo.DeleteHost(ctx, scopeId, id) if err != nil { - return false, errors.Wrap(err, op, errors.WithMsg("unable to delete host")) + 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/mask_manager.go b/internal/servers/controller/handlers/mask_manager.go index d09b33856..5f0ab6b84 100644 --- a/internal/servers/controller/handlers/mask_manager.go +++ b/internal/servers/controller/handlers/mask_manager.go @@ -4,7 +4,6 @@ import ( "fmt" "strings" - "github.com/hashicorp/boundary/internal/errors" "google.golang.org/protobuf/proto" "google.golang.org/protobuf/reflect/protoreflect" "google.golang.org/protobuf/types/descriptorpb" @@ -18,7 +17,6 @@ type MaskManager map[string]string // protos assuming they are both using the mask_mapping custom option. Error is returned if no mappings are // found or if one of the passed protos has a mapping that doesn't reciprocate. func NewMaskManager(dest protoreflect.ProtoMessage, src ...protoreflect.ProtoMessage) (MaskManager, error) { - const op = "handlers.NewMaskManager" srcToDest, err := mapFromProto(src...) if err != nil { return nil, err @@ -32,7 +30,7 @@ func NewMaskManager(dest protoreflect.ProtoMessage, src ...protoreflect.ProtoMes for k, v := range srcToDest { ov, ok := destToSrc[v] if !ok || ov != k { - return nil, errors.New(errors.Encode, op, fmt.Sprintf("mapping src field %q maps to %q, dest %q maps to %q", k, v, v, ov)) + return nil, fmt.Errorf("mapping src field %q maps to %q, dest %q maps to %q", k, v, v, ov) } result[k] = v } @@ -40,19 +38,18 @@ func NewMaskManager(dest protoreflect.ProtoMessage, src ...protoreflect.ProtoMes // Now check to make sure there aren't any dangling dest mappings. for k, v := range destToSrc { if ov, ok := srcToDest[v]; !ok || ov != k { - return nil, errors.New(errors.Encrypt, op, fmt.Sprintf("mapping src field %q maps to %q, dest %q maps to %q", k, v, v, ov)) + return nil, fmt.Errorf("mapping dest field %q maps to %q, src %q maps to %q", k, v, v, ov) } } if len(result) == 0 { - return nil, errors.New(errors.InvalidParameter, op, "mask mapping generated is zero") + return nil, fmt.Errorf("size of mask mapping is 0") } return result, nil } func mapFromProto(ps ...protoreflect.ProtoMessage) (map[string]string, error) { - const op = "handlers.mapFromProto" mapping := make(map[string]string) for _, p := range ps { m := p.ProtoReflect() @@ -62,7 +59,7 @@ func mapFromProto(ps ...protoreflect.ProtoMessage) (map[string]string, error) { opts := f.Options().(*descriptorpb.FieldOptions) if nameMap := proto.GetExtension(opts, pb.E_MaskMapping).(*pb.MaskMapping); !proto.Equal(nameMap, &pb.MaskMapping{}) && nameMap != nil { if _, ok := mapping[nameMap.GetThis()]; ok { - return nil, errors.New(errors.InvalidParameter, op, fmt.Sprintf("duplicate mapping from field %q with the mapping key %q", f.Name(), nameMap.GetThis())) + return nil, fmt.Errorf("duplicate mapping from field %q with the mapping key %q", f.Name(), nameMap.GetThis()) } mapping[nameMap.GetThis()] = nameMap.GetThat() } diff --git a/internal/servers/controller/handlers/roles/role_service.go b/internal/servers/controller/handlers/roles/role_service.go index ac1c83863..a6f8f09f2 100644 --- a/internal/servers/controller/handlers/roles/role_service.go +++ b/internal/servers/controller/handlers/roles/role_service.go @@ -63,9 +63,8 @@ type Service struct { // NewService returns a role service which handles role related requests to boundary. func NewService(repo common.IamRepoFactory) (Service, error) { - const op = "roles.NewService" if repo == nil { - return Service{}, errors.New(errors.InvalidParameter, op, "missing iam repository") + return Service{}, fmt.Errorf("nil iam repository provided") } return Service{repoFn: repo}, nil } @@ -324,7 +323,6 @@ func (s Service) getFromRepo(ctx context.Context, id string) (*pb.Role, error) { } func (s Service) createInRepo(ctx context.Context, scopeId string, item *pb.Role) (*pb.Role, error) { - const op = "roles.(Service).createInRepo" var opts []iam.Option if item.GetName() != nil { opts = append(opts, iam.WithName(item.GetName().GetValue())) @@ -345,7 +343,7 @@ func (s Service) createInRepo(ctx context.Context, scopeId string, item *pb.Role } out, err := repo.CreateRole(ctx, u) if err != nil { - return nil, errors.Wrap(err, op) + return nil, fmt.Errorf("unable to create role: %w", err) } if out == nil { return nil, handlers.ApiErrorWithCodeAndMessage(codes.Internal, "Unable to create role but no error returned from repository.") @@ -354,7 +352,6 @@ func (s Service) createInRepo(ctx context.Context, scopeId string, item *pb.Role } func (s Service) updateInRepo(ctx context.Context, scopeId, id string, mask []string, item *pb.Role) (*pb.Role, error) { - const op = "roles.(Service).updateInRepo" var opts []iam.Option if desc := item.GetDescription(); desc != nil { opts = append(opts, iam.WithDescription(desc.GetValue())) @@ -382,7 +379,7 @@ func (s Service) updateInRepo(ctx context.Context, scopeId, id string, mask []st } out, pr, gr, rowsUpdated, err := repo.UpdateRole(ctx, u, version, dbMask) if err != nil { - return nil, errors.Wrap(err, op) + return nil, fmt.Errorf("unable to update role: %w", err) } if rowsUpdated == 0 { return nil, handlers.NotFoundErrorf("Role %q doesn't exist or incorrect version provided.", id) @@ -391,7 +388,6 @@ func (s Service) updateInRepo(ctx context.Context, scopeId, id string, mask []st } func (s Service) deleteFromRepo(ctx context.Context, id string) (bool, error) { - const op = "roles.(Service).deleteFromRepo" repo, err := s.repoFn() if err != nil { return false, err @@ -401,7 +397,7 @@ func (s Service) deleteFromRepo(ctx context.Context, id string) (bool, error) { if errors.IsNotFoundError(err) { return false, nil } - return false, errors.Wrap(err, op, errors.WithMsg("unable to delete role")) + return false, fmt.Errorf("unable to delete role: %w", err) } return rows > 0, nil } @@ -424,7 +420,6 @@ func (s Service) listFromRepo(ctx context.Context, scopeIds []string) ([]*pb.Rol } func (s Service) addPrinciplesInRepo(ctx context.Context, roleId string, principalIds []string, version uint32) (*pb.Role, error) { - const op = "roles.(Service).addPrincpleInRepo" repo, err := s.repoFn() if err != nil { return nil, err @@ -436,7 +431,7 @@ func (s Service) addPrinciplesInRepo(ctx context.Context, roleId string, princip } out, pr, roleGrants, err := repo.LookupRole(ctx, roleId) if err != nil { - return nil, errors.Wrap(err, op, errors.WithMsg("unable to look up role after adding principals")) + return nil, fmt.Errorf("unable to look up role after adding principals: %w", err) } if out == nil { return nil, handlers.ApiErrorWithCodeAndMessage(codes.Internal, "Unable to lookup role after adding principals to it.") @@ -445,7 +440,6 @@ func (s Service) addPrinciplesInRepo(ctx context.Context, roleId string, princip } func (s Service) setPrinciplesInRepo(ctx context.Context, roleId string, principalIds []string, version uint32) (*pb.Role, error) { - const op = "roles.(Service).setPrinciplesInRepo" repo, err := s.repoFn() if err != nil { return nil, err @@ -457,7 +451,7 @@ func (s Service) setPrinciplesInRepo(ctx context.Context, roleId string, princip } out, pr, roleGrants, err := repo.LookupRole(ctx, roleId) if err != nil { - return nil, errors.Wrap(err, op, errors.WithMsg("unable to look up role after setting principals")) + return nil, fmt.Errorf("unable to look up role after setting principals: %w", err) } if out == nil { return nil, handlers.ApiErrorWithCodeAndMessage(codes.Internal, "Unable to lookup role after setting principals for it.") @@ -466,7 +460,6 @@ func (s Service) setPrinciplesInRepo(ctx context.Context, roleId string, princip } func (s Service) removePrinciplesInRepo(ctx context.Context, roleId string, principalIds []string, version uint32) (*pb.Role, error) { - const op = "roles.(Service).removePrinciplesInRepo" repo, err := s.repoFn() if err != nil { return nil, err @@ -478,7 +471,7 @@ func (s Service) removePrinciplesInRepo(ctx context.Context, roleId string, prin } out, pr, roleGrants, err := repo.LookupRole(ctx, roleId) if err != nil { - return nil, errors.Wrap(err, op, errors.WithMsg("unable to look up role after removing principals")) + return nil, fmt.Errorf("unable to look up role after removing principals: %w", err) } if out == nil { return nil, handlers.ApiErrorWithCodeAndMessage(codes.Internal, "Unable to lookup role after removing principals from it.") @@ -487,7 +480,6 @@ func (s Service) removePrinciplesInRepo(ctx context.Context, roleId string, prin } func (s Service) addGrantsInRepo(ctx context.Context, roleId string, grants []string, version uint32) (*pb.Role, error) { - const op = "service.(Service).addGrantsInRepo" repo, err := s.repoFn() if err != nil { return nil, err @@ -499,7 +491,7 @@ func (s Service) addGrantsInRepo(ctx context.Context, roleId string, grants []st } out, pr, roleGrants, err := repo.LookupRole(ctx, roleId) if err != nil { - return nil, errors.Wrap(err, op, errors.WithMsg("unable to look up role after adding grants")) + return nil, fmt.Errorf("unable to look up role after adding grants: %w", err) } if out == nil { return nil, handlers.ApiErrorWithCodeAndMessage(codes.Internal, "Unable to lookup role after adding grants to it.") @@ -508,7 +500,6 @@ func (s Service) addGrantsInRepo(ctx context.Context, roleId string, grants []st } func (s Service) setGrantsInRepo(ctx context.Context, roleId string, grants []string, version uint32) (*pb.Role, error) { - const op = "roles.(Service).setGrantsInRepo" repo, err := s.repoFn() if err != nil { return nil, err @@ -524,7 +515,7 @@ func (s Service) setGrantsInRepo(ctx context.Context, roleId string, grants []st } out, pr, roleGrants, err := repo.LookupRole(ctx, roleId) if err != nil { - return nil, errors.Wrap(err, op, errors.WithMsg("unable to look up role after setting grants")) + return nil, fmt.Errorf("unable to look up role after setting grants: %w", err) } if out == nil { return nil, handlers.ApiErrorWithCodeAndMessage(codes.Internal, "Unable to lookup role after setting grants on it.") @@ -533,7 +524,6 @@ func (s Service) setGrantsInRepo(ctx context.Context, roleId string, grants []st } func (s Service) removeGrantsInRepo(ctx context.Context, roleId string, grants []string, version uint32) (*pb.Role, error) { - const op = "roles.(Service).removeGrantsInRepo" repo, err := s.repoFn() if err != nil { return nil, err @@ -545,7 +535,7 @@ func (s Service) removeGrantsInRepo(ctx context.Context, roleId string, grants [ } out, pr, roleGrants, err := repo.LookupRole(ctx, roleId) if err != nil { - return nil, errors.Wrap(err, op, errors.WithMsg("uable to look up role after removing grant")) + return nil, fmt.Errorf("unable to look up role after removing grants: %w", err) } if out == nil { return nil, handlers.ApiErrorWithCodeAndMessage(codes.Internal, "Unable to lookup role after removing grants from it.") diff --git a/internal/servers/controller/handlers/scopes/scope_service.go b/internal/servers/controller/handlers/scopes/scope_service.go index 84e0752c8..bd2db6258 100644 --- a/internal/servers/controller/handlers/scopes/scope_service.go +++ b/internal/servers/controller/handlers/scopes/scope_service.go @@ -7,7 +7,6 @@ import ( "strings" "github.com/hashicorp/boundary/internal/auth" - "github.com/hashicorp/boundary/internal/errors" pb "github.com/hashicorp/boundary/internal/gen/controller/api/resources/scopes" pbs "github.com/hashicorp/boundary/internal/gen/controller/api/services" "github.com/hashicorp/boundary/internal/iam" @@ -85,9 +84,8 @@ type Service struct { // NewService returns a project service which handles project related requests to boundary. func NewService(repo common.IamRepoFactory) (Service, error) { - const op = "scopes.(Service).NewService" if repo == nil { - return Service{}, errors.New(errors.InvalidParameter, op, "missing iam repository") + return Service{}, fmt.Errorf("nil iam repository provided") } return Service{repoFn: repo}, nil } @@ -286,7 +284,6 @@ func (s Service) getFromRepo(ctx context.Context, id string) (*pb.Scope, error) } func (s Service) createInRepo(ctx context.Context, authResults auth.VerifyResults, req *pbs.CreateScopeRequest) (*pb.Scope, error) { - const op = "scopes.(Service).createInRepo" item := req.GetItem() var opts []iam.Option if item.GetName() != nil { @@ -316,7 +313,7 @@ func (s Service) createInRepo(ctx context.Context, authResults auth.VerifyResult } out, err := repo.CreateScope(ctx, iamScope, authResults.UserId, opts...) if err != nil { - return nil, errors.Wrap(err, op, errors.WithMsg("unable to create scope")) + return nil, fmt.Errorf("unable to create scope: %w", err) } if out == nil { return nil, handlers.ApiErrorWithCodeAndMessage(codes.Internal, "Unable to create scope but no error returned from repository.") @@ -325,7 +322,6 @@ func (s Service) createInRepo(ctx context.Context, authResults auth.VerifyResult } func (s Service) updateInRepo(ctx context.Context, parentScope *pb.ScopeInfo, scopeId string, mask []string, item *pb.Scope) (*pb.Scope, error) { - const op = "scope.(Service).updateInRepo" var opts []iam.Option if desc := item.GetDescription(); desc != nil { opts = append(opts, iam.WithDescription(desc.GetValue())) @@ -357,7 +353,7 @@ func (s Service) updateInRepo(ctx context.Context, parentScope *pb.ScopeInfo, sc } out, rowsUpdated, err := repo.UpdateScope(ctx, iamScope, version, dbMask) if err != nil { - return nil, errors.Wrap(err, op, errors.WithMsg("unable to update project")) + return nil, fmt.Errorf("unable to update project: %w", err) } if rowsUpdated == 0 { return nil, handlers.NotFoundErrorf("Scope %q doesn't exist or incorrect version provided.", scopeId) @@ -366,14 +362,13 @@ func (s Service) updateInRepo(ctx context.Context, parentScope *pb.ScopeInfo, sc } func (s Service) deleteFromRepo(ctx context.Context, scopeId string) (bool, error) { - const op = "scope.(Service).deleteFromRepo" repo, err := s.repoFn() if err != nil { return false, err } rows, err := repo.DeleteScope(ctx, scopeId) if err != nil { - return false, errors.Wrap(err, op, errors.WithMsg("unable to delete scope")) + return false, fmt.Errorf("unable to delete scope: %w", err) } return rows > 0, nil } diff --git a/internal/servers/controller/handlers/sessions/session_service.go b/internal/servers/controller/handlers/sessions/session_service.go index b87cae5c3..d89352a1a 100644 --- a/internal/servers/controller/handlers/sessions/session_service.go +++ b/internal/servers/controller/handlers/sessions/session_service.go @@ -47,12 +47,11 @@ type Service struct { // NewService returns a session service which handles session related requests to boundary. func NewService(repoFn common.SessionRepoFactory, iamRepoFn common.IamRepoFactory) (Service, error) { - const op = "sessions.NewService" if repoFn == nil { - return Service{}, errors.New(errors.InvalidParameter, op, "missing session repository") + return Service{}, fmt.Errorf("nil session repository provided") } if iamRepoFn == nil { - return Service{}, errors.New(errors.InvalidParameter, op, "missing iam repository") + return Service{}, fmt.Errorf("nil iam repository provided") } return Service{repoFn: repoFn, iamRepoFn: iamRepoFn}, nil } @@ -236,14 +235,13 @@ func (s Service) listFromRepo(ctx context.Context, opts ...session.Option) ([]*p } func (s Service) cancelInRepo(ctx context.Context, id string, version uint32) (*pb.Session, error) { - const op = "sessions.(Service).cancelInRepo" repo, err := s.repoFn() if err != nil { return nil, err } out, err := repo.CancelSession(ctx, id, version) if err != nil { - return nil, errors.Wrap(err, op, errors.WithMsg("unable to update session")) + return nil, fmt.Errorf("unable to update session: %w", err) } return toProto(out), nil } diff --git a/internal/servers/controller/handlers/targets/target_service.go b/internal/servers/controller/handlers/targets/target_service.go index 8f37ea0cf..e7b6f6dde 100644 --- a/internal/servers/controller/handlers/targets/target_service.go +++ b/internal/servers/controller/handlers/targets/target_service.go @@ -88,21 +88,20 @@ func NewService( serversRepoFn common.ServersRepoFactory, sessionRepoFn common.SessionRepoFactory, staticHostRepoFn common.StaticRepoFactory) (Service, error) { - const op = "targets.NewService" if repoFn == nil { - return Service{}, errors.New(errors.InvalidParameter, op, "missing target repository") + return Service{}, fmt.Errorf("nil target repository provided") } if iamRepoFn == nil { - return Service{}, errors.New(errors.InvalidParameter, op, "missing iam repository") + return Service{}, fmt.Errorf("nil iam repository provided") } if serversRepoFn == nil { - return Service{}, errors.New(errors.InvalidParameter, op, "missing servers repository") + return Service{}, fmt.Errorf("nil servers repository provided") } if sessionRepoFn == nil { - return Service{}, errors.New(errors.InvalidParameter, op, "missing session repository") + return Service{}, fmt.Errorf("nil session repository provided") } if staticHostRepoFn == nil { - return Service{}, errors.New(errors.InvalidParameter, op, "missing static host repository") + return Service{}, fmt.Errorf("nil static host repository provided") } return Service{ repoFn: repoFn, @@ -296,7 +295,6 @@ func (s Service) RemoveTargetHostSets(ctx context.Context, req *pbs.RemoveTarget } func (s Service) AuthorizeSession(ctx context.Context, req *pbs.AuthorizeSessionRequest) (*pbs.AuthorizeSessionResponse, error) { - const op = "targets.(Service).AuthorizeSession" if err := validateAuthorizeSessionRequest(req); err != nil { return nil, err } @@ -498,7 +496,7 @@ HostSetIterationLoop: case host.StaticSubtype: h, err := staticHostRepo.LookupHost(ctx, chosenId.hostId) if err != nil { - return nil, errors.New(errors.InvalidParameter, op, "errors looking up host") + return nil, fmt.Errorf("error looking up host: %w", err) } endpointHost = h.Address if endpointHost == "" { @@ -592,7 +590,6 @@ func (s Service) getFromRepo(ctx context.Context, id string) (*pb.Target, error) } func (s Service) createInRepo(ctx context.Context, item *pb.Target) (*pb.Target, error) { - const op = "targets.(Service).createInRepo" opts := []target.Option{target.WithName(item.GetName().GetValue())} if item.GetDescription() != nil { opts = append(opts, target.WithDescription(item.GetDescription().GetValue())) @@ -623,7 +620,7 @@ func (s Service) createInRepo(ctx context.Context, item *pb.Target) (*pb.Target, } out, m, err := repo.CreateTcpTarget(ctx, u) if err != nil { - return nil, errors.Wrap(err, op, errors.WithMsg("unable to create target")) + return nil, fmt.Errorf("unable to create target: %w", err) } if out == nil { return nil, handlers.ApiErrorWithCodeAndMessage(codes.Internal, "Unable to create target but no error returned from repository.") @@ -632,7 +629,6 @@ func (s Service) createInRepo(ctx context.Context, item *pb.Target) (*pb.Target, } func (s Service) updateInRepo(ctx context.Context, scopeId, id string, mask []string, item *pb.Target) (*pb.Target, error) { - const op = "targets.(Service).updateInRepo" var opts []target.Option if desc := item.GetDescription(); desc != nil { opts = append(opts, target.WithDescription(desc.GetValue())) @@ -672,7 +668,7 @@ func (s Service) updateInRepo(ctx context.Context, scopeId, id string, mask []st } out, m, rowsUpdated, err := repo.UpdateTcpTarget(ctx, u, version, dbMask) if err != nil { - return nil, errors.Wrap(err, op, errors.WithMsg("unable to update target")) + return nil, fmt.Errorf("unable to update target: %w", err) } if rowsUpdated == 0 { return nil, handlers.NotFoundErrorf("Target %q not found or incorrect version provided.", id) @@ -681,7 +677,6 @@ func (s Service) updateInRepo(ctx context.Context, scopeId, id string, mask []st } func (s Service) deleteFromRepo(ctx context.Context, id string) (bool, error) { - const op = "targets.(Service).deleteFromRepo" repo, err := s.repoFn() if err != nil { return false, err @@ -691,7 +686,7 @@ func (s Service) deleteFromRepo(ctx context.Context, id string) (bool, error) { if errors.IsNotFoundError(err) { return false, nil } - return false, errors.Wrap(err, op, errors.WithMsg("unable to delete target")) + return false, fmt.Errorf("unable to delete target: %w", err) } return rows > 0, nil } @@ -733,7 +728,6 @@ func (s Service) addInRepo(ctx context.Context, targetId string, hostSetId []str } func (s Service) setInRepo(ctx context.Context, targetId string, hostSetIds []string, version uint32) (*pb.Target, error) { - const op = "targets.(Service).setInRepo" repo, err := s.repoFn() if err != nil { return nil, err @@ -746,7 +740,7 @@ func (s Service) setInRepo(ctx context.Context, targetId string, hostSetIds []st out, m, err := repo.LookupTarget(ctx, targetId) if err != nil { - return nil, errors.Wrap(err, op, errors.WithMsg("unable to look up target after setting host sets")) + return nil, fmt.Errorf("unable to look up target after setting host sets: %w", err) } if out == nil { return nil, handlers.ApiErrorWithCodeAndMessage(codes.Internal, "Unable to lookup target after setting host sets for it.") @@ -755,7 +749,6 @@ func (s Service) setInRepo(ctx context.Context, targetId string, hostSetIds []st } func (s Service) removeInRepo(ctx context.Context, targetId string, hostSetIds []string, version uint32) (*pb.Target, error) { - const op = "targets.(Service).removeInRepo" repo, err := s.repoFn() if err != nil { return nil, err @@ -767,7 +760,7 @@ func (s Service) removeInRepo(ctx context.Context, targetId string, hostSetIds [ } out, m, err := repo.LookupTarget(ctx, targetId) if err != nil { - return nil, errors.Wrap(err, op, errors.WithMsg("unable to look up target after removing host sets")) + return nil, fmt.Errorf("unable to look up target after removing host sets: %w", err) } if out == nil { return nil, handlers.ApiErrorWithCodeAndMessage(codes.Internal, "Unable to lookup target after removing host sets from it.") diff --git a/internal/servers/controller/handlers/users/user_service.go b/internal/servers/controller/handlers/users/user_service.go index 8b0fcb782..15a68f5ae 100644 --- a/internal/servers/controller/handlers/users/user_service.go +++ b/internal/servers/controller/handlers/users/user_service.go @@ -61,9 +61,8 @@ type Service struct { // NewService returns a user service which handles user related requests to boundary. func NewService(repo common.IamRepoFactory) (Service, error) { - const op = "users.NewService" if repo == nil { - return Service{}, errors.New(errors.InvalidParameter, op, "missing iam repository") + return Service{}, fmt.Errorf("nil iam repository provided") } return Service{repoFn: repo}, nil } @@ -267,7 +266,6 @@ func (s Service) getFromRepo(ctx context.Context, id string) (*pb.User, error) { } func (s Service) createInRepo(ctx context.Context, orgId string, item *pb.User) (*pb.User, error) { - const op = "users.(Service).createInRepo" var opts []iam.Option if item.GetName() != nil { opts = append(opts, iam.WithName(item.GetName().GetValue())) @@ -285,7 +283,7 @@ func (s Service) createInRepo(ctx context.Context, orgId string, item *pb.User) } out, err := repo.CreateUser(ctx, u) if err != nil { - return nil, errors.Wrap(err, op, errors.WithMsg("unable to create user")) + return nil, fmt.Errorf("unable to create user: %w", err) } if out == nil { return nil, handlers.ApiErrorWithCodeAndMessage(codes.Internal, "Unable to create user but no error returned from repository.") @@ -294,7 +292,6 @@ func (s Service) createInRepo(ctx context.Context, orgId string, item *pb.User) } func (s Service) updateInRepo(ctx context.Context, orgId, id string, mask []string, item *pb.User) (*pb.User, error) { - const op = "users.(Service).updateInRepo" var opts []iam.Option if desc := item.GetDescription(); desc != nil { opts = append(opts, iam.WithDescription(desc.GetValue())) @@ -318,7 +315,7 @@ func (s Service) updateInRepo(ctx context.Context, orgId, id string, mask []stri } out, accts, rowsUpdated, err := repo.UpdateUser(ctx, u, version, dbMask) if err != nil { - return nil, errors.Wrap(err, op, errors.WithMsg("unable to update user")) + return nil, fmt.Errorf("unable to update user: %w", err) } if rowsUpdated == 0 { return nil, handlers.NotFoundErrorf("User %q doesn't exist or incorrect version provided.", id) @@ -327,7 +324,6 @@ func (s Service) updateInRepo(ctx context.Context, orgId, id string, mask []stri } func (s Service) deleteFromRepo(ctx context.Context, id string) (bool, error) { - const op = "users.(Service).deleteFromRepo" repo, err := s.repoFn() if err != nil { return false, err @@ -337,7 +333,7 @@ func (s Service) deleteFromRepo(ctx context.Context, id string) (bool, error) { if errors.IsNotFoundError(err) { return false, nil } - return false, errors.Wrap(err, op, errors.WithMsg("unable to delete user")) + return false, fmt.Errorf("unable to delete user: %w", err) } return rows > 0, nil } @@ -359,7 +355,6 @@ func (s Service) listFromRepo(ctx context.Context, scopeIds []string) ([]*pb.Use } func (s Service) addInRepo(ctx context.Context, userId string, accountIds []string, version uint32) (*pb.User, error) { - const op = "users.(Service).addInRepo" repo, err := s.repoFn() if err != nil { return nil, err @@ -370,7 +365,7 @@ func (s Service) addInRepo(ctx context.Context, userId string, accountIds []stri } out, accts, err := repo.LookupUser(ctx, userId) if err != nil { - return nil, errors.Wrap(err, op, errors.WithMsg("unable to look up user after adding accounts")) + return nil, fmt.Errorf("unable to look up user after adding accounts: %w", err) } if out == nil { return nil, handlers.ApiErrorWithCodeAndMessage(codes.Internal, "Unable to lookup user after adding accounts to it.") @@ -379,7 +374,6 @@ func (s Service) addInRepo(ctx context.Context, userId string, accountIds []stri } func (s Service) setInRepo(ctx context.Context, userId string, accountIds []string, version uint32) (*pb.User, error) { - const op = "users.(Service).setInRepo" repo, err := s.repoFn() if err != nil { return nil, err @@ -390,7 +384,7 @@ func (s Service) setInRepo(ctx context.Context, userId string, accountIds []stri } out, accts, err := repo.LookupUser(ctx, userId) if err != nil { - return nil, errors.Wrap(err, op, errors.WithMsg("unable to look up user after setting accounts")) + return nil, fmt.Errorf("unable to look up user after setting accounts: %w", err) } if out == nil { return nil, handlers.ApiErrorWithCodeAndMessage(codes.Internal, "Unable to lookup user after setting accounts for it.") @@ -399,7 +393,6 @@ func (s Service) setInRepo(ctx context.Context, userId string, accountIds []stri } func (s Service) removeInRepo(ctx context.Context, userId string, accountIds []string, version uint32) (*pb.User, error) { - const op = "users.(Service).removeInRepo" repo, err := s.repoFn() if err != nil { return nil, err @@ -410,7 +403,7 @@ func (s Service) removeInRepo(ctx context.Context, userId string, accountIds []s } out, accts, err := repo.LookupUser(ctx, userId) if err != nil { - return nil, errors.Wrap(err, op, errors.WithMsg("unable to look up user after removing accounts")) + return nil, fmt.Errorf("unable to look up user after removing accounts: %w", err) } if out == nil { return nil, handlers.ApiErrorWithCodeAndMessage(codes.Internal, "Unable to lookup user after removing accounts from it.")