style: Centralise decision for stale marker

src/core/error/mod.rs

@@ -287,3 +287,7 @@ impl From<reqwest::Error> for FormattedReqwestError {
 impl From<reqwest::Error> for Error {
 	fn from(err: reqwest::Error) -> Self { Self::Reqwest(err.into()) }
 }
+
+impl From<FormattedReqwestError> for reqwest::Error {
+	fn from(val: FormattedReqwestError) -> Self { val.0 }
+}

src/service/federation/execute.rs

@@ -186,12 +186,23 @@ impl super::Service {
 				self.handle_response::<T>(dest, actual, &method, &url, response)
 					.await,
 			| Err(error) => {
-				if error.is_connect() {
+				// This awful wrapping hack is required because `reqwest::Error` does not
+				// implement `Clone`, but we need to convert it into a local error type to pass
+				// to should_mark_stale, but handle_error then itself expects the original
+				// reqwest error.
+				// handle_error *could* just take the wrapped error, but it'd have to unwrap it
+				// anyway.
+				let wrapped = Error::Reqwest(error.into());
+				if self.should_mark_stale(&wrapped) {
 					debug_info!("{dest} is unhealthy & stale due to a connect error");
 					self.mark_destination_stale(dest);
 					self.hit_unhealthy(dest.to_owned());
 				}
-				Err(handle_error(actual, &method, &url, error).expect_err("always returns error"))
+				let Error::Reqwest(unwrapped) = wrapped else {
+					unreachable!("wrapped reqwest error must unwrap to a reqwest error");
+				};
+				Err(handle_error(actual, &method, &url, unwrapped.into())
+					.expect_err("always returns error"))
 			},
 		}
 	}

src/service/federation/mod.rs

@@ -151,7 +151,9 @@ impl Service {
 					retry_after: Some(RetryAfter::Delay(retry_after)),
 				})),
 				format!(
-					"Remote server is currently unhealthy (not retrying for another {} seconds)",
+					"Remote server {} is currently unhealthy (not retrying for another {} \
+					 seconds)",
+					server_name,
 					retry_after.as_secs()
 				)
 				.into(),
@@ -173,6 +175,18 @@ impl Service {
 			.insert(server_name.to_owned());
 	}
 
+	/// Determines whether a destination should be marked "stale" depending on
+	/// the returned error.
+	pub fn should_mark_stale(&self, error: &Error) -> bool {
+		if let Error::Reqwest(error) = error {
+			if error.is_connect() {
+				return true;
+			}
+		}
+
+		error.status_code() == StatusCode::MISDIRECTED_REQUEST
+	}
+
 	/// Returns a clone of the internal remote health tracking map.
 	pub fn remote_health(&self) -> HashMap<OwnedServerName, (u32, u64)> {
 		self.remote_health.read().clone()

src/service/sending/sender.rs

@@ -145,9 +145,7 @@ impl Service {
 			| Ok(dest) => self.handle_response_ok(&dest, futures, statuses).await,
 			| Err((dest, e)) => {
 				if let Destination::Federation(dest) = &dest {
-					if let Error::Reqwest(e) = &e
-						&& e.is_connect()
-					{
+					if self.services.federation.should_mark_stale(&e) {
 						debug!("{dest} is now unhealthy & stale due to a connect error: {e:?}");
 						self.services.federation.mark_destination_stale(dest);
 						self.services.federation.hit_unhealthy(dest.clone());