Skip to content

Commit e4ae3ff

Browse files
authoredAug 2, 2019
teams-service: cut down "create team" error strings (#1134)
* teams-service: cut down "create team" error strings There' some decent guidance here: https://github.com/golang/go/wiki/Errors Error strings should - start with lowercase - be specific: if create fails, don't say that create failed - not end with a period ...because they might be wrapped, as it happens in this case: When this error bubbles up in the UI, it'll say > Could not create team: unable to create team: a team with name "..." already exists.. * teams-service: nitpick "purge team subject" error I've never seen this happen, so we shouldn't sweat it; but status.Error(codes.Xyz, fmt.Sprintf(..., ...)) should really be status.Errorf(codes.Xyz, ..., ...) Signed-off-by: Stephan Renatus <srenatus@chef.io>
1 parent 93282b0 commit e4ae3ff

File tree

2 files changed

+5
-17
lines changed

2 files changed

+5
-17
lines changed
 

‎components/teams-service/server/v1/server.go

+2-8
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ package v1
22

33
import (
44
"context"
5-
"fmt"
65

76
"google.golang.org/grpc/codes"
87
"google.golang.org/grpc/status"
@@ -60,10 +59,7 @@ func (s *Server) CreateTeam(ctx context.Context, req *teams.CreateTeamReq) (*tea
6059
var err error
6160
if team, err = s.service.Storage.StoreTeam(ctx, req.Name, req.Description); err != nil {
6261
if err == storage.ErrConflict {
63-
return nil, status.Errorf(
64-
codes.AlreadyExists,
65-
"unable to create team: a team with name %q already exists.",
66-
req.Name)
62+
return nil, status.Errorf(codes.AlreadyExists, "team with name %q already exists", req.Name)
6763
}
6864
return nil, status.Error(codes.Internal, err.Error())
6965
}
@@ -129,9 +125,7 @@ func (s *Server) DeleteTeam(ctx context.Context, req *teams.DeleteTeamReq) (*tea
129125
})
130126
if err != nil {
131127
s.service.Logger.Warnf("failed to purge subjects on team delete: %s", err.Error())
132-
return nil, status.Error(codes.Internal,
133-
fmt.Sprintf("the team named %s with id %s was successfully deleted but its "+
134-
"subject could not be purged from the policies: %s", team.Name, team.ID, err.Error()))
128+
return nil, status.Errorf(codes.Internal, "failed to purge team %q from policies: %s", team.ID, err.Error())
135129
}
136130

137131
return &teams.DeleteTeamResp{

‎components/teams-service/server/v2/server.go

+3-9
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ package v2
22

33
import (
44
"context"
5-
"fmt"
65

76
"google.golang.org/grpc/codes"
87
"google.golang.org/grpc/status"
@@ -64,10 +63,7 @@ func (s *Server) CreateTeam(ctx context.Context,
6463
var err error
6564
if team, err = s.service.Storage.StoreTeamWithProjects(ctx, req.Id, req.Name, req.Projects); err != nil {
6665
if err == storage.ErrConflict {
67-
return nil, status.Errorf(
68-
codes.AlreadyExists,
69-
"unable to create team: a team with id %q already exists.",
70-
req.Id)
66+
return nil, status.Errorf(codes.AlreadyExists, "team with ID %q already exists", req.Id)
7167
}
7268
return nil, status.Error(codes.Internal, err.Error())
7369
}
@@ -95,9 +91,7 @@ func (s *Server) DeleteTeam(ctx context.Context, req *teams.DeleteTeamReq) (*tea
9591
})
9692
if err != nil {
9793
s.service.Logger.Warnf("failed to purge subjects on team delete: %s", err.Error())
98-
return nil, status.Error(codes.Internal,
99-
fmt.Sprintf("the team named %q with id %q was successfully deleted but its "+
100-
"subject could not be purged from the policies: %s", team.Name, req.Id, err.Error()))
94+
return nil, status.Errorf(codes.Internal, "failed to purge team %q from policies: %s", req.Id, err.Error())
10195
}
10296

10397
return &teams.DeleteTeamResp{
@@ -127,7 +121,7 @@ func (s *Server) AddTeamMembers(ctx context.Context,
127121
req *teams.AddTeamMembersReq) (*teams.AddTeamMembersResp, error) {
128122

129123
if len(req.UserIds) == 0 {
130-
return nil, status.Error(codes.InvalidArgument, "missing user ids")
124+
return nil, status.Error(codes.InvalidArgument, "missing user IDs")
131125
}
132126
// TODO (tc): The storage interface is still using V1 verbiage, so
133127
// name is really the ID in V2 terms. We'll refactor at GA when V1 is removed.

0 commit comments

Comments
 (0)
Please sign in to comment.