refactor some additional errors

Signed-off-by: Jason Volk <jason@zemos.net>

src/admin/debug/commands.rs

@@ -7,7 +7,7 @@ use std::{
 
 use api::client::validate_and_add_event_id;
 use conduit::{
-	debug, info, log,
+	debug, debug_error, err, info, log,
 	log::{capture, Capture},
 	utils, warn, Error, PduEvent, Result,
 };
@@ -575,8 +575,8 @@ pub(super) async fn force_set_room_state_from_server(
 		};
 
 		let pdu = PduEvent::from_id_val(&event_id, value.clone()).map_err(|e| {
-			warn!("Invalid PDU in fetching remote room state PDUs response: {} {:?}", e, value);
+			debug_error!("Invalid PDU in fetching remote room state PDUs response: {value:#?}");
-			Error::BadServerResponse("Invalid PDU in send_join response.")
+			err!(BadServerResponse(debug_error!("Invalid PDU in send_join response: {e:?}")))
 		})?;
 
 		self.services

src/api/client/keys.rs

@@ -4,7 +4,7 @@ use std::{
 };
 
 use axum::extract::State;
-use conduit::{debug, utils, utils::math::continue_exponential_backoff_secs, Error, Result};
+use conduit::{utils, utils::math::continue_exponential_backoff_secs, Err, Error, Result};
 use futures_util::{stream::FuturesUnordered, StreamExt};
 use ruma::{
 	api::{
@@ -362,8 +362,7 @@ pub(crate) async fn get_keys_helper<F: Fn(&UserId) -> bool + Send>(
 				const MIN: u64 = 5 * 60;
 				const MAX: u64 = 60 * 60 * 24;
 				if continue_exponential_backoff_secs(MIN, MAX, time.elapsed(), *tries) {
-					debug!("Backing off query from {server:?}");
+					return (server, Err!(BadServerResponse("bad query from {server:?}, still backing off")));
-					return (server, Err(Error::BadServerResponse("bad query, still backing off")));
 				}
 			}
 

src/api/client/media.rs

@@ -5,13 +5,13 @@ use std::time::Duration;
 use axum::extract::State;
 use axum_client_ip::InsecureClientIp;
 use conduit::{
-	debug_warn, error,
+	debug_warn, err, error,
 	utils::{
 		self,
 		content_disposition::{content_disposition_type, make_content_disposition, sanitise_filename},
 		math::ruma_from_usize,
 	},
-	warn, Error, Result,
+	warn, Err, Error, Result,
 };
 use ruma::api::client::{
 	error::{ErrorKind, RetryAfter},
@@ -70,8 +70,9 @@ pub(crate) async fn get_media_preview_route(
 
 	let url = &body.url;
 	if !services.media.url_preview_allowed(url) {
-		warn!(%sender_user, "URL is not allowed to be previewed: {url}");
+		return Err!(Request(Forbidden(
-		return Err(Error::BadRequest(ErrorKind::forbidden(), "URL is not allowed to be previewed"));
+			warn!(%sender_user, %url, "URL is not allowed to be previewed")
+		)));
 	}
 
 	match services.media.get_url_preview(url).await {
@@ -90,7 +91,6 @@ pub(crate) async fn get_media_preview_route(
 		},
 		Err(e) => {
 			warn!(%sender_user, "Failed to generate a URL preview: {e}");
-
 			// there doesn't seem to be an agreed-upon error code in the spec.
 			// the only response codes in the preview_url spec page are 200 and 429.
 			Err(Error::BadRequest(
@@ -222,10 +222,7 @@ pub(crate) async fn get_content_route(
 			body.timeout_ms,
 		)
 		.await
-		.map_err(|e| {
+		.map_err(|e| err!(Request(NotFound(debug_warn!("Fetching media `{mxc}` failed: {e:?}")))))?;
-			debug_warn!("Fetching media `{}` failed: {:?}", mxc, e);
-			Error::BadRequest(ErrorKind::NotFound, "Remote media error.")
-		})?;
 
 		let content_disposition = Some(make_content_disposition(
 			&response.content_type,
@@ -241,7 +238,7 @@ pub(crate) async fn get_content_route(
 			cache_control: Some(CACHE_CONTROL_IMMUTABLE.to_owned()),
 		})
 	} else {
-		Err(Error::BadRequest(ErrorKind::NotFound, "Media not found."))
+		Err!(Request(NotFound("Media not found.")))
 	}
 }
 
@@ -328,13 +325,10 @@ pub(crate) async fn get_content_as_filename_route(
 					cache_control: Some(CACHE_CONTROL_IMMUTABLE.into()),
 				})
 			},
-			Err(e) => {
+			Err(e) => Err!(Request(NotFound(debug_warn!("Fetching media `{mxc}` failed: {e:?}")))),
-				debug_warn!("Fetching media `{}` failed: {:?}", mxc, e);
-				Err(Error::BadRequest(ErrorKind::NotFound, "Remote media error."))
-			},
 		}
 	} else {
-		Err(Error::BadRequest(ErrorKind::NotFound, "Media not found."))
+		Err!(Request(NotFound("Media not found.")))
 	}
 }
 
@@ -385,10 +379,10 @@ pub(crate) async fn get_content_thumbnail_route(
 			&mxc,
 			body.width
 				.try_into()
-				.map_err(|_| Error::BadRequest(ErrorKind::InvalidParam, "Width is invalid."))?,
+				.map_err(|e| err!(Request(InvalidParam("Width is invalid: {e:?}"))))?,
 			body.height
 				.try_into()
-				.map_err(|_| Error::BadRequest(ErrorKind::InvalidParam, "Height is invalid."))?,
+				.map_err(|e| err!(Request(InvalidParam("Height is invalid: {e:?}"))))?,
 		)
 		.await?
 	{
@@ -411,7 +405,7 @@ pub(crate) async fn get_content_thumbnail_route(
 			// we'll lie to the client and say the blocked server's media was not found and
 			// log. the client has no way of telling anyways so this is a security bonus.
 			debug_warn!("Received request for media `{}` on blocklisted server", mxc);
-			return Err(Error::BadRequest(ErrorKind::NotFound, "Media not found."));
+			return Err!(Request(NotFound("Media not found.")));
 		}
 
 		match services
@@ -460,13 +454,10 @@ pub(crate) async fn get_content_thumbnail_route(
 					content_disposition,
 				})
 			},
-			Err(e) => {
+			Err(e) => Err!(Request(NotFound(debug_warn!("Fetching media `{mxc}` failed: {e:?}")))),
-				debug_warn!("Fetching media `{}` failed: {:?}", mxc, e);
-				Err(Error::BadRequest(ErrorKind::NotFound, "Remote media error."))
-			},
 		}
 	} else {
-		Err(Error::BadRequest(ErrorKind::NotFound, "Media not found."))
+		Err!(Request(NotFound("Media not found.")))
 	}
 }
 
@@ -504,7 +495,7 @@ async fn get_remote_content(
 		// we'll lie to the client and say the blocked server's media was not found and
 		// log. the client has no way of telling anyways so this is a security bonus.
 		debug_warn!("Received request for media `{mxc}` on blocklisted server");
-		return Err(Error::BadRequest(ErrorKind::NotFound, "Media not found."));
+		return Err!(Request(NotFound("Media not found.")));
 	}
 
 	let content_response = services

src/api/client/membership.rs

@@ -8,11 +8,11 @@ use std::{
 use axum::extract::State;
 use axum_client_ip::InsecureClientIp;
 use conduit::{
-	debug, debug_warn, error, info,
+	debug, debug_error, debug_warn, err, error, info,
 	pdu::{gen_event_id_canonical_json, PduBuilder},
 	trace, utils,
 	utils::math::continue_exponential_backoff_secs,
-	warn, Error, PduEvent, Result,
+	warn, Err, Error, PduEvent, Result,
 };
 use ruma::{
 	api::{
@@ -705,11 +705,11 @@ async fn join_room_by_id_helper_remote(
 		{
 			room_version
 		},
-		_ => return Err(Error::BadServerResponse("Room version is not supported")),
+		_ => return Err!(BadServerResponse("Room version is not supported")),
 	};
 
 	let mut join_event_stub: CanonicalJsonObject = serde_json::from_str(make_join_response.event.get())
-		.map_err(|_| Error::BadServerResponse("Invalid make_join event json received from server."))?;
+		.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")
@@ -876,7 +876,7 @@ async fn join_room_by_id_helper_remote(
 
 	info!("Parsing join event");
 	let parsed_join_pdu = PduEvent::from_id_val(event_id, join_event.clone())
-		.map_err(|_| Error::BadServerResponse("Invalid join event PDU."))?;
+		.map_err(|e| err!(BadServerResponse("Invalid join event PDU: {e:?}")))?;
 
 	let mut state = HashMap::new();
 	let pub_key_map = RwLock::new(BTreeMap::new());
@@ -899,8 +899,8 @@ async fn join_room_by_id_helper_remote(
 		};
 
 		let pdu = PduEvent::from_id_val(&event_id, value.clone()).map_err(|e| {
-			warn!("Invalid PDU in send_join response: {} {:?}", e, value);
+			debug_warn!("Invalid PDU in send_join response: {value:#?}");
-			Error::BadServerResponse("Invalid PDU in send_join response.")
+			err!(BadServerResponse("Invalid PDU in send_join response: {e:?}"))
 		})?;
 
 		services.rooms.outlier.add_pdu_outlier(&event_id, &value)?;
@@ -1122,10 +1122,10 @@ async fn join_room_by_id_helper_local(
 			{
 				room_version_id
 			},
-			_ => return Err(Error::BadServerResponse("Room version is not supported")),
+			_ => return Err!(BadServerResponse("Room version is not supported")),
 		};
 		let mut join_event_stub: CanonicalJsonObject = serde_json::from_str(make_join_response.event.get())
-			.map_err(|_| Error::BadServerResponse("Invalid make_join event json received from server."))?;
+			.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| {
@@ -1250,7 +1250,7 @@ async fn join_room_by_id_helper_local(
 async fn make_join_request(
 	services: &Services, sender_user: &UserId, room_id: &RoomId, servers: &[OwnedServerName],
 ) -> Result<(federation::membership::prepare_join_event::v1::Response, OwnedServerName)> {
-	let mut make_join_response_and_server = Err(Error::BadServerResponse("No server available to assist in joining."));
+	let mut make_join_response_and_server = Err!(BadServerResponse("No server available to assist in joining."));
 
 	let mut make_join_counter: u16 = 0;
 	let mut incompatible_room_version_count: u8 = 0;
@@ -1291,8 +1291,7 @@ async fn make_join_request(
 					"15 servers have responded with M_INCOMPATIBLE_ROOM_VERSION or M_UNSUPPORTED_ROOM_VERSION, \
 					 assuming that Conduwuit does not support the room {room_id}: {e}"
 				);
-				make_join_response_and_server =
+				make_join_response_and_server = Err!(BadServerResponse("Room version is not supported by Conduwuit"));
-					Err(Error::BadServerResponse("Room version is not supported by Conduwuit"));
 				return make_join_response_and_server;
 			}
 
@@ -1300,8 +1299,7 @@ async fn make_join_request(
 				warn!(
 					"50 servers failed to provide valid make_join response, assuming no server can assist in joining."
 				);
-				make_join_response_and_server =
+				make_join_response_and_server = Err!(BadServerResponse("No server available to assist in joining."));
-					Err(Error::BadServerResponse("No server available to assist in joining."));
 				return make_join_response_and_server;
 			}
 		}
@@ -1321,8 +1319,8 @@ pub async fn validate_and_add_event_id(
 	pub_key_map: &RwLock<BTreeMap<String, BTreeMap<String, Base64>>>,
 ) -> Result<(OwnedEventId, CanonicalJsonObject)> {
 	let mut value: CanonicalJsonObject = serde_json::from_str(pdu.get()).map_err(|e| {
-		error!("Invalid PDU in server response: {:?}: {:?}", pdu, e);
+		debug_error!("Invalid PDU in server response: {pdu:#?}");
-		Error::BadServerResponse("Invalid PDU in server response")
+		err!(BadServerResponse("Invalid PDU in server response: {e:?}"))
 	})?;
 	let event_id = EventId::parse(format!(
 		"${}",
@@ -1358,15 +1356,15 @@ pub async fn validate_and_add_event_id(
 		const MIN: u64 = 60 * 5;
 		const MAX: u64 = 60 * 60 * 24;
 		if continue_exponential_backoff_secs(MIN, MAX, time.elapsed(), *tries) {
-			debug!("Backing off from {event_id}");
+			return Err!(BadServerResponse("bad event {event_id:?}, still backing off"));
-			return Err(Error::BadServerResponse("bad event, still backing off"));
 		}
 	}
 
 	if let Err(e) = ruma::signatures::verify_event(&*pub_key_map.read().await, &value, room_version) {
-		warn!("Event {} failed verification {:?} {}", event_id, pdu, e);
+		debug_error!("Event {event_id} failed verification {pdu:#?}");
+		let e = Err!(BadServerResponse(debug_error!("Event {event_id} failed verification: {e:?}")));
 		back_off(event_id).await;
-		return Err(Error::BadServerResponse("Event failed verification."));
+		return e;
 	}
 
 	value.insert("event_id".to_owned(), CanonicalJsonValue::String(event_id.as_str().to_owned()));
@@ -1648,7 +1646,7 @@ pub async fn leave_room(services: &Services, user_id: &UserId, room_id: &RoomId,
 }
 
 async fn remote_leave_room(services: &Services, user_id: &UserId, room_id: &RoomId) -> Result<()> {
-	let mut make_leave_response_and_server = Err(Error::BadServerResponse("No server available to assist in leaving."));
+	let mut make_leave_response_and_server = Err!(BadServerResponse("No server available to assist in leaving."));
 
 	let invite_state = services
 		.rooms
@@ -1705,11 +1703,11 @@ async fn remote_leave_room(services: &Services, user_id: &UserId, room_id: &Room
 		{
 			version
 		},
-		_ => return Err(Error::BadServerResponse("Room version is not supported")),
+		_ => return Err!(BadServerResponse("Room version is not supported")),
 	};
 
 	let mut leave_event_stub = serde_json::from_str::<CanonicalJsonObject>(make_leave_response.event.get())
-		.map_err(|_| Error::BadServerResponse("Invalid make_leave event json received from server."))?;
+		.map_err(|e| err!(BadServerResponse("Invalid make_leave event json received from server: {e:?}")))?;
 
 	// TODO: Is origin needed?
 	leave_event_stub.insert(

src/core/error/mod.rs

@@ -86,13 +86,13 @@ pub enum Error {
 	// conduwuit
 	#[error("Arithmetic operation failed: {0}")]
 	Arithmetic(Cow<'static, str>),
+	#[error("{0}")]
+	BadServerResponse(Cow<'static, str>),
 	#[error("There was a problem with the '{0}' directive in your configuration: {1}")]
 	Config(&'static str, Cow<'static, str>),
 	#[error("{0}")]
 	Database(Cow<'static, str>),
 	#[error("{0}")]
-	BadServerResponse(&'static str),
-	#[error("{0}")]
 	Conflict(&'static str), // This is only needed for when a room alias already exists
 
 	// unique / untyped

src/core/pdu/mod.rs

@@ -23,7 +23,7 @@ use serde_json::{
 	value::{to_raw_value, RawValue as RawJsonValue},
 };
 
-use crate::{warn, Error};
+use crate::{err, warn, Error};
 
 #[derive(Deserialize)]
 struct ExtractRedactedBecause {
@@ -396,10 +396,8 @@ impl Ord for PduEvent {
 pub fn gen_event_id_canonical_json(
 	pdu: &RawJsonValue, room_version_id: &RoomVersionId,
 ) -> crate::Result<(OwnedEventId, CanonicalJsonObject)> {
-	let value: CanonicalJsonObject = serde_json::from_str(pdu.get()).map_err(|e| {
+	let value: CanonicalJsonObject = serde_json::from_str(pdu.get())
-		warn!("Error parsing incoming event {:?}: {:?}", pdu, e);
+		.map_err(|e| err!(BadServerResponse(warn!("Error parsing incoming event: {e:?}"))))?;
-		Error::BadServerResponse("Invalid PDU in server response")
-	})?;
 
 	let event_id = format!(
 		"${}",

src/service/media/preview.rs

@@ -1,10 +1,9 @@
 use std::{io::Cursor, time::SystemTime};
 
-use conduit::{debug, utils, warn, Error, Result};
+use conduit::{debug, utils, warn, Err, Result};
 use conduit_core::implement;
 use image::ImageReader as ImgReader;
 use ipaddress::IPAddress;
-use ruma::api::client::error::ErrorKind;
 use serde::Serialize;
 use url::Url;
 use webpage::HTML;
@@ -89,7 +88,7 @@ pub async fn get_url_preview(&self, url: &str) -> Result<UrlPreviewData> {
 async fn request_url_preview(&self, url: &str) -> Result<UrlPreviewData> {
 	if let Ok(ip) = IPAddress::parse(url) {
 		if !self.services.globals.valid_cidr_range(&ip) {
-			return Err(Error::BadServerResponse("Requesting from this address is forbidden"));
+			return Err!(BadServerResponse("Requesting from this address is forbidden"));
 		}
 	}
 
@@ -99,7 +98,7 @@ async fn request_url_preview(&self, url: &str) -> Result<UrlPreviewData> {
 	if let Some(remote_addr) = response.remote_addr() {
 		if let Ok(ip) = IPAddress::parse(remote_addr.ip().to_string()) {
 			if !self.services.globals.valid_cidr_range(&ip) {
-				return Err(Error::BadServerResponse("Requesting from this address is forbidden"));
+				return Err!(BadServerResponse("Requesting from this address is forbidden"));
 			}
 		}
 	}
@@ -109,12 +108,12 @@ async fn request_url_preview(&self, url: &str) -> Result<UrlPreviewData> {
 		.get(reqwest::header::CONTENT_TYPE)
 		.and_then(|x| x.to_str().ok())
 	else {
-		return Err(Error::BadRequest(ErrorKind::Unknown, "Unknown Content-Type"));
+		return Err!(Request(Unknown("Unknown Content-Type")));
 	};
 	let data = match content_type {
 		html if html.starts_with("text/html") => self.download_html(url).await?,
 		img if img.starts_with("image/") => self.download_image(url).await?,
-		_ => return Err(Error::BadRequest(ErrorKind::Unknown, "Unsupported Content-Type")),
+		_ => return Err!(Request(Unknown("Unsupported Content-Type"))),
 	};
 
 	self.set_url_preview(url, &data).await?;
@@ -142,7 +141,7 @@ async fn download_html(&self, url: &str) -> Result<UrlPreviewData> {
 	}
 	let body = String::from_utf8_lossy(&bytes);
 	let Ok(html) = HTML::from_string(body.to_string(), Some(url.to_owned())) else {
-		return Err(Error::BadRequest(ErrorKind::Unknown, "Failed to parse HTML"));
+		return Err!(Request(Unknown("Failed to parse HTML")));
 	};
 
 	let mut data = match html.opengraph.images.first() {

src/service/pusher/mod.rs

@@ -3,7 +3,7 @@ mod data;
 use std::{fmt::Debug, mem, sync::Arc};
 
 use bytes::BytesMut;
-use conduit::{debug_info, info, trace, utils::string_from_bytes, warn, Error, PduEvent, Result};
+use conduit::{debug_error, err, trace, utils::string_from_bytes, warn, Err, PduEvent, Result};
 use ipaddress::IPAddress;
 use ruma::{
 	api::{
@@ -84,8 +84,9 @@ impl Service {
 		let http_request = request
 			.try_into_http_request::<BytesMut>(&dest, SendAccessToken::IfRequired(""), &VERSIONS)
 			.map_err(|e| {
-				warn!("Failed to find destination {dest} for push gateway: {e}");
+				err!(BadServerResponse(warn!(
-				Error::BadServerResponse("Invalid push gateway destination")
+					"Failed to find destination {dest} for push gateway: {e}"
+				)))
 			})?
 			.map(BytesMut::freeze);
 
@@ -95,7 +96,7 @@ impl Service {
 			trace!("Checking request URL for IP");
 			if let Ok(ip) = IPAddress::parse(url_host) {
 				if !self.services.globals.valid_cidr_range(&ip) {
-					return Err(Error::BadServerResponse("Not allowed to send requests to this IP"));
+					return Err!(BadServerResponse("Not allowed to send requests to this IP"));
 				}
 			}
 		}
@@ -110,7 +111,7 @@ impl Service {
 				if let Some(remote_addr) = response.remote_addr() {
 					if let Ok(ip) = IPAddress::parse(remote_addr.ip().to_string()) {
 						if !self.services.globals.valid_cidr_range(&ip) {
-							return Err(Error::BadServerResponse("Not allowed to send requests to this IP"));
+							return Err!(BadServerResponse("Not allowed to send requests to this IP"));
 						}
 					}
 				}
@@ -129,10 +130,10 @@ impl Service {
 				let body = response.bytes().await?; // TODO: handle timeout
 
 				if !status.is_success() {
-					info!("Push gateway {dest} returned unsuccessful HTTP response ({status})");
+					debug_error!("Push gateway response body: {:?}", string_from_bytes(&body));
-					debug_info!("Push gateway response body: {:?}", string_from_bytes(&body));
+					return Err!(BadServerResponse(error!(
-
+						"Push gateway {dest} returned unsuccessful HTTP response: {status}"
-					return Err(Error::BadServerResponse("Push gateway returned unsuccessful response"));
+					)));
 				}
 
 				let response = T::IncomingResponse::try_from_http_response(
@@ -140,13 +141,11 @@ impl Service {
 						.body(body)
 						.expect("reqwest body is valid http body"),
 				);
-				response.map_err(|e| {
+				response
-					warn!("Push gateway {dest} returned invalid response bytes: {e}");
+					.map_err(|e| err!(BadServerResponse(error!("Push gateway {dest} returned invalid response: {e}"))))
-					Error::BadServerResponse("Push gateway returned bad/invalid response")
-				})
 			},
 			Err(e) => {
-				warn!("Could not send request to pusher {dest}: {e}");
+				debug_error!("Could not send request to pusher {dest}: {e}");
 				Err(e.into())
 			},
 		}
@@ -165,7 +164,7 @@ impl Service {
 			.room_state_get(&pdu.room_id, &StateEventType::RoomPowerLevels, "")?
 			.map(|ev| {
 				serde_json::from_str(ev.content.get())
-					.map_err(|_| Error::bad_database("invalid m.room.power_levels event"))
+					.map_err(|e| err!(Database("invalid m.room.power_levels event: {e:?}")))
 			})
 			.transpose()?
 			.unwrap_or_default();
@@ -181,8 +180,8 @@ impl Service {
 			};
 
 			if notify.is_some() {
-				return Err(Error::bad_database(
+				return Err!(Database(
-					r#"Malformed pushrule contains more than one of these actions: ["dont_notify", "notify", "coalesce"]"#,
+					r#"Malformed pushrule contains more than one of these actions: ["dont_notify", "notify", "coalesce"]"#
 				));
 			}
 

src/service/resolver/actual.rs

@@ -4,7 +4,7 @@ use std::{
 	sync::Arc,
 };
 
-use conduit::{debug, debug_error, debug_info, debug_warn, trace, Err, Error, Result};
+use conduit::{debug, debug_error, debug_info, debug_warn, err, trace, Err, Result};
 use hickory_resolver::{error::ResolveError, lookup::SrvLookup};
 use ipaddress::IPAddress;
 use ruma::ServerName;
@@ -329,10 +329,8 @@ impl super::Service {
 			dest.is_ip_literal() || !IPAddress::is_valid(dest.host()),
 			"Destination is not an IP literal."
 		);
-		let ip = IPAddress::parse(dest.host()).map_err(|e| {
+		let ip = IPAddress::parse(dest.host())
-			debug_error!("Failed to parse IP literal from string: {}", e);
+			.map_err(|e| err!(BadServerResponse(debug_error!("Failed to parse IP literal from string: {e}"))))?;
-			Error::BadServerResponse("Invalid IP address")
-		})?;
 
 		self.validate_ip(&ip)?;
 
@@ -341,7 +339,7 @@ impl super::Service {
 
 	pub(crate) fn validate_ip(&self, ip: &IPAddress) -> Result<()> {
 		if !self.services.globals.valid_cidr_range(ip) {
-			return Err(Error::BadServerResponse("Not allowed to send requests to this IP"));
+			return Err!(BadServerResponse("Not allowed to send requests to this IP"));
 		}
 
 		Ok(())

src/service/rooms/event_handler/parse_incoming_pdu.rs

@@ -1,18 +1,18 @@
-use conduit::{pdu::gen_event_id_canonical_json, warn, Err, Error, Result};
+use conduit::{debug_warn, err, pdu::gen_event_id_canonical_json, Err, Result};
-use ruma::{api::client::error::ErrorKind, CanonicalJsonObject, OwnedEventId, OwnedRoomId, RoomId};
+use ruma::{CanonicalJsonObject, OwnedEventId, OwnedRoomId, RoomId};
 use serde_json::value::RawValue as RawJsonValue;
 
 impl super::Service {
 	pub fn parse_incoming_pdu(&self, pdu: &RawJsonValue) -> Result<(OwnedEventId, CanonicalJsonObject, OwnedRoomId)> {
 		let value: CanonicalJsonObject = serde_json::from_str(pdu.get()).map_err(|e| {
-			warn!("Error parsing incoming event {pdu:?}: {e:?}");
+			debug_warn!("Error parsing incoming event {pdu:#?}");
-			Error::BadServerResponse("Invalid PDU in server response")
+			err!(BadServerResponse("Error parsing incoming event {e:?}"))
 		})?;
 
 		let room_id: OwnedRoomId = value
 			.get("room_id")
 			.and_then(|id| RoomId::parse(id.as_str()?).ok())
-			.ok_or(Error::BadRequest(ErrorKind::InvalidParam, "Invalid room id in pdu"))?;
+			.ok_or(err!(Request(InvalidParam("Invalid room id in pdu"))))?;
 
 		let Ok(room_version_id) = self.services.state.get_room_version(&room_id) else {
 			return Err!("Server is not in room {room_id}");

src/service/sending/appservice.rs

@@ -1,7 +1,7 @@
 use std::{fmt::Debug, mem};
 
 use bytes::BytesMut;
-use conduit::{debug_error, trace, utils, warn, Error, Result};
+use conduit::{debug_error, err, trace, utils, warn, Err, Result};
 use reqwest::Client;
 use ruma::api::{appservice::Registration, IncomingResponse, MatrixVersion, OutgoingRequest, SendAccessToken};
 
@@ -26,10 +26,7 @@ where
 	let hs_token = registration.hs_token.as_str();
 	let mut http_request = request
 		.try_into_http_request::<BytesMut>(&dest, SendAccessToken::IfRequired(hs_token), &VERSIONS)
-		.map_err(|e| {
+		.map_err(|e| err!(BadServerResponse(warn!("Failed to find destination {dest}: {e}"))))?
-			warn!("Failed to find destination {dest}: {e}");
-			Error::BadServerResponse("Invalid appservice destination")
-		})?
 		.map(BytesMut::freeze);
 
 	let mut parts = http_request.uri().clone().into_parts();
@@ -69,13 +66,11 @@ where
 	let body = response.bytes().await?; // TODO: handle timeout
 
 	if !status.is_success() {
-		warn!(
+		debug_error!("Appservice response bytes: {:?}", utils::string_from_bytes(&body));
+		return Err!(BadServerResponse(error!(
 			"Appservice \"{}\" returned unsuccessful HTTP response {status} at {dest}",
 			registration.id
-		);
+		)));
-		debug_error!("Appservice response bytes: {:?}", utils::string_from_bytes(&body));
-
-		return Err(Error::BadServerResponse("Appservice returned unsuccessful HTTP response"));
 	}
 
 	let response = T::IncomingResponse::try_from_http_response(
@@ -85,7 +80,9 @@ where
 	);
 
 	response.map(Some).map_err(|e| {
-		warn!("Appservice \"{}\" returned invalid response bytes {dest}: {e}", registration.id);
+		err!(BadServerResponse(error!(
-		Error::BadServerResponse("Appservice returned bad/invalid response")
+			"Appservice \"{}\" returned invalid response bytes {dest}: {e}",
+			registration.id
+		)))
 	})
 }

src/service/sending/send.rs

@@ -1,7 +1,7 @@
 use std::{fmt::Debug, mem};
 
 use conduit::{
-	debug, debug_error, debug_warn, error::inspect_debug_log, trace, utils::string::EMPTY, Err, Error, Result,
+	debug, debug_error, debug_warn, err, error::inspect_debug_log, trace, utils::string::EMPTY, Err, Error, Result,
 };
 use http::{header::AUTHORIZATION, HeaderValue};
 use ipaddress::IPAddress;
@@ -62,7 +62,7 @@ impl super::Service {
 		trace!("Preparing request");
 		let mut http_request = req
 			.try_into_http_request::<Vec<u8>>(&actual.string, SATIR, &VERSIONS)
-			.map_err(|_| Error::BadServerResponse("Invalid destination"))?;
+			.map_err(|e| err!(BadServerResponse("Invalid destination: {e:?}")))?;
 
 		sign_request::<T>(&self.services.globals, dest, &mut http_request);
 
@@ -139,10 +139,7 @@ where
 		);
 	}
 
-	match response {
+	response.map_err(|e| err!(BadServerResponse("Server returned bad 200 response: {e:?}")))
-		Err(_) => Err(Error::BadServerResponse("Server returned bad 200 response.")),
-		Ok(response) => Ok(response),
-	}
 }
 
 fn handle_error<T>(

src/service/server_keys/mod.rs

@@ -4,7 +4,7 @@ use std::{
 	time::{Duration, SystemTime},
 };
 
-use conduit::{debug, error, info, trace, warn, Error, Result};
+use conduit::{debug, debug_error, debug_warn, err, error, info, trace, warn, Err, Result};
 use futures_util::{stream::FuturesUnordered, StreamExt};
 use ruma::{
 	api::federation::{
@@ -57,13 +57,13 @@ impl Service {
 		for event in events {
 			for (signature_server, signature) in event
 				.get("signatures")
-				.ok_or(Error::BadServerResponse("No signatures in server response pdu."))?
+				.ok_or(err!(BadServerResponse("No signatures in server response pdu.")))?
 				.as_object()
-				.ok_or(Error::BadServerResponse("Invalid signatures object in server response pdu."))?
+				.ok_or(err!(BadServerResponse("Invalid signatures object in server response pdu.")))?
 			{
-				let signature_object = signature.as_object().ok_or(Error::BadServerResponse(
+				let signature_object = signature.as_object().ok_or(err!(BadServerResponse(
 					"Invalid signatures content object in server response pdu.",
-				))?;
+				)))?;
 
 				for signature_id in signature_object.keys() {
 					server_key_ids
@@ -94,10 +94,12 @@ impl Service {
 			.map(|(signature_server, signature_ids)| async {
 				let fetch_res = self
 					.fetch_signing_keys_for_server(
-						signature_server.as_str().try_into().map_err(|_| {
+						signature_server.as_str().try_into().map_err(|e| {
 							(
 								signature_server.clone(),
-								Error::BadServerResponse("Invalid servername in signatures of server response pdu."),
+								err!(BadServerResponse(
+									"Invalid servername in signatures of server response pdu: {e:?}"
+								)),
 							)
 						})?,
 						signature_ids.into_iter().collect(), // HashSet to Vec
@@ -107,7 +109,9 @@ impl Service {
 				match fetch_res {
 					Ok(keys) => Ok((signature_server, keys)),
 					Err(e) => {
-						warn!("Signature verification failed: Could not fetch signing key for {signature_server}: {e}",);
+						debug_error!(
+							"Signature verification failed: Could not fetch signing key for {signature_server}: {e}",
+						);
 						Err((signature_server, e))
 					},
 				}
@@ -123,7 +127,7 @@ impl Service {
 						.insert(signature_server.clone(), keys);
 				},
 				Err((signature_server, e)) => {
-					warn!("Failed to fetch keys for {}: {:?}", signature_server, e);
+					debug_warn!("Failed to fetch keys for {signature_server}: {e:?}");
 				},
 			}
 		}
@@ -141,35 +145,37 @@ impl Service {
 		pub_key_map: &mut RwLockWriteGuard<'_, BTreeMap<String, BTreeMap<String, Base64>>>,
 	) -> Result<()> {
 		let value: CanonicalJsonObject = serde_json::from_str(pdu.get()).map_err(|e| {
-			error!("Invalid PDU in server response: {:?}: {:?}", pdu, e);
+			debug_error!("Invalid PDU in server response: {pdu:#?}");
-			Error::BadServerResponse("Invalid PDU in server response")
+			err!(BadServerResponse(error!("Invalid PDU in server response: {e:?}")))
 		})?;
 
 		let signatures = value
 			.get("signatures")
-			.ok_or(Error::BadServerResponse("No signatures in server response pdu."))?
+			.ok_or(err!(BadServerResponse("No signatures in server response pdu.")))?
 			.as_object()
-			.ok_or(Error::BadServerResponse("Invalid signatures object in server response pdu."))?;
+			.ok_or(err!(BadServerResponse("Invalid signatures object in server response pdu.")))?;
 
 		for (signature_server, signature) in signatures {
-			let signature_object = signature.as_object().ok_or(Error::BadServerResponse(
+			let signature_object = signature.as_object().ok_or(err!(BadServerResponse(
 				"Invalid signatures content object in server response pdu.",
-			))?;
+			)))?;
 
 			let signature_ids = signature_object.keys().cloned().collect::<Vec<_>>();
 
 			let contains_all_ids =
 				|keys: &BTreeMap<String, Base64>| signature_ids.iter().all(|id| keys.contains_key(id));
 
-			let origin = <&ServerName>::try_from(signature_server.as_str())
+			let origin = <&ServerName>::try_from(signature_server.as_str()).map_err(|e| {
-				.map_err(|_| Error::BadServerResponse("Invalid servername in signatures of server response pdu."))?;
+				err!(BadServerResponse(
+					"Invalid servername in signatures of server response pdu: {e:?}"
+				))
+			})?;
 
 			if servers.contains_key(origin) || pub_key_map.contains_key(origin.as_str()) {
 				continue;
 			}
 
-			debug!("Loading signing keys for {}", origin);
+			debug!("Loading signing keys for {origin}");
-
 			let result: BTreeMap<_, _> = self
 				.services
 				.globals
@@ -179,7 +185,7 @@ impl Service {
 				.collect();
 
 			if !contains_all_ids(&result) {
-				debug!("Signing key not loaded for {}", origin);
+				debug_warn!("Signing key not loaded for {origin}");
 				servers.insert(origin.to_owned(), BTreeMap::new());
 			}
 
@@ -196,7 +202,7 @@ impl Service {
 		pub_key_map: &RwLock<BTreeMap<String, BTreeMap<String, Base64>>>,
 	) -> Result<()> {
 		for server in self.services.globals.trusted_servers() {
-			debug!("Asking batch signing keys from trusted server {}", server);
+			debug!("Asking batch signing keys from trusted server {server}");
 			match self
 				.services
 				.sending
@@ -209,14 +215,16 @@ impl Service {
 				.await
 			{
 				Ok(keys) => {
-					debug!("Got signing keys: {:?}", keys);
+					debug!("Got signing keys: {keys:?}");
 					let mut pkm = pub_key_map.write().await;
 					for k in keys.server_keys {
 						let k = match k.deserialize() {
 							Ok(key) => key,
 							Err(e) => {
-								warn!("Received error {e} while fetching keys from trusted server {server}");
+								warn!(
-								warn!("{}", k.into_json());
+									"Received error {e} while fetching keys from trusted server {server}: {:#?}",
+									k.into_json()
+								);
 								continue;
 							},
 						};
@@ -236,13 +244,10 @@ impl Service {
 						pkm.insert(k.server_name.to_string(), result);
 					}
 				},
-				Err(e) => {
+				Err(e) => error!(
-					warn!(
 					"Failed sending batched key request to trusted key server {server} for the remote servers \
-						 {:?}: {e}",
+					 {servers:?}: {e}"
-						servers
+				),
-					);
-				},
 			}
 		}
 
@@ -478,7 +483,6 @@ impl Service {
 			}
 		} else {
 			info!("query_trusted_key_servers_first is set to false, querying {origin} first");
-
 			debug!("Asking {origin} for their signing keys over federation");
 			if let Some(server_key) = self
 				.services
@@ -536,7 +540,7 @@ impl Service {
 							.filter_map(|e| e.deserialize().ok())
 							.collect::<Vec<_>>()
 					}) {
-					debug!("Got signing keys: {:?}", server_keys);
+					debug!("Got signing keys: {server_keys:?}");
 					for k in server_keys {
 						self.services
 							.globals
@@ -561,7 +565,6 @@ impl Service {
 			}
 		}
 
-		warn!("Failed to find public key for server: {origin}");
+		Err!(BadServerResponse(warn!("Failed to find public key for server {origin:?}")))
-		Err(Error::BadServerResponse("Failed to find public key for server"))
 	}
 }