return first if response is already committed in DefaultHTTPErrorHandler

Author: Mohammad Alian

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

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)