refactor fed membership endpoints, add missing checks, some cleanup, reduce line width

Signed-off-by: strawberry <strawberry@puppygock.gay>

src/api/client/membership.rs

@@ -7,12 +7,12 @@ use std::{
 use axum::extract::State;
 use axum_client_ip::InsecureClientIp;
 use conduit::{
-	debug, debug_info, debug_warn, err, error, info, pdu,
+	debug, debug_info, debug_warn, err, error, info,
-	pdu::{gen_event_id_canonical_json, PduBuilder},
+	pdu::{self, gen_event_id_canonical_json, PduBuilder},
 	result::FlatOk,
-	trace, utils,
+	trace,
-	utils::{shuffle, IterStream, ReadyExt},
+	utils::{self, shuffle, IterStream, ReadyExt},
-	warn, Err, Error, PduEvent, Result,
+	warn, Err, PduEvent, Result,
 };
 use futures::{join, FutureExt, StreamExt};
 use ruma::{
@@ -153,21 +153,14 @@ async fn banned_room_check(
 ///   rules locally
 /// - If the server does not know about the room: asks other servers over
 ///   federation
-#[tracing::instrument(skip_all, fields(%client_ip), name = "join")]
+#[tracing::instrument(skip_all, fields(%client), name = "join")]
 pub(crate) async fn join_room_by_id_route(
-	State(services): State<crate::State>, InsecureClientIp(client_ip): InsecureClientIp,
+	State(services): State<crate::State>, InsecureClientIp(client): InsecureClientIp,
 	body: Ruma<join_room_by_id::v3::Request>,
 ) -> Result<join_room_by_id::v3::Response> {
 	let sender_user = body.sender_user();
 
-	banned_room_check(
+	banned_room_check(&services, sender_user, Some(&body.room_id), body.room_id.server_name(), client).await?;
-		&services,
-		sender_user,
-		Some(&body.room_id),
-		body.room_id.server_name(),
-		client_ip,
-	)
-	.await?;
 
 	// There is no body.server_name for /roomId/join
 	let mut servers: Vec<_> = services
@@ -354,10 +347,7 @@ pub(crate) async fn invite_user_route(
 			"User {sender_user} is not an admin and attempted to send an invite to room {}",
 			&body.room_id
 		);
-		return Err(Error::BadRequest(
+		return Err!(Request(Forbidden("Invites are not allowed on this server.")));
-			ErrorKind::forbidden(),
-			"Invites are not allowed on this server.",
-		));
 	}
 
 	banned_room_check(&services, sender_user, Some(&body.room_id), body.room_id.server_name(), client).await?;
@@ -388,7 +378,7 @@ pub(crate) async fn invite_user_route(
 
 		Ok(invite_user::v3::Response {})
 	} else {
-		Err(Error::BadRequest(ErrorKind::NotFound, "User not found."))
+		Err!(Request(NotFound("User not found.")))
 	}
 }
 
@@ -686,6 +676,18 @@ pub async fn join_room_by_id_helper(
 		});
 	}
 
+	if let Ok(membership) = services
+		.rooms
+		.state_accessor
+		.get_member(room_id, sender_user)
+		.await
+	{
+		if membership.membership == MembershipState::Ban {
+			debug_warn!("{sender_user} is banned from {room_id} but attempted to join");
+			return Err!(Request(Forbidden("You are banned from the room.")));
+		}
+	}
+
 	let server_in_room = services
 		.rooms
 		.state_cache
@@ -730,19 +732,29 @@ async fn join_room_by_id_helper_remote(
 		));
 	}
 
-	let mut join_event_stub: CanonicalJsonObject = serde_json::from_str(make_join_response.event.get())
+	let mut join_event_stub: CanonicalJsonObject =
-		.map_err(|e| err!(BadServerResponse("Invalid make_join event json received from server: {e:?}")))?;
+		serde_json::from_str(make_join_response.event.get()).map_err(|e| {
+			err!(BadServerResponse(warn!(
+				"Invalid make_join event json received from server: {e:?}"
+			)))
+		})?;
 
-	let join_authorized_via_users_server = join_event_stub
+	let join_authorized_via_users_server = {
+		use RoomVersionId::*;
+		if !matches!(room_version_id, V1 | V2 | V3 | V4 | V5 | V6 | V7) {
+			join_event_stub
 				.get("content")
 				.map(|s| {
 					s.as_object()?
 						.get("join_authorised_via_users_server")?
 						.as_str()
 				})
-		.and_then(|s| OwnedUserId::try_from(s.unwrap_or_default()).ok());
+				.and_then(|s| OwnedUserId::try_from(s.unwrap_or_default()).ok())
+		} else {
+			None
+		}
+	};
 
-	// TODO: Is origin needed?
 	join_event_stub.insert(
 		"origin".to_owned(),
 		CanonicalJsonValue::String(services.globals.server_name().as_str().to_owned()),
@@ -811,47 +823,30 @@ async fn join_room_by_id_helper_remote(
 	info!("send_join finished");
 
 	if join_authorized_via_users_server.is_some() {
-		use RoomVersionId::*;
-		match &room_version_id {
-			V1 | V2 | V3 | V4 | V5 | V6 | V7 => {
-				warn!(
-					"Found `join_authorised_via_users_server` but room {} is version {}. Ignoring.",
-					room_id, &room_version_id
-				);
-			},
-			// only room versions 8 and above using `join_authorized_via_users_server` (restricted joins) need to
-			// validate and send signatures
-			_ => {
 		if let Some(signed_raw) = &send_join_response.room_state.event {
 			debug_info!(
-						"There is a signed event. This room is probably using restricted joins. Adding signature to \
+				"There is a signed event with join_authorized_via_users_server. This room is probably using \
-						 our event"
+				 restricted joins. Adding signature to our event"
 			);
-					let Ok((signed_event_id, signed_value)) = gen_event_id_canonical_json(signed_raw, &room_version_id)
+
-					else {
+			let (signed_event_id, signed_value) = gen_event_id_canonical_json(signed_raw, &room_version_id)
-						// Event could not be converted to canonical json
+				.map_err(|e| err!(Request(BadJson(warn!("Could not convert event to canonical JSON: {e}")))))?;
-						return Err(Error::BadRequest(
-							ErrorKind::InvalidParam,
-							"Could not convert event to canonical json.",
-						));
-					};
 
 			if signed_event_id != event_id {
-						return Err(Error::BadRequest(
+				return Err!(Request(BadJson(
-							ErrorKind::InvalidParam,
+					warn!(%signed_event_id, %event_id, "Server {remote_server} sent event with wrong event ID")
-							"Server sent event with wrong event id",
+				)));
-						));
 			}
 
 			match signed_value["signatures"]
 				.as_object()
-						.ok_or(Error::BadRequest(
+				.ok_or_else(|| err!(BadServerResponse(warn!("Server {remote_server} sent invalid signatures type"))))
-							ErrorKind::InvalidParam,
-							"Server sent invalid signatures type",
-						))
 				.and_then(|e| {
-							e.get(remote_server.as_str())
+					e.get(remote_server.as_str()).ok_or_else(|| {
-								.ok_or(Error::BadRequest(ErrorKind::InvalidParam, "Server did not send its signature"))
+						err!(BadServerResponse(warn!(
+							"Server {remote_server} did not send its signature for a restricted room"
+						)))
+					})
 				}) {
 				Ok(signature) => {
 					join_event
@@ -863,14 +858,12 @@ async fn join_room_by_id_helper_remote(
 				},
 				Err(e) => {
 					warn!(
-								"Server {remote_server} sent invalid signature in sendjoin signatures for event \
+						"Server {remote_server} sent invalid signature in send_join signatures for event \
 						 {signed_value:?}: {e:?}",
 					);
 				},
 			}
 		}
-			},
-		}
 	}
 
 	services
@@ -1041,14 +1034,13 @@ async fn join_room_by_id_helper_local(
 	services: &Services, sender_user: &UserId, room_id: &RoomId, reason: Option<String>, servers: &[OwnedServerName],
 	_third_party_signed: Option<&ThirdPartySigned>, state_lock: RoomMutexGuard,
 ) -> Result {
-	debug!("We can join locally");
+	debug_info!("We can join locally");
 
 	let join_rules_event_content = services
 		.rooms
 		.state_accessor
-		.room_state_get_content(room_id, &StateEventType::RoomJoinRules, "")
+		.room_state_get_content::<RoomJoinRulesEventContent>(room_id, &StateEventType::RoomJoinRules, "")
-		.await
+		.await;
-		.map(|content: RoomJoinRulesEventContent| content);
 
 	let restriction_rooms = match join_rules_event_content {
 		Ok(RoomJoinRulesEventContent {
@@ -1064,17 +1056,7 @@ async fn join_room_by_id_helper_local(
 		_ => Vec::new(),
 	};
 
-	let local_members: Vec<_> = services
+	let join_authorized_via_users_server: Option<OwnedUserId> = {
-		.rooms
-		.state_cache
-		.room_members(room_id)
-		.ready_filter(|user| services.globals.user_is_local(user))
-		.map(ToOwned::to_owned)
-		.collect()
-		.await;
-
-	let mut join_authorized_via_users_server: Option<OwnedUserId> = None;
-
 		if restriction_rooms
 			.iter()
 			.stream()
@@ -1086,18 +1068,24 @@ async fn join_room_by_id_helper_local(
 			})
 			.await
 		{
-		for user in local_members {
+			services
-			if services
+				.rooms
+				.state_cache
+				.local_users_in_room(room_id)
+				.filter(|user| {
+					services
 						.rooms
 						.state_accessor
-				.user_can_invite(room_id, &user, sender_user, &state_lock)
+						.user_can_invite(room_id, user, sender_user, &state_lock)
+				})
+				.boxed()
+				.next()
 				.await
-			{
+				.map(ToOwned::to_owned)
-				join_authorized_via_users_server = Some(user);
+		} else {
-				break;
+			None
-			}
-		}
 		}
+	};
 
 	let content = RoomMemberEventContent {
 		displayname: services.users.displayname(sender_user).await.ok(),
@@ -1109,7 +1097,7 @@ async fn join_room_by_id_helper_local(
 	};
 
 	// Try normal join first
-	let error = match services
+	let Err(error) = services
 		.rooms
 		.timeline
 		.build_and_append_pdu(
@@ -1119,18 +1107,21 @@ async fn join_room_by_id_helper_local(
 			&state_lock,
 		)
 		.await
-	{
+	else {
-		Ok(_) => return Ok(()),
+		return Ok(());
-		Err(e) => e,
 	};
 
-	if !restriction_rooms.is_empty()
+	if restriction_rooms.is_empty()
-		&& servers
+		&& (servers.is_empty() || servers.len() == 1 && services.globals.server_is_ours(&servers[0]))
-			.iter()
-			.any(|server_name| !services.globals.server_is_ours(server_name))
 	{
+		return Err(error);
+	}
+
 	warn!("We couldn't do the join locally, maybe federation can help to satisfy the restricted join requirements");
-		let (make_join_response, remote_server) = make_join_request(services, sender_user, room_id, servers).await?;
+	let Ok((make_join_response, remote_server)) = make_join_request(services, sender_user, room_id, servers).await
+	else {
+		return Err(error);
+	};
 
 	let Some(room_version_id) = make_join_response.room_version else {
 		return Err!(BadServerResponse("Remote room version is not supported by conduwuit"));
@@ -1144,6 +1135,7 @@ async fn join_room_by_id_helper_local(
 
 	let mut join_event_stub: CanonicalJsonObject = serde_json::from_str(make_join_response.event.get())
 		.map_err(|e| err!(BadServerResponse("Invalid make_join event json received from server: {e:?}")))?;
+
 	let join_authorized_via_users_server = join_event_stub
 		.get("content")
 		.map(|s| {
@@ -1152,7 +1144,7 @@ async fn join_room_by_id_helper_local(
 				.as_str()
 		})
 		.and_then(|s| OwnedUserId::try_from(s.unwrap_or_default()).ok());
-		// TODO: Is origin needed?
+
 	join_event_stub.insert(
 		"origin".to_owned(),
 		CanonicalJsonValue::String(services.globals.server_name().as_str().to_owned()),
@@ -1219,19 +1211,13 @@ async fn join_room_by_id_helper_local(
 		.await?;
 
 	if let Some(signed_raw) = send_join_response.room_state.event {
-			let Ok((signed_event_id, signed_value)) = gen_event_id_canonical_json(&signed_raw, &room_version_id) else {
+		let (signed_event_id, signed_value) = gen_event_id_canonical_json(&signed_raw, &room_version_id)
-				// Event could not be converted to canonical json
+			.map_err(|e| err!(Request(BadJson(warn!("Could not convert event to canonical JSON: {e}")))))?;
-				return Err(Error::BadRequest(
-					ErrorKind::InvalidParam,
-					"Could not convert event to canonical json.",
-				));
-			};
 
 		if signed_event_id != event_id {
-				return Err(Error::BadRequest(
+			return Err!(Request(BadJson(
-					ErrorKind::InvalidParam,
+				warn!(%signed_event_id, %event_id, "Server {remote_server} sent event with wrong event ID")
-					"Server sent event with wrong event id",
+			)));
-				));
 		}
 
 		drop(state_lock);
@@ -1243,9 +1229,6 @@ async fn join_room_by_id_helper_local(
 	} else {
 		return Err(error);
 	}
-	} else {
-		return Err(error);
-	}
 
 	Ok(())
 }
@@ -1317,13 +1300,10 @@ async fn make_join_request(
 pub(crate) async fn invite_helper(
 	services: &Services, sender_user: &UserId, user_id: &UserId, room_id: &RoomId, reason: Option<String>,
 	is_direct: bool,
-) -> Result<()> {
+) -> Result {
 	if !services.users.is_admin(sender_user).await && services.globals.block_non_admin_invites() {
 		info!("User {sender_user} is not an admin and attempted to send an invite to room {room_id}");
-		return Err(Error::BadRequest(
+		return Err!(Request(Forbidden("Invites are not allowed on this server.")));
-			ErrorKind::forbidden(),
-			"Invites are not allowed on this server.",
-		));
 	}
 
 	if !services.globals.user_is_local(user_id) {
@@ -1382,30 +1362,24 @@ pub(crate) async fn invite_helper(
 
 		// We do not add the event_id field to the pdu here because of signature and
 		// hashes checks
-		let Ok((event_id, value)) = gen_event_id_canonical_json(&response.event, &room_version_id) else {
+		let (event_id, value) = gen_event_id_canonical_json(&response.event, &room_version_id)
-			// Event could not be converted to canonical json
+			.map_err(|e| err!(Request(BadJson(warn!("Could not convert event to canonical JSON: {e}")))))?;
-			return Err(Error::BadRequest(
-				ErrorKind::InvalidParam,
-				"Could not convert event to canonical json.",
-			));
-		};
 
-		if *pdu.event_id != *event_id {
+		if pdu.event_id != event_id {
-			warn!(
+			return Err!(Request(BadJson(
-				"Server {} changed invite event, that's not allowed in the spec: ours: {pdu_json:?}, theirs: {value:?}",
+				warn!(%pdu.event_id, %event_id, "Server {} sent event with wrong event ID", user_id.server_name())
-				user_id.server_name(),
+			)));
-			);
 		}
 
 		let origin: OwnedServerName = serde_json::from_value(
 			serde_json::to_value(
 				value
 					.get("origin")
-					.ok_or(Error::BadRequest(ErrorKind::InvalidParam, "Event needs an origin field."))?,
+					.ok_or_else(|| err!(Request(BadJson("Event missing origin field."))))?,
 			)
 			.expect("CanonicalJson is valid json value"),
 		)
-		.map_err(|_| Error::BadRequest(ErrorKind::InvalidParam, "Origin field is invalid."))?;
+		.map_err(|e| err!(Request(BadJson(warn!("Origin field in event is not a valid server name: {e}")))))?;
 
 		let pdu_id = services
 			.rooms
@@ -1414,8 +1388,7 @@ pub(crate) async fn invite_helper(
 			.await?
 			.ok_or_else(|| err!(Request(InvalidParam("Could not accept incoming PDU as timeline event."))))?;
 
-		services.sending.send_pdu_room(room_id, &pdu_id).await?;
+		return services.sending.send_pdu_room(room_id, &pdu_id).await;
-		return Ok(());
 	}
 
 	if !services
@@ -1424,10 +1397,9 @@ pub(crate) async fn invite_helper(
 		.is_joined(sender_user, room_id)
 		.await
 	{
-		return Err(Error::BadRequest(
+		return Err!(Request(Forbidden(
-			ErrorKind::forbidden(),
+			"You must be joined in the room you are trying to invite from."
-			"You don't have permission to view this room.",
+		)));
-		));
 	}
 
 	let state_lock = services.rooms.state.mutex.lock(room_id).await;
@@ -1599,7 +1571,11 @@ async fn remote_leave_room(services: &Services, user_id: &UserId, room_id: &Room
 			.map(|user| user.server_name().to_owned()),
 	);
 
-	debug!("servers in remote_leave_room: {servers:?}");
+	if let Some(room_id_server_name) = room_id.server_name() {
+		servers.insert(room_id_server_name.to_owned());
+	}
+
+	debug_info!("servers in remote_leave_room: {servers:?}");
 
 	for remote_server in servers {
 		let make_leave_response = services

src/api/client/user_directory.rs

@@ -21,7 +21,7 @@ pub(crate) async fn search_users_route(
 	State(services): State<crate::State>, body: Ruma<search_users::v3::Request>,
 ) -> Result<search_users::v3::Response> {
 	let sender_user = body.sender_user.as_ref().expect("user is authenticated");
-	let limit = usize::try_from(body.limit).unwrap_or(10); // default limit is 10
+	let limit = usize::try_from(body.limit).map_or(10, usize::from).min(100); // default limit is 10
 
 	let users = services.users.stream().filter_map(|user_id| async {
 		// Filter out buggy users (they should not exist, but you never know...)

src/api/server/invite.rs

@@ -145,7 +145,6 @@ pub(crate) async fn create_invite_route(
 				true,
 			)
 			.await?;
-	}
 
 		for appservice in services.appservice.read().await.values() {
 			if appservice.is_user_match(&invited_user) {
@@ -165,6 +164,7 @@ pub(crate) async fn create_invite_route(
 					.await?;
 			}
 		}
+	}
 
 	Ok(create_invite::v2::Response {
 		event: services

src/api/server/make_join.rs

@@ -1,8 +1,5 @@
 use axum::extract::State;
-use conduit::{
+use conduit::{debug_info, utils::IterStream, warn, Err};
-	utils::{IterStream, ReadyExt},
-	warn,
-};
 use futures::StreamExt;
 use ruma::{
 	api::{client::error::ErrorKind, federation::membership::prepare_join_event},
@@ -13,7 +10,7 @@ use ruma::{
 		},
 		StateEventType,
 	},
-	CanonicalJsonObject, RoomId, RoomVersionId, UserId,
+	CanonicalJsonObject, OwnedUserId, RoomId, RoomVersionId, UserId,
 };
 use serde_json::value::to_raw_value;
 
@@ -29,14 +26,11 @@ pub(crate) async fn create_join_event_template_route(
 	State(services): State<crate::State>, body: Ruma<prepare_join_event::v1::Request>,
 ) -> Result<prepare_join_event::v1::Response> {
 	if !services.rooms.metadata.exists(&body.room_id).await {
-		return Err(Error::BadRequest(ErrorKind::NotFound, "Room is unknown to this server."));
+		return Err!(Request(NotFound("Room is unknown to this server.")));
 	}
 
 	if body.user_id.server_name() != body.origin() {
-		return Err(Error::BadRequest(
+		return Err!(Request(BadJson("Not allowed to join on behalf of another server/user.")));
-			ErrorKind::InvalidParam,
-			"Not allowed to join on behalf of another server/user",
-		));
 	}
 
 	// ACL check origin server
@@ -59,10 +53,7 @@ pub(crate) async fn create_join_event_template_route(
 			&body.user_id,
 			&body.room_id,
 		);
-		return Err(Error::BadRequest(
+		return Err!(Request(Forbidden("Server is banned on this homeserver.")));
-			ErrorKind::forbidden(),
-			"Server is banned on this homeserver.",
-		));
 	}
 
 	if let Some(server) = body.room_id.server_name() {
@@ -72,10 +63,9 @@ pub(crate) async fn create_join_event_template_route(
 			.forbidden_remote_server_names
 			.contains(&server.to_owned())
 		{
-			return Err(Error::BadRequest(
+			return Err!(Request(Forbidden(warn!(
-				ErrorKind::forbidden(),
+				"Room ID server name {server} is banned on this homeserver."
-				"Server is banned on this homeserver.",
+			))));
-			));
 		}
 	}
 
@@ -91,18 +81,16 @@ pub(crate) async fn create_join_event_template_route(
 
 	let state_lock = services.rooms.state.mutex.lock(&body.room_id).await;
 
-	let join_authorized_via_users_server = if (services
+	let join_authorized_via_users_server: Option<OwnedUserId> = {
+		use RoomVersionId::*;
+		if matches!(room_version_id, V1 | V2 | V3 | V4 | V5 | V6 | V7) {
+			// room version does not support restricted join rules
+			None
+		} else if user_can_perform_restricted_join(&services, &body.user_id, &body.room_id, &room_version_id).await? {
+			let Some(auth_user) = services
 				.rooms
 				.state_cache
-		.is_left(&body.user_id, &body.room_id)
+				.local_users_in_room(&body.room_id)
-		.await)
-		&& user_can_perform_restricted_join(&services, &body.user_id, &body.room_id, &room_version_id).await?
-	{
-		let auth_user = services
-			.rooms
-			.state_cache
-			.room_members(&body.room_id)
-			.ready_filter(|user| user.server_name() == services.globals.server_name())
 				.filter(|user| {
 					services
 						.rooms
@@ -112,18 +100,16 @@ pub(crate) async fn create_join_event_template_route(
 				.boxed()
 				.next()
 				.await
-			.map(ToOwned::to_owned);
+				.map(ToOwned::to_owned)
-
+			else {
-		if auth_user.is_some() {
+				return Err!(Request(UnableToGrantJoin(
-			auth_user
+					"No user on this server is able to assist in joining."
-		} else {
+				)));
-			return Err(Error::BadRequest(
+			};
-				ErrorKind::UnableToGrantJoin,
+			Some(auth_user)
-				"No user on this server is able to assist in joining.",
-			));
-		}
 		} else {
 			None
+		}
 	};
 
 	let (_pdu, mut pdu_json) = services
@@ -155,37 +141,39 @@ pub(crate) async fn create_join_event_template_route(
 }
 
 /// Checks whether the given user can join the given room via a restricted join.
-/// This doesn't check the current user's membership. This should be done
-/// externally, either by using the state cache or attempting to authorize the
-/// event.
 pub(crate) async fn user_can_perform_restricted_join(
 	services: &Services, user_id: &UserId, room_id: &RoomId, room_version_id: &RoomVersionId,
 ) -> Result<bool> {
 	use RoomVersionId::*;
 
-	let join_rules_event = services
+	// restricted rooms are not supported on <=v7
-		.rooms
-		.state_accessor
-		.room_state_get(room_id, &StateEventType::RoomJoinRules, "")
-		.await;
-
-	let Ok(Ok(join_rules_event_content)) = join_rules_event.as_ref().map(|join_rules_event| {
-		serde_json::from_str::<RoomJoinRulesEventContent>(join_rules_event.content.get()).map_err(|e| {
-			warn!("Invalid join rules event in database: {e}");
-			Error::bad_database("Invalid join rules event in database")
-		})
-	}) else {
-		return Ok(false);
-	};
-
 	if matches!(room_version_id, V1 | V2 | V3 | V4 | V5 | V6 | V7) {
 		return Ok(false);
 	}
 
+	if services.rooms.state_cache.is_joined(user_id, room_id).await {
+		// joining user is already joined, there is nothing we need to do
+		return Ok(false);
+	}
+
+	let Ok(join_rules_event_content) = services
+		.rooms
+		.state_accessor
+		.room_state_get_content::<RoomJoinRulesEventContent>(room_id, &StateEventType::RoomJoinRules, "")
+		.await
+	else {
+		return Ok(false);
+	};
+
 	let (JoinRule::Restricted(r) | JoinRule::KnockRestricted(r)) = join_rules_event_content.join_rule else {
 		return Ok(false);
 	};
 
+	if r.allow.is_empty() {
+		debug_info!("{room_id} is restricted but the allow key is empty");
+		return Ok(false);
+	}
+
 	if r.allow
 		.iter()
 		.filter_map(|rule| {
@@ -201,22 +189,20 @@ pub(crate) async fn user_can_perform_restricted_join(
 	{
 		Ok(true)
 	} else {
-		Err(Error::BadRequest(
+		Err!(Request(UnableToAuthorizeJoin(
-			ErrorKind::UnableToAuthorizeJoin,
+			"Joining user is not known to be in any required room."
-			"User is not known to be in any required room.",
+		)))
-		))
 	}
 }
 
-pub(crate) fn maybe_strip_event_id(pdu_json: &mut CanonicalJsonObject, room_version_id: &RoomVersionId) -> Result<()> {
+pub(crate) fn maybe_strip_event_id(pdu_json: &mut CanonicalJsonObject, room_version_id: &RoomVersionId) -> Result {
 	use RoomVersionId::*;
 
 	match room_version_id {
-		V1 | V2 => {},
+		V1 | V2 => Ok(()),
 		_ => {
 			pdu_json.remove("event_id");
-		},
-	};
-
 			Ok(())
+		},
+	}
 }

src/api/server/make_knock.rs

@@ -18,14 +18,11 @@ pub(crate) async fn create_knock_event_template_route(
 	State(services): State<crate::State>, body: Ruma<create_knock_event_template::v1::Request>,
 ) -> Result<create_knock_event_template::v1::Response> {
 	if !services.rooms.metadata.exists(&body.room_id).await {
-		return Err(Error::BadRequest(ErrorKind::NotFound, "Room is unknown to this server."));
+		return Err!(Request(NotFound("Room is unknown to this server.")));
 	}
 
 	if body.user_id.server_name() != body.origin() {
-		return Err(Error::BadRequest(
+		return Err!(Request(BadJson("Not allowed to knock on behalf of another server/user.")));
-			ErrorKind::InvalidParam,
-			"Not allowed to knock on behalf of another server/user",
-		));
 	}
 
 	// ACL check origin server

src/api/server/make_leave.rs

@@ -1,7 +1,7 @@
 use axum::extract::State;
-use conduit::{Error, Result};
+use conduit::{Err, Result};
 use ruma::{
-	api::{client::error::ErrorKind, federation::membership::prepare_leave_event},
+	api::federation::membership::prepare_leave_event,
 	events::room::member::{MembershipState, RoomMemberEventContent},
 };
 use serde_json::value::to_raw_value;
@@ -16,14 +16,11 @@ pub(crate) async fn create_leave_event_template_route(
 	State(services): State<crate::State>, body: Ruma<prepare_leave_event::v1::Request>,
 ) -> Result<prepare_leave_event::v1::Response> {
 	if !services.rooms.metadata.exists(&body.room_id).await {
-		return Err(Error::BadRequest(ErrorKind::NotFound, "Room is unknown to this server."));
+		return Err!(Request(NotFound("Room is unknown to this server.")));
 	}
 
 	if body.user_id.server_name() != body.origin() {
-		return Err(Error::BadRequest(
+		return Err!(Request(BadJson("Not allowed to leave on behalf of another server/user.")));
-			ErrorKind::InvalidParam,
-			"Not allowed to leave on behalf of another server/user",
-		));
 	}
 
 	// ACL check origin

src/api/server/send_join.rs

@@ -7,16 +7,16 @@ use conduit::{
 	err,
 	pdu::gen_event_id_canonical_json,
 	utils::stream::{IterStream, TryBroadbandExt},
-	warn, Error, Result,
+	warn, Err, Result,
 };
 use futures::{FutureExt, StreamExt, TryStreamExt};
 use ruma::{
-	api::{client::error::ErrorKind, federation::membership::create_join_event},
+	api::federation::membership::create_join_event,
 	events::{
 		room::member::{MembershipState, RoomMemberEventContent},
 		StateEventType,
 	},
-	CanonicalJsonValue, OwnedEventId, OwnedServerName, OwnedUserId, RoomId, ServerName,
+	CanonicalJsonValue, OwnedEventId, OwnedRoomId, OwnedServerName, OwnedUserId, RoomId, ServerName,
 };
 use serde_json::value::{to_raw_value, RawValue as RawJsonValue};
 use service::Services;
@@ -28,7 +28,7 @@ async fn create_join_event(
 	services: &Services, origin: &ServerName, room_id: &RoomId, pdu: &RawJsonValue,
 ) -> Result<create_join_event::v1::RoomState> {
 	if !services.rooms.metadata.exists(room_id).await {
-		return Err(Error::BadRequest(ErrorKind::NotFound, "Room is unknown to this server."));
+		return Err!(Request(NotFound("Room is unknown to this server.")));
 	}
 
 	// ACL check origin server
@@ -45,7 +45,7 @@ async fn create_join_event(
 		.state
 		.get_room_shortstatehash(room_id)
 		.await
-		.map_err(|_| err!(Request(NotFound("Event state not found."))))?;
+		.map_err(|e| err!(Request(NotFound(error!("Room has no state: {e}")))))?;
 
 	// We do not add the event_id field to the pdu here because of signature and
 	// hashes checks
@@ -53,53 +53,62 @@ async fn create_join_event(
 
 	let Ok((event_id, mut value)) = gen_event_id_canonical_json(pdu, &room_version_id) else {
 		// Event could not be converted to canonical json
-		return Err(Error::BadRequest(
+		return Err!(Request(BadJson("Could not convert event to canonical json.")));
-			ErrorKind::InvalidParam,
-			"Could not convert event to canonical json.",
-		));
 	};
 
+	let event_room_id: OwnedRoomId = serde_json::from_value(
+		serde_json::to_value(
+			value
+				.get("room_id")
+				.ok_or_else(|| err!(Request(BadJson("Event missing room_id property."))))?,
+		)
+		.expect("CanonicalJson is valid json value"),
+	)
+	.map_err(|e| err!(Request(BadJson(warn!("room_id field is not a valid room ID: {e}")))))?;
+
+	if event_room_id != room_id {
+		return Err!(Request(BadJson("Event room_id does not match request path room ID.")));
+	}
+
 	let event_type: StateEventType = serde_json::from_value(
 		value
 			.get("type")
-			.ok_or_else(|| Error::BadRequest(ErrorKind::InvalidParam, "Event missing type property."))?
+			.ok_or_else(|| err!(Request(BadJson("Event missing type property."))))?
 			.clone()
 			.into(),
 	)
-	.map_err(|_| Error::BadRequest(ErrorKind::InvalidParam, "Event has invalid event type."))?;
+	.map_err(|e| err!(Request(BadJson(warn!("Event has invalid state event type: {e}")))))?;
 
 	if event_type != StateEventType::RoomMember {
-		return Err(Error::BadRequest(
+		return Err!(Request(BadJson(
-			ErrorKind::InvalidParam,
+			"Not allowed to send non-membership state event to join endpoint."
-			"Not allowed to send non-membership state event to join endpoint.",
+		)));
-		));
 	}
 
 	let content: RoomMemberEventContent = serde_json::from_value(
 		value
 			.get("content")
-			.ok_or_else(|| Error::BadRequest(ErrorKind::InvalidParam, "Event missing content property"))?
+			.ok_or_else(|| err!(Request(BadJson("Event missing content property"))))?
 			.clone()
 			.into(),
 	)
-	.map_err(|_| Error::BadRequest(ErrorKind::InvalidParam, "Event content is empty or invalid"))?;
+	.map_err(|e| err!(Request(BadJson(warn!("Event content is empty or invalid: {e}")))))?;
 
 	if content.membership != MembershipState::Join {
-		return Err(Error::BadRequest(
+		return Err!(Request(BadJson(
-			ErrorKind::InvalidParam,
+			"Not allowed to send a non-join membership event to join endpoint."
-			"Not allowed to send a non-join membership event to join endpoint.",
+		)));
-		));
 	}
 
-	// ACL check sender server name
+	// ACL check sender user server name
 	let sender: OwnedUserId = serde_json::from_value(
 		value
 			.get("sender")
-			.ok_or_else(|| Error::BadRequest(ErrorKind::InvalidParam, "Event missing sender property."))?
+			.ok_or_else(|| err!(Request(BadJson("Event missing sender property."))))?
 			.clone()
 			.into(),
 	)
-	.map_err(|_| Error::BadRequest(ErrorKind::BadJson, "sender is not a valid user ID."))?;
+	.map_err(|e| err!(Request(BadJson(warn!("sender property is not a valid user ID: {e}")))))?;
 
 	services
 		.rooms
@@ -109,50 +118,71 @@ async fn create_join_event(
 
 	// check if origin server is trying to send for another server
 	if sender.server_name() != origin {
-		return Err(Error::BadRequest(
+		return Err!(Request(Forbidden("Not allowed to join on behalf of another server.")));
-			ErrorKind::InvalidParam,
-			"Not allowed to join on behalf of another server.",
-		));
 	}
 
 	let state_key: OwnedUserId = serde_json::from_value(
 		value
 			.get("state_key")
-			.ok_or_else(|| Error::BadRequest(ErrorKind::InvalidParam, "Event missing state_key property."))?
+			.ok_or_else(|| err!(Request(BadJson("Event missing state_key property."))))?
 			.clone()
 			.into(),
 	)
-	.map_err(|_| Error::BadRequest(ErrorKind::BadJson, "state_key is invalid or not a user ID."))?;
+	.map_err(|e| err!(Request(BadJson(warn!("State key is not a valid user ID: {e}")))))?;
 
 	if state_key != sender {
-		return Err(Error::BadRequest(
+		return Err!(Request(BadJson("State key does not match sender user.")));
-			ErrorKind::InvalidParam,
-			"State key does not match sender user",
-		));
 	};
 
-	if content
+	if let Some(authorising_user) = content.join_authorized_via_users_server {
-		.join_authorized_via_users_server
+		use ruma::RoomVersionId::*;
-		.is_some_and(|user| services.globals.user_is_local(&user))
+
-		&& super::user_can_perform_restricted_join(services, &sender, room_id, &room_version_id)
+		if !matches!(room_version_id, V1 | V2 | V3 | V4 | V5 | V6 | V7) {
+			return Err!(Request(InvalidParam(
+				"Room version {room_version_id} does not support restricted rooms but \
+				 join_authorised_via_users_server ({authorising_user}) was found in the event."
+			)));
+		}
+
+		if !services.globals.user_is_local(&authorising_user) {
+			return Err!(Request(InvalidParam(
+				"Cannot authorise membership event through {authorising_user} as they do not belong to this homeserver"
+			)));
+		}
+
+		if !services
+			.rooms
+			.state_cache
+			.is_joined(&authorising_user, room_id)
 			.await
-			.unwrap_or_default()
 		{
+			return Err!(Request(InvalidParam(
+				"Authorising user {authorising_user} is not in the room you are trying to join, they cannot authorise \
+				 your join."
+			)));
+		}
+
+		if !super::user_can_perform_restricted_join(services, &state_key, room_id, &room_version_id).await? {
+			return Err!(Request(UnableToAuthorizeJoin(
+				"Joining user did not pass restricted room's rules."
+			)));
+		}
+	}
+
 	services
 		.server_keys
 		.hash_and_sign_event(&mut value, &room_version_id)
-			.map_err(|e| err!(Request(InvalidParam("Failed to sign event: {e}"))))?;
+		.map_err(|e| err!(Request(InvalidParam(warn!("Failed to sign send_join event: {e}")))))?;
-	}
 
 	let origin: OwnedServerName = serde_json::from_value(
 		serde_json::to_value(
 			value
 				.get("origin")
-				.ok_or_else(|| Error::BadRequest(ErrorKind::InvalidParam, "Event missing origin property."))?,
+				.ok_or_else(|| err!(Request(BadJson("Event missing origin property."))))?,
 		)
 		.expect("CanonicalJson is valid json value"),
 	)
-	.map_err(|_| Error::BadRequest(ErrorKind::InvalidParam, "origin is not a server name."))?;
+	.map_err(|e| err!(Request(BadJson(warn!("origin field is not a valid server name: {e}")))))?;
 
 	let mutex_lock = services
 		.rooms
@@ -214,7 +244,6 @@ async fn create_join_event(
 	Ok(create_join_event::v1::RoomState {
 		auth_chain,
 		state,
-		// Event field is required if the room version supports restricted join rules.
 		event: to_raw_value(&CanonicalJsonValue::Object(value)).ok(),
 	})
 }
@@ -232,14 +261,12 @@ pub(crate) async fn create_join_event_v1_route(
 		.contains(body.origin())
 	{
 		warn!(
-			"Server {} tried joining room ID {} who has a server name that is globally forbidden. Rejecting.",
+			"Server {} tried joining room ID {} through us who has a server name that is globally forbidden. \
+			 Rejecting.",
 			body.origin(),
 			&body.room_id,
 		);
-		return Err(Error::BadRequest(
+		return Err!(Request(Forbidden("Server is banned on this homeserver.")));
-			ErrorKind::forbidden(),
-			"Server is banned on this homeserver.",
-		));
 	}
 
 	if let Some(server) = body.room_id.server_name() {
@@ -250,14 +277,14 @@ pub(crate) async fn create_join_event_v1_route(
 			.contains(&server.to_owned())
 		{
 			warn!(
-				"Server {} tried joining room ID {} which has a server name that is globally forbidden. Rejecting.",
+				"Server {} tried joining room ID {} through us which has a server name that is globally forbidden. \
+				 Rejecting.",
 				body.origin(),
 				&body.room_id,
 			);
-			return Err(Error::BadRequest(
+			return Err!(Request(Forbidden(warn!(
-				ErrorKind::forbidden(),
+				"Room ID server name {server} is banned on this homeserver."
-				"Server is banned on this homeserver.",
+			))));
-			));
 		}
 	}
 
@@ -282,10 +309,7 @@ pub(crate) async fn create_join_event_v2_route(
 		.forbidden_remote_server_names
 		.contains(body.origin())
 	{
-		return Err(Error::BadRequest(
+		return Err!(Request(Forbidden("Server is banned on this homeserver.")));
-			ErrorKind::forbidden(),
-			"Server is banned on this homeserver.",
-		));
 	}
 
 	if let Some(server) = body.room_id.server_name() {
@@ -295,10 +319,15 @@ pub(crate) async fn create_join_event_v2_route(
 			.forbidden_remote_server_names
 			.contains(&server.to_owned())
 		{
-			return Err(Error::BadRequest(
+			warn!(
-				ErrorKind::forbidden(),
+				"Server {} tried joining room ID {} through us which has a server name that is globally forbidden. \
-				"Server is banned on this homeserver.",
+				 Rejecting.",
-			));
+				body.origin(),
+				&body.room_id,
+			);
+			return Err!(Request(Forbidden(warn!(
+				"Room ID server name {server} is banned on this homeserver."
+			))));
 		}
 	}
 

src/api/server/send_knock.rs

@@ -121,10 +121,7 @@ pub(crate) async fn create_knock_event_v1_route(
 
 	// check if origin server is trying to send for another server
 	if sender.server_name() != body.origin() {
-		return Err(Error::BadRequest(
+		return Err!(Request(BadJson("Not allowed to knock on behalf of another server/user.")));
-			ErrorKind::InvalidParam,
-			"Not allowed to knock on behalf of another server.",
-		));
 	}
 
 	let state_key: OwnedUserId = serde_json::from_value(

src/api/server/send_leave.rs

@@ -1,14 +1,14 @@
 #![allow(deprecated)]
 
 use axum::extract::State;
-use conduit::{err, utils::ReadyExt, Error, Result};
+use conduit::{err, Err, Result};
 use ruma::{
-	api::{client::error::ErrorKind, federation::membership::create_leave_event},
+	api::federation::membership::create_leave_event,
 	events::{
 		room::member::{MembershipState, RoomMemberEventContent},
 		StateEventType,
 	},
-	OwnedUserId, RoomId, ServerName,
+	OwnedRoomId, OwnedUserId, RoomId, ServerName,
 };
 use serde_json::value::RawValue as RawJsonValue;
 
@@ -39,11 +39,9 @@ pub(crate) async fn create_leave_event_v2_route(
 	Ok(create_leave_event::v2::Response::new())
 }
 
-async fn create_leave_event(
+async fn create_leave_event(services: &Services, origin: &ServerName, room_id: &RoomId, pdu: &RawJsonValue) -> Result {
-	services: &Services, origin: &ServerName, room_id: &RoomId, pdu: &RawJsonValue,
-) -> Result<()> {
 	if !services.rooms.metadata.exists(room_id).await {
-		return Err(Error::BadRequest(ErrorKind::NotFound, "Room is unknown to this server."));
+		return Err!(Request(NotFound("Room is unknown to this server.")));
 	}
 
 	// ACL check origin
@@ -58,53 +56,62 @@ async fn create_leave_event(
 	let room_version_id = services.rooms.state.get_room_version(room_id).await?;
 	let Ok((event_id, value)) = gen_event_id_canonical_json(pdu, &room_version_id) else {
 		// Event could not be converted to canonical json
-		return Err(Error::BadRequest(
+		return Err!(Request(BadJson("Could not convert event to canonical json.")));
-			ErrorKind::InvalidParam,
-			"Could not convert event to canonical json.",
-		));
 	};
 
+	let event_room_id: OwnedRoomId = serde_json::from_value(
+		serde_json::to_value(
+			value
+				.get("room_id")
+				.ok_or_else(|| err!(Request(BadJson("Event missing room_id property."))))?,
+		)
+		.expect("CanonicalJson is valid json value"),
+	)
+	.map_err(|e| err!(Request(BadJson(warn!("room_id field is not a valid room ID: {e}")))))?;
+
+	if event_room_id != room_id {
+		return Err!(Request(BadJson("Event room_id does not match request path room ID.")));
+	}
+
 	let content: RoomMemberEventContent = serde_json::from_value(
 		value
 			.get("content")
-			.ok_or_else(|| Error::BadRequest(ErrorKind::InvalidParam, "Event missing content property"))?
+			.ok_or_else(|| err!(Request(BadJson("Event missing content property."))))?
 			.clone()
 			.into(),
 	)
-	.map_err(|_| Error::BadRequest(ErrorKind::InvalidParam, "Event content is empty or invalid"))?;
+	.map_err(|e| err!(Request(BadJson(warn!("Event content is empty or invalid: {e}")))))?;
 
 	if content.membership != MembershipState::Leave {
-		return Err(Error::BadRequest(
+		return Err!(Request(BadJson(
-			ErrorKind::InvalidParam,
+			"Not allowed to send a non-leave membership event to leave endpoint."
-			"Not allowed to send a non-leave membership event to leave endpoint.",
+		)));
-		));
 	}
 
 	let event_type: StateEventType = serde_json::from_value(
 		value
 			.get("type")
-			.ok_or_else(|| Error::BadRequest(ErrorKind::InvalidParam, "Event missing type property."))?
+			.ok_or_else(|| err!(Request(BadJson("Event missing type property."))))?
 			.clone()
 			.into(),
 	)
-	.map_err(|_| Error::BadRequest(ErrorKind::BadJson, "Event does not have a valid state event type."))?;
+	.map_err(|e| err!(Request(BadJson(warn!("Event has invalid state event type: {e}")))))?;
 
 	if event_type != StateEventType::RoomMember {
-		return Err(Error::BadRequest(
+		return Err!(Request(BadJson(
-			ErrorKind::InvalidParam,
+			"Not allowed to send non-membership state event to leave endpoint."
-			"Not allowed to send non-membership state event to leave endpoint.",
+		)));
-		));
 	}
 
 	// ACL check sender server name
 	let sender: OwnedUserId = serde_json::from_value(
 		value
 			.get("sender")
-			.ok_or_else(|| Error::BadRequest(ErrorKind::InvalidParam, "Event missing sender property."))?
+			.ok_or_else(|| err!(Request(BadJson("Event missing sender property."))))?
 			.clone()
 			.into(),
 	)
-	.map_err(|_| Error::BadRequest(ErrorKind::BadJson, "User ID in sender is invalid."))?;
+	.map_err(|e| err!(Request(BadJson(warn!("sender property is not a valid user ID: {e}")))))?;
 
 	services
 		.rooms
@@ -113,26 +120,20 @@ async fn create_leave_event(
 		.await?;
 
 	if sender.server_name() != origin {
-		return Err(Error::BadRequest(
+		return Err!(Request(BadJson("Not allowed to leave on behalf of another server/user.")));
-			ErrorKind::InvalidParam,
-			"Not allowed to leave on behalf of another server.",
-		));
 	}
 
 	let state_key: OwnedUserId = serde_json::from_value(
 		value
 			.get("state_key")
-			.ok_or_else(|| Error::BadRequest(ErrorKind::InvalidParam, "Event missing state_key property."))?
+			.ok_or_else(|| err!(Request(BadJson("Event missing state_key property."))))?
 			.clone()
 			.into(),
 	)
-	.map_err(|_| Error::BadRequest(ErrorKind::BadJson, "state_key is invalid or not a user ID"))?;
+	.map_err(|e| err!(Request(BadJson(warn!("State key is not a valid user ID: {e}")))))?;
 
 	if state_key != sender {
-		return Err(Error::BadRequest(
+		return Err!(Request(BadJson("State key does not match sender user.")));
-			ErrorKind::InvalidParam,
-			"state_key does not match sender user.",
-		));
 	}
 
 	let mutex_lock = services
@@ -151,11 +152,5 @@ async fn create_leave_event(
 
 	drop(mutex_lock);
 
-	let servers = services
+	services.sending.send_pdu_room(room_id, &pdu_id).await
-		.rooms
-		.state_cache
-		.room_servers(room_id)
-		.ready_filter(|server| !services.globals.server_is_ours(server));
-
-	services.sending.send_pdu_servers(servers, &pdu_id).await
 }

src/service/pusher/mod.rs

@@ -227,7 +227,7 @@ impl Service {
 
 		for action in self
 			.get_actions(user, &ruleset, &power_levels, &pdu.to_sync_room_event(), &pdu.room_id)
-			.await?
+			.await
 		{
 			let n = match action {
 				Action::Notify => true,
@@ -259,7 +259,7 @@ impl Service {
 	pub async fn get_actions<'a>(
 		&self, user: &UserId, ruleset: &'a Ruleset, power_levels: &RoomPowerLevelsEventContent,
 		pdu: &Raw<AnySyncTimelineEvent>, room_id: &RoomId,
-	) -> Result<&'a [Action]> {
+	) -> &'a [Action] {
 		let power_levels = PushConditionPowerLevelsCtx {
 			users: power_levels.users.clone(),
 			users_default: power_levels.users_default,
@@ -290,7 +290,7 @@ impl Service {
 			power_levels: Some(power_levels),
 		};
 
-		Ok(ruleset.get_actions(pdu, &ctx))
+		ruleset.get_actions(pdu, &ctx)
 	}
 
 	#[tracing::instrument(skip(self, unread, pusher, tweaks, event))]

src/service/rooms/state_cache/mod.rs

@@ -4,7 +4,7 @@ use std::{
 };
 
 use conduit::{
-	err, is_not_empty,
+	is_not_empty,
 	result::LogErr,
 	utils::{stream::TryIgnore, ReadyExt, StreamTools},
 	warn, Result,
@@ -600,11 +600,11 @@ impl Service {
 			.map(|(_, servers): KeyVal<'_>| *servers.last().expect("at least one server"))
 	}
 
-	/// Gets up to three servers that are likely to be in the room in the
+	/// Gets up to five servers that are likely to be in the room in the
 	/// distant future.
 	///
-	/// See <https://spec.matrix.org/v1.10/appendices/#routing>
+	/// See <https://spec.matrix.org/latest/appendices/#routing>
-	#[tracing::instrument(skip(self))]
+	#[tracing::instrument(skip(self), level = "debug")]
 	pub async fn servers_route_via(&self, room_id: &RoomId) -> Result<Vec<OwnedServerName>> {
 		let most_powerful_user_server = self
 			.services
@@ -618,8 +618,7 @@ impl Service {
 					.max_by_key(|(_, power)| *power)
 					.and_then(|x| (x.1 >= &int!(50)).then_some(x))
 					.map(|(user, _power)| user.server_name().to_owned())
-			})
+			});
-			.map_err(|e| err!(Database(error!(?e, "Invalid power levels event content in database."))))?;
 
 		let mut servers: Vec<OwnedServerName> = self
 			.room_members(room_id)
@@ -629,12 +628,12 @@ impl Service {
 			.sorted_by_key(|(_, users)| *users)
 			.map(|(server, _)| server)
 			.rev()
-			.take(3)
+			.take(5)
 			.collect();
 
-		if let Some(server) = most_powerful_user_server {
+		if let Ok(Some(server)) = most_powerful_user_server {
 			servers.insert(0, server);
-			servers.truncate(3);
+			servers.truncate(5);
 		}
 
 		Ok(servers)

src/service/rooms/timeline/mod.rs

@@ -417,7 +417,7 @@ impl Service {
 				.services
 				.pusher
 				.get_actions(user, &rules_for_user, &power_levels, &sync_pdu, &pdu.room_id)
-				.await?
+				.await
 			{
 				match action {
 					Action::Notify => notify = true,
@@ -769,10 +769,8 @@ impl Service {
 		}
 
 		// Hash and sign
-		let mut pdu_json = utils::to_canonical_object(&pdu).map_err(|e| {
+		let mut pdu_json = utils::to_canonical_object(&pdu)
-			error!("Failed to convert PDU to canonical JSON: {e}");
+			.map_err(|e| err!(Request(BadJson(warn!("Failed to convert PDU to canonical JSON: {e}")))))?;
-			Error::bad_database("Failed to convert PDU to canonical JSON.")
-		})?;
 
 		// room v3 and above removed the "event_id" field from remote PDU format
 		match room_version_id {
@@ -794,8 +792,10 @@ impl Service {
 			.hash_and_sign_event(&mut pdu_json, &room_version_id)
 		{
 			return match e {
-				Error::Signatures(ruma::signatures::Error::PduSize) => Err!(Request(TooLarge("Message is too long"))),
+				Error::Signatures(ruma::signatures::Error::PduSize) => {
-				_ => Err!(Request(Unknown("Signing event failed"))),
+					Err!(Request(TooLarge("Message/PDU is too long (exceeds 65535 bytes)")))
+				},
+				_ => Err!(Request(Unknown(warn!("Signing event failed: {e}")))),
 			};
 		}