From c325292082b40351d4dda28f742de8eed864c40b Mon Sep 17 00:00:00 2001 From: Tony Abboud Date: Thu, 14 Nov 2024 13:55:14 -0500 Subject: [PATCH 1/4] WIP: Add ConjureErrorDecoder --- .../httpclient/request_params.go | 12 ++++++++++++ .../response_error_decoder_middleware.go | 12 ++++++++++-- .../errors/conjure_error_decoder.go | 19 +++++++++++++++++++ conjure-go-contract/errors/unmarshal.go | 16 ++++++++++++++++ 4 files changed, 57 insertions(+), 2 deletions(-) create mode 100644 conjure-go-contract/errors/conjure_error_decoder.go diff --git a/conjure-go-client/httpclient/request_params.go b/conjure-go-client/httpclient/request_params.go index 904b4a2f..b541b1e4 100644 --- a/conjure-go-client/httpclient/request_params.go +++ b/conjure-go-client/httpclient/request_params.go @@ -23,6 +23,7 @@ import ( "time" "github.com/palantir/conjure-go-runtime/v2/conjure-go-contract/codecs" + "github.com/palantir/conjure-go-runtime/v2/conjure-go-contract/errors" werror "github.com/palantir/witchcraft-go-error" ) @@ -230,3 +231,14 @@ func WithRequestTimeout(timeout time.Duration) RequestParam { return nil }) } + +func WithRequestConjureErrorDecoder(ced errors.ConjureErrorDecoder) RequestParam { + return requestParamFunc(func(b *requestBuilder) error { + b.errorDecoderMiddleware = errorDecoderMiddleware{ + errorDecoder: restErrorDecoder{ + conjureErrorDecoder: ced, + }, + } + return nil + }) +} diff --git a/conjure-go-client/httpclient/response_error_decoder_middleware.go b/conjure-go-client/httpclient/response_error_decoder_middleware.go index 603ea36a..829e43fc 100644 --- a/conjure-go-client/httpclient/response_error_decoder_middleware.go +++ b/conjure-go-client/httpclient/response_error_decoder_middleware.go @@ -66,7 +66,9 @@ func (e errorDecoderMiddleware) RoundTrip(req *http.Request, next http.RoundTrip // If the response has a Content-Type containing 'application/json', we attempt // to unmarshal the error as a conjure error. See TestErrorDecoderMiddlewares for // example error messages and parameters. -type restErrorDecoder struct{} +type restErrorDecoder struct { + conjureErrorDecoder errors.ConjureErrorDecoder +} var _ ErrorDecoder = restErrorDecoder{} @@ -102,7 +104,13 @@ func (d restErrorDecoder) DecodeError(resp *http.Response) error { if isJSON := strings.Contains(resp.Header.Get("Content-Type"), codecs.JSON.ContentType()); !isJSON { return werror.Error(resp.Status, wSafeParams, wUnsafeParams, werror.UnsafeParam("responseBody", string(body))) } - conjureErr, jsonErr := errors.UnmarshalError(body) + var conjureErr errors.Error + var jsonErr error + if d.conjureErrorDecoder != nil { + conjureErr, jsonErr = errors.UnmarshalErrorWithDecoder(d.conjureErrorDecoder, body) + } else { + conjureErr, jsonErr = errors.UnmarshalError(body) + } if jsonErr != nil { return werror.Error(resp.Status, wSafeParams, wUnsafeParams, werror.UnsafeParam("responseBody", string(body))) } diff --git a/conjure-go-contract/errors/conjure_error_decoder.go b/conjure-go-contract/errors/conjure_error_decoder.go new file mode 100644 index 00000000..6ec98f07 --- /dev/null +++ b/conjure-go-contract/errors/conjure_error_decoder.go @@ -0,0 +1,19 @@ +// Copyright (c) 2024 Palantir Technologies. 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 errors + +type ConjureErrorDecoder interface { + DecodeConjureError(name string, body []byte) (Error, error) +} diff --git a/conjure-go-contract/errors/unmarshal.go b/conjure-go-contract/errors/unmarshal.go index a0921f2c..22c4e540 100644 --- a/conjure-go-contract/errors/unmarshal.go +++ b/conjure-go-contract/errors/unmarshal.go @@ -46,3 +46,19 @@ func UnmarshalError(body []byte) (Error, error) { // Cast should never panic, as we've verified in RegisterErrorType return instance.(Error), nil } + +// UnmarshalErrorWithDecoder attempts to deserialize the message to a known implementation of Error +// using the provided ConjureErrorDecoder. +func UnmarshalErrorWithDecoder(ced ConjureErrorDecoder, body []byte) (Error, error) { + var name struct { + Name string `json:"errorName"` + } + if err := codecs.JSON.Unmarshal(body, &name); err != nil { + return nil, werror.Wrap(err, "failed to unmarshal body as conjure error") + } + cErr, err := ced.DecodeConjureError(name.Name, body) + if err != nil { + return nil, werror.Wrap(err, "failed to decode body using ConjureErrorDecoder") + } + return cErr, nil +} From f52ba8f67d5498949a32900144477f280712c364 Mon Sep 17 00:00:00 2001 From: Brad Moylan Date: Mon, 25 Nov 2024 17:42:03 -0800 Subject: [PATCH 2/4] add ReflectTypeConjureErrorDecoder --- .../errors/error_type_registry.go | 50 ++++++++++++++++--- conjure-go-contract/errors/unmarshal.go | 24 +-------- 2 files changed, 46 insertions(+), 28 deletions(-) diff --git a/conjure-go-contract/errors/error_type_registry.go b/conjure-go-contract/errors/error_type_registry.go index 95975cd3..c1dc6a90 100644 --- a/conjure-go-contract/errors/error_type_registry.go +++ b/conjure-go-contract/errors/error_type_registry.go @@ -17,9 +17,12 @@ package errors import ( "fmt" "reflect" + + "github.com/palantir/conjure-go-runtime/v2/conjure-go-contract/codecs" + werror "github.com/palantir/witchcraft-go-error" ) -var registry = map[string]reflect.Type{} +var globalRegistry = NewReflectTypeConjureErrorDecoder() var errorInterfaceType = reflect.TypeOf((*Error)(nil)).Elem() @@ -27,11 +30,46 @@ var errorInterfaceType = reflect.TypeOf((*Error)(nil)).Elem() // The type should be a struct type whose pointer implements Error. // Panics if name is already registered or *type does not implement Error. func RegisterErrorType(name string, typ reflect.Type) { - if existing, exists := registry[name]; exists { - panic(fmt.Sprintf("ErrorName %v already registered as %v", name, existing)) + if err := globalRegistry.RegisterErrorType(name, typ); err != nil { + panic(err.Error()) + } +} + +// NewReflectTypeConjureErrorDecoder returns a new ConjureErrorDecoder that uses reflection to convert JSON errors to their go types. +func NewReflectTypeConjureErrorDecoder() *ReflectTypeConjureErrorDecoder { + return &ReflectTypeConjureErrorDecoder{registry: make(map[string]reflect.Type)} +} + +// ReflectTypeConjureErrorDecoder is a ConjureErrorDecoder that uses reflection to convert JSON errors to their go types. +// It stores a mapping of error name to +type ReflectTypeConjureErrorDecoder struct { + registry map[string]reflect.Type +} + +func (d *ReflectTypeConjureErrorDecoder) RegisterErrorType(name string, typ reflect.Type) error { + if existing, exists := d.registry[name]; exists { + return fmt.Errorf("ErrorName %v already registered as %v", name, existing) + } + if ptr := reflect.PointerTo(typ); !ptr.Implements(errorInterfaceType) { + return fmt.Errorf("Error type %v does not implement errors.Error interface", ptr) + } + d.registry[name] = typ + return nil +} + +func (d *ReflectTypeConjureErrorDecoder) DecodeConjureError(errorName string, body []byte) (Error, error) { + typ, ok := d.registry[errorName] + if !ok { + // Unrecognized error name, fall back to genericError + typ = reflect.TypeOf(genericError{}) + } + instance := reflect.New(typ).Interface() + if err := codecs.JSON.Unmarshal(body, &instance); err != nil { + return nil, werror.Wrap(err, "failed to unmarshal body using registered type", werror.SafeParam("type", typ.String())) } - if ptr := reflect.PtrTo(typ); !ptr.Implements(errorInterfaceType) { - panic(fmt.Sprintf("Error type %v does not implement errors.Error interface", ptr)) + cerr, ok := instance.(Error) + if !ok { + return nil, werror.Error("unmarshaled type does not implement errors.Error interface", werror.SafeParam("type", typ.String())) } - registry[name] = typ + return cerr, nil } diff --git a/conjure-go-contract/errors/unmarshal.go b/conjure-go-contract/errors/unmarshal.go index 22c4e540..f023b2d8 100644 --- a/conjure-go-contract/errors/unmarshal.go +++ b/conjure-go-contract/errors/unmarshal.go @@ -15,8 +15,6 @@ package errors import ( - "reflect" - "github.com/palantir/conjure-go-runtime/v2/conjure-go-contract/codecs" werror "github.com/palantir/witchcraft-go-error" ) @@ -26,25 +24,7 @@ import ( // If the ErrorName is not recognized, a genericError is returned with all params marked unsafe. // If we fail to unmarshal to a generic SerializableError or to the type specified by ErrorName, an error is returned. func UnmarshalError(body []byte) (Error, error) { - var name struct { - Name string `json:"errorName"` - } - if err := codecs.JSON.Unmarshal(body, &name); err != nil { - return nil, werror.Wrap(err, "failed to unmarshal body as conjure error") - } - typ, ok := registry[name.Name] - if !ok { - // Unrecognized error name, fall back to genericError - typ = reflect.TypeOf(genericError{}) - } - - instance := reflect.New(typ).Interface() - if err := codecs.JSON.Unmarshal(body, &instance); err != nil { - return nil, werror.Wrap(err, "failed to unmarshal body using registered type", werror.SafeParam("type", typ.String())) - } - - // Cast should never panic, as we've verified in RegisterErrorType - return instance.(Error), nil + return UnmarshalErrorWithDecoder(globalRegistry, body) } // UnmarshalErrorWithDecoder attempts to deserialize the message to a known implementation of Error @@ -58,7 +38,7 @@ func UnmarshalErrorWithDecoder(ced ConjureErrorDecoder, body []byte) (Error, err } cErr, err := ced.DecodeConjureError(name.Name, body) if err != nil { - return nil, werror.Wrap(err, "failed to decode body using ConjureErrorDecoder") + return nil, werror.Convert(err) } return cErr, nil } From e134ab4e64501cd4c009dd61c8ee6c7df4f736b5 Mon Sep 17 00:00:00 2001 From: svc-changelog Date: Wed, 27 Nov 2024 17:43:39 +0000 Subject: [PATCH 3/4] Add generated changelog entries --- changelog/@unreleased/pr-724.v2.yml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 changelog/@unreleased/pr-724.v2.yml diff --git a/changelog/@unreleased/pr-724.v2.yml b/changelog/@unreleased/pr-724.v2.yml new file mode 100644 index 00000000..6288764b --- /dev/null +++ b/changelog/@unreleased/pr-724.v2.yml @@ -0,0 +1,6 @@ +type: feature +feature: + description: Add errors.ConjureErrorDecoder interface & instantiable registry type + for customizing error deserialization + links: + - https://github.com/palantir/conjure-go-runtime/pull/724 From 94e97db7ae0824b99e33862558e034ee1e1feef9 Mon Sep 17 00:00:00 2001 From: Brad Moylan Date: Wed, 27 Nov 2024 13:24:08 -0800 Subject: [PATCH 4/4] godoc --- conjure-go-contract/errors/error_type_registry.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/conjure-go-contract/errors/error_type_registry.go b/conjure-go-contract/errors/error_type_registry.go index c1dc6a90..4b8757ce 100644 --- a/conjure-go-contract/errors/error_type_registry.go +++ b/conjure-go-contract/errors/error_type_registry.go @@ -41,7 +41,7 @@ func NewReflectTypeConjureErrorDecoder() *ReflectTypeConjureErrorDecoder { } // ReflectTypeConjureErrorDecoder is a ConjureErrorDecoder that uses reflection to convert JSON errors to their go types. -// It stores a mapping of error name to +// It stores a mapping of serialized error name to the go type that should be used to unmarshal the error. type ReflectTypeConjureErrorDecoder struct { registry map[string]reflect.Type }