Skip to content

refactor: change error definition in account PKG to Bucketeer error #2125

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
kakcy merged 5 commits into from
Sep 30, 2025
Merged

refactor: change error definition in account PKG to Bucketeer error #2125

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2baf31b
refactor: change error definition in account PKG to Bucketeer error
kakcy Sep 10, 2025
dbab187
Merge branch 'main' into refactor-error-handle-account-3
kakcy Sep 16, 2025
483e20d
Merge branch 'main' into refactor-error-handle-account-3
kakcy Sep 17, 2025
8a9fa45
Merge branch 'main' into refactor-error-handle-account-3
kakcy Sep 19, 2025
df90b6d
Merge branch 'main' into refactor-error-handle-account-3
kakcy Sep 26, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 7 additions & 56 deletions pkg/account/api/admin_account.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (

"github.com/bucketeer-io/bucketeer/v2/pkg/account/domain"
v2as "github.com/bucketeer-io/bucketeer/v2/pkg/account/storage/v2"
"github.com/bucketeer-io/bucketeer/v2/pkg/api/api"
"github.com/bucketeer-io/bucketeer/v2/pkg/locale"
"github.com/bucketeer-io/bucketeer/v2/pkg/log"
"github.com/bucketeer-io/bucketeer/v2/pkg/rpc"
Expand Down Expand Up @@ -70,29 +71,15 @@ func (s *AccountService) GetMe(
"Failed to get project list",
log.FieldsFromIncomingContext(ctx).AddFields(zap.Error(err))...,
)
dt, err := statusInternal.WithDetails(&errdetails.LocalizedMessage{
Locale: localizer.GetLocale(),
Message: localizer.MustLocalize(locale.InternalServerError),
})
if err != nil {
return nil, statusInternal.Err()
}
return nil, dt.Err()
return nil, api.NewGRPCStatus(err).Err()
}
environments, err := s.listEnvironmentsByOrganizationID(ctx, req.OrganizationId)
if err != nil {
s.logger.Error(
"Failed to get environment list",
log.FieldsFromIncomingContext(ctx).AddFields(zap.Error(err))...,
)
dt, err := statusInternal.WithDetails(&errdetails.LocalizedMessage{
Locale: localizer.GetLocale(),
Message: localizer.MustLocalize(locale.InternalServerError),
})
if err != nil {
return nil, statusInternal.Err()
}
return nil, dt.Err()

}
organization, err := s.getOrganization(ctx, req.OrganizationId)
if err != nil {
Expand All @@ -117,14 +104,7 @@ func (s *AccountService) GetMe(
"Failed to get organization",
log.FieldsFromIncomingContext(ctx).AddFields(zap.Error(err))...,
)
dt, err := statusInternal.WithDetails(&errdetails.LocalizedMessage{
Locale: localizer.GetLocale(),
Message: localizer.MustLocalize(locale.InternalServerError),
})
if err != nil {
return nil, statusInternal.Err()
}
return nil, dt.Err()
return nil, api.NewGRPCStatus(err).Err()
}
// system admin account response
sysAdminAccount, err := s.getSystemAdminAccountV2(ctx, t.Email, localizer)
Expand Down Expand Up @@ -231,14 +211,7 @@ func (s *AccountService) getAccount(
}
return nil, dt.Err()
}
dt, err := statusInternal.WithDetails(&errdetails.LocalizedMessage{
Locale: localizer.GetLocale(),
Message: localizer.MustLocalize(locale.InternalServerError),
})
if err != nil {
return nil, statusInternal.Err()
}
return nil, dt.Err()
return nil, api.NewGRPCStatus(err).Err()
}
if account.Disabled {
s.logger.Error("Account is disabled",
Expand Down Expand Up @@ -381,14 +354,7 @@ func (s *AccountService) getMyOrganizations(
"Failed to get accounts with organization",
log.FieldsFromIncomingContext(ctx).AddFields(zap.Error(err))...,
)
dt, err := statusInternal.WithDetails(&errdetails.LocalizedMessage{
Locale: localizer.GetLocale(),
Message: localizer.MustLocalize(locale.InternalServerError),
})
if err != nil {
return nil, statusInternal.Err()
}
return nil, dt.Err()
return nil, api.NewGRPCStatus(err).Err()
}
if s.containsSystemAdminOrganization(accountsWithOrg) {
resp, err := s.environmentClient.ListOrganizations(
Expand All @@ -402,14 +368,7 @@ func (s *AccountService) getMyOrganizations(
"Failed to get organizations",
log.FieldsFromIncomingContext(ctx).AddFields(zap.Error(err))...,
)
dt, err := statusInternal.WithDetails(&errdetails.LocalizedMessage{
Locale: localizer.GetLocale(),
Message: localizer.MustLocalize(locale.InternalServerError),
})
if err != nil {
return nil, statusInternal.Err()
}
return nil, dt.Err()
return nil, api.NewGRPCStatus(err).Err()
}
return resp.Organizations, nil
}
Expand Down Expand Up @@ -480,14 +439,6 @@ func (s *AccountService) getSystemAdminAccountV2(
zap.String("email", email),
)...,
)
dt, err := statusInternal.WithDetails(&errdetails.LocalizedMessage{
Locale: localizer.GetLocale(),
Message: localizer.MustLocalize(locale.InternalServerError),
})
if err != nil {
return nil, statusInternal.Err()
}
return nil, dt.Err()
}
return account, nil
}
Expand Down
44 changes: 14 additions & 30 deletions pkg/account/api/admin_account_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ package api

import (
"context"
"errors"
"testing"
"time"

Expand All @@ -28,6 +27,8 @@ import (
gstatus "google.golang.org/grpc/status"

"github.com/bucketeer-io/bucketeer/v2/pkg/account/domain"
"github.com/bucketeer-io/bucketeer/v2/pkg/api/api"
pkgErr "github.com/bucketeer-io/bucketeer/v2/pkg/error"

v2as "github.com/bucketeer-io/bucketeer/v2/pkg/account/storage/v2"
accstoragemock "github.com/bucketeer-io/bucketeer/v2/pkg/account/storage/v2/mock"
Expand Down Expand Up @@ -91,12 +92,12 @@ func TestGetMeMySQL(t *testing.T) {
gomock.Any(),
).Return(
nil,
createError(statusInternal, localizer.MustLocalize(locale.InternalServerError)),
pkgErr.NewErrorInternal(pkgErr.AccountPackageName, "internal"),
)
},
input: &accountproto.GetMeRequest{},
expected: nil,
expectedErr: createError(statusInternal, localizer.MustLocalize(locale.InternalServerError)),
expectedErr: api.NewGRPCStatus(pkgErr.NewErrorInternal(pkgErr.AccountPackageName, "internal")).Err(),
},
{
desc: "err: account is disabled",
Expand Down Expand Up @@ -352,15 +353,6 @@ func TestGetMyOrganizationsMySQL(t *testing.T) {
ctx = metadata.NewIncomingContext(ctx, metadata.MD{
"accept-language": []string{"ja"},
})
localizer := locale.NewLocalizer(ctx)
createError := func(status *gstatus.Status, msg string) error {
st, err := status.WithDetails(&errdetails.LocalizedMessage{
Locale: localizer.GetLocale(),
Message: msg,
})
require.NoError(t, err)
return st.Err()
}

patterns := []struct {
desc string
Expand All @@ -374,11 +366,11 @@ func TestGetMyOrganizationsMySQL(t *testing.T) {
setup: func(s *AccountService) {
s.accountStorage.(*accstoragemock.MockAccountStorage).EXPECT().GetAccountsWithOrganization(
gomock.Any(), gomock.Any(),
).Return(nil, errors.New("test"))
).Return(nil, pkgErr.NewErrorInternal(pkgErr.AccountPackageName, "internal"))
},
input: &accountproto.GetMyOrganizationsRequest{},
expected: nil,
expectedErr: createError(statusInternal, localizer.MustLocalize(locale.InternalServerError)),
expectedErr: api.NewGRPCStatus(pkgErr.NewErrorInternal(pkgErr.AccountPackageName, "internal")).Err(),
},
{
desc: "errInternal: GetOrganizations from environment service",
Expand All @@ -398,11 +390,11 @@ func TestGetMyOrganizationsMySQL(t *testing.T) {
}, nil)
s.environmentClient.(*ecmock.MockClient).EXPECT().ListOrganizations(
gomock.Any(), gomock.Any(),
).Return(nil, errors.New("test"))
).Return(nil, pkgErr.NewErrorInternal(pkgErr.AccountPackageName, "internal"))
},
input: &accountproto.GetMyOrganizationsRequest{},
expected: nil,
expectedErr: createError(statusInternal, localizer.MustLocalize(locale.InternalServerError)),
expectedErr: api.NewGRPCStatus(pkgErr.NewErrorInternal(pkgErr.AccountPackageName, "internal")).Err(),
},
{
desc: "success",
Expand Down Expand Up @@ -502,11 +494,11 @@ func TestGetMyOrganizationsByEmailMySQL(t *testing.T) {
setup: func(s *AccountService) {
s.accountStorage.(*accstoragemock.MockAccountStorage).EXPECT().GetAccountsWithOrganization(
gomock.Any(), gomock.Any(),
).Return(nil, errors.New("test"))
).Return(nil, pkgErr.NewErrorInternal(pkgErr.AccountPackageName, "internal"))
},
input: &accountproto.GetMyOrganizationsByEmailRequest{Email: "bucketeer@example.com"},
expected: nil,
expectedErr: createError(statusInternal, localizer.MustLocalize(locale.InternalServerError)),
expectedErr: api.NewGRPCStatus(pkgErr.NewErrorInternal(pkgErr.AccountPackageName, "internal")).Err(),
},
{
desc: "errInternal: GetOrganizations from environment service",
Expand All @@ -526,11 +518,11 @@ func TestGetMyOrganizationsByEmailMySQL(t *testing.T) {
}, nil)
s.environmentClient.(*ecmock.MockClient).EXPECT().ListOrganizations(
gomock.Any(), gomock.Any(),
).Return(nil, errors.New("test"))
).Return(nil, pkgErr.NewErrorInternal(pkgErr.AccountPackageName, "internal"))
},
input: &accountproto.GetMyOrganizationsByEmailRequest{Email: "bucketeer@example.com"},
expected: nil,
expectedErr: createError(statusInternal, localizer.MustLocalize(locale.InternalServerError)),
expectedErr: api.NewGRPCStatus(pkgErr.NewErrorInternal(pkgErr.AccountPackageName, "internal")).Err(),
},
{
desc: "success",
Expand Down Expand Up @@ -603,14 +595,6 @@ func TestGetMyOrganizationsAdminRole(t *testing.T) {
"accept-language": []string{"ja"},
})
localizer := locale.NewLocalizer(ctx)
createError := func(status *gstatus.Status, msg string) error {
st, err := status.WithDetails(&errdetails.LocalizedMessage{
Locale: localizer.GetLocale(),
Message: msg,
})
require.NoError(t, err)
return st.Err()
}

patterns := []struct {
desc string
Expand Down Expand Up @@ -857,11 +841,11 @@ func TestGetMyOrganizationsAdminRole(t *testing.T) {
setup: func(s *AccountService) {
s.accountStorage.(*accstoragemock.MockAccountStorage).EXPECT().GetAccountsWithOrganization(
gomock.Any(), "user@example.com",
).Return(nil, errors.New("database error"))
).Return(nil, pkgErr.NewErrorInternal(pkgErr.AccountPackageName, "internal"))
},
email: "user@example.com",
expected: nil,
expectedErr: createError(statusInternal, localizer.MustLocalize(locale.InternalServerError)),
expectedErr: api.NewGRPCStatus(pkgErr.NewErrorInternal(pkgErr.AccountPackageName, "internal")).Err(),
},
{
desc: "success: system admin gets all organizations",
Expand Down
37 changes: 5 additions & 32 deletions pkg/account/api/api.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"google.golang.org/protobuf/types/known/wrapperspb"

v2 "github.com/bucketeer-io/bucketeer/v2/pkg/account/storage/v2"
"github.com/bucketeer-io/bucketeer/v2/pkg/api/api"
auditlogstorage "github.com/bucketeer-io/bucketeer/v2/pkg/auditlog/storage/v2"
environmentclient "github.com/bucketeer-io/bucketeer/v2/pkg/environment/client"
"github.com/bucketeer-io/bucketeer/v2/pkg/locale"
Expand Down Expand Up @@ -216,14 +217,7 @@ func (s *AccountService) checkSystemAdminRole(
"Failed to check role",
log.FieldsFromIncomingContext(ctx).AddFields(zap.Error(err))...,
)
dt, err := statusInternal.WithDetails(&errdetails.LocalizedMessage{
Locale: localizer.GetLocale(),
Message: localizer.MustLocalize(locale.InternalServerError),
})
if err != nil {
return nil, statusInternal.Err()
}
return nil, dt.Err()
return nil, api.NewGRPCStatus(err).Err()
}
}
return editor, nil
Expand Down Expand Up @@ -289,14 +283,7 @@ func (s *AccountService) checkEnvironmentRole(
zap.String("environmentId", environmentId),
)...,
)
dt, err := statusInternal.WithDetails(&errdetails.LocalizedMessage{
Locale: localizer.GetLocale(),
Message: localizer.MustLocalize(locale.InternalServerError),
})
if err != nil {
return nil, statusInternal.Err()
}
return nil, dt.Err()
return nil, api.NewGRPCStatus(err).Err()
}
}
return editor, nil
Expand Down Expand Up @@ -357,14 +344,7 @@ func (s *AccountService) checkOrganizationRole(
zap.String("organizationID", organizationID),
)...,
)
dt, err := statusInternal.WithDetails(&errdetails.LocalizedMessage{
Locale: localizer.GetLocale(),
Message: localizer.MustLocalize(locale.InternalServerError),
})
if err != nil {
return nil, statusInternal.Err()
}
return nil, dt.Err()
return nil, api.NewGRPCStatus(err).Err()
}
}
return editor, nil
Expand Down Expand Up @@ -425,14 +405,7 @@ func (s *AccountService) checkOrganizationRoleByEnvironmentID(
zap.String("environmentID", environmentID),
)...,
)
dt, err := statusInternal.WithDetails(&errdetails.LocalizedMessage{
Locale: localizer.GetLocale(),
Message: localizer.MustLocalize(locale.InternalServerError),
})
if err != nil {
return nil, statusInternal.Err()
}
return nil, dt.Err()
return nil, api.NewGRPCStatus(err).Err()
}
}
return editor, nil
Expand Down
Loading