Description
Issue Description
While trying to write a custom HTTP error handler, I discovered a relatively new and seemingly incorrect semantic in the DefaultHTTPErrorHandler. I am still using echo v3, and the old semantic seems to be sensible, while the new one does not:
In the old semantic (I am on v3.3.9), the code does this:
if he, ok := err.(*HTTPError); ok {
code = he.Code
msg = he.Message
if he.Internal != nil {
err = fmt.Errorf("%v, %v", err, he.Internal)
}
} else if e.Debug {
msg = err.Error()
} else {
msg = http.StatusText(code)
}
if _, ok := msg.(string); ok {
msg = Map{"message": msg}
}
That is to say, if the error is an HTTPError, then we use its Code and Message. If the error is some other type of error, then in debug mode, we use err.Error(). This seems reasonable. It's useful to rember that HTTPError.Message is of type interface{}. And so it is reasonable to pass it anything that could be marshalled to JSON.
In the new code, (I am testing with v4.1.13), introduced by commit ed51400 the logic is different:
he, ok := err.(*HTTPError)
if ok {
if he.Internal != nil {
if herr, ok := he.Internal.(*HTTPError); ok {
he = herr
}
}
} else {
he = &HTTPError{
Code: http.StatusInternalServerError,
Message: http.StatusText(http.StatusInternalServerError),
}
}
// Issue #1426
code := he.Code
message := he.Message
if e.Debug { // <-------------------- unconditional overwrite of message
message = err.Error()
} else if m, ok := message.(string); ok {
message = Map{"message": m}
}
This means that toggling on Debug ALWAYS overwrites message (which may be a map full of an arbitrary set of things). This seems to run counter to what a developer would reasonably expect.
Checklist
- Dependencies installed
- No typos
- Searched existing issues and docs
Expected behaviour
e.Debug should not modify the semantics of error messages which have a developer-supplied payload
Actual behaviour
e.Debug now wipes out customized error Message payload
Steps to reproduce
Run server below. Hit endpoint with e.g. curl http://localhost:9999
Working code to debug
package main
import (
"github.com/labstack/echo/v4"
)
func main() {
// Echo instance
e := echo.New()
e.Debug = true
// Routes
e.GET("/", hello)
// Start server
e.Logger.Fatal(e.Start(":9999"))
}
// Handler
func hello(c echo.Context) error {
myError := make(map[string]string)
myError["error"] = "Bad thing happened"
myError["reason"] = "no one knows why"
return echo.NewHTTPError(500, myError)
}In the above code, if e.Debug is set to False, then curl against this endpoint produces:
{"error":"Bad thing happened","reason":"no one knows why"}
Which seems to be the expected result. But when debug=True, it produces:
"code=500, message=map[error:Bad thing happened reason:no one knows why], internal=\u003cnil\u003e"
Which seems like an unexpected result-- the presence of Debug is changing the error semantics of API. This means that the Debug server can't be used with clients which expect to interpret specific fields from API errors.
Version/commit
4.1.13