Skip to content

Commit 258235a

Browse files
kakcykakcy
authored
refactor: api error handling in Push and Tag packages (#2112)
1 parent 550d25b commit 258235a

File tree

11 files changed

+134
-189
lines changed

11 files changed

+134
-189
lines changed

‎pkg/api/api/grpc_status.go‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,8 @@ func convertErrorReason(errorType pkgErr.ErrorType) string {
104104
return "INVALID_ARGUMENT_NOT_MATCH_FORMAT"
105105
case pkgErr.ErrorTypeInvalidArgUnknown:
106106
return "INVALID_ARGUMENT"
107+
case pkgErr.ErrorTypeInvalidArgDuplicated:
108+
return "INVALID_ARGUMENT_DUPLICATED"
107109
case pkgErr.ErrorTypeNotFound:
108110
return "NOT_FOUND"
109111
case pkgErr.ErrorTypeAlreadyExists:
@@ -112,6 +114,8 @@ func convertErrorReason(errorType pkgErr.ErrorType) string {
112114
return "UNAUTHENTICATED"
113115
case pkgErr.ErrorTypePermissionDenied:
114116
return "PERMISSION_DENIED"
117+
case pkgErr.ErrorTypeFailedPrecondition:
118+
return "FAILED_PRECONDITION"
115119
case pkgErr.ErrorTypeUnexpectedAffectedRows:
116120
return "UNEXPECTED_AFFECTED_ROWS"
117121
case pkgErr.ErrorTypeInternal:
@@ -126,14 +130,17 @@ func convertStatusCode(errorType pkgErr.ErrorType) codes.Code {
126130
case pkgErr.ErrorTypeInvalidArgUnknown,
127131
pkgErr.ErrorTypeInvalidArgEmpty,
128132
pkgErr.ErrorTypeInvalidArgNil,
129-
pkgErr.ErrorTypeInvalidArgNotMatchFormat:
133+
pkgErr.ErrorTypeInvalidArgNotMatchFormat,
134+
pkgErr.ErrorTypeInvalidArgDuplicated:
130135
return codes.InvalidArgument
131136
case pkgErr.ErrorTypeNotFound:
132137
return codes.NotFound
133138
case pkgErr.ErrorTypeAlreadyExists:
134139
return codes.AlreadyExists
135140
case pkgErr.ErrorTypeUnauthenticated:
136141
return codes.Unauthenticated
142+
case pkgErr.ErrorTypeFailedPrecondition:
143+
return codes.FailedPrecondition
137144
case pkgErr.ErrorTypePermissionDenied:
138145
return codes.PermissionDenied
139146
case pkgErr.ErrorTypeUnexpectedAffectedRows:

‎pkg/error/error.go‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,14 @@ import (
2323
const (
2424
AccountPackageName = "account"
2525
NotificationPackageName = "notification"
26+
PushPackageName = "push"
27+
TagPackageName = "tag"
2628

2729
invalidTypeUnknown = "unknown"
2830
invalidTypeEmpty = "empty"
2931
invalidTypeNil = "nil"
3032
invalidTypeNotMatchFormat = "not_match_format"
33+
invalidTypeDuplicated = "duplicated"
3134

3235
invalidPrefix = "invalid"
3336
)
@@ -41,10 +44,12 @@ const (
4144
ErrorTypePermissionDenied ErrorType = "permission_denied"
4245
ErrorTypeUnexpectedAffectedRows ErrorType = "unexpected_affected_rows"
4346
ErrorTypeInternal ErrorType = "internal"
47+
ErrorTypeFailedPrecondition ErrorType = "failed_precondition"
4448
ErrorTypeInvalidArgUnknown ErrorType = invalidPrefix + "_" + invalidTypeUnknown
4549
ErrorTypeInvalidArgEmpty ErrorType = invalidPrefix + "_" + invalidTypeEmpty
4650
ErrorTypeInvalidArgNil ErrorType = invalidPrefix + "_" + invalidTypeNil
4751
ErrorTypeInvalidArgNotMatchFormat ErrorType = invalidPrefix + "_" + invalidTypeNotMatchFormat
52+
ErrorTypeInvalidArgDuplicated ErrorType = invalidPrefix + "_" + invalidTypeDuplicated
4853
)
4954

5055
// Base error - no field needed
@@ -153,3 +158,11 @@ func NewErrorInvalidArgNil(pkg string, message string, field string) *BktFieldEr
153158
func NewErrorInvalidArgNotMatchFormat(pkg string, message string, field string) *BktFieldError {
154159
return newBktFieldError(pkg, ErrorTypeInvalidArgNotMatchFormat, message, field)
155160
}
161+
162+
func NewErrorInvalidArgDuplicated(pkg string, message string, field string) *BktFieldError {
163+
return newBktFieldError(pkg, ErrorTypeInvalidArgDuplicated, message, field)
164+
}
165+
166+
func NewErrorFailedPrecondition(pkg string, message string) *BktError {
167+
return newBktError(pkg, ErrorTypeFailedPrecondition, message)
168+
}

‎pkg/push/api/api.go‎

Lines changed: 16 additions & 120 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import (
3030
"google.golang.org/protobuf/types/known/wrapperspb"
3131

3232
accountclient "github.com/bucketeer-io/bucketeer/pkg/account/client"
33+
"github.com/bucketeer-io/bucketeer/pkg/api/api"
3334
domainevent "github.com/bucketeer-io/bucketeer/pkg/domainevent/domain"
3435
experimentclient "github.com/bucketeer-io/bucketeer/pkg/experiment/client"
3536
featureclient "github.com/bucketeer-io/bucketeer/pkg/feature/client"
@@ -134,25 +135,11 @@ func (s *PushService) CreatePush(
134135
zap.Strings("tags", req.Command.Tags),
135136
)...,
136137
)
137-
dt, err := statusInternal.WithDetails(&errdetails.LocalizedMessage{
138-
Locale: localizer.GetLocale(),
139-
Message: localizer.MustLocalize(locale.InternalServerError),
140-
})
141-
if err != nil {
142-
return nil, statusInternal.Err()
143-
}
144-
return nil, dt.Err()
138+
return nil, api.NewGRPCStatus(err).Err()
145139
}
146140
pushes, err := s.listAllPushes(ctx, req.EnvironmentId, localizer)
147141
if err != nil {
148-
dt, err := statusInternal.WithDetails(&errdetails.LocalizedMessage{
149-
Locale: localizer.GetLocale(),
150-
Message: localizer.MustLocalize(locale.InternalServerError),
151-
})
152-
if err != nil {
153-
return nil, statusInternal.Err()
154-
}
155-
return nil, dt.Err()
142+
return nil, api.NewGRPCStatus(err).Err()
156143
}
157144
if err := s.checkFCMServiceAccount(ctx, pushes, req.Command.FcmServiceAccount, localizer); err != nil {
158145
return nil, err
@@ -177,14 +164,7 @@ func (s *PushService) CreatePush(
177164
zap.Strings("tags", req.Command.Tags),
178165
)...,
179166
)
180-
dt, err := statusInternal.WithDetails(&errdetails.LocalizedMessage{
181-
Locale: localizer.GetLocale(),
182-
Message: localizer.MustLocalize(locale.InternalServerError),
183-
})
184-
if err != nil {
185-
return nil, statusInternal.Err()
186-
}
187-
return nil, dt.Err()
167+
return nil, api.NewGRPCStatus(err).Err()
188168
}
189169
err = s.mysqlClient.RunInTransactionV2(ctx, func(contextWithTx context.Context, _ mysql.Transaction) error {
190170
if err := s.pushStorage.CreatePush(contextWithTx, push, req.EnvironmentId); err != nil {
@@ -217,14 +197,7 @@ func (s *PushService) CreatePush(
217197
zap.String("environmentId", req.EnvironmentId),
218198
)...,
219199
)
220-
dt, err := statusInternal.WithDetails(&errdetails.LocalizedMessage{
221-
Locale: localizer.GetLocale(),
222-
Message: localizer.MustLocalize(locale.InternalServerError),
223-
})
224-
if err != nil {
225-
return nil, statusInternal.Err()
226-
}
227-
return nil, dt.Err()
200+
return nil, api.NewGRPCStatus(err).Err()
228201
}
229202

230203
// For security reasons we remove the service account from the API response
@@ -260,25 +233,11 @@ func (s *PushService) createPushNoCommand(
260233
zap.Strings("tags", req.Tags),
261234
)...,
262235
)
263-
dt, err := statusInternal.WithDetails(&errdetails.LocalizedMessage{
264-
Locale: localizer.GetLocale(),
265-
Message: localizer.MustLocalize(locale.InternalServerError),
266-
})
267-
if err != nil {
268-
return nil, statusInternal.Err()
269-
}
270-
return nil, dt.Err()
236+
return nil, api.NewGRPCStatus(err).Err()
271237
}
272238
pushes, err := s.listAllPushes(ctx, req.EnvironmentId, localizer)
273239
if err != nil {
274-
dt, err := statusInternal.WithDetails(&errdetails.LocalizedMessage{
275-
Locale: localizer.GetLocale(),
276-
Message: localizer.MustLocalize(locale.InternalServerError),
277-
})
278-
if err != nil {
279-
return nil, statusInternal.Err()
280-
}
281-
return nil, dt.Err()
240+
return nil, api.NewGRPCStatus(err).Err()
282241
}
283242
if err := s.checkFCMServiceAccount(ctx, pushes, req.FcmServiceAccount, localizer); err != nil {
284243
return nil, err
@@ -304,14 +263,7 @@ func (s *PushService) createPushNoCommand(
304263
zap.Strings("tags", req.Tags),
305264
)...,
306265
)
307-
dt, err := statusInternal.WithDetails(&errdetails.LocalizedMessage{
308-
Locale: localizer.GetLocale(),
309-
Message: localizer.MustLocalize(locale.InternalServerError),
310-
})
311-
if err != nil {
312-
return nil, statusInternal.Err()
313-
}
314-
return nil, dt.Err()
266+
return nil, api.NewGRPCStatus(err).Err()
315267
}
316268

317269
var event *eventproto.Event
@@ -361,14 +313,7 @@ func (s *PushService) createPushNoCommand(
361313
zap.String("name", req.Name),
362314
)...,
363315
)
364-
dt, err := statusInternal.WithDetails(&errdetails.LocalizedMessage{
365-
Locale: localizer.GetLocale(),
366-
Message: localizer.MustLocalize(locale.InternalServerError),
367-
})
368-
if err != nil {
369-
return nil, statusInternal.Err()
370-
}
371-
return nil, dt.Err()
316+
return nil, api.NewGRPCStatus(err).Err()
372317
}
373318

374319
// For security reasons we remove the service account from the API response
@@ -494,14 +439,7 @@ func (s *PushService) UpdatePush(
494439
zap.String("id", req.Id),
495440
)...,
496441
)
497-
dt, err := statusInternal.WithDetails(&errdetails.LocalizedMessage{
498-
Locale: localizer.GetLocale(),
499-
Message: localizer.MustLocalize(locale.InternalServerError),
500-
})
501-
if err != nil {
502-
return nil, statusInternal.Err()
503-
}
504-
return nil, dt.Err()
442+
return nil, api.NewGRPCStatus(err).Err()
505443
}
506444

507445
if updatedPushPb != nil {
@@ -585,14 +523,7 @@ func (s *PushService) updatePushNoCommand(
585523
zap.String("id", req.Id),
586524
)...,
587525
)
588-
dt, err := statusInternal.WithDetails(&errdetails.LocalizedMessage{
589-
Locale: localizer.GetLocale(),
590-
Message: localizer.MustLocalize(locale.InternalServerError),
591-
})
592-
if err != nil {
593-
return nil, statusInternal.Err()
594-
}
595-
return nil, dt.Err()
526+
return nil, api.NewGRPCStatus(err).Err()
596527
}
597528

598529
if updatedPushPb != nil {
@@ -685,14 +616,7 @@ func (s *PushService) validateAddPushTagsCommand(
685616
}
686617
pushes, err := s.listAllPushes(ctx, req.EnvironmentId, localizer)
687618
if err != nil {
688-
dt, err := statusInternal.WithDetails(&errdetails.LocalizedMessage{
689-
Locale: localizer.GetLocale(),
690-
Message: localizer.MustLocalize(locale.InternalServerError),
691-
})
692-
if err != nil {
693-
return statusInternal.Err()
694-
}
695-
return dt.Err()
619+
return api.NewGRPCStatus(err).Err()
696620
}
697621
err = s.containsTags(pushes, req.AddPushTagsCommand.Tags, localizer)
698622
if err != nil {
@@ -715,14 +639,7 @@ func (s *PushService) validateAddPushTagsCommand(
715639
zap.Strings("tags", req.AddPushTagsCommand.Tags),
716640
)...,
717641
)
718-
dt, err := statusInternal.WithDetails(&errdetails.LocalizedMessage{
719-
Locale: localizer.GetLocale(),
720-
Message: localizer.MustLocalize(locale.InternalServerError),
721-
})
722-
if err != nil {
723-
return statusInternal.Err()
724-
}
725-
return dt.Err()
642+
return api.NewGRPCStatus(err).Err()
726643
}
727644
return nil
728645
}
@@ -798,14 +715,7 @@ func (s *PushService) DeletePush(
798715
zap.String("environmentId", req.EnvironmentId),
799716
)...,
800717
)
801-
dt, err := statusInternal.WithDetails(&errdetails.LocalizedMessage{
802-
Locale: localizer.GetLocale(),
803-
Message: localizer.MustLocalize(locale.InternalServerError),
804-
})
805-
if err != nil {
806-
return nil, statusInternal.Err()
807-
}
808-
return nil, dt.Err()
718+
return nil, api.NewGRPCStatus(err).Err()
809719
}
810720
return &pushproto.DeletePushResponse{}, nil
811721
}
@@ -1319,14 +1229,7 @@ func (s *PushService) checkEnvironmentRole(
13191229
zap.String("environmentId", environmentId),
13201230
)...,
13211231
)
1322-
dt, err := statusInternal.WithDetails(&errdetails.LocalizedMessage{
1323-
Locale: localizer.GetLocale(),
1324-
Message: localizer.MustLocalize(locale.InternalServerError),
1325-
})
1326-
if err != nil {
1327-
return nil, statusInternal.Err()
1328-
}
1329-
return nil, dt.Err()
1232+
return nil, api.NewGRPCStatus(err).Err()
13301233
}
13311234
}
13321235
return editor, nil
@@ -1390,14 +1293,7 @@ func (s *PushService) checkOrganizationRole(
13901293
zap.String("organizationID", organizationID),
13911294
)...,
13921295
)
1393-
dt, err := statusInternal.WithDetails(&errdetails.LocalizedMessage{
1394-
Locale: localizer.GetLocale(),
1395-
Message: localizer.MustLocalize(locale.InternalServerError),
1396-
})
1397-
if err != nil {
1398-
return nil, statusInternal.Err()
1399-
}
1400-
return nil, dt.Err()
1296+
return nil, api.NewGRPCStatus(err).Err()
14011297
}
14021298
}
14031299
return editor, nil

‎pkg/push/api/error.go‎

Lines changed: 40 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -15,26 +15,47 @@
1515
package api
1616

1717
import (
18-
"google.golang.org/grpc/codes"
19-
gstatus "google.golang.org/grpc/status"
18+
"github.com/bucketeer-io/bucketeer/pkg/api/api"
19+
err "github.com/bucketeer-io/bucketeer/pkg/error"
2020
)
2121

2222
var (
23-
statusInternal = gstatus.New(codes.Internal, "push: internal")
24-
statusIDRequired = gstatus.New(codes.InvalidArgument, "push: id must be specified")
25-
statusNameRequired = gstatus.New(codes.InvalidArgument, "push: name must be specified")
26-
statusFCMServiceAccountRequired = gstatus.New(
27-
codes.InvalidArgument,
28-
"push: fcm service account must be specified",
29-
)
30-
statusFCMServiceAccountInvalid = gstatus.New(codes.InvalidArgument, "push: fcm service account is invalid")
31-
statusTagsRequired = gstatus.New(codes.InvalidArgument, "push: tags must be specified")
32-
statusInvalidCursor = gstatus.New(codes.InvalidArgument, "push: cursor is invalid")
33-
statusInvalidOrderBy = gstatus.New(codes.InvalidArgument, "push: order_by is invalid")
34-
statusNotFound = gstatus.New(codes.NotFound, "push: not found")
35-
statusAlreadyExists = gstatus.New(codes.AlreadyExists, "push: already exists")
36-
statusFCMServiceAccountAlreadyExists = gstatus.New(codes.AlreadyExists, "push: fcm service account already exists")
37-
statusTagAlreadyExists = gstatus.New(codes.AlreadyExists, "push: tag already exists")
38-
statusUnauthenticated = gstatus.New(codes.Unauthenticated, "push: unauthenticated")
39-
statusPermissionDenied = gstatus.New(codes.PermissionDenied, "push: permission denied")
23+
statusInternal = api.NewGRPCStatus(err.NewErrorInternal(err.PushPackageName, "internal"))
24+
statusIDRequired = api.NewGRPCStatus(
25+
err.NewErrorInvalidArgEmpty(err.PushPackageName, "id must be specified", "id"),
26+
)
27+
statusNameRequired = api.NewGRPCStatus(
28+
err.NewErrorInvalidArgEmpty(err.PushPackageName, "name must be specified", "name"),
29+
)
30+
statusFCMServiceAccountRequired = api.NewGRPCStatus(
31+
err.NewErrorInvalidArgEmpty(err.PushPackageName, "fcm service account must be specified", "fcm_service_account"),
32+
)
33+
statusFCMServiceAccountInvalid = api.NewGRPCStatus(
34+
err.NewErrorInvalidArgNotMatchFormat(err.PushPackageName, "fcm service account is invalid", "fcm_service_account"),
35+
)
36+
statusTagsRequired = api.NewGRPCStatus(
37+
err.NewErrorInvalidArgEmpty(err.PushPackageName, "tags must be specified", "tags"),
38+
)
39+
statusInvalidCursor = api.NewGRPCStatus(
40+
err.NewErrorInvalidArgNotMatchFormat(err.PushPackageName, "cursor is invalid", "cursor"),
41+
)
42+
statusInvalidOrderBy = api.NewGRPCStatus(
43+
err.NewErrorInvalidArgUnknown(err.PushPackageName, "order_by is invalid", "order_by"),
44+
)
45+
statusNotFound = api.NewGRPCStatus(
46+
err.NewErrorNotFound(err.PushPackageName, "not found", "push"),
47+
)
48+
statusAlreadyExists = api.NewGRPCStatus(
49+
err.NewErrorAlreadyExists(err.PushPackageName, "already exists"),
50+
)
51+
statusFCMServiceAccountAlreadyExists = api.NewGRPCStatus(
52+
err.NewErrorAlreadyExists(err.PushPackageName, "fcm service account already exists"),
53+
)
54+
statusTagAlreadyExists = api.NewGRPCStatus(
55+
err.NewErrorAlreadyExists(err.PushPackageName, "tag already exists"),
56+
)
57+
statusUnauthenticated = api.NewGRPCStatus(
58+
err.NewErrorUnauthenticated(err.PushPackageName, "unauthenticated"),
59+
)
60+
statusPermissionDenied = api.NewGRPCStatus(err.NewErrorPermissionDenied(err.PushPackageName, "permission denied"))
4061
)

‎pkg/push/command/command.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,12 @@ package command
1616

1717
import (
1818
"context"
19-
"errors"
19+
20+
pkgErr "github.com/bucketeer-io/bucketeer/pkg/error"
2021
)
2122

2223
var (
23-
errUnknownCommand = errors.New("command: unknown command")
24+
errUnknownCommand = pkgErr.NewErrorInvalidArgUnknown(pkgErr.PushPackageName, "unknown command", "command")
2425
)
2526

2627
type Command interface{}

0 commit comments

Comments
 (0)