Revert "Unify server pinging implementations between app and backend (#5510)" (#5558)

Author: aecsocket

Cargo.lock

@@ -423,11 +423,11 @@ pub struct JavaServerPingData {
     /// Reported version protocol number of the server.
     pub version_protocol: i32,
     /// Description/MOTD of the server as shown in the server list.
-    pub description: Option<serde_json::Value>,
+    pub description: String,
     /// Number of players online at the time.
-    pub players_online: Option<i32>,
+    pub players_online: i32,
     /// Maximum number of players allowed on the server.
-    pub players_max: Option<i32>,
+    pub players_max: i32,
 }
 
 component::relations! {

apps/labrinth/src/models/exp/minecraft.rs

@@ -6,6 +6,7 @@ use crate::models::exp;
 use crate::models::ids::ProjectId;
 use crate::models::projects::ProjectStatus;
 use crate::{database::PgPool, util::error::Context};
+use async_minecraft_ping::ServerDescription;
 use chrono::{TimeDelta, Utc};
 use clickhouse::{Client, Row};
 use serde::Serialize;
@@ -106,16 +107,11 @@ impl ServerPingQueue {
                     project_id: project_id.0,
                     address: ping.address.clone(),
                     latency_ms: data.map(|d| d.latency.as_millis() as u32),
-                    description: data.and_then(|d| {
-                        d.description.as_ref().map(|d| {
-                            serde_json::to_string(&d)
-                                .expect("serialization should not fail")
-                        })
-                    }),
+                    description: data.map(|d| d.description.clone()),
                     version_name: data.map(|d| d.version_name.clone()),
                     version_protocol: data.map(|d| d.version_protocol),
-                    players_online: data.and_then(|d| d.players_online),
-                    players_max: data.and_then(|d| d.players_max),
+                    players_online: data.map(|d| d.players_online),
+                    players_max: data.map(|d| d.players_max),
                 };
 
                 ch.write(&row)
@@ -260,27 +256,45 @@ pub async fn ping_server(
         .map(|duration| duration.min(default_duration))
         .unwrap_or(default_duration);
 
-    let conn = async_minecraft_ping::ConnectionConfig::build(address)
-        .with_srv_lookup()
-        .with_timeout(timeout)
-        .connect()
-        .await
-        .wrap_err("failed to connect to server")?;
+    let (address, port) = match address.rsplit_once(':') {
+        Some((addr, port)) => {
+            let port = port.parse::<u16>().wrap_err("invalid port number")?;
+            (addr, port)
+        }
+        None => (address, 25565),
+    };
+
+    let task = async move {
+        let conn = async_minecraft_ping::ConnectionConfig::build(address)
+            .with_port(port)
+            .with_srv_lookup()
+            .connect()
+            .await
+            .wrap_err("failed to connect to server")?;
 
-    let status = conn
-        .status()
+        let status = conn
+            .status()
+            .await
+            .wrap_err("failed to get server status")?
+            .status;
+
+        eyre::Ok(exp::minecraft::JavaServerPingData {
+            latency: start.elapsed(),
+            version_name: status.version.name,
+            version_protocol: status.version.protocol,
+            description: match status.description {
+                ServerDescription::Plain(text)
+                | ServerDescription::Object { text } => text,
+            },
+            players_online: status.players.online,
+            players_max: status.players.max,
+        })
+    };
+
+    tokio::time::timeout(timeout, task)
         .await
-        .wrap_err("failed to get server status")?
-        .status;
-
-    eyre::Ok(exp::minecraft::JavaServerPingData {
-        latency: start.elapsed(),
-        version_name: status.version.name,
-        version_protocol: status.version.protocol,
-        description: status.description,
-        players_online: status.players.online,
-        players_max: status.players.max,
-    })
+        .map_err(eyre::Error::new)
+        .flatten()
 }
 
 #[derive(Debug, Row, Serialize, Clone)]

apps/labrinth/src/queue/server_ping.rs

@@ -37,7 +37,6 @@ dunce = { workspace = true }
 either = { workspace = true }
 encoding_rs = { workspace = true }
 enumset = { workspace = true }
-eyre = { workspace = true }
 flate2 = { workspace = true }
 fs4 = { workspace = true, features = ["tokio"] }
 futures = { workspace = true, features = ["alloc", "async-await"] }

packages/app-lib/Cargo.toml

@@ -6,13 +6,14 @@ use crate::state::attached_world_data::AttachedWorldData;
 use crate::state::{
     Profile, ProfileInstallStage, attached_world_data, server_join_log,
 };
-use crate::util::io;
 use crate::util::protocol_version::OLD_PROTOCOL_VERSIONS;
 pub use crate::util::protocol_version::ProtocolVersion;
 pub use crate::util::server_ping::{
     ServerGameProfile, ServerPlayers, ServerStatus, ServerVersion,
 };
-use crate::{Context, ErrorKind, Result, State, launcher};
+use crate::util::{io, server_ping};
+use crate::{Error, ErrorKind, Result, State, launcher};
+use async_minecraft_ping::ServerDescription;
 use async_walkdir::WalkDir;
 use async_zip::{Compression, ZipEntryBuilder};
 use chrono::{DateTime, Local, TimeZone, Utc};
@@ -909,54 +910,67 @@ pub async fn get_server_status(
         "Pinging {address} with protocol version {protocol_version:?}"
     );
 
-    // get_server_status_old(address, protocol_version).await
-    get_server_status_new(address, protocol_version).await
-}
-
-// async fn _get_server_status_old(
-//     address: &str,
-//     protocol_version: Option<ProtocolVersion>,
-// ) -> Result<ServerStatus> {
-//     let (original_host, original_port) = parse_server_address(address)?;
-//     let (host, port) =
-//         resolve_server_address(original_host, original_port).await?;
-//     tracing::debug!(
-//         "Pinging {address} with protocol version {protocol_version:?}"
-//     );
-//     server_ping::get_server_status(
-//         &(&host as &str, port),
-//         (original_host, original_port),
-//         protocol_version,
-//     )
-//     .await
-
-async fn get_server_status_new(
+    get_server_status_old(address, protocol_version).await
+    // get_server_status_new(address, protocol_version).await
+}
+
+async fn get_server_status_old(
+    address: &str,
+    protocol_version: Option<ProtocolVersion>,
+) -> Result<ServerStatus> {
+    let (original_host, original_port) = parse_server_address(address)?;
+    let (host, port) =
+        resolve_server_address(original_host, original_port).await?;
+    tracing::debug!(
+        "Pinging {address} with protocol version {protocol_version:?}"
+    );
+    server_ping::get_server_status(
+        &(&host as &str, port),
+        (original_host, original_port),
+        protocol_version,
+    )
+    .await
+}
+
+async fn _get_server_status_new(
     address: &str,
     protocol_version: Option<ProtocolVersion>,
 ) -> Result<ServerStatus> {
+    let (address, port) = match address.rsplit_once(':') {
+        Some((addr, port)) => {
+            let port = port.parse::<u16>().map_err(|_err| {
+                Error::from(ErrorKind::InputError("invalid port number".into()))
+            })?;
+            (addr, port)
+        }
+        None => (address, 25565),
+    };
+
     let mut builder = async_minecraft_ping::ConnectionConfig::build(address)
+        .with_port(port)
         .with_srv_lookup();
 
     if let Some(version) = protocol_version {
         builder = builder.with_protocol_version(version.version as usize)
     }
 
-    let conn = builder
-        .connect()
-        .await
-        .wrap_err("failed to connect to server")?;
+    let conn = builder.connect().await.map_err(|_err| {
+        Error::from(ErrorKind::InputError("failed to connect to server".into()))
+    })?;
 
-    let ping_conn = conn
-        .status()
-        .await
-        .wrap_err("failed to get server status")?;
+    let ping_conn = conn.status().await.map_err(|_err| {
+        Error::from(ErrorKind::InputError("failed to get server status".into()))
+    })?;
     let status = &ping_conn.status;
-    let description = status.description.as_ref().map(|d| {
-        let json =
-            serde_json::to_string(d).expect("serializing should not fail");
-        RawValue::from_string(json)
-            .expect("converting to `RawValue` should not fail")
-    });
+    let description = match &status.description {
+        ServerDescription::Plain(text) => {
+            serde_json::value::to_raw_value(&text).ok()
+        }
+        ServerDescription::Object { text } => {
+            // TODO: `text` always seems to be empty?
+            RawValue::from_string(text.clone()).ok()
+        }
+    };
 
     let players = ServerPlayers {
         max: status.players.max,
@@ -986,10 +1000,9 @@ async fn get_server_status_new(
     let latency = {
         let start = Instant::now();
         let ping_magic = Utc::now().timestamp_millis().cast_unsigned();
-        ping_conn
-            .ping(ping_magic)
-            .await
-            .wrap_err("failed to do ping")?;
+        ping_conn.ping(ping_magic).await.map_err(|_err| {
+            Error::from(ErrorKind::InputError("failed to do ping".into()))
+        })?;
         start.elapsed().as_millis() as i64
     };
 

packages/app-lib/src/api/worlds.rs

@@ -1,9 +1,5 @@
 //! Theseus error type
-use std::{
-    convert::Infallible,
-    fmt::{Debug, Display},
-    sync::Arc,
-};
+use std::sync::Arc;
 
 use crate::{profile, util};
 use data_url::DataUrlError;
@@ -224,41 +220,4 @@ impl ErrorKind {
     }
 }
 
-pub type Result<T, E = Error> = core::result::Result<T, E>;
-
-pub trait Context<T, E>: Sized {
-    fn wrap_err_with<D>(self, f: impl FnOnce() -> D) -> Result<T, Error>
-    where
-        D: Send + Sync + Debug + Display + 'static;
-
-    #[inline]
-    fn wrap_err<D>(self, msg: D) -> Result<T, Error>
-    where
-        D: Send + Sync + Debug + Display + 'static,
-    {
-        self.wrap_err_with(|| msg)
-    }
-}
-
-impl<T, E> Context<T, E> for Result<T, E>
-where
-    Self: eyre::WrapErr<T, E>,
-{
-    fn wrap_err_with<D>(self, f: impl FnOnce() -> D) -> Result<T, Error>
-    where
-        D: Send + Sync + Debug + Display + 'static,
-    {
-        eyre::WrapErr::wrap_err_with(self, f).map_err(|err| {
-            Error::from(ErrorKind::OtherError(format!("{err:#}")))
-        })
-    }
-}
-
-impl<T> Context<T, Infallible> for Option<T> {
-    fn wrap_err_with<D>(self, f: impl FnOnce() -> D) -> Result<T, Error>
-    where
-        D: Send + Sync + Debug + Display + 'static,
-    {
-        self.ok_or_else(|| Error::from(ErrorKind::OtherError(f().to_string())))
-    }
-}
+pub type Result<T> = core::result::Result<T, Error>;

packages/app-lib/src/error.rs

@@ -318,7 +318,7 @@ macro_rules! get_resource_file {
                 Ok(dir) => dir,
                 Err(e) => {
                     break 'get_resource_file $crate::Result::Err(
-                        $crate::Error::from($crate::util::io::IOError::from(e)),
+                        $crate::util::io::IOError::from(e).into(),
                     );
                 }
             };

packages/app-lib/src/util/io.rs

@@ -1,3 +1,53 @@
+use crate::ErrorKind;
+use crate::error::Result;
+use crate::util::protocol_version::ProtocolVersion;
+use serde::{Deserialize, Serialize};
+use serde_json::value::RawValue;
+use std::time::Duration;
+use tokio::net::ToSocketAddrs;
+use tokio::select;
+use url::Url;
+
+#[derive(Deserialize, Serialize, Debug, Clone)]
+#[serde(rename_all = "camelCase")]
+pub struct ServerStatus {
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub description: Option<Box<RawValue>>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub players: Option<ServerPlayers>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub version: Option<ServerVersion>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub favicon: Option<Url>,
+    #[serde(default)]
+    pub enforces_secure_chat: bool,
+
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub ping: Option<i64>,
+}
+
+#[derive(Deserialize, Serialize, Debug, Clone)]
+pub struct ServerPlayers {
+    pub max: i32,
+    pub online: i32,
+    #[serde(default)]
+    pub sample: Vec<ServerGameProfile>,
+}
+
+#[derive(Deserialize, Serialize, Debug, Clone)]
+pub struct ServerGameProfile {
+    pub id: String,
+    pub name: String,
+}
+
+#[derive(Deserialize, Serialize, Debug, Clone)]
+pub struct ServerVersion {
+    pub name: String,
+    pub protocol: i32,
+    #[serde(skip_deserializing)]
+    pub legacy: bool,
+}
+
 pub async fn get_server_status(
     address: &impl ToSocketAddrs,
     original_address: (&str, u16),
@@ -255,8 +305,8 @@ mod legacy {
             }),
             description: parts.next().and_then(|x| to_raw_value(x).ok()),
             players: Some(ServerPlayers {
-                online: parts.next().and_then(|x| x.parse().ok()),
-                max: parts.next().and_then(|x| x.parse().ok()),
+                online: parts.next().and_then(|x| x.parse().ok()).unwrap_or(-1),
+                max: parts.next().and_then(|x| x.parse().ok()).unwrap_or(-1),
                 sample: vec![],
             }),
             favicon: None,