From 411d1c9258b29f3d72f416506743850c948cdbff Mon Sep 17 00:00:00 2001 From: Yourim Cha Date: Fri, 18 Oct 2024 14:02:20 +0900 Subject: [PATCH] Introduce RichError struct to extend error responses with metadata for Auth webhook --- api/converter/errors.go | 19 ++++++++++++ internal/richerror/richerror.go | 28 +++++++++++++++++ server/rpc/auth/webhook.go | 7 ++++- server/rpc/connecthelper/status.go | 43 +++++++++++++++++++++++++++ test/integration/auth_webhook_test.go | 7 +++-- 5 files changed, 101 insertions(+), 3 deletions(-) create mode 100644 internal/richerror/richerror.go diff --git a/api/converter/errors.go b/api/converter/errors.go index 69ec75f4e..a89b0bfb1 100644 --- a/api/converter/errors.go +++ b/api/converter/errors.go @@ -25,3 +25,22 @@ func ErrorCodeOf(err error) string { } return "" } + +// ErrorMetadataOf returns the error metadata of the given error. +func ErrorMetadataOf(err error) map[string]string { + var connectErr *connect.Error + if !errors.As(err, &connectErr) { + return nil + } + for _, detail := range connectErr.Details() { + msg, valueErr := detail.Value() + if valueErr != nil { + continue + } + + if errorInfo, ok := msg.(*errdetails.ErrorInfo); ok { + return errorInfo.GetMetadata() + } + } + return nil +} diff --git a/internal/richerror/richerror.go b/internal/richerror/richerror.go new file mode 100644 index 000000000..b8e8f9fe6 --- /dev/null +++ b/internal/richerror/richerror.go @@ -0,0 +1,28 @@ +/* + * Copyright 2024 The Yorkie Authors. All rights reserved. + * + * 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 richerror provides a rich error type that can be used to wrap errors +package richerror + +// RichError is an error type that can be used to wrap errors with additional metadata +type RichError struct { + Err error + Metadata map[string]string +} + +func (e RichError) Error() string { + return e.Err.Error() +} diff --git a/server/rpc/auth/webhook.go b/server/rpc/auth/webhook.go index a25314a75..d9f4a198e 100644 --- a/server/rpc/auth/webhook.go +++ b/server/rpc/auth/webhook.go @@ -28,6 +28,7 @@ import ( "time" "github.com/yorkie-team/yorkie/api/types" + "github.com/yorkie-team/yorkie/internal/richerror" "github.com/yorkie-team/yorkie/server/backend" "github.com/yorkie-team/yorkie/server/logging" ) @@ -108,7 +109,11 @@ func verifyAccess( return resp.StatusCode, fmt.Errorf("%s: %w", authResp.Message, ErrPermissionDenied) } if authResp.Code == types.CodeUnauthenticated { - return resp.StatusCode, fmt.Errorf("%s: %w", authResp.Message, ErrUnauthenticated) + richError := &richerror.RichError{ + Err: ErrUnauthenticated, + Metadata: map[string]string{"message": authResp.Message}, + } + return resp.StatusCode, richError } return resp.StatusCode, fmt.Errorf("%d: %w", authResp.Code, ErrUnexpectedResponse) diff --git a/server/rpc/connecthelper/status.go b/server/rpc/connecthelper/status.go index 39e8ef8ad..bc9ce3ca8 100644 --- a/server/rpc/connecthelper/status.go +++ b/server/rpc/connecthelper/status.go @@ -26,6 +26,7 @@ import ( "github.com/yorkie-team/yorkie/api/converter" "github.com/yorkie-team/yorkie/api/types" + "github.com/yorkie-team/yorkie/internal/richerror" "github.com/yorkie-team/yorkie/internal/validation" "github.com/yorkie-team/yorkie/pkg/document/key" "github.com/yorkie-team/yorkie/pkg/document/time" @@ -185,6 +186,44 @@ func errorToConnectError(err error) (*connect.Error, bool) { return connectErr, true } +// richErrorToConnectError returns connect.Error from the given rich error. +func richErrorToConnectError(err error) (*connect.Error, bool) { + var richError *richerror.RichError + if !errors.As(err, &richError) { + return nil, false + } + + // NOTE(hackerwins): This prevents panic when the cause is an unhashable + // error. + var connectCode connect.Code + var ok bool + defer func() { + if r := recover(); r != nil { + ok = false + } + }() + + connectCode, ok = errorToConnectCode[richError.Err] + if !ok { + return nil, false + } + + connectErr := connect.NewError(connectCode, err) + if code, ok := errorToCode[richError.Err]; ok { + errorInfo := &errdetails.ErrorInfo{ + Metadata: map[string]string{"code": code}, + } + for key, value := range richError.Metadata { + errorInfo.Metadata[key] = value + } + if detail, detailErr := connect.NewErrorDetail(errorInfo); detailErr == nil { + connectErr.AddDetail(detail) + } + } + + return connectErr, true +} + // structErrorToConnectError returns connect.Error from the given struct error. func structErrorToConnectError(err error) (*connect.Error, bool) { var invalidFieldsError *validation.StructError @@ -231,6 +270,10 @@ func ToStatusError(err error) error { return nil } + if err, ok := richErrorToConnectError(err); ok { + return err + } + if err, ok := errorToConnectError(err); ok { return err } diff --git a/test/integration/auth_webhook_test.go b/test/integration/auth_webhook_test.go index 8b1ed4742..6126ee68a 100644 --- a/test/integration/auth_webhook_test.go +++ b/test/integration/auth_webhook_test.go @@ -53,6 +53,9 @@ func newAuthServer(t *testing.T) (*httptest.Server, string) { res.Code = types.CodeOK } else if req.Token == "not allowed token" { res.Code = types.CodePermissionDenied + } else if req.Token == "" { + res.Code = types.CodeUnauthenticated + res.Message = "no token" } else { res.Code = types.CodeUnauthenticated res.Message = "invalid token" @@ -147,7 +150,7 @@ func TestProjectAuthWebhook(t *testing.T) { defer func() { assert.NoError(t, cliWithoutToken.Close()) }() err = cliWithoutToken.Activate(ctx) assert.Equal(t, connect.CodeUnauthenticated, connect.CodeOf(err)) - assert.Equal(t, connecthelper.CodeOf(auth.ErrUnauthenticated), converter.ErrorCodeOf(err)) + assert.Equal(t, map[string]string{"code": connecthelper.CodeOf(auth.ErrUnauthenticated), "message": "no token"}, converter.ErrorMetadataOf(err)) // client with invalid token cliWithInvalidToken, err := client.Dial( @@ -159,7 +162,7 @@ func TestProjectAuthWebhook(t *testing.T) { defer func() { assert.NoError(t, cliWithInvalidToken.Close()) }() err = cliWithInvalidToken.Activate(ctx) assert.Equal(t, connect.CodeUnauthenticated, connect.CodeOf(err)) - assert.Equal(t, connecthelper.CodeOf(auth.ErrUnauthenticated), converter.ErrorCodeOf(err)) + assert.Equal(t, map[string]string{"code": connecthelper.CodeOf(auth.ErrUnauthenticated), "message": "invalid token"}, converter.ErrorMetadataOf(err)) }) t.Run("permission denied response test", func(t *testing.T) {