Skip to content

Commit

Permalink
return first if response is already committed in DefaultHTTPErrorHandler
Browse files Browse the repository at this point in the history
  • Loading branch information
Mohammad Alian authored and aldas committed Aug 12, 2021
1 parent 499097e commit 7d41537
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 9 deletions.
21 changes: 12 additions & 9 deletions echo.go
Original file line number Diff line number Diff line change
Expand Up @@ -358,6 +358,11 @@ func (e *Echo) Routers() map[string]*Router {
// DefaultHTTPErrorHandler is the default HTTP error handler. It sends a JSON response
// with status code.
func (e *Echo) DefaultHTTPErrorHandler(err error, c Context) {

if c.Response().Committed {
return
}

he, ok := err.(*HTTPError)
if ok {
if he.Internal != nil {
Expand All @@ -384,15 +389,13 @@ func (e *Echo) DefaultHTTPErrorHandler(err error, c Context) {
}

// Send response
if !c.Response().Committed {
if c.Request().Method == http.MethodHead { // Issue #608
err = c.NoContent(he.Code)
} else {
err = c.JSON(code, message)
}
if err != nil {
e.Logger.Error(err)
}
if c.Request().Method == http.MethodHead { // Issue #608
err = c.NoContent(he.Code)
} else {
err = c.JSON(code, message)
}
if err != nil {
e.Logger.Error(err)
}
}

Expand Down
17 changes: 17 additions & 0 deletions echo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1124,6 +1124,15 @@ func TestDefaultHTTPErrorHandler(t *testing.T) {
"error": "stackinfo",
})
})
e.Any("/early-return", func(c Context) error {
c.String(http.StatusOK, "OK")
return errors.New("ERROR")
})
e.GET("/internal-error", func(c Context) error {
err := errors.New("internal error message body")
return NewHTTPError(http.StatusBadRequest).SetInternal(err)
})

// With Debug=true plain response contains error message
c, b := request(http.MethodGet, "/plain", e)
assert.Equal(t, http.StatusInternalServerError, c)
Expand All @@ -1136,6 +1145,14 @@ func TestDefaultHTTPErrorHandler(t *testing.T) {
c, b = request(http.MethodGet, "/servererror", e)
assert.Equal(t, http.StatusInternalServerError, c)
assert.Equal(t, "{\n \"code\": 33,\n \"error\": \"stackinfo\",\n \"message\": \"Something bad happened\"\n}\n", b)
// if the body is already set HTTPErrorHandler should not add anything to response body
c, b = request(http.MethodGet, "/early-return", e)
assert.Equal(t, http.StatusOK, c)
assert.Equal(t, "OK", b)
// internal error should be reflected in the message
c, b = request(http.MethodGet, "/internal-error", e)
assert.Equal(t, http.StatusBadRequest, c)
assert.Equal(t, "{\n \"error\": \"code=400, message=Bad Request, internal=internal error message body\",\n \"message\": \"Bad Request\"\n}\n", b)

e.Debug = false
// With Debug=false the error response is shortened
Expand Down

0 comments on commit 7d41537

Please sign in to comment.