src-d
Logg original errors
context: https://github.com/src-d/code-annotation/pull/49#discussion_r164770518
When an error is returned by a handler.RequestProcessFunc, the original error should be logged in the STDERR
proposal:
serializer.NewHTTPError should wrap the original error, so handler.write could log it
+1 to the proposal with the wrapper.
Only one small idea, maybe you don't need to change serializer.NewHTTPError (it accepts status and message). Maybe it's better to create serializer.NewError(err error) which will set status to http.InternalServerError and title http.StatusText(InternalServerError).
Feel free to implement it in any other way though!
Thank you! More or less, yes, that would be the way I thought.
The only issue I see with setting the serializer.httpError.Status to http.InternalServerError for all errors created with serializer.NewError(err error), is that in some cases we're mapping the passed errors to any different http status (i.e. when strconv.Atoi fails converting the URL part, the error is http.StatusBadRequest instead of http.InternalServerError)
I'll ping you with a short proposal when ready, so we'll be able to discuss based on that :D
I checked the source code and it looks like we return correct error everywhere now. For our current use cases when we return not 500 we don't need to log the original error.
The wrapper itself still can be useful though. But because we don't have any place right now where we really need it, I would suggest to move this issue to backlog. @bzz
One thing I would propose is to keep the error messages in the log, but remove them from the 500 we send back to the client. Just in case some DB error includes data a user should not see.
Yeah. I meant to work like that. Misread code. Fix is here: https://github.com/src-d/code-annotation/pull/84