diff --git a/api/errors.go b/api/errors.go index 04fddaa..eff7059 100644 --- a/api/errors.go +++ b/api/errors.go @@ -27,17 +27,21 @@ func Error(err interface{}) Reply { rep := Reply{Success: false} switch err := err.(type) { case validation.Errors: - if len(err) == 1 { - rep.Error = err.Error() - } else { - rep.Error = fmt.Sprintf("%d validation errors occurred", len(err)) - rep.ErrorDetail = make(ErrorDetail, 0, len(err)) - for _, verr := range err { - rep.ErrorDetail = append(rep.ErrorDetail, &DetailError{ - Field: verr.Field(), - Error: verr.Error(), - }) - } + rep.Error = fmt.Sprintf("%d validation errors occurred", len(err)) + rep.ErrorDetail = make(ErrorDetail, 0, len(err)) + for _, verr := range err { + rep.ErrorDetail = append(rep.ErrorDetail, &DetailError{ + Field: verr.Field(), + Error: verr.Error(), + }) + } + case *validation.FieldError: + rep.Error = err.Error() + rep.ErrorDetail = ErrorDetail{ + &DetailError{ + Field: err.Field(), + Error: err.Error(), + }, } case error: rep.Error = err.Error() @@ -62,26 +66,34 @@ func Error(err interface{}) Reply { // Status Errors //=========================================================================== -// ErrorReply decodes an error response from the API call. -type ErrorReply struct { - StatusCode int - Reply Reply +// Wraps an error with an HTTP status code. +type StatusError struct { + Code int + Err error +} + +func (e *StatusError) Error() string { + return fmt.Sprintf("[%d] %s", e.Code, e.Err.Error()) +} + +func (e *StatusError) Unwrap() error { + return e.Err } -func (e *ErrorReply) Error() string { - return fmt.Sprintf("[%d] %s", e.StatusCode, e.Reply.Error) +func (e *StatusError) Reply() Reply { + return Error(e.Err) } -// ErrorStatus returns the HTTP status code from an error or 500 if the error is not a StatusError. -func ErrorStatus(err error) int { +// StatusCode returns the HTTP status code from an error or 500 if the error is not a StatusError. +func StatusCode(err error) int { if err == nil { return http.StatusOK } - if e, ok := err.(*ErrorReply); !ok || e.StatusCode < 100 || e.StatusCode >= 600 { + if e, ok := err.(*StatusError); !ok || e.Code < 100 || e.Code >= 600 { return http.StatusInternalServerError } else { - return e.StatusCode + return e.Code } } diff --git a/api/errors_test.go b/api/errors_test.go index 960c29d..e7e82c3 100644 --- a/api/errors_test.go +++ b/api/errors_test.go @@ -1 +1,193 @@ package api_test + +import ( + "encoding/json" + "errors" + "fmt" + "net/http" + "testing" + + "go.rtnl.ai/x/api" + "go.rtnl.ai/x/assert" + "go.rtnl.ai/x/validation" +) + +func TestSentinelReplies(t *testing.T) { + assert.Equal(t, api.Reply{Success: false}, api.Unsuccessful) + assert.Equal(t, api.Reply{Success: false, Error: "resource not found"}, api.NotFound) + assert.Equal(t, api.Reply{Success: false, Error: "method not allowed"}, api.NotAllowed) +} + +func TestError(t *testing.T) { + t.Run("Nil", func(t *testing.T) { + assert.Equal(t, api.Unsuccessful, api.Error(nil)) + }) + + t.Run("ValidationErrors", func(t *testing.T) { + verrs := validation.Errors{ + validation.Missing("email"), + validation.Incorrect("age", "must not be negative"), + } + + rep := api.Error(verrs) + assert.False(t, rep.Success) + assert.Equal(t, "2 validation errors occurred", rep.Error) + assert.Equal(t, api.ErrorDetail{ + {Field: "email", Error: "missing email: this field is required"}, + {Field: "age", Error: "invalid age: must not be negative"}, + }, rep.ErrorDetail) + }) + + t.Run("EmptyValidationErrors", func(t *testing.T) { + rep := api.Error(validation.Errors{}) + assert.False(t, rep.Success) + assert.Equal(t, "0 validation errors occurred", rep.Error) + assert.Equal(t, api.ErrorDetail{}, rep.ErrorDetail) + }) + + t.Run("FieldError", func(t *testing.T) { + ferr := validation.Missing("email") + rep := api.Error(ferr) + assert.False(t, rep.Success) + assert.Equal(t, "missing email: this field is required", rep.Error) + assert.Equal(t, api.ErrorDetail{ + {Field: "email", Error: "missing email: this field is required"}, + }, rep.ErrorDetail) + }) + + t.Run("Error", func(t *testing.T) { + rep := api.Error(errors.New("something broke")) + assert.False(t, rep.Success) + assert.Equal(t, "something broke", rep.Error) + assert.Nil(t, rep.ErrorDetail) + }) + + t.Run("String", func(t *testing.T) { + rep := api.Error("plain string error") + assert.False(t, rep.Success) + assert.Equal(t, "plain string error", rep.Error) + assert.Nil(t, rep.ErrorDetail) + }) + + t.Run("Stringer", func(t *testing.T) { + rep := api.Error(testStringer("from stringer")) + assert.False(t, rep.Success) + assert.Equal(t, "from stringer", rep.Error) + assert.Nil(t, rep.ErrorDetail) + }) + + t.Run("JSONMarshaler", func(t *testing.T) { + rep := api.Error(testMarshaler(`{"code":"bad_request"}`)) + assert.False(t, rep.Success) + assert.Equal(t, `{"code":"bad_request"}`, rep.Error) + assert.Nil(t, rep.ErrorDetail) + }) + + t.Run("JSONMarshalerPanic", func(t *testing.T) { + m := failingMarshaler{} + assert.PanicsWithValue(t, m, func() { + api.Error(m) + }) + }) + + t.Run("Unhandled", func(t *testing.T) { + rep := api.Error(42) + assert.False(t, rep.Success) + assert.Equal(t, "unhandled error response", rep.Error) + assert.Nil(t, rep.ErrorDetail) + }) + + t.Run("ErrorTakesPrecedenceOverStringer", func(t *testing.T) { + rep := api.Error(errorStringer{}) + assert.Equal(t, "from error", rep.Error) + }) +} + +func TestStatusError(t *testing.T) { + inner := errors.New("resource missing") + err := &api.StatusError{Code: http.StatusNotFound, Err: inner} + + t.Run("Error", func(t *testing.T) { + assert.EqualError(t, err, "[404] resource missing") + }) + + t.Run("Unwrap", func(t *testing.T) { + assert.Equal(t, inner, err.Unwrap()) + assert.ErrorIs(t, err, inner) + }) + + t.Run("Reply", func(t *testing.T) { + rep := err.Reply() + assert.False(t, rep.Success) + assert.Equal(t, "resource missing", rep.Error) + assert.Nil(t, rep.ErrorDetail) + }) + + t.Run("ReplyValidationErrors", func(t *testing.T) { + verrs := validation.Errors{validation.Missing("query")} + serr := &api.StatusError{Code: http.StatusBadRequest, Err: verrs} + + rep := serr.Reply() + assert.False(t, rep.Success) + assert.Equal(t, "1 validation errors occurred", rep.Error) + assert.Equal(t, api.ErrorDetail{ + {Field: "query", Error: "missing query: this field is required"}, + }, rep.ErrorDetail) + }) +} + +func TestStatusCode(t *testing.T) { + tests := []struct { + name string + err error + code int + }{ + {"Nil", nil, http.StatusOK}, + {"PlainError", errors.New("boom"), http.StatusInternalServerError}, + {"NotFound", &api.StatusError{Code: http.StatusNotFound, Err: errors.New("missing")}, http.StatusNotFound}, + {"BadRequest", &api.StatusError{Code: http.StatusBadRequest, Err: errors.New("invalid")}, http.StatusBadRequest}, + {"MinValid", &api.StatusError{Code: 100, Err: errors.New("continue")}, 100}, + {"MaxValid", &api.StatusError{Code: 599, Err: errors.New("timeout")}, 599}, + {"BelowMin", &api.StatusError{Code: 99, Err: errors.New("too low")}, http.StatusInternalServerError}, + {"AboveMax", &api.StatusError{Code: 600, Err: errors.New("too high")}, http.StatusInternalServerError}, + {"ZeroCode", &api.StatusError{Code: 0, Err: errors.New("unset")}, http.StatusInternalServerError}, + {"Wrapped", fmt.Errorf("wrap: %w", &api.StatusError{Code: http.StatusNotFound, Err: errors.New("missing")}), http.StatusInternalServerError}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + assert.Equal(t, tc.code, api.StatusCode(tc.err)) + }) + } +} + +func TestDetailErrorJSON(t *testing.T) { + detail := api.ErrorDetail{ + {Field: "email", Error: "this field is required"}, + } + + data, err := json.Marshal(detail) + assert.Ok(t, err) + assert.Equal(t, `[{"field":"email","error":"this field is required"}]`, string(data)) +} + +type testStringer string + +func (s testStringer) String() string { return string(s) } + +type testMarshaler string + +func (m testMarshaler) MarshalJSON() ([]byte, error) { + return []byte(m), nil +} + +type failingMarshaler struct{} + +func (failingMarshaler) MarshalJSON() ([]byte, error) { + return nil, errors.New("marshal failed") +} + +type errorStringer struct{} + +func (errorStringer) Error() string { return "from error" } +func (errorStringer) String() string { return "from stringer" } diff --git a/probez/client.go b/probez/client.go index 55e700d..9ecdba0 100644 --- a/probez/client.go +++ b/probez/client.go @@ -196,21 +196,19 @@ func Do(req *http.Request) (rep *http.Response, err error) { return nil, err } - if rep.StatusCode < 200 || rep.StatusCode >= 300 { + if rep.StatusCode < 200 || rep.StatusCode >= 400 { defer rep.Body.Close() var body string - if data, err := io.ReadAll(rep.Body); err == nil { + if data, err := io.ReadAll(rep.Body); err != nil || len(data) == 0 { body = http.StatusText(rep.StatusCode) } else { body = string(data) } - err = &api.ErrorReply{ - StatusCode: rep.StatusCode, - Reply: api.Reply{ - Error: body, - }, + err = &api.StatusError{ + Code: rep.StatusCode, + Err: errors.New(body), } } diff --git a/probez/probez.go b/probez/probez.go index e7a3815..5164671 100644 --- a/probez/probez.go +++ b/probez/probez.go @@ -32,7 +32,7 @@ const ( // This handler implements the http.Handler interface, but in common practice, users // should add the Healthz, Livez, and Readyz http.HandlerFuncs to their own muxer or // router. If you're using an http.ServeMux you can use the Handler.Mux function to -// automatically addd the routes. +// automatically add the routes. type Handler struct { healthy *atomic.Value ready *atomic.Value