style: Fix typo

me and cargo check are oops now

.pre-commit-config.yaml

@@ -23,7 +23,7 @@ repos:
         - id: check-added-large-files
 
   - repo: https://github.com/crate-ci/typos
-    rev: v1.40.0
+    rev: v1.41.0
     hooks:
       - id: typos
       - id: typos
@@ -31,7 +31,7 @@ repos:
         stages: [commit-msg]
 
   - repo: https://github.com/crate-ci/committed
-    rev: v1.1.8
+    rev: v1.1.9
     hooks:
     - id: committed
 

changelog.d/1257.bugfix

@@ -0,0 +1 @@
+Fixed unreliable room summary fetching and improved error messages. Contributed by @nex.

src/api/client/room/summary.rs

@@ -1,11 +1,11 @@
 use axum::extract::State;
 use axum_client_ip::InsecureClientIp;
 use conduwuit::{
-	Err, Result, debug_warn, trace,
+	Err, Result, debug, debug_warn, info, trace,
 	utils::{IterStream, future::TryExtExt},
 };
 use futures::{
-	FutureExt, StreamExt,
+	FutureExt, StreamExt, TryFutureExt,
 	future::{OptionFuture, join3},
 	stream::FuturesUnordered,
 };
@@ -79,9 +79,15 @@ async fn room_summary_response(
 		.server_in_room(services.globals.server_name(), room_id)
 		.await
 	{
-		return local_room_summary_response(services, room_id, sender_user)
+		match local_room_summary_response(services, room_id, sender_user)
 			.boxed()
-			.await;
+			.await
+		{
+			| Ok(response) => return Ok(response),
+			| Err(e) => {
+				debug_warn!("Failed to get local room summary: {e:?}, falling back to remote");
+			},
+		}
 	}
 
 	let room =
@@ -111,26 +117,27 @@ async fn local_room_summary_response(
 	sender_user: Option<&UserId>,
 ) -> Result<get_summary::msc3266::Response> {
 	trace!(?sender_user, "Sending local room summary response for {room_id:?}");
-	let join_rule = services.rooms.state_accessor.get_join_rules(room_id);
-
-	let world_readable = services.rooms.state_accessor.is_world_readable(room_id);
-
-	let guest_can_join = services.rooms.state_accessor.guest_can_join(room_id);
-
-	let (join_rule, world_readable, guest_can_join) =
-		join3(join_rule, world_readable, guest_can_join).await;
-
-	trace!("{join_rule:?}, {world_readable:?}, {guest_can_join:?}");
-	user_can_see_summary(
-		services,
-		room_id,
-		&join_rule.clone().into(),
-		guest_can_join,
-		world_readable,
-		join_rule.allowed_rooms(),
-		sender_user,
+	let (join_rule, world_readable, guest_can_join) = join3(
+		services.rooms.state_accessor.get_join_rules(room_id),
+		services.rooms.state_accessor.is_world_readable(room_id),
+		services.rooms.state_accessor.guest_can_join(room_id),
 	)
-	.await?;
+	.await;
+
+	// Synapse allows server admins to bypass visibility checks.
+	// That seems neat so we'll copy that behaviour.
+	if sender_user.is_none() || !services.users.is_admin(sender_user.unwrap()).await {
+		user_can_see_summary(
+			services,
+			room_id,
+			&join_rule.clone().into(),
+			guest_can_join,
+			world_readable,
+			join_rule.allowed_rooms(),
+			sender_user,
+		)
+		.await?;
+	}
 
 	let canonical_alias = services
 		.rooms
@@ -231,15 +238,27 @@ async fn remote_room_summary_hierarchy_response(
 			"Federaton of room {room_id} is currently disabled on this server."
 		)));
 	}
+	if servers.is_empty() {
+		return Err!(Request(MissingParam(
+			"No servers were provided to fetch the room over federation"
+		)));
+	}
 
 	let request = get_hierarchy::v1::Request::new(room_id.to_owned());
 
 	let mut requests: FuturesUnordered<_> = servers
 		.iter()
 		.map(|server| {
+			info!("Fetching room summary for {room_id} from server {server}");
 			services
 				.sending
 				.send_federation_request(server, request.clone())
+				.inspect_ok(move |v| {
+					debug!("Fetched room summary for {room_id} from server {server}: {v:?}");
+				})
+				.inspect_err(move |e| {
+					info!("Failed to fetch room summary for {room_id} from server {server}: {e}");
+				})
 		})
 		.collect();
 
@@ -255,23 +274,23 @@ async fn remote_room_summary_hierarchy_response(
 			continue;
 		}
 
-		return user_can_see_summary(
-			services,
-			room_id,
-			&room.join_rule,
-			room.guest_can_join,
-			room.world_readable,
-			room.allowed_room_ids.iter().map(AsRef::as_ref),
-			sender_user,
-		)
-		.await
-		.map(|()| room);
+		if sender_user.is_none() || !services.users.is_admin(sender_user.unwrap()).await {
+			return user_can_see_summary(
+				services,
+				room_id,
+				&room.join_rule,
+				room.guest_can_join,
+				room.world_readable,
+				room.allowed_room_ids.iter().map(AsRef::as_ref),
+				sender_user,
+			)
+			.await
+			.map(|()| room);
+		}
+		return Ok(room);
 	}
 
-	Err!(Request(NotFound(
-		"Room is unknown to this server and was unable to fetch over federation with the \
-		 provided servers available"
-	)))
+	Err!(Request(NotFound("Room not found or is not accessible")))
 }
 
 async fn user_can_see_summary<'a, I>(
@@ -311,21 +330,14 @@ async fn user_can_see_summary<'a, I>(
 				return Ok(());
 			}
 
-			Err!(Request(Forbidden(
-				"Room is not world readable, not publicly accessible/joinable, restricted room \
-				 conditions not met, and guest access is forbidden. Not allowed to see details \
-				 of this room."
-			)))
+			Err!(Request(Forbidden("Room is not accessible")))
 		},
 		| None => {
 			if is_public_room || world_readable {
 				return Ok(());
 			}
 
-			Err!(Request(Forbidden(
-				"Room is not world readable or publicly accessible/joinable, authentication is \
-				 required"
-			)))
+			Err!(Request(Forbidden("Room is not accessible")))
 		},
 	}
 }

src/service/pusher/mod.rs

@@ -427,20 +427,20 @@ async fn send_notice<E>(
 				}
 
 				let d = vec![device];
-				let mut notifi = Notification::new(d);
+				let mut notify = Notification::new(d);
 
-				notifi.event_id = Some(event.event_id().to_owned());
-				notifi.room_id = Some(event.room_id().unwrap().to_owned());
+				notify.event_id = Some(event.event_id().to_owned());
+				notify.room_id = Some(event.room_id().unwrap().to_owned());
 				if http
 					.data
 					.get("org.matrix.msc4076.disable_badge_count")
 					.is_none() && http.data.get("disable_badge_count").is_none()
 				{
-					notifi.counts = NotificationCounts::new(unread, uint!(0));
+					notify.counts = NotificationCounts::new(unread, uint!(0));
 				} else {
 					// counts will not be serialised if it's the default (0, 0)
 					// skip_serializing_if = "NotificationCounts::is_default"
-					notifi.counts = NotificationCounts::default();
+					notify.counts = NotificationCounts::default();
 				}
 
 				if !event_id_only {
@@ -449,30 +449,30 @@ async fn send_notice<E>(
 							.iter()
 							.any(|t| matches!(t, Tweak::Highlight(true) | Tweak::Sound(_)))
 					{
-						notifi.prio = NotificationPriority::High;
+						notify.prio = NotificationPriority::High;
 					} else {
-						notifi.prio = NotificationPriority::Low;
+						notify.prio = NotificationPriority::Low;
 					}
-					notifi.sender = Some(event.sender().to_owned());
-					notifi.event_type = Some(event.kind().to_owned());
-					notifi.content = serde_json::value::to_raw_value(event.content()).ok();
+					notify.sender = Some(event.sender().to_owned());
+					notify.event_type = Some(event.kind().to_owned());
+					notify.content = serde_json::value::to_raw_value(event.content()).ok();
 
 					if *event.kind() == TimelineEventType::RoomMember {
-						notifi.user_is_target =
+						notify.user_is_target =
 							event.state_key() == Some(event.sender().as_str());
 					}
 
-					notifi.sender_display_name =
+					notify.sender_display_name =
 						self.services.users.displayname(event.sender()).await.ok();
 
-					notifi.room_name = self
+					notify.room_name = self
 						.services
 						.state_accessor
 						.get_name(event.room_id().unwrap())
 						.await
 						.ok();
 
-					notifi.room_alias = self
+					notify.room_alias = self
 						.services
 						.state_accessor
 						.get_canonical_alias(event.room_id().unwrap())
@@ -480,7 +480,7 @@ async fn send_notice<E>(
 						.ok();
 				}
 
-				self.send_request(&http.url, send_event_notification::v1::Request::new(notifi))
+				self.send_request(&http.url, send_event_notification::v1::Request::new(notify))
 					.await?;
 
 				Ok(())