echo icon indicating copy to clipboard operation
echo copied to clipboard

DefaultHTTPErrorHandler: return error if marshalling JSON fails

Open georgmu opened this issue 4 years ago • 1 comments

Before this commit, when passing a message to echo.NewHTTPError() that is not serializable, no error is returned, but a 200 Ok response with no content. I triggered this behavior by accident when using err.Error instead of err.Error() as message.

I added test cases which trigger the problem for both c.JSON() as well as echo.NewHttpError(). With the fix, both of them now return the same result (500 Internal Server Error).

One thing open is to maybe close the whole connection if even the sending of the new error message fails.

georgmu avatar Nov 05 '21 09:11 georgmu

This is probably not that easy fix. c.JSON can fail because of other reasons other than object not being serializable. ala something wrong has happened to response writer. In that case we would pretty much try now write twice to broken writer and log twice that problem.

Probably DefaultJSONSerializer should check error type and if that error is type of json.UnsupportedTypeError then return some Echo specific error type that global handler knows to be worth handling.

aldas avatar Nov 14 '21 18:11 aldas