From a04ecbbb447798a88643bc4002f628b13fb2f409 Mon Sep 17 00:00:00 2001 From: Frederic BIDON Date: Sat, 6 Dec 2025 11:59:27 +0100 Subject: [PATCH] chore(lint): relinted code base according to strengthened rules. Following the refactoring of error switch statements, added tests cases to cover all new code branches. NOTE: remember not to apply the auto fix for errlint, as it embarks into ambitious code rewrites that are broken. Signed-off-by: Frederic BIDON --- api.go | 107 +++++++++++++++++++++++++++++++------------------ api_test.go | 24 ++++++++++- auth.go | 2 +- headers.go | 14 +++---- middleware.go | 2 +- parsing.go | 8 ++-- schema.go | 88 +++++++++++++++++++++++----------------- schema_test.go | 23 +++++++++++ 8 files changed, 178 insertions(+), 90 deletions(-) diff --git a/api.go b/api.go index d39233b..d169882 100644 --- a/api.go +++ b/api.go @@ -5,6 +5,7 @@ package errors import ( "encoding/json" + "errors" "fmt" "net/http" "reflect" @@ -12,9 +13,11 @@ import ( ) // DefaultHTTPCode is used when the error Code cannot be used as an HTTP code. +// +//nolint:gochecknoglobals // it should have been a constant in the first place, but now it is mutable so we have to leave it here or introduce a breaking change. var DefaultHTTPCode = http.StatusUnprocessableEntity -// Error represents a error interface all swagger framework errors implement +// Error represents a error interface all swagger framework errors implement. type Error interface { error Code() int32 @@ -33,7 +36,7 @@ func (a *apiError) Code() int32 { return a.code } -// MarshalJSON implements the JSON encoding interface +// MarshalJSON implements the JSON encoding interface. func (a apiError) MarshalJSON() ([]byte, error) { return json.Marshal(map[string]any{ "code": a.code, @@ -41,7 +44,7 @@ func (a apiError) MarshalJSON() ([]byte, error) { }) } -// New creates a new API error with a code and a message +// New creates a new API error with a code and a message. func New(code int32, message string, args ...any) Error { if len(args) > 0 { return &apiError{ @@ -55,7 +58,7 @@ func New(code int32, message string, args ...any) Error { } } -// NotFound creates a new not found error +// NotFound creates a new not found error. func NotFound(message string, args ...any) Error { if message == "" { message = "Not found" @@ -63,12 +66,12 @@ func NotFound(message string, args ...any) Error { return New(http.StatusNotFound, message, args...) } -// NotImplemented creates a new not implemented error +// NotImplemented creates a new not implemented error. func NotImplemented(message string) Error { return New(http.StatusNotImplemented, "%s", message) } -// MethodNotAllowedError represents an error for when the path matches but the method doesn't +// MethodNotAllowedError represents an error for when the path matches but the method doesn't. type MethodNotAllowedError struct { code int32 Allowed []string @@ -79,12 +82,12 @@ func (m *MethodNotAllowedError) Error() string { return m.message } -// Code the error code +// Code the error code. func (m *MethodNotAllowedError) Code() int32 { return m.code } -// MarshalJSON implements the JSON encoding interface +// MarshalJSON implements the JSON encoding interface. func (m MethodNotAllowedError) MarshalJSON() ([]byte, error) { return json.Marshal(map[string]any{ "code": m.code, @@ -104,25 +107,33 @@ func errorAsJSON(err Error) []byte { func flattenComposite(errs *CompositeError) *CompositeError { var res []error - for _, er := range errs.Errors { - switch e := er.(type) { - case *CompositeError: - if e != nil && len(e.Errors) > 0 { - flat := flattenComposite(e) - if len(flat.Errors) > 0 { - res = append(res, flat.Errors...) - } - } - default: - if e != nil { - res = append(res, e) - } + + for _, err := range errs.Errors { + if err == nil { + continue + } + + e := &CompositeError{} + if !errors.As(err, &e) { + res = append(res, err) + + continue + } + + if len(e.Errors) == 0 { + res = append(res, e) + + continue } + + flat := flattenComposite(e) + res = append(res, flat.Errors...) } + return CompositeValidationError(res...) } -// MethodNotAllowed creates a new method not allowed error +// MethodNotAllowed creates a new method not allowed error. func MethodNotAllowed(requested string, allow []string) Error { msg := fmt.Sprintf("method %s is not allowed, but [%s] are", requested, strings.Join(allow, ",")) return &MethodNotAllowedError{ @@ -132,39 +143,55 @@ func MethodNotAllowed(requested string, allow []string) Error { } } -// ServeError implements the http error handler interface +// ServeError implements the http error handler interface. func ServeError(rw http.ResponseWriter, r *http.Request, err error) { rw.Header().Set("Content-Type", "application/json") - switch e := err.(type) { - case *CompositeError: - er := flattenComposite(e) + + if err == nil { + rw.WriteHeader(http.StatusInternalServerError) + _, _ = rw.Write(errorAsJSON(New(http.StatusInternalServerError, "Unknown error"))) + + return + } + + errComposite := &CompositeError{} + errMethodNotAllowed := &MethodNotAllowedError{} + var errError Error + + switch { + case errors.As(err, &errComposite): + er := flattenComposite(errComposite) // strips composite errors to first element only if len(er.Errors) > 0 { ServeError(rw, r, er.Errors[0]) - } else { - // guard against empty CompositeError (invalid construct) - ServeError(rw, r, nil) + + return } - case *MethodNotAllowedError: - rw.Header().Add("Allow", strings.Join(e.Allowed, ",")) - rw.WriteHeader(asHTTPCode(int(e.Code()))) + + // guard against empty CompositeError (invalid construct) + ServeError(rw, r, nil) + + case errors.As(err, &errMethodNotAllowed): + rw.Header().Add("Allow", strings.Join(errMethodNotAllowed.Allowed, ",")) + rw.WriteHeader(asHTTPCode(int(errMethodNotAllowed.Code()))) if r == nil || r.Method != http.MethodHead { - _, _ = rw.Write(errorAsJSON(e)) + _, _ = rw.Write(errorAsJSON(errMethodNotAllowed)) } - case Error: - value := reflect.ValueOf(e) + + case errors.As(err, &errError): + value := reflect.ValueOf(errError) if value.Kind() == reflect.Ptr && value.IsNil() { rw.WriteHeader(http.StatusInternalServerError) _, _ = rw.Write(errorAsJSON(New(http.StatusInternalServerError, "Unknown error"))) + return } - rw.WriteHeader(asHTTPCode(int(e.Code()))) + + rw.WriteHeader(asHTTPCode(int(errError.Code()))) if r == nil || r.Method != http.MethodHead { - _, _ = rw.Write(errorAsJSON(e)) + _, _ = rw.Write(errorAsJSON(errError)) } - case nil: - rw.WriteHeader(http.StatusInternalServerError) - _, _ = rw.Write(errorAsJSON(New(http.StatusInternalServerError, "Unknown error"))) + default: rw.WriteHeader(http.StatusInternalServerError) if r == nil || r.Method != http.MethodHead { diff --git a/api_test.go b/api_test.go index c526d13..272ba1e 100644 --- a/api_test.go +++ b/api_test.go @@ -98,7 +98,7 @@ func TestServeError(t *testing.T) { ) }) - t.Run("with composite erors", func(t *testing.T) { + t.Run("with composite errors", func(t *testing.T) { t.Run("unrecognized - return internal error with first error only - the second error is ignored", func(t *testing.T) { compositeErr := &CompositeError{ Errors: []error{ @@ -169,6 +169,28 @@ func TestServeError(t *testing.T) { ) }) + t.Run("check guard against nil members in a CompositeError", func(t *testing.T) { + compositeErr := &CompositeError{ + Errors: []error{ + New(600, "myApiError"), + nil, + New(601, "myOtherApiError"), + }, + } + t.Run("flatten CompositeError should strip nil members", func(t *testing.T) { + flat := flattenComposite(compositeErr) + require.Len(t, flat.Errors, 2) + }) + + recorder := httptest.NewRecorder() + ServeError(recorder, nil, compositeErr) + assert.Equal(t, CompositeErrorCode, recorder.Code) + assert.JSONEq(t, + `{"code":600,"message":"myApiError"}`, + recorder.Body.String(), + ) + }) + t.Run("check guard against nil type", func(t *testing.T) { recorder := httptest.NewRecorder() ServeError(recorder, nil, nil) diff --git a/auth.go b/auth.go index 08de582..1173b56 100644 --- a/auth.go +++ b/auth.go @@ -5,7 +5,7 @@ package errors import "net/http" -// Unauthenticated returns an unauthenticated error +// Unauthenticated returns an unauthenticated error. func Unauthenticated(scheme string) Error { return New(http.StatusUnauthorized, "unauthenticated for %s", scheme) } diff --git a/headers.go b/headers.go index 2d837c3..0f0b1db 100644 --- a/headers.go +++ b/headers.go @@ -9,8 +9,8 @@ import ( "net/http" ) -// Validation represents a failure of a precondition -type Validation struct { //nolint: errname +// Validation represents a failure of a precondition. +type Validation struct { //nolint: errname // changing the name to abide by the naming rule would bring a breaking change. code int32 Name string In string @@ -23,12 +23,12 @@ func (e *Validation) Error() string { return e.message } -// Code the error code +// Code the error code. func (e *Validation) Code() int32 { return e.code } -// MarshalJSON implements the JSON encoding interface +// MarshalJSON implements the JSON encoding interface. func (e Validation) MarshalJSON() ([]byte, error) { return json.Marshal(map[string]any{ "code": e.code, @@ -40,7 +40,7 @@ func (e Validation) MarshalJSON() ([]byte, error) { }) } -// ValidateName sets the name for a validation or updates it for a nested property +// ValidateName sets the name for a validation or updates it for a nested property. func (e *Validation) ValidateName(name string) *Validation { if name != "" { if e.Name == "" { @@ -59,7 +59,7 @@ const ( responseFormatFail = `unsupported media type requested, only %v are available` ) -// InvalidContentType error for an invalid content type +// InvalidContentType error for an invalid content type. func InvalidContentType(value string, allowed []string) *Validation { values := make([]any, 0, len(allowed)) for _, v := range allowed { @@ -75,7 +75,7 @@ func InvalidContentType(value string, allowed []string) *Validation { } } -// InvalidResponseFormat error for an unacceptable response format request +// InvalidResponseFormat error for an unacceptable response format request. func InvalidResponseFormat(value string, allowed []string) *Validation { values := make([]any, 0, len(allowed)) for _, v := range allowed { diff --git a/middleware.go b/middleware.go index c434e59..9bd2c03 100644 --- a/middleware.go +++ b/middleware.go @@ -10,7 +10,7 @@ import ( ) // APIVerificationFailed is an error that contains all the missing info for a mismatched section -// between the api registrations and the api spec +// between the api registrations and the api spec. type APIVerificationFailed struct { //nolint: errname Section string `json:"section,omitempty"` MissingSpecification []string `json:"missingSpecification,omitempty"` diff --git a/parsing.go b/parsing.go index ea2a7c6..25e5ac4 100644 --- a/parsing.go +++ b/parsing.go @@ -9,7 +9,7 @@ import ( "net/http" ) -// ParseError represents a parsing error +// ParseError represents a parsing error. type ParseError struct { code int32 Name string @@ -19,7 +19,7 @@ type ParseError struct { message string } -// NewParseError creates a new parse error +// NewParseError creates a new parse error. func NewParseError(name, in, value string, reason error) *ParseError { var msg string if in == "" { @@ -41,12 +41,12 @@ func (e *ParseError) Error() string { return e.message } -// Code returns the http status code for this error +// Code returns the http status code for this error. func (e *ParseError) Code() int32 { return e.code } -// MarshalJSON implements the JSON encoding interface +// MarshalJSON implements the JSON encoding interface. func (e ParseError) MarshalJSON() ([]byte, error) { var reason string if e.Reason != nil { diff --git a/schema.go b/schema.go index e59ca4f..ede3f6f 100644 --- a/schema.go +++ b/schema.go @@ -5,6 +5,7 @@ package errors import ( "encoding/json" + "errors" "fmt" "net/http" "strings" @@ -62,13 +63,13 @@ const ( const maximumValidHTTPCode = 600 // All code responses can be used to differentiate errors for different handling -// by the consuming program +// by the consuming program. const ( // CompositeErrorCode remains 422 for backwards-compatibility - // and to separate it from validation errors with cause + // and to separate it from validation errors with cause. CompositeErrorCode = http.StatusUnprocessableEntity - // InvalidTypeCode is used for any subclass of invalid types + // InvalidTypeCode is used for any subclass of invalid types. InvalidTypeCode = maximumValidHTTPCode + iota RequiredFailCode TooLongFailCode @@ -90,14 +91,14 @@ const ( ReadOnlyFailCode ) -// CompositeError is an error that groups several errors together +// CompositeError is an error that groups several errors together. type CompositeError struct { Errors []error code int32 message string } -// Code for this error +// Code for this error. func (c *CompositeError) Code() int32 { return c.code } @@ -106,6 +107,9 @@ func (c *CompositeError) Error() string { if len(c.Errors) > 0 { msgs := []string{c.message + ":"} for _, e := range c.Errors { + if e == nil { + continue + } msgs = append(msgs, e.Error()) } return strings.Join(msgs, "\n") @@ -117,7 +121,7 @@ func (c *CompositeError) Unwrap() []error { return c.Errors } -// MarshalJSON implements the JSON encoding interface +// MarshalJSON implements the JSON encoding interface. func (c CompositeError) MarshalJSON() ([]byte, error) { return json.Marshal(map[string]any{ "code": c.code, @@ -126,7 +130,7 @@ func (c CompositeError) MarshalJSON() ([]byte, error) { }) } -// CompositeValidationError an error to wrap a bunch of other errors +// CompositeValidationError an error to wrap a bunch of other errors. func CompositeValidationError(errors ...error) *CompositeError { return &CompositeError{ code: CompositeErrorCode, @@ -135,20 +139,33 @@ func CompositeValidationError(errors ...error) *CompositeError { } } -// ValidateName recursively sets the name for all validations or updates them for nested properties +// ValidateName recursively sets the name for all validations or updates them for nested properties. func (c *CompositeError) ValidateName(name string) *CompositeError { for i, e := range c.Errors { - if ve, ok := e.(*Validation); ok { - c.Errors[i] = ve.ValidateName(name) - } else if ce, ok := e.(*CompositeError); ok { + if e == nil { + continue + } + + ce := &CompositeError{} + if errors.As(e, &ce) { c.Errors[i] = ce.ValidateName(name) + + continue + } + + ve := &Validation{} + if errors.As(e, &ve) { + c.Errors[i] = ve.ValidateName(name) + + continue } + } return c } -// FailedAllPatternProperties an error for when the property doesn't match a pattern +// FailedAllPatternProperties an error for when the property doesn't match a pattern. func FailedAllPatternProperties(name, in, key string) *Validation { msg := fmt.Sprintf(failedAllPatternProps, name, key, in) if in == "" { @@ -163,7 +180,7 @@ func FailedAllPatternProperties(name, in, key string) *Validation { } } -// PropertyNotAllowed an error for when the property doesn't match a pattern +// PropertyNotAllowed an error for when the property doesn't match a pattern. func PropertyNotAllowed(name, in, key string) *Validation { msg := fmt.Sprintf(unallowedProperty, name, key, in) if in == "" { @@ -178,7 +195,7 @@ func PropertyNotAllowed(name, in, key string) *Validation { } } -// TooFewProperties an error for an object with too few properties +// TooFewProperties an error for an object with too few properties. func TooFewProperties(name, in string, n int64) *Validation { msg := fmt.Sprintf(tooFewProperties, name, in, n) if in == "" { @@ -193,7 +210,7 @@ func TooFewProperties(name, in string, n int64) *Validation { } } -// TooManyProperties an error for an object with too many properties +// TooManyProperties an error for an object with too many properties. func TooManyProperties(name, in string, n int64) *Validation { msg := fmt.Sprintf(tooManyProperties, name, in, n) if in == "" { @@ -208,7 +225,7 @@ func TooManyProperties(name, in string, n int64) *Validation { } } -// AdditionalItemsNotAllowed an error for invalid additional items +// AdditionalItemsNotAllowed an error for invalid additional items. func AdditionalItemsNotAllowed(name, in string) *Validation { msg := fmt.Sprintf(noAdditionalItems, name, in) if in == "" { @@ -222,7 +239,7 @@ func AdditionalItemsNotAllowed(name, in string) *Validation { } } -// InvalidCollectionFormat another flavor of invalid type error +// InvalidCollectionFormat another flavor of invalid type error. func InvalidCollectionFormat(name, in, format string) *Validation { return &Validation{ code: InvalidTypeCode, @@ -233,7 +250,7 @@ func InvalidCollectionFormat(name, in, format string) *Validation { } } -// InvalidTypeName an error for when the type is invalid +// InvalidTypeName an error for when the type is invalid. func InvalidTypeName(typeName string) *Validation { return &Validation{ code: InvalidTypeCode, @@ -242,7 +259,7 @@ func InvalidTypeName(typeName string) *Validation { } } -// InvalidType creates an error for when the type is invalid +// InvalidType creates an error for when the type is invalid. func InvalidType(name, in, typeName string, value any) *Validation { var message string @@ -273,10 +290,9 @@ func InvalidType(name, in, typeName string, value any) *Validation { Value: value, message: message, } - } -// DuplicateItems error for when an array contains duplicates +// DuplicateItems error for when an array contains duplicates. func DuplicateItems(name, in string) *Validation { msg := fmt.Sprintf(uniqueFail, name, in) if in == "" { @@ -290,7 +306,7 @@ func DuplicateItems(name, in string) *Validation { } } -// TooManyItems error for when an array contains too many items +// TooManyItems error for when an array contains too many items. func TooManyItems(name, in string, maximum int64, value any) *Validation { msg := fmt.Sprintf(maximumItemsFail, name, in, maximum) if in == "" { @@ -306,7 +322,7 @@ func TooManyItems(name, in string, maximum int64, value any) *Validation { } } -// TooFewItems error for when an array contains too few items +// TooFewItems error for when an array contains too few items. func TooFewItems(name, in string, minimum int64, value any) *Validation { msg := fmt.Sprintf(minItemsFail, name, in, minimum) if in == "" { @@ -321,7 +337,7 @@ func TooFewItems(name, in string, minimum int64, value any) *Validation { } } -// ExceedsMaximumInt error for when maximumimum validation fails +// ExceedsMaximumInt error for when maximumimum validation fails. func ExceedsMaximumInt(name, in string, maximum int64, exclusive bool, value any) *Validation { var message string if in == "" { @@ -346,7 +362,7 @@ func ExceedsMaximumInt(name, in string, maximum int64, exclusive bool, value any } } -// ExceedsMaximumUint error for when maximumimum validation fails +// ExceedsMaximumUint error for when maximumimum validation fails. func ExceedsMaximumUint(name, in string, maximum uint64, exclusive bool, value any) *Validation { var message string if in == "" { @@ -371,7 +387,7 @@ func ExceedsMaximumUint(name, in string, maximum uint64, exclusive bool, value a } } -// ExceedsMaximum error for when maximumimum validation fails +// ExceedsMaximum error for when maximumimum validation fails. func ExceedsMaximum(name, in string, maximum float64, exclusive bool, value any) *Validation { var message string if in == "" { @@ -396,7 +412,7 @@ func ExceedsMaximum(name, in string, maximum float64, exclusive bool, value any) } } -// ExceedsMinimumInt error for when minimum validation fails +// ExceedsMinimumInt error for when minimum validation fails. func ExceedsMinimumInt(name, in string, minimum int64, exclusive bool, value any) *Validation { var message string if in == "" { @@ -421,7 +437,7 @@ func ExceedsMinimumInt(name, in string, minimum int64, exclusive bool, value any } } -// ExceedsMinimumUint error for when minimum validation fails +// ExceedsMinimumUint error for when minimum validation fails. func ExceedsMinimumUint(name, in string, minimum uint64, exclusive bool, value any) *Validation { var message string if in == "" { @@ -446,7 +462,7 @@ func ExceedsMinimumUint(name, in string, minimum uint64, exclusive bool, value a } } -// ExceedsMinimum error for when minimum validation fails +// ExceedsMinimum error for when minimum validation fails. func ExceedsMinimum(name, in string, minimum float64, exclusive bool, value any) *Validation { var message string if in == "" { @@ -471,7 +487,7 @@ func ExceedsMinimum(name, in string, minimum float64, exclusive bool, value any) } } -// NotMultipleOf error for when multiple of validation fails +// NotMultipleOf error for when multiple of validation fails. func NotMultipleOf(name, in string, multiple, value any) *Validation { var msg string if in == "" { @@ -488,7 +504,7 @@ func NotMultipleOf(name, in string, multiple, value any) *Validation { } } -// EnumFail error for when an enum validation fails +// EnumFail error for when an enum validation fails. func EnumFail(name, in string, value any, values []any) *Validation { var msg string if in == "" { @@ -507,7 +523,7 @@ func EnumFail(name, in string, value any, values []any) *Validation { } } -// Required error for when a value is missing +// Required error for when a value is missing. func Required(name, in string, value any) *Validation { var msg string if in == "" { @@ -524,7 +540,7 @@ func Required(name, in string, value any) *Validation { } } -// ReadOnly error for when a value is present in request +// ReadOnly error for when a value is present in request. func ReadOnly(name, in string, value any) *Validation { var msg string if in == "" { @@ -541,7 +557,7 @@ func ReadOnly(name, in string, value any) *Validation { } } -// TooLong error for when a string is too long +// TooLong error for when a string is too long. func TooLong(name, in string, maximum int64, value any) *Validation { var msg string if in == "" { @@ -558,7 +574,7 @@ func TooLong(name, in string, maximum int64, value any) *Validation { } } -// TooShort error for when a string is too short +// TooShort error for when a string is too short. func TooShort(name, in string, minimum int64, value any) *Validation { var msg string if in == "" { @@ -596,7 +612,7 @@ func FailedPattern(name, in, pattern string, value any) *Validation { } // MultipleOfMustBePositive error for when a -// multipleOf factor is negative +// multipleOf factor is negative. func MultipleOfMustBePositive(name, in string, factor any) *Validation { return &Validation{ code: MultipleOfMustBePositiveCode, diff --git a/schema_test.go b/schema_test.go index af33121..8a99784 100644 --- a/schema_test.go +++ b/schema_test.go @@ -402,6 +402,29 @@ new-namey is an invalid type name` assert.Equal(t, expectedMessage, err.Error()) }) + t.Run("ValidateName for a CompositeError should skip nil members", func(t *testing.T) { + err := CompositeValidationError( + InvalidContentType("text/html", []string{"application/json"}), + nil, + CompositeValidationError( + InvalidTypeName("z"), + nil, + errors.New("another error"), + ), + errors.New("some error"), + ) + mutated := err.ValidateName("another-name") + const expectedMessage = `validation failure list: +another-name.unsupported media type "text/html", only [application/json] are allowed +validation failure list: +another-namez is an invalid type name +another error +some error` + + require.Len(t, mutated.Errors, len(err.Errors)) + assert.Equal(t, expectedMessage, mutated.Error()) + }) + t.Run("with PropertyNotAllowed", func(t *testing.T) { err = PropertyNotAllowed("path", "body", "key") require.Error(t, err)