From 8ccff1e40f2229679d807e08dbd037dea783b268 Mon Sep 17 00:00:00 2001 From: Kegsay Date: Fri, 10 Mar 2017 16:50:41 +0000 Subject: [PATCH] Log fatal errors at error level and return generic 500s (#34) Previously, the error responses: - were not valid matrix errors (no `errcode`) - returned the `err.Error()` message which may contain sensitive information. - did not get logged (at all, let alone set the level correctly). Now the error responses: - return valid matrix errors (`M_UNKNOWN`) - return a generic "Internal Server Error" string - get logged at `ERROR` level. --- .../matrix-org/dendrite/clientapi/httputil/httputil.go | 8 ++++++++ .../matrix-org/dendrite/clientapi/writers/createroom.go | 8 ++++---- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/src/github.com/matrix-org/dendrite/clientapi/httputil/httputil.go b/src/github.com/matrix-org/dendrite/clientapi/httputil/httputil.go index d1f6c4d5..255ad189 100644 --- a/src/github.com/matrix-org/dendrite/clientapi/httputil/httputil.go +++ b/src/github.com/matrix-org/dendrite/clientapi/httputil/httputil.go @@ -23,3 +23,11 @@ func UnmarshalJSONRequest(req *http.Request, iface interface{}) *util.JSONRespon } return nil } + +// LogThenError logs the given error then returns a matrix-compliant 500 internal server error response. +// This should be used to log fatal errors which require investigation. It should not be used +// to log client validation errors, etc. +func LogThenError(req *http.Request, err error) util.JSONResponse { + util.GetLogger(req.Context()).WithError(err).Error("request failed") + return jsonerror.InternalServerError() +} diff --git a/src/github.com/matrix-org/dendrite/clientapi/writers/createroom.go b/src/github.com/matrix-org/dendrite/clientapi/writers/createroom.go index 9f7f7efd..77c112c2 100644 --- a/src/github.com/matrix-org/dendrite/clientapi/writers/createroom.go +++ b/src/github.com/matrix-org/dendrite/clientapi/writers/createroom.go @@ -168,11 +168,11 @@ func createRoom(req *http.Request, cfg config.ClientAPI, roomID string, producer } ev, err := buildEvent(&builder, builtEventMap, cfg) if err != nil { - return util.ErrorResponse(err) + return httputil.LogThenError(req, err) } if err := gomatrixserverlib.Allowed(*ev, &authEvents); err != nil { - return util.ErrorResponse(err) + return httputil.LogThenError(req, err) } // Add the event to the list of auth events @@ -183,10 +183,10 @@ func createRoom(req *http.Request, cfg config.ClientAPI, roomID string, producer // send events to the room server msgs, err := eventsToMessages(builtEvents, cfg.ClientAPIOutputTopic) if err != nil { - return util.ErrorResponse(err) + return httputil.LogThenError(req, err) } if err = producer.SendMessages(msgs); err != nil { - return util.ErrorResponse(err) + return httputil.LogThenError(req, err) } return util.JSONResponse{