From aef87b5463dc25b78f8efa91c97a5a72ac523249 Mon Sep 17 00:00:00 2001 From: Kenta Kozuka Date: Mon, 4 Sep 2023 12:39:13 +0900 Subject: [PATCH 1/4] Display MySQL user defined error in UI Signed-off-by: Kenta Kozuka --- pkg/app/server/grpcapi/grpcapi.go | 18 +++++++++++------- pkg/datastore/datastore.go | 1 + pkg/datastore/mysql/mysql.go | 10 ++++++++-- .../components/settings-page/api-key/index.tsx | 16 +++++++++++----- web/src/hooks/redux.ts | 2 ++ 5 files changed, 33 insertions(+), 14 deletions(-) diff --git a/pkg/app/server/grpcapi/grpcapi.go b/pkg/app/server/grpcapi/grpcapi.go index a88b81747d..9b65f0ad6e 100644 --- a/pkg/app/server/grpcapi/grpcapi.go +++ b/pkg/app/server/grpcapi/grpcapi.go @@ -166,18 +166,22 @@ func getEncriptionKey(se *model.Piped_SecretEncryption) ([]byte, error) { } func gRPCStoreError(err error, msg string) error { - switch err { - case nil: + if err == nil { return nil - case datastore.ErrNotFound, filestore.ErrNotFound, stagelogstore.ErrNotFound: + } + if errors.Is(err, datastore.ErrNotFound) || errors.Is(err, filestore.ErrNotFound) || errors.Is(err, stagelogstore.ErrNotFound) { return status.Error(codes.NotFound, fmt.Sprintf("Entity was not found to %s", msg)) - case datastore.ErrInvalidArgument: + } + if errors.Is(err, datastore.ErrInvalidArgument) { return status.Error(codes.InvalidArgument, fmt.Sprintf("Invalid argument to %s", msg)) - case datastore.ErrAlreadyExists: + } + if errors.Is(err, datastore.ErrAlreadyExists) { return status.Error(codes.AlreadyExists, fmt.Sprintf("Entity already exists to %s", msg)) - default: - return status.Error(codes.Internal, fmt.Sprintf("Failed to %s", msg)) } + if errors.Is(err, datastore.ErrUserDefined) { + return status.Error(codes.FailedPrecondition, err.Error()) + } + return status.Error(codes.Internal, fmt.Sprintf("Failed to %s", msg)) } func makeUnregisteredAppsCacheKey(projectID string) string { diff --git a/pkg/datastore/datastore.go b/pkg/datastore/datastore.go index 9951b0263a..eddc677701 100644 --- a/pkg/datastore/datastore.go +++ b/pkg/datastore/datastore.go @@ -67,6 +67,7 @@ var ( ErrInternal = errors.New("internal") ErrUnimplemented = errors.New("unimplemented") ErrUnsupported = errors.New("unsupported") + ErrUserDefined = errors.New("user defined error") ) type Commander string diff --git a/pkg/datastore/mysql/mysql.go b/pkg/datastore/mysql/mysql.go index 328cebeaed..4d3506ee97 100644 --- a/pkg/datastore/mysql/mysql.go +++ b/pkg/datastore/mysql/mysql.go @@ -30,6 +30,7 @@ import ( ) const mysqlErrorCodeDuplicateEntry = 1062 +const mysqlErrorCodeUserDefined = 1644 // MySQL client wrapper type MySQL struct { @@ -164,8 +165,13 @@ func (m *MySQL) Create(ctx context.Context, col datastore.Collection, id string, } _, err = stmt.ExecContext(ctx, makeRowID(id), data) - if mysqlErr, ok := err.(*mysql.MySQLError); ok && mysqlErr.Number == mysqlErrorCodeDuplicateEntry { - return datastore.ErrAlreadyExists + if mysqlErr, ok := err.(*mysql.MySQLError); ok { + if mysqlErr.Number == mysqlErrorCodeDuplicateEntry { + return datastore.ErrAlreadyExists + } + if mysqlErr.Number == mysqlErrorCodeUserDefined { + return fmt.Errorf("%w: %s", datastore.ErrUserDefined, mysqlErr.Message) + } } if err != nil { m.logger.Error("failed to create entity", diff --git a/web/src/components/settings-page/api-key/index.tsx b/web/src/components/settings-page/api-key/index.tsx index 741dbf7f55..fa6247a02e 100644 --- a/web/src/components/settings-page/api-key/index.tsx +++ b/web/src/components/settings-page/api-key/index.tsx @@ -26,7 +26,7 @@ import { DISABLE_API_KEY_SUCCESS, GENERATE_API_KEY_SUCCESS, } from "~/constants/toast-text"; -import { useAppDispatch, useAppSelector } from "~/hooks/redux"; +import { unwrapResult, useAppDispatch, useAppSelector } from "~/hooks/redux"; import { APIKey, disableAPIKey, @@ -96,10 +96,16 @@ export const APIKeyPage: FC = memo(function APIKeyPage() { const handleSubmit = useCallback( (values: { name: string; role: APIKey.Role }) => { - dispatch(generateAPIKey(values)).then(() => { - dispatch(fetchAPIKeys({ enabled: true })); - dispatch(addToast({ message: GENERATE_API_KEY_SUCCESS })); - }); + dispatch(generateAPIKey(values)) + .then(unwrapResult) + .then(() => { + console.log("handleSubmit.then"); + dispatch(fetchAPIKeys({ enabled: true })); + dispatch( + addToast({ message: GENERATE_API_KEY_SUCCESS, severity: "success" }) + ); + }) + .catch(() => {}); }, [dispatch] ); diff --git a/web/src/hooks/redux.ts b/web/src/hooks/redux.ts index 1e8a64ad8a..a4e1eb619d 100644 --- a/web/src/hooks/redux.ts +++ b/web/src/hooks/redux.ts @@ -1,4 +1,5 @@ // @see https://redux-toolkit.js.org/tutorials/typescript#define-typed-hooks +import { unwrapResult } from "@reduxjs/toolkit"; import { shallowEqual, TypedUseSelectorHook, @@ -15,3 +16,4 @@ export const useShallowEqualSelector: TypedUseSelectorHook = ( ) => { return useSelector(selector, shallowEqual); }; +export { unwrapResult }; From d34fcfbaaa78df207f3c2222811398de33d78f62 Mon Sep 17 00:00:00 2001 From: Kenta Kozuka Date: Mon, 4 Sep 2023 12:45:34 +0900 Subject: [PATCH 2/4] Fix Unexpected empty arrow function Signed-off-by: Kenta Kozuka --- web/src/components/settings-page/api-key/index.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/src/components/settings-page/api-key/index.tsx b/web/src/components/settings-page/api-key/index.tsx index fa6247a02e..28e007cf27 100644 --- a/web/src/components/settings-page/api-key/index.tsx +++ b/web/src/components/settings-page/api-key/index.tsx @@ -105,7 +105,7 @@ export const APIKeyPage: FC = memo(function APIKeyPage() { addToast({ message: GENERATE_API_KEY_SUCCESS, severity: "success" }) ); }) - .catch(() => {}); + .catch(() => undefined); }, [dispatch] ); From 5984a3df94b11bc34dc2e700929b5b3fce9b3bd2 Mon Sep 17 00:00:00 2001 From: Kenta Kozuka Date: Thu, 7 Sep 2023 10:33:49 +0900 Subject: [PATCH 3/4] Add tests Signed-off-by: Kenta Kozuka --- pkg/app/server/grpcapi/grpcapi_test.go | 90 ++++++++++++++++++++++++++ 1 file changed, 90 insertions(+) create mode 100644 pkg/app/server/grpcapi/grpcapi_test.go diff --git a/pkg/app/server/grpcapi/grpcapi_test.go b/pkg/app/server/grpcapi/grpcapi_test.go new file mode 100644 index 0000000000..7791e506fd --- /dev/null +++ b/pkg/app/server/grpcapi/grpcapi_test.go @@ -0,0 +1,90 @@ +// Copyright 2023 The PipeCD Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package grpcapi + +import ( + "errors" + "fmt" + "testing" + + "github.com/pipe-cd/pipecd/pkg/datastore" + "github.com/pipe-cd/pipecd/pkg/filestore" + "github.com/stretchr/testify/assert" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" +) + +func TestGRPCStoreError(t *testing.T) { + t.Parallel() + tests := []struct { + name string + inputErr error + inputMsg string + expected error + }{ + { + name: "datastore not found error", + inputErr: datastore.ErrNotFound, + inputMsg: "datastore", + expected: status.Error(codes.NotFound, "Entity was not found to datastore"), + }, + { + name: "filestore not found error", + inputErr: filestore.ErrNotFound, + inputMsg: "filestore", + expected: status.Error(codes.NotFound, "Entity was not found to filestore"), + }, + { + name: "stagelogstore not found error", + inputErr: filestore.ErrNotFound, + inputMsg: "stagelogstore", + expected: status.Error(codes.NotFound, "Entity was not found to stagelogstore"), + }, + { + name: "datastore invalid argument error", + inputErr: datastore.ErrInvalidArgument, + inputMsg: "datastore", + expected: status.Error(codes.InvalidArgument, "Invalid argument to datastore"), + }, + { + name: "datastore already exists error", + inputErr: datastore.ErrAlreadyExists, + inputMsg: "datastore", + expected: status.Error(codes.AlreadyExists, "Entity already exists to datastore"), + }, + { + name: "user defined error", + inputErr: datastore.ErrUserDefined, + expected: status.Error(codes.FailedPrecondition, "user defined error"), + }, + { + name: "user defined error with message", + inputErr: fmt.Errorf("%w: %s", datastore.ErrUserDefined, "test"), + expected: status.Error(codes.FailedPrecondition, "user defined error: test"), + }, + { + name: "internal error", + inputErr: errors.New("internal error"), + inputMsg: "test", + expected: status.Error(codes.Internal, "Failed to test"), + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := gRPCStoreError(tt.inputErr, tt.inputMsg) + assert.Equal(t, tt.expected, err) + }) + } +} From 794bc92c3a916cba2248c27855c15fa18094a771 Mon Sep 17 00:00:00 2001 From: Kenta Kozuka Date: Thu, 7 Sep 2023 10:40:19 +0900 Subject: [PATCH 4/4] Run subtests in parallel Signed-off-by: Kenta Kozuka --- pkg/app/server/grpcapi/grpcapi_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/app/server/grpcapi/grpcapi_test.go b/pkg/app/server/grpcapi/grpcapi_test.go index 7791e506fd..58def4f62b 100644 --- a/pkg/app/server/grpcapi/grpcapi_test.go +++ b/pkg/app/server/grpcapi/grpcapi_test.go @@ -82,7 +82,9 @@ func TestGRPCStoreError(t *testing.T) { }, } for _, tt := range tests { + tt := tt t.Run(tt.name, func(t *testing.T) { + t.Parallel() err := gRPCStoreError(tt.inputErr, tt.inputMsg) assert.Equal(t, tt.expected, err) })