Skip to content

Commit fbfe216

Browse files
gnuletikgnuletik
authored
fix(DefaultHTTPErrorHandler): return error message when message is an error (#2456)
* fix(DefaultHTTPErrorHandler): return error message when message is an error
1 parent 8e425c0 commit fbfe216

File tree

2 files changed

+141
-60
lines changed

2 files changed

+141
-60
lines changed

‎echo.go

+8-1
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ package echo
3939
import (
4040
stdContext "context"
4141
"crypto/tls"
42+
"encoding/json"
4243
"errors"
4344
"fmt"
4445
"io"
@@ -438,12 +439,18 @@ func (e *Echo) DefaultHTTPErrorHandler(err error, c Context) {
438439
// Issue #1426
439440
code := he.Code
440441
message := he.Message
441-
if m, ok := he.Message.(string); ok {
442+
443+
switch m := he.Message.(type) {
444+
case string:
442445
if e.Debug {
443446
message = Map{"message": m, "error": err.Error()}
444447
} else {
445448
message = Map{"message": m}
446449
}
450+
case json.Marshaler:
451+
// do nothing - this type knows how to format itself to JSON
452+
case error:
453+
message = Map{"message": m.Error()}
447454
}
448455

449456
// Send response

‎echo_test.go

+133-59
Original file line numberDiff line numberDiff line change
@@ -1286,67 +1286,141 @@ func TestHTTPError_Unwrap(t *testing.T) {
12861286
})
12871287
}
12881288

1289+
type customError struct {
1290+
s string
1291+
}
1292+
1293+
func (ce *customError) MarshalJSON() ([]byte, error) {
1294+
return []byte(fmt.Sprintf(`{"x":"%v"}`, ce.s)), nil
1295+
}
1296+
1297+
func (ce *customError) Error() string {
1298+
return ce.s
1299+
}
1300+
12891301
func TestDefaultHTTPErrorHandler(t *testing.T) {
1290-
e := New()
1291-
e.Debug = true
1292-
e.Any("/plain", func(c Context) error {
1293-
return errors.New("an error occurred")
1294-
})
1295-
e.Any("/badrequest", func(c Context) error {
1296-
return NewHTTPError(http.StatusBadRequest, "Invalid request")
1297-
})
1298-
e.Any("/servererror", func(c Context) error {
1299-
return NewHTTPError(http.StatusInternalServerError, map[string]interface{}{
1300-
"code": 33,
1301-
"message": "Something bad happened",
1302-
"error": "stackinfo",
1303-
})
1304-
})
1305-
e.Any("/early-return", func(c Context) error {
1306-
err := c.String(http.StatusOK, "OK")
1307-
if err != nil {
1308-
assert.Fail(t, err.Error())
1309-
}
1310-
return errors.New("ERROR")
1311-
})
1312-
e.GET("/internal-error", func(c Context) error {
1313-
err := errors.New("internal error message body")
1314-
return NewHTTPError(http.StatusBadRequest).SetInternal(err)
1315-
})
1302+
var testCases = []struct {
1303+
name string
1304+
givenDebug bool
1305+
whenPath string
1306+
expectCode int
1307+
expectBody string
1308+
}{
1309+
{
1310+
name: "with Debug=true plain response contains error message",
1311+
givenDebug: true,
1312+
whenPath: "/plain",
1313+
expectCode: http.StatusInternalServerError,
1314+
expectBody: "{\n \"error\": \"an error occurred\",\n \"message\": \"Internal Server Error\"\n}\n",
1315+
},
1316+
{
1317+
name: "with Debug=true special handling for HTTPError",
1318+
givenDebug: true,
1319+
whenPath: "/badrequest",
1320+
expectCode: http.StatusBadRequest,
1321+
expectBody: "{\n \"error\": \"code=400, message=Invalid request\",\n \"message\": \"Invalid request\"\n}\n",
1322+
},
1323+
{
1324+
name: "with Debug=true complex errors are serialized to pretty JSON",
1325+
givenDebug: true,
1326+
whenPath: "/servererror",
1327+
expectCode: http.StatusInternalServerError,
1328+
expectBody: "{\n \"code\": 33,\n \"error\": \"stackinfo\",\n \"message\": \"Something bad happened\"\n}\n",
1329+
},
1330+
{
1331+
name: "with Debug=true if the body is already set HTTPErrorHandler should not add anything to response body",
1332+
givenDebug: true,
1333+
whenPath: "/early-return",
1334+
expectCode: http.StatusOK,
1335+
expectBody: "OK",
1336+
},
1337+
{
1338+
name: "with Debug=true internal error should be reflected in the message",
1339+
givenDebug: true,
1340+
whenPath: "/internal-error",
1341+
expectCode: http.StatusBadRequest,
1342+
expectBody: "{\n \"error\": \"code=400, message=Bad Request, internal=internal error message body\",\n \"message\": \"Bad Request\"\n}\n",
1343+
},
1344+
{
1345+
name: "with Debug=false the error response is shortened",
1346+
whenPath: "/plain",
1347+
expectCode: http.StatusInternalServerError,
1348+
expectBody: "{\"message\":\"Internal Server Error\"}\n",
1349+
},
1350+
{
1351+
name: "with Debug=false the error response is shortened",
1352+
whenPath: "/badrequest",
1353+
expectCode: http.StatusBadRequest,
1354+
expectBody: "{\"message\":\"Invalid request\"}\n",
1355+
},
1356+
{
1357+
name: "with Debug=false No difference for error response with non plain string errors",
1358+
whenPath: "/servererror",
1359+
expectCode: http.StatusInternalServerError,
1360+
expectBody: "{\"code\":33,\"error\":\"stackinfo\",\"message\":\"Something bad happened\"}\n",
1361+
},
1362+
{
1363+
name: "with Debug=false when httpError contains an error",
1364+
whenPath: "/error-in-httperror",
1365+
expectCode: http.StatusBadRequest,
1366+
expectBody: "{\"message\":\"error in httperror\"}\n",
1367+
},
1368+
{
1369+
name: "with Debug=false when httpError contains an error",
1370+
whenPath: "/customerror-in-httperror",
1371+
expectCode: http.StatusBadRequest,
1372+
expectBody: "{\"x\":\"custom error msg\"}\n",
1373+
},
1374+
}
1375+
for _, tc := range testCases {
1376+
t.Run(tc.name, func(t *testing.T) {
1377+
e := New()
1378+
e.Debug = tc.givenDebug // With Debug=true plain response contains error message
13161379

1317-
// With Debug=true plain response contains error message
1318-
c, b := request(http.MethodGet, "/plain", e)
1319-
assert.Equal(t, http.StatusInternalServerError, c)
1320-
assert.Equal(t, "{\n \"error\": \"an error occurred\",\n \"message\": \"Internal Server Error\"\n}\n", b)
1321-
// and special handling for HTTPError
1322-
c, b = request(http.MethodGet, "/badrequest", e)
1323-
assert.Equal(t, http.StatusBadRequest, c)
1324-
assert.Equal(t, "{\n \"error\": \"code=400, message=Invalid request\",\n \"message\": \"Invalid request\"\n}\n", b)
1325-
// complex errors are serialized to pretty JSON
1326-
c, b = request(http.MethodGet, "/servererror", e)
1327-
assert.Equal(t, http.StatusInternalServerError, c)
1328-
assert.Equal(t, "{\n \"code\": 33,\n \"error\": \"stackinfo\",\n \"message\": \"Something bad happened\"\n}\n", b)
1329-
// if the body is already set HTTPErrorHandler should not add anything to response body
1330-
c, b = request(http.MethodGet, "/early-return", e)
1331-
assert.Equal(t, http.StatusOK, c)
1332-
assert.Equal(t, "OK", b)
1333-
// internal error should be reflected in the message
1334-
c, b = request(http.MethodGet, "/internal-error", e)
1335-
assert.Equal(t, http.StatusBadRequest, c)
1336-
assert.Equal(t, "{\n \"error\": \"code=400, message=Bad Request, internal=internal error message body\",\n \"message\": \"Bad Request\"\n}\n", b)
1337-
1338-
e.Debug = false
1339-
// With Debug=false the error response is shortened
1340-
c, b = request(http.MethodGet, "/plain", e)
1341-
assert.Equal(t, http.StatusInternalServerError, c)
1342-
assert.Equal(t, "{\"message\":\"Internal Server Error\"}\n", b)
1343-
c, b = request(http.MethodGet, "/badrequest", e)
1344-
assert.Equal(t, http.StatusBadRequest, c)
1345-
assert.Equal(t, "{\"message\":\"Invalid request\"}\n", b)
1346-
// No difference for error response with non plain string errors
1347-
c, b = request(http.MethodGet, "/servererror", e)
1348-
assert.Equal(t, http.StatusInternalServerError, c)
1349-
assert.Equal(t, "{\"code\":33,\"error\":\"stackinfo\",\"message\":\"Something bad happened\"}\n", b)
1380+
e.Any("/plain", func(c Context) error {
1381+
return errors.New("an error occurred")
1382+
})
1383+
1384+
e.Any("/badrequest", func(c Context) error { // and special handling for HTTPError
1385+
return NewHTTPError(http.StatusBadRequest, "Invalid request")
1386+
})
1387+
1388+
e.Any("/servererror", func(c Context) error { // complex errors are serialized to pretty JSON
1389+
return NewHTTPError(http.StatusInternalServerError, map[string]interface{}{
1390+
"code": 33,
1391+
"message": "Something bad happened",
1392+
"error": "stackinfo",
1393+
})
1394+
})
1395+
1396+
// if the body is already set HTTPErrorHandler should not add anything to response body
1397+
e.Any("/early-return", func(c Context) error {
1398+
err := c.String(http.StatusOK, "OK")
1399+
if err != nil {
1400+
assert.Fail(t, err.Error())
1401+
}
1402+
return errors.New("ERROR")
1403+
})
1404+
1405+
// internal error should be reflected in the message
1406+
e.GET("/internal-error", func(c Context) error {
1407+
err := errors.New("internal error message body")
1408+
return NewHTTPError(http.StatusBadRequest).SetInternal(err)
1409+
})
1410+
1411+
e.GET("/error-in-httperror", func(c Context) error {
1412+
return NewHTTPError(http.StatusBadRequest, errors.New("error in httperror"))
1413+
})
1414+
1415+
e.GET("/customerror-in-httperror", func(c Context) error {
1416+
return NewHTTPError(http.StatusBadRequest, &customError{s: "custom error msg"})
1417+
})
1418+
1419+
c, b := request(http.MethodGet, tc.whenPath, e)
1420+
assert.Equal(t, tc.expectCode, c)
1421+
assert.Equal(t, tc.expectBody, b)
1422+
})
1423+
}
13501424
}
13511425

13521426
func TestEchoClose(t *testing.T) {

0 commit comments

Comments
 (0)