[FIX] Update API Errors

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
 	}
 }
 

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" }

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),
 		}
 	}
 

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