Address review comments

clientapi/routing/directory.go

@@ -236,8 +236,8 @@ func RemoveLocalAlias(
 	validRoomID, err := spec.NewRoomID(roomIDRes.RoomID)
 	if err != nil {
 		return util.JSONResponse{
-			Code: http.StatusNotFound,
+			Code: http.StatusBadRequest,
-			JSON: spec.NotFound("The alias does not exist."),
+			JSON: spec.BadJSON("The roomID is invalid."),
 		}
 	}
 	deviceSenderID, err := rsAPI.QuerySenderIDForUser(req.Context(), *validRoomID, *userID)

roomserver/internal/input/input_events.go

@@ -840,16 +840,17 @@ func (r *Inputer) kickGuests(ctx context.Context, event gomatrixserverlib.PDU, r
 		return err
 	}
 
+	validRoomID, err := spec.NewRoomID(event.RoomID())
+	if err != nil {
+		return err
+	}
+
 	prevEvents := latestRes.LatestEvents
 	for _, memberEvent := range memberEvents {
 		if memberEvent.StateKey() == nil {
 			continue
 		}
 
-		validRoomID, err := spec.NewRoomID(memberEvent.RoomID())
-		if err != nil {
-			continue
-		}
 		memberUserID, err := r.Queryer.QueryUserIDForSender(ctx, *validRoomID, spec.SenderID(*memberEvent.StateKey()))
 		if err != nil {
 			continue

roomserver/internal/perform/perform_create_room.go

@@ -66,9 +66,9 @@ func (c *Creator) PerformCreateRoom(ctx context.Context, userID spec.UserID, roo
 	var senderID spec.SenderID
 	if createRequest.RoomVersion == gomatrixserverlib.RoomVersionPseudoIDs {
 		// create user room key if needed
-		key, err := c.RSAPI.GetOrCreateUserRoomPrivateKey(ctx, userID, roomID)
+		key, keyErr := c.RSAPI.GetOrCreateUserRoomPrivateKey(ctx, userID, roomID)
-		if err != nil {
+		if keyErr != nil {
-			util.GetLogger(ctx).WithError(err).Error("GetOrCreateUserRoomPrivateKey failed")
+			util.GetLogger(ctx).WithError(keyErr).Error("GetOrCreateUserRoomPrivateKey failed")
 			return "", &util.JSONResponse{
 				Code: http.StatusInternalServerError,
 				JSON: spec.InternalServerError{},

roomserver/internal/perform/perform_join.go

@@ -204,10 +204,10 @@ func (r *Joiner) performJoinRoomByID(
 				checkInvitePending = true
 			} else {
 				// create user room key if needed
-				key, err := r.RSAPI.GetOrCreateUserRoomPrivateKey(ctx, *userID, *roomID)
+				key, keyErr := r.RSAPI.GetOrCreateUserRoomPrivateKey(ctx, *userID, *roomID)
-				if err != nil {
+				if keyErr != nil {
-					util.GetLogger(ctx).WithError(err).Error("GetOrCreateUserRoomPrivateKey failed")
+					util.GetLogger(ctx).WithError(keyErr).Error("GetOrCreateUserRoomPrivateKey failed")
-					return "", "", fmt.Errorf("GetOrCreateUserRoomPrivateKey failed: %w", err)
+					return "", "", fmt.Errorf("GetOrCreateUserRoomPrivateKey failed: %w", keyErr)
 				}
 				senderID = spec.SenderID(spec.Base64Bytes(key).Encode())
 			}
@@ -222,8 +222,8 @@ func (r *Joiner) performJoinRoomByID(
 	// Force a federated join if we're dealing with a pending invite
 	// and we aren't in the room.
 	if checkInvitePending {
-		isInvitePending, inviteSender, _, inviteEvent, err := helpers.IsInvitePending(ctx, r.DB, req.RoomIDOrAlias, senderID)
+		isInvitePending, inviteSender, _, inviteEvent, inviteErr := helpers.IsInvitePending(ctx, r.DB, req.RoomIDOrAlias, senderID)
-		if err == nil && !serverInRoom && isInvitePending {
+		if inviteErr == nil && !serverInRoom && isInvitePending {
 			inviter, queryErr := r.RSAPI.QueryUserIDForSender(ctx, *roomID, inviteSender)
 			if queryErr != nil {
 				return "", "", fmt.Errorf("r.RSAPI.QueryUserIDForSender: %w", queryErr)

syncapi/internal/keychange.go

@@ -170,14 +170,14 @@ func TrackChangedUsers(
 			return nil, nil, err
 		}
 		for roomID, state := range stateRes.Rooms {
+			validRoomID, roomErr := spec.NewRoomID(roomID)
+			if roomErr != nil {
+				continue
+			}
 			for tuple, membership := range state {
 				if membership != spec.Join {
 					continue
 				}
-				validRoomID, roomErr := spec.NewRoomID(roomID)
-				if roomErr != nil {
-					continue
-				}
 				user, queryErr := rsAPI.QueryUserIDForSender(ctx, *validRoomID, spec.SenderID(tuple.StateKey))
 				if queryErr != nil || user == nil {
 					continue
@@ -220,16 +220,16 @@ func TrackChangedUsers(
 		return nil, left, err
 	}
 	for roomID, state := range stateRes.Rooms {
+		validRoomID, err := spec.NewRoomID(roomID)
+		if err != nil {
+			continue
+		}
 		for tuple, membership := range state {
 			if membership != spec.Join {
 				continue
 			}
 			// new user who we weren't previously sharing rooms with
 			if _, ok := queryRes.UserIDsToCount[tuple.StateKey]; !ok {
-				validRoomID, err := spec.NewRoomID(roomID)
-				if err != nil {
-					continue
-				}
 				user, err := rsAPI.QueryUserIDForSender(ctx, *validRoomID, spec.SenderID(tuple.StateKey))
 				if err != nil || user == nil {
 					continue

syncapi/routing/relations.go

@@ -110,15 +110,16 @@ func Relations(
 		return util.ErrorResponse(err)
 	}
 
+	validRoomID, err := spec.NewRoomID(roomID)
+	if err != nil {
+		return util.ErrorResponse(err)
+	}
+
 	// Convert the events into client events, and optionally filter based on the event
 	// type if it was specified.
 	res.Chunk = make([]synctypes.ClientEvent, 0, len(filteredEvents))
 	for _, event := range filteredEvents {
 		sender := spec.UserID{}
-		validRoomID, err := spec.NewRoomID(event.RoomID())
-		if err != nil {
-			continue
-		}
 		userID, err := rsAPI.QueryUserIDForSender(req.Context(), *validRoomID, event.SenderID())
 		if err == nil && userID != nil {
 			sender = *userID