Repository: labstack/echo
Pull request #2456 (2023-05-26 17:59:40)

fix(DefaultHTTPErrorHandler): return error message when message is an error
The current behavior when returning a go `error` from a echo handler like this:

```go
return echo.NewHTTPError(status.BadRequest, errors.New("error in httperror"))
```

Is that echo returns an empty JSON object `{}`.

See the failing test result, without the fix:

```
    echo_test.go:1356:
        	Error Trace:	/~/echo/echo_test.go:1356
        	Error:      	Not equal:
        	            	expected: "{\"message\":\"error in httperror\"}\n"
        	            	actual  : "{}\n"

        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -1,2 +1,2 @@
        	            	-{"message":"error in httperror"}
        	            	+{}

        	Test:       	TestDefaultHTTPErrorHandler
```

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/labstack/echo/pull/2456?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=labstack) Report
Patch coverage: **`100.00`**% and no project coverage change.
> Comparison is base [(`8e425c0`)](https://app.codecov.io/gh/labstack/echo/commit/8e425c04311cc1efb896e7d5a7d7cbcafbf03a60?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=labstack) 92.85% compared to head [(`94f9c21`)](https://app.codecov.io/gh/labstack/echo/pull/2456?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=labstack) 92.86%.

<details><summary>Additional details and impacted files</summary>


```diff
@@           Coverage Diff           @@
##           master    #2456   +/-   ##
=======================================
  Coverage   92.85%   92.86%           
=======================================
  Files          39       39           
  Lines        4563     4567    +4     
=======================================
+ Hits         4237     4241    +4     
  Misses        237      237           
  Partials       89       89           
```


| [Impacted Files](https://app.codecov.io/gh/labstack/echo/pull/2456?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=labstack) | Coverage Δ | |
|---|---|---|
| [echo.go](https://app.codecov.io/gh/labstack/echo/pull/2456?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=labstack#diff-ZWNoby5nbw==) | `95.51% <100.00%> (+0.04%)` | :arrow_up: |


</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/labstack/echo/pull/2456?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=labstack).   
:loudspeaker: Do you have feedback about the report comment? [Let us know in this issue](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=labstack).


Patch:
diff --git a/echo.go b/echo.go
index 9028b7a..e216354 100644
--- a/echo.go
+++ b/echo.go
@@ -39,6 +39,7 @@ package echo
 import (
 	stdContext "context"
 	"crypto/tls"
+	"encoding/json"
 	"errors"
 	"fmt"
 	"io"
@@ -438,12 +439,18 @@ func (e *Echo) DefaultHTTPErrorHandler(err error, c Context) {
 	// Issue #1426
 	code := he.Code
 	message := he.Message
-	if m, ok := he.Message.(string); ok {
+
+	switch m := he.Message.(type) {
+	case string:
 		if e.Debug {
 			message = Map{"message": m, "error": err.Error()}
 		} else {
 			message = Map{"message": m}
 		}
+	case json.Marshaler:
+		// do nothing - this type knows how to format itself to JSON
+	case error:
+		message = Map{"message": m.Error()}
 	}
 
 	// Send response


Test patch:
diff --git a/echo_test.go b/echo_test.go
index eab25db..a352e40 100644
--- a/echo_test.go
+++ b/echo_test.go
@@ -1286,67 +1286,141 @@ func TestHTTPError_Unwrap(t *testing.T) {
 	})
 }
 
+type customError struct {
+	s string
+}
+
+func (ce *customError) MarshalJSON() ([]byte, error) {
+	return []byte(fmt.Sprintf(`{"x":"%v"}`, ce.s)), nil
+}
+
+func (ce *customError) Error() string {
+	return ce.s
+}
+
 func TestDefaultHTTPErrorHandler(t *testing.T) {
-	e := New()
-	e.Debug = true
-	e.Any("/plain", func(c Context) error {
-		return errors.New("an error occurred")
-	})
-	e.Any("/badrequest", func(c Context) error {
-		return NewHTTPError(http.StatusBadRequest, "Invalid request")
-	})
-	e.Any("/servererror", func(c Context) error {
-		return NewHTTPError(http.StatusInternalServerError, map[string]interface{}{
-			"code":    33,
-			"message": "Something bad happened",
-			"error":   "stackinfo",
-		})
-	})
-	e.Any("/early-return", func(c Context) error {
-		err := c.String(http.StatusOK, "OK")
-		if err != nil {
-			assert.Fail(t, err.Error())
-		}
-		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)
-	})
+	var testCases = []struct {
+		name       string
+		givenDebug bool
+		whenPath   string
+		expectCode int
+		expectBody string
+	}{
+		{
+			name:       "with Debug=true plain response contains error message",
+			givenDebug: true,
+			whenPath:   "/plain",
+			expectCode: http.StatusInternalServerError,
+			expectBody: "{\n  \"error\": \"an error occurred\",\n  \"message\": \"Internal Server Error\"\n}\n",
+		},
+		{
+			name:       "with Debug=true special handling for HTTPError",
+			givenDebug: true,
+			whenPath:   "/badrequest",
+			expectCode: http.StatusBadRequest,
+			expectBody: "{\n  \"error\": \"code=400, message=Invalid request\",\n  \"message\": \"Invalid request\"\n}\n",
+		},
+		{
+			name:       "with Debug=true complex errors are serialized to pretty JSON",
+			givenDebug: true,
+			whenPath:   "/servererror",
+			expectCode: http.StatusInternalServerError,
+			expectBody: "{\n  \"code\": 33,\n  \"error\": \"stackinfo\",\n  \"message\": \"Something bad happened\"\n}\n",
+		},
+		{
+			name:       "with Debug=true if the body is already set HTTPErrorHandler should not add anything to response body",
+			givenDebug: true,
+			whenPath:   "/early-return",
+			expectCode: http.StatusOK,
+			expectBody: "OK",
+		},
+		{
+			name:       "with Debug=true internal error should be reflected in the message",
+			givenDebug: true,
+			whenPath:   "/internal-error",
+			expectCode: http.StatusBadRequest,
+			expectBody: "{\n  \"error\": \"code=400, message=Bad Request, internal=internal error message body\",\n  \"message\": \"Bad Request\"\n}\n",
+		},
+		{
+			name:       "with Debug=false the error response is shortened",
+			whenPath:   "/plain",
+			expectCode: http.StatusInternalServerError,
+			expectBody: "{\"message\":\"Internal Server Error\"}\n",
+		},
+		{
+			name:       "with Debug=false the error response is shortened",
+			whenPath:   "/badrequest",
+			expectCode: http.StatusBadRequest,
+			expectBody: "{\"message\":\"Invalid request\"}\n",
+		},
+		{
+			name:       "with Debug=false No difference for error response with non plain string errors",
+			whenPath:   "/servererror",
+			expectCode: http.StatusInternalServerError,
+			expectBody: "{\"code\":33,\"error\":\"stackinfo\",\"message\":\"Something bad happened\"}\n",
+		},
+		{
+			name:       "with Debug=false when httpError contains an error",
+			whenPath:   "/error-in-httperror",
+			expectCode: http.StatusBadRequest,
+			expectBody: "{\"message\":\"error in httperror\"}\n",
+		},
+		{
+			name:       "with Debug=false when httpError contains an error",
+			whenPath:   "/customerror-in-httperror",
+			expectCode: http.StatusBadRequest,
+			expectBody: "{\"x\":\"custom error msg\"}\n",
+		},
+	}
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			e := New()
+			e.Debug = tc.givenDebug // With Debug=true plain response contains error message
 
-	// With Debug=true plain response contains error message
-	c, b := request(http.MethodGet, "/plain", e)
-	assert.Equal(t, http.StatusInternalServerError, c)
-	assert.Equal(t, "{\n  \"error\": \"an error occurred\",\n  \"message\": \"Internal Server Error\"\n}\n", b)
-	// and special handling for HTTPError
-	c, b = request(http.MethodGet, "/badrequest", e)
-	assert.Equal(t, http.StatusBadRequest, c)
-	assert.Equal(t, "{\n  \"error\": \"code=400, message=Invalid request\",\n  \"message\": \"Invalid request\"\n}\n", b)
-	// complex errors are serialized to pretty JSON
-	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
-	c, b = request(http.MethodGet, "/plain", e)
-	assert.Equal(t, http.StatusInternalServerError, c)
-	assert.Equal(t, "{\"message\":\"Internal Server Error\"}\n", b)
-	c, b = request(http.MethodGet, "/badrequest", e)
-	assert.Equal(t, http.StatusBadRequest, c)
-	assert.Equal(t, "{\"message\":\"Invalid request\"}\n", b)
-	// No difference for error response with non plain string errors
-	c, b = request(http.MethodGet, "/servererror", e)
-	assert.Equal(t, http.StatusInternalServerError, c)
-	assert.Equal(t, "{\"code\":33,\"error\":\"stackinfo\",\"message\":\"Something bad happened\"}\n", b)
+			e.Any("/plain", func(c Context) error {
+				return errors.New("an error occurred")
+			})
+
+			e.Any("/badrequest", func(c Context) error { // and special handling for HTTPError
+				return NewHTTPError(http.StatusBadRequest, "Invalid request")
+			})
+
+			e.Any("/servererror", func(c Context) error { // complex errors are serialized to pretty JSON
+				return NewHTTPError(http.StatusInternalServerError, map[string]interface{}{
+					"code":    33,
+					"message": "Something bad happened",
+					"error":   "stackinfo",
+				})
+			})
+
+			// if the body is already set HTTPErrorHandler should not add anything to response body
+			e.Any("/early-return", func(c Context) error {
+				err := c.String(http.StatusOK, "OK")
+				if err != nil {
+					assert.Fail(t, err.Error())
+				}
+				return errors.New("ERROR")
+			})
+
+			// internal error should be reflected in the message
+			e.GET("/internal-error", func(c Context) error {
+				err := errors.New("internal error message body")
+				return NewHTTPError(http.StatusBadRequest).SetInternal(err)
+			})
+
+			e.GET("/error-in-httperror", func(c Context) error {
+				return NewHTTPError(http.StatusBadRequest, errors.New("error in httperror"))
+			})
+
+			e.GET("/customerror-in-httperror", func(c Context) error {
+				return NewHTTPError(http.StatusBadRequest, &customError{s: "custom error msg"})
+			})
+
+			c, b := request(http.MethodGet, tc.whenPath, e)
+			assert.Equal(t, tc.expectCode, c)
+			assert.Equal(t, tc.expectBody, b)
+		})
+	}
 }
 
 func TestEchoClose(t *testing.T) {