From e25457b8e94fb1860adc5cf67368b4043551d69f Mon Sep 17 00:00:00 2001 From: Mohammad Alian Date: Thu, 12 Aug 2021 17:54:48 +0900 Subject: [PATCH] return first if response is already committed in DefaultHTTPErrorHandler --- echo.go | 21 ++++++++++++--------- echo_test.go | 29 +++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 9 deletions(-) diff --git a/echo.go b/echo.go index 246a62256..a28fa0c1a 100644 --- a/echo.go +++ b/echo.go @@ -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 { @@ -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) } } diff --git a/echo_test.go b/echo_test.go index dc553490b..5868bc50e 100644 --- a/echo_test.go +++ b/echo_test.go @@ -17,6 +17,7 @@ import ( "testing" "time" + `github.com/labstack/gommon/log` "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "golang.org/x/net/http2" @@ -252,6 +253,34 @@ func TestEchoFile(t *testing.T) { assert.NotEmpty(t, b) } +func TestEchoErrorHandlerEarlyReturn(t *testing.T) { + e := New() + + // Routes + e.GET("/", func(c Context) error { + c.String(http.StatusOK, "OK") + return errors.New("ERROR") + }) + + c, b := request(http.MethodGet, "/", e) + assert.Equal(t, http.StatusOK, c) + assert.Equal(t, "OK", b) +} + +func TestEchoErrorHandlerInternalError(t *testing.T) { + e := New() + err := errors.New("internal error") + e.Logger.SetLevel(log.DEBUG) + // Routes + e.GET("/", func(c Context) error { + return NewHTTPError(http.StatusBadRequest).SetInternal(err) + }) + + c, b := request(http.MethodGet, "/", e) + assert.Equal(t, http.StatusBadRequest, c) + assert.Equal(t, `{"message":"Bad Request"}`+"\n", b) +} + func TestEchoMiddleware(t *testing.T) { e := New() buf := new(bytes.Buffer)