Remove httputil.LogThenError so that the line numbers are reported pr…

…operly - make error reporting slightly more useful (#879)

clientapi/auth/auth.go

@@ -26,7 +26,6 @@ import (
 
 	"github.com/matrix-org/dendrite/appservice/types"
 	"github.com/matrix-org/dendrite/clientapi/auth/authtypes"
-	"github.com/matrix-org/dendrite/clientapi/httputil"
 	"github.com/matrix-org/dendrite/clientapi/jsonerror"
 	"github.com/matrix-org/dendrite/clientapi/userutil"
 	"github.com/matrix-org/dendrite/common/config"
@@ -166,7 +165,8 @@ func verifyAccessToken(req *http.Request, deviceDB DeviceDatabase) (device *auth
 				JSON: jsonerror.UnknownToken("Unknown token"),
 			}
 		} else {
-			jsonErr := httputil.LogThenError(req, err)
+			util.GetLogger(req.Context()).WithError(err).Error("deviceDB.GetDeviceByAccessToken failed")
+			jsonErr := jsonerror.InternalServerError()
 			resErr = &jsonErr
 		}
 	}

clientapi/httputil/httputil.go

@@ -36,11 +36,3 @@ 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()
-}

clientapi/routing/account_data.go

@@ -20,7 +20,6 @@ import (
 
 	"github.com/matrix-org/dendrite/clientapi/auth/authtypes"
 	"github.com/matrix-org/dendrite/clientapi/auth/storage/accounts"
-	"github.com/matrix-org/dendrite/clientapi/httputil"
 	"github.com/matrix-org/dendrite/clientapi/jsonerror"
 	"github.com/matrix-org/dendrite/clientapi/producers"
 	"github.com/matrix-org/gomatrixserverlib"
@@ -42,7 +41,8 @@ func GetAccountData(
 
 	localpart, _, err := gomatrixserverlib.SplitID('@', userID)
 	if err != nil {
-		return httputil.LogThenError(req, err)
+		util.GetLogger(req.Context()).WithError(err).Error("gomatrixserverlib.SplitID failed")
+		return jsonerror.InternalServerError()
 	}
 
 	if data, err := accountDB.GetAccountDataByType(
@@ -74,24 +74,28 @@ func SaveAccountData(
 
 	localpart, _, err := gomatrixserverlib.SplitID('@', userID)
 	if err != nil {
-		return httputil.LogThenError(req, err)
+		util.GetLogger(req.Context()).WithError(err).Error("gomatrixserverlib.SplitID failed")
+		return jsonerror.InternalServerError()
 	}
 
 	defer req.Body.Close() // nolint: errcheck
 
 	body, err := ioutil.ReadAll(req.Body)
 	if err != nil {
-		return httputil.LogThenError(req, err)
+		util.GetLogger(req.Context()).WithError(err).Error("ioutil.ReadAll failed")
+		return jsonerror.InternalServerError()
 	}
 
 	if err := accountDB.SaveAccountData(
 		req.Context(), localpart, roomID, dataType, string(body),
 	); err != nil {
-		return httputil.LogThenError(req, err)
+		util.GetLogger(req.Context()).WithError(err).Error("accountDB.SaveAccountData failed")
+		return jsonerror.InternalServerError()
 	}
 
 	if err := syncProducer.SendData(userID, roomID, dataType); err != nil {
-		return httputil.LogThenError(req, err)
+		util.GetLogger(req.Context()).WithError(err).Error("syncProducer.SendData failed")
+		return jsonerror.InternalServerError()
 	}
 
 	return util.JSONResponse{

clientapi/routing/auth_fallback.go

@@ -19,7 +19,6 @@ import (
 	"net/http"
 
 	"github.com/matrix-org/dendrite/clientapi/auth/authtypes"
-	"github.com/matrix-org/dendrite/clientapi/httputil"
 	"github.com/matrix-org/dendrite/clientapi/jsonerror"
 	"github.com/matrix-org/dendrite/common/config"
 	"github.com/matrix-org/util"
@@ -151,7 +150,8 @@ func AuthFallback(
 			clientIP := req.RemoteAddr
 			err := req.ParseForm()
 			if err != nil {
-				res := httputil.LogThenError(req, err)
+				util.GetLogger(req.Context()).WithError(err).Error("req.ParseForm failed")
+				res := jsonerror.InternalServerError()
 				return &res
 			}
 
@@ -203,7 +203,8 @@ func writeHTTPMessage(
 	w.WriteHeader(header)
 	_, err := w.Write([]byte(message))
 	if err != nil {
-		res := httputil.LogThenError(req, err)
+		util.GetLogger(req.Context()).WithError(err).Error("w.Write failed")
+		res := jsonerror.InternalServerError()
 		return &res
 	}
 	return nil

clientapi/routing/capabilities.go

@@ -17,7 +17,7 @@ package routing
 import (
 	"net/http"
 
-	"github.com/matrix-org/dendrite/clientapi/httputil"
+	"github.com/matrix-org/dendrite/clientapi/jsonerror"
 	roomserverAPI "github.com/matrix-org/dendrite/roomserver/api"
 
 	"github.com/matrix-org/util"
@@ -35,7 +35,8 @@ func GetCapabilities(
 		&roomVersionsQueryReq,
 		&roomVersionsQueryRes,
 	); err != nil {
-		return httputil.LogThenError(req, err)
+		util.GetLogger(req.Context()).WithError(err).Error("queryAPI.QueryRoomVersionCapabilities failed")
+		return jsonerror.InternalServerError()
 	}
 
 	response := map[string]interface{}{

clientapi/routing/createroom.go

@@ -193,7 +193,8 @@ func createRoom(
 
 	profile, err := appserviceAPI.RetrieveUserProfile(req.Context(), userID, asAPI, accountDB)
 	if err != nil {
-		return httputil.LogThenError(req, err)
+		util.GetLogger(req.Context()).WithError(err).Error("appserviceAPI.RetrieveUserProfile failed")
+		return jsonerror.InternalServerError()
 	}
 
 	membershipContent := gomatrixserverlib.MemberContent{
@@ -276,33 +277,38 @@ func createRoom(
 		}
 		err = builder.SetContent(e.Content)
 		if err != nil {
-			return httputil.LogThenError(req, err)
+			util.GetLogger(req.Context()).WithError(err).Error("builder.SetContent failed")
+			return jsonerror.InternalServerError()
 		}
 		if i > 0 {
 			builder.PrevEvents = []gomatrixserverlib.EventReference{builtEvents[i-1].EventReference()}
 		}
 		var ev *gomatrixserverlib.Event
 		ev, err = buildEvent(&builder, &authEvents, cfg, evTime)
 		if err != nil {
-			return httputil.LogThenError(req, err)
+			util.GetLogger(req.Context()).WithError(err).Error("buildEvent failed")
+			return jsonerror.InternalServerError()
 		}
 
 		if err = gomatrixserverlib.Allowed(*ev, &authEvents); err != nil {
-			return httputil.LogThenError(req, err)
+			util.GetLogger(req.Context()).WithError(err).Error("gomatrixserverlib.Allowed failed")
+			return jsonerror.InternalServerError()
 		}
 
 		// Add the event to the list of auth events
 		builtEvents = append(builtEvents, *ev)
 		err = authEvents.AddEvent(ev)
 		if err != nil {
-			return httputil.LogThenError(req, err)
+			util.GetLogger(req.Context()).WithError(err).Error("authEvents.AddEvent failed")
+			return jsonerror.InternalServerError()
 		}
 	}
 
 	// send events to the room server
 	_, err = producer.SendEvents(req.Context(), builtEvents, cfg.Matrix.ServerName, nil)
 	if err != nil {
-		return httputil.LogThenError(req, err)
+		util.GetLogger(req.Context()).WithError(err).Error("producer.SendEvents failed")
+		return jsonerror.InternalServerError()
 	}
 
 	// TODO(#269): Reserve room alias while we create the room. This stops us
@@ -321,7 +327,8 @@ func createRoom(
 		var aliasResp roomserverAPI.SetRoomAliasResponse
 		err = aliasAPI.SetRoomAlias(req.Context(), &aliasReq, &aliasResp)
 		if err != nil {
-			return httputil.LogThenError(req, err)
+			util.GetLogger(req.Context()).WithError(err).Error("aliasAPI.SetRoomAlias failed")
+			return jsonerror.InternalServerError()
 		}
 
 		if aliasResp.AliasExists {

clientapi/routing/device.go

@@ -21,7 +21,6 @@ import (
 
 	"github.com/matrix-org/dendrite/clientapi/auth/authtypes"
 	"github.com/matrix-org/dendrite/clientapi/auth/storage/devices"
-	"github.com/matrix-org/dendrite/clientapi/httputil"
 	"github.com/matrix-org/dendrite/clientapi/jsonerror"
 	"github.com/matrix-org/gomatrixserverlib"
 	"github.com/matrix-org/util"
@@ -51,7 +50,8 @@ func GetDeviceByID(
 ) util.JSONResponse {
 	localpart, _, err := gomatrixserverlib.SplitID('@', device.UserID)
 	if err != nil {
-		return httputil.LogThenError(req, err)
+		util.GetLogger(req.Context()).WithError(err).Error("gomatrixserverlib.SplitID failed")
+		return jsonerror.InternalServerError()
 	}
 
 	ctx := req.Context()
@@ -62,7 +62,8 @@ func GetDeviceByID(
 			JSON: jsonerror.NotFound("Unknown device"),
 		}
 	} else if err != nil {
-		return httputil.LogThenError(req, err)
+		util.GetLogger(req.Context()).WithError(err).Error("deviceDB.GetDeviceByID failed")
+		return jsonerror.InternalServerError()
 	}
 
 	return util.JSONResponse{
@@ -80,14 +81,16 @@ func GetDevicesByLocalpart(
 ) util.JSONResponse {
 	localpart, _, err := gomatrixserverlib.SplitID('@', device.UserID)
 	if err != nil {
-		return httputil.LogThenError(req, err)
+		util.GetLogger(req.Context()).WithError(err).Error("gomatrixserverlib.SplitID failed")
+		return jsonerror.InternalServerError()
 	}
 
 	ctx := req.Context()
 	deviceList, err := deviceDB.GetDevicesByLocalpart(ctx, localpart)
 
 	if err != nil {
-		return httputil.LogThenError(req, err)
+		util.GetLogger(req.Context()).WithError(err).Error("deviceDB.GetDevicesByLocalpart failed")
+		return jsonerror.InternalServerError()
 	}
 
 	res := devicesJSON{}
@@ -112,7 +115,8 @@ func UpdateDeviceByID(
 ) util.JSONResponse {
 	localpart, _, err := gomatrixserverlib.SplitID('@', device.UserID)
 	if err != nil {
-		return httputil.LogThenError(req, err)
+		util.GetLogger(req.Context()).WithError(err).Error("gomatrixserverlib.SplitID failed")
+		return jsonerror.InternalServerError()
 	}
 
 	ctx := req.Context()
@@ -123,7 +127,8 @@ func UpdateDeviceByID(
 			JSON: jsonerror.NotFound("Unknown device"),
 		}
 	} else if err != nil {
-		return httputil.LogThenError(req, err)
+		util.GetLogger(req.Context()).WithError(err).Error("deviceDB.GetDeviceByID failed")
+		return jsonerror.InternalServerError()
 	}
 
 	if dev.UserID != device.UserID {
@@ -138,11 +143,13 @@ func UpdateDeviceByID(
 	payload := deviceUpdateJSON{}
 
 	if err := json.NewDecoder(req.Body).Decode(&payload); err != nil {
-		return httputil.LogThenError(req, err)
+		util.GetLogger(req.Context()).WithError(err).Error("json.NewDecoder.Decode failed")
+		return jsonerror.InternalServerError()
 	}
 
 	if err := deviceDB.UpdateDevice(ctx, localpart, deviceID, payload.DisplayName); err != nil {
-		return httputil.LogThenError(req, err)
+		util.GetLogger(req.Context()).WithError(err).Error("deviceDB.UpdateDevice failed")
+		return jsonerror.InternalServerError()
 	}
 
 	return util.JSONResponse{
@@ -158,14 +165,16 @@ func DeleteDeviceById(
 ) util.JSONResponse {
 	localpart, _, err := gomatrixserverlib.SplitID('@', device.UserID)
 	if err != nil {
-		return httputil.LogThenError(req, err)
+		util.GetLogger(req.Context()).WithError(err).Error("gomatrixserverlib.SplitID failed")
+		return jsonerror.InternalServerError()
 	}
 	ctx := req.Context()
 
 	defer req.Body.Close() // nolint: errcheck
 
 	if err := deviceDB.RemoveDevice(ctx, deviceID, localpart); err != nil {
-		return httputil.LogThenError(req, err)
+		util.GetLogger(req.Context()).WithError(err).Error("deviceDB.RemoveDevice failed")
+		return jsonerror.InternalServerError()
 	}
 
 	return util.JSONResponse{
@@ -180,20 +189,23 @@ func DeleteDevices(
 ) util.JSONResponse {
 	localpart, _, err := gomatrixserverlib.SplitID('@', device.UserID)
 	if err != nil {
-		return httputil.LogThenError(req, err)
+		util.GetLogger(req.Context()).WithError(err).Error("gomatrixserverlib.SplitID failed")
+		return jsonerror.InternalServerError()
 	}
 
 	ctx := req.Context()
 	payload := devicesDeleteJSON{}
 
 	if err := json.NewDecoder(req.Body).Decode(&payload); err != nil {
-		return httputil.LogThenError(req, err)
+		util.GetLogger(req.Context()).WithError(err).Error("json.NewDecoder.Decode failed")
+		return jsonerror.InternalServerError()
 	}
 
 	defer req.Body.Close() // nolint: errcheck
 
 	if err := deviceDB.RemoveDevices(ctx, localpart, payload.Devices); err != nil {
-		return httputil.LogThenError(req, err)
+		util.GetLogger(req.Context()).WithError(err).Error("deviceDB.RemoveDevices failed")
+		return jsonerror.InternalServerError()
 	}
 
 	return util.JSONResponse{

clientapi/routing/directory.go

@@ -63,7 +63,8 @@ func DirectoryRoom(
 	queryReq := roomserverAPI.GetRoomIDForAliasRequest{Alias: roomAlias}
 	var queryRes roomserverAPI.GetRoomIDForAliasResponse
 	if err = rsAPI.GetRoomIDForAlias(req.Context(), &queryReq, &queryRes); err != nil {
-		return httputil.LogThenError(req, err)
+		util.GetLogger(req.Context()).WithError(err).Error("rsAPI.GetRoomIDForAlias failed")
+		return jsonerror.InternalServerError()
 	}
 
 	res.RoomID = queryRes.RoomID
@@ -76,7 +77,8 @@ func DirectoryRoom(
 			if fedErr != nil {
 				// TODO: Return 502 if the remote server errored.
 				// TODO: Return 504 if the remote server timed out.
-				return httputil.LogThenError(req, fedErr)
+				util.GetLogger(req.Context()).WithError(err).Error("federation.LookupRoomAlias failed")
+				return jsonerror.InternalServerError()
 			}
 			res.RoomID = fedRes.RoomID
 			res.fillServers(fedRes.Servers)
@@ -94,7 +96,8 @@ func DirectoryRoom(
 		joinedHostsReq := federationSenderAPI.QueryJoinedHostServerNamesInRoomRequest{RoomID: res.RoomID}
 		var joinedHostsRes federationSenderAPI.QueryJoinedHostServerNamesInRoomResponse
 		if err = fedSenderAPI.QueryJoinedHostServerNamesInRoom(req.Context(), &joinedHostsReq, &joinedHostsRes); err != nil {
-			return httputil.LogThenError(req, err)
+			util.GetLogger(req.Context()).WithError(err).Error("fedSenderAPI.QueryJoinedHostServerNamesInRoom failed")
+			return jsonerror.InternalServerError()
 		}
 		res.fillServers(joinedHostsRes.ServerNames)
 	}
@@ -165,7 +168,8 @@ func SetLocalAlias(
 	}
 	var queryRes roomserverAPI.SetRoomAliasResponse
 	if err := aliasAPI.SetRoomAlias(req.Context(), &queryReq, &queryRes); err != nil {
-		return httputil.LogThenError(req, err)
+		util.GetLogger(req.Context()).WithError(err).Error("aliasAPI.SetRoomAlias failed")
+		return jsonerror.InternalServerError()
 	}
 
 	if queryRes.AliasExists {
@@ -194,7 +198,8 @@ func RemoveLocalAlias(
 	}
 	var creatorQueryRes roomserverAPI.GetCreatorIDForAliasResponse
 	if err := aliasAPI.GetCreatorIDForAlias(req.Context(), &creatorQueryReq, &creatorQueryRes); err != nil {
-		return httputil.LogThenError(req, err)
+		util.GetLogger(req.Context()).WithError(err).Error("aliasAPI.GetCreatorIDForAlias failed")
+		return jsonerror.InternalServerError()
 	}
 
 	if creatorQueryRes.UserID == "" {
@@ -218,7 +223,8 @@ func RemoveLocalAlias(
 	}
 	var queryRes roomserverAPI.RemoveRoomAliasResponse
 	if err := aliasAPI.RemoveRoomAlias(req.Context(), &queryReq, &queryRes); err != nil {
-		return httputil.LogThenError(req, err)
+		util.GetLogger(req.Context()).WithError(err).Error("aliasAPI.RemoveRoomAlias failed")
+		return jsonerror.InternalServerError()
 	}
 
 	return util.JSONResponse{