Skip to content

Commit

Permalink
Validation should return 400 on fail (#30)
Browse files Browse the repository at this point in the history
New BadRequestError type : returns 400 status code on Validation failure
Instead of 500 previously

---------

Co-authored-by: EwenQuim <ewen.quimerch@gmail.com>
  • Loading branch information
rizerkrof and EwenQuim authored Dec 18, 2023
1 parent 63a68ab commit 5a961ee
Show file tree
Hide file tree
Showing 12 changed files with 84 additions and 45 deletions.
26 changes: 15 additions & 11 deletions ctx.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ const (

// Ctx is the context of the request.
// It contains the request body, the path parameters, the query parameters, and the http request.
// Please do not use a pointer type as parameter.
type Ctx[B any] interface {
// Body returns the body of the request.
// If (*B) implements [InTransformer], it will be transformed after deserialization.
Expand Down Expand Up @@ -79,6 +80,7 @@ type Ctx[B any] interface {
Pass() ClassicContext
}

// NewContext returns a new context. It is used internally by Fuego. You probably want to use Ctx[B] instead.
func NewContext[B any](w http.ResponseWriter, r *http.Request, options readOptions) *Context[B] {
c := &Context[B]{
ClassicContext: ClassicContext{
Expand All @@ -94,6 +96,7 @@ func NewContext[B any](w http.ResponseWriter, r *http.Request, options readOptio
return c
}

// Context is used internally by Fuego. You probably want to use Ctx[B] instead. Please do not use a pointer type as parameter.
type Context[BodyType any] struct {
body *BodyType
ClassicContext
Expand All @@ -103,22 +106,22 @@ func (c *Context[B]) Pass() ClassicContext {
return c.ClassicContext
}

// ClassicContext for the request. BodyType is the type of the request body. Please do not use a pointer type as parameter.
// ClassicContext is used internally by Fuego. Please do not use a pointer type as parameter.
type ClassicContext struct {
request *http.Request
response http.ResponseWriter
pathParams map[string]string

fs fs.FS
templates *template.Template
templatesParsed bool
fs fs.FS
templates *template.Template

readOptions readOptions
}

func (c ClassicContext) Body() (any, error) {
panic("this method should not be called. It probably happened because you passed the context to another controller with the Pass method.")
}

func (c ClassicContext) MustBody() any {
b, err := c.Body()
if err != nil {
Expand Down Expand Up @@ -146,8 +149,11 @@ type readOptions struct {
LogBody bool
}

var _ Ctx[any] = &Context[any]{} // Check that Context implements Ctx.
var _ Ctx[any] = &ClassicContext{} // Check that Context implements Ctx.
var (
_ Ctx[any] = &Context[any]{} // Check that Context implements Ctx.
_ Ctx[string] = &Context[string]{} // Check that Context implements Ctx.
_ Ctx[any] = &ClassicContext{} // Check that Context implements Ctx.
)

// Context returns the context of the request.
// Same as c.Request().Context().
Expand All @@ -174,14 +180,13 @@ func (c ClassicContext) Pass() ClassicContext {
// the need to parse the templates on each request but also preventing
// to dynamically use new templates.
func (c ClassicContext) Render(templateToExecute string, data any, layoutsGlobs ...string) (HTML, error) {
if !c.templatesParsed &&
(strings.Contains(templateToExecute, "/") || strings.Contains(templateToExecute, "*")) {
if strings.Contains(templateToExecute, "/") || strings.Contains(templateToExecute, "*") {

layoutsGlobs = append(layoutsGlobs, templateToExecute) // To override all blocks defined in the main template
cloned := template.Must(c.templates.Clone())
tmpl, err := cloned.ParseFS(c.fs, layoutsGlobs...)
if err != nil {
return "", ErrorResponse{
return "", HTTPError{
StatusCode: http.StatusInternalServerError,
Message: fmt.Errorf("error parsing template '%s': %w", layoutsGlobs, err).Error(),
MoreInfo: map[string]any{
Expand All @@ -191,7 +196,6 @@ func (c ClassicContext) Render(templateToExecute string, data any, layoutsGlobs
}
}
c.templates = template.Must(tmpl.Clone())
c.templatesParsed = true
}

// Get only last template name (for example, with partials/nav/main/nav.partial.html, get nav.partial.html)
Expand All @@ -201,7 +205,7 @@ func (c ClassicContext) Render(templateToExecute string, data any, layoutsGlobs
c.response.Header().Set("Content-Type", "text/html; charset=utf-8")
err := c.templates.ExecuteTemplate(c.response, templateToExecute, data)
if err != nil {
return "", ErrorResponse{
return "", HTTPError{
StatusCode: http.StatusInternalServerError,
Message: fmt.Errorf("error executing template '%s': %w", templateToExecute, err).Error(),
MoreInfo: map[string]any{
Expand Down
10 changes: 5 additions & 5 deletions deserialization.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,18 +45,18 @@ func readJSON[B any](input io.Reader, options readOptions) (B, error) {
}
err := dec.Decode(&body)
if err != nil {
return body, fmt.Errorf("cannot decode request body: %w", err)
return body, BadRequestError{Message: "cannot decode request body: " + err.Error()}
}
slog.Debug("Decoded body", "body", body)

body, err = transform(body)
if err != nil {
return body, fmt.Errorf("cannot transform request body: %w", err)
return body, BadRequestError{Message: "cannot transform request body: " + err.Error()}
}

err = validate(body)
if err != nil {
return body, fmt.Errorf("cannot validate request body: %w", err)
return body, BadRequestError{Message: "cannot validate request body: " + err.Error()}
}

return body, nil
Expand All @@ -73,7 +73,7 @@ func readString[B ~string](input io.Reader, options readOptions) (B, error) {
// Read the request body.
readBody, err := io.ReadAll(input)
if err != nil {
return "", fmt.Errorf("cannot read request body: %w", err)
return "", BadRequestError{Message: "cannot read request body: " + err.Error()}
}

body := B(readBody)
Expand Down Expand Up @@ -150,7 +150,7 @@ func transform[B any](body B) (B, error) {
if inTransformerBody, ok := any(&body).(InTransformer); ok {
err := inTransformerBody.InTransform()
if err != nil {
return body, fmt.Errorf("cannot transform request body: %w", err)
return body, BadRequestError{Message: "cannot transform request body: " + err.Error()}
}
body = *any(inTransformerBody).(*B)

Expand Down
6 changes: 3 additions & 3 deletions deserialization_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ func TestReadJSON(t *testing.T) {

t.Run("cannot read invalid JSON", func(t *testing.T) {
_, err := ReadJSON[TestBody](input)
require.Error(t, err)
require.ErrorAs(t, err, &BadRequestError{}, "Expected a BadRequestError")
})

t.Run("cannot deserialize JSON to wrong struct", func(t *testing.T) {
Expand All @@ -36,7 +36,7 @@ func TestReadJSON(t *testing.T) {
// Missing C bool
}
_, err := ReadJSON[WrongBody](input)
require.Error(t, err)
require.ErrorAs(t, err, &BadRequestError{}, "Expected a BadRequestError")
})
}

Expand Down Expand Up @@ -127,7 +127,7 @@ func TestInTransformStringWithError(t *testing.T) {
t.Run("ReadString", func(t *testing.T) {
input := strings.NewReader(`coucou`)
body, err := ReadString[transformableStringWithError](input)
require.Error(t, err)
require.ErrorAs(t, err, &BadRequestError{}, "Expected a BadRequestError")
require.Equal(t, transformableStringWithError("transformed coucou"), body)
})
}
Expand Down
44 changes: 35 additions & 9 deletions errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,37 +20,63 @@ type ErrorWithInfo interface {
Info() map[string]any
}

// ErrorResponse is the error response used by the serialization part of the framework.
type ErrorResponse struct {
// HTTPError is the error response used by the serialization part of the framework.
type HTTPError struct {
Err error `json:",omitempty"` // backend developer readable error message
Message string `json:"error" xml:"Error"` // human readable error message
StatusCode int `json:"-" xml:"-"` // http status code
MoreInfo map[string]any `json:"info,omitempty" xml:"Info,omitempty"` // additional info
}

func (e ErrorResponse) Error() string {
var (
_ ErrorWithInfo = HTTPError{}
_ ErrorWithStatus = HTTPError{}
)

func (e HTTPError) Error() string {
return e.Message
}

var _ ErrorWithStatus = ErrorResponse{}
func (e HTTPError) Info() map[string]any {
return e.MoreInfo
}

func (e ErrorResponse) Status() int {
func (e HTTPError) Status() int {
if e.StatusCode == 0 {
return http.StatusInternalServerError
}
return e.StatusCode
}

var _ ErrorWithInfo = ErrorResponse{}
// BadRequestError is an error used to return a 400 status code.
type BadRequestError struct {
Err error // developer readable error message
Message string `json:"error" xml:"Error"` // human readable error message
MoreInfo map[string]any `json:"info,omitempty" xml:"Info,omitempty"` // additional info
}

var (
_ ErrorWithInfo = BadRequestError{}
_ ErrorWithStatus = BadRequestError{}
)

func (e BadRequestError) Error() string {
return e.Message
}

func (e ErrorResponse) Info() map[string]any {
func (e BadRequestError) Info() map[string]any {
return e.MoreInfo
}

func (e BadRequestError) Status() int {
return http.StatusBadRequest
}

// ErrorHandler is the default error handler used by the framework.
// It transforms any error into the unified error type [ErrorResponse],
// It transforms any error into the unified error type [HTTPError],
// Using the [ErrorWithStatus] and [ErrorWithInfo] interfaces.
func ErrorHandler(err error) error {
errResponse := ErrorResponse{
errResponse := HTTPError{
Message: err.Error(),
}

Expand Down
20 changes: 10 additions & 10 deletions errors_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,35 +20,35 @@ func TestErrorHandler(t *testing.T) {
err := errors.New("test error")

errResponse := ErrorHandler(err)
require.ErrorAs(t, errResponse, &ErrorResponse{})
require.ErrorAs(t, errResponse, &HTTPError{})
require.Equal(t, "test error", errResponse.Error())
require.Equal(t, http.StatusInternalServerError, errResponse.(ErrorResponse).Status())
require.Nil(t, errResponse.(ErrorResponse).Info())
require.Equal(t, http.StatusInternalServerError, errResponse.(HTTPError).Status())
require.Nil(t, errResponse.(HTTPError).Info())
})

t.Run("error with status ", func(t *testing.T) {
err := myError{
status: http.StatusNotFound,
}
errResponse := ErrorHandler(err)
require.ErrorAs(t, errResponse, &ErrorResponse{})
require.ErrorAs(t, errResponse, &HTTPError{})
require.Equal(t, "test error", errResponse.Error())
require.Equal(t, http.StatusNotFound, errResponse.(ErrorResponse).Status())
require.Nil(t, errResponse.(ErrorResponse).Info())
require.Equal(t, http.StatusNotFound, errResponse.(HTTPError).Status())
require.Nil(t, errResponse.(HTTPError).Info())
})

t.Run("error with status and info", func(t *testing.T) {
err := ErrorResponse{
err := HTTPError{
Message: "test error",
StatusCode: http.StatusNotFound,
MoreInfo: map[string]any{
"test": "info",
},
}
errResponse := ErrorHandler(err)
require.ErrorAs(t, errResponse, &ErrorResponse{})
require.ErrorAs(t, errResponse, &HTTPError{})
require.Equal(t, "test error", errResponse.Error())
require.Equal(t, http.StatusNotFound, errResponse.(ErrorResponse).Status())
require.NotNil(t, errResponse.(ErrorResponse).Info())
require.Equal(t, http.StatusNotFound, errResponse.(HTTPError).Status())
require.NotNil(t, errResponse.(HTTPError).Info())
})
}
11 changes: 11 additions & 0 deletions examples/simple-crud/errors_custom.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package main

import "net/http"

type MyError struct {
Err error // developer readable error message
}

func (e MyError) Status() int {
return http.StatusTeapot
}
1 change: 0 additions & 1 deletion examples/simple-crud/store/ingredient.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,5 +60,4 @@ func (i Ingredient) Months() string {
}

return strings.Join(months, ", ")

}
1 change: 0 additions & 1 deletion examples/simple-crud/views/admin.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,6 @@ func (rs Ressource) adminAddDosing(c fuego.Ctx[store.CreateDosingParams]) (any,
}

func (rs Ressource) adminIngredients(c fuego.Ctx[any]) (any, error) {

searchParams := components.SearchParams{
Name: c.QueryParam("name"),
PerPage: c.QueryParamInt("perPage", 10),
Expand Down
2 changes: 1 addition & 1 deletion middleware/basicauth/basicauth.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ func New(config Config) func(http.Handler) http.Handler {
return
}

err := fuego.ErrorResponse{
err := fuego.HTTPError{
Message: "unauthorized",
StatusCode: http.StatusUnauthorized,
}
Expand Down
2 changes: 1 addition & 1 deletion options_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,6 @@ func TestWithXML(t *testing.T) {

require.Equal(t, 500, recorder.Code)
require.Equal(t, "application/xml", recorder.Header().Get("Content-Type"))
require.Equal(t, "<ErrorResponse><Error>error</Error></ErrorResponse>", recorder.Body.String())
require.Equal(t, "<HTTPError><Error>error</Error></HTTPError>", recorder.Body.String())
})
}
2 changes: 1 addition & 1 deletion serialization.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ func SendJSON(w http.ResponseWriter, ans any) {
// If the error implements ErrorWithStatus, the status code will be set.
func SendJSONError(w http.ResponseWriter, err error) {
status := http.StatusInternalServerError
errorStatus := ErrorResponse{
errorStatus := HTTPError{
Message: err.Error(),
}
if errors.As(err, &errorStatus) {
Expand Down
4 changes: 2 additions & 2 deletions serialization_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,11 @@ func TestXML(t *testing.T) {

t.Run("can serialize xml error", func(t *testing.T) {
w := httptest.NewRecorder()
err := ErrorResponse{Message: "Hello World"}
err := HTTPError{Message: "Hello World"}
SendXMLError(w, err)
body := w.Body.String()

require.Equal(t, `<ErrorResponse><Error>Hello World</Error></ErrorResponse>`, body)
require.Equal(t, `<HTTPError><Error>Hello World</Error></HTTPError>`, body)
})
}

Expand Down

0 comments on commit 5a961ee

Please sign in to comment.