feat: Add connect timeout to TimeoutSettings (#158)

Because TcpSocket connects in Socket::new TimeoutSettings are now
required for Socket::new. Since we already have TimeoutSettings there
Sockets are now expected to apply timeout settings in Socket::new.

CHANGELOG.md

@@ -28,6 +28,10 @@ Game:
 Protocols:
 - Valve: Removed `SteamApp` due to it not being really useful at all, replaced all instances with `Engine`.
 
+Query:
+- Added a connection timeout to TimeoutSettings (at the moment this only applies to TCP)
+  - Sockets are now expected to apply timeout settings in new()
+
 # 0.4.1 - 13/10/2023
 ### Changes:
 Game:

crates/lib/examples/generic.rs

@@ -45,6 +45,7 @@ fn main() {
         let timeout_settings = TimeoutSettings::new(
             TimeoutSettings::default().get_read(),
             TimeoutSettings::default().get_write(),
+            TimeoutSettings::default().get_connect(),
             2,
         )
         .unwrap();
@@ -90,6 +91,7 @@ mod test {
     fn test_game(game_name: &str) {
         let timeout_settings = Some(
             TimeoutSettings::new(
+                Some(Duration::from_nanos(1)),
                 Some(Duration::from_nanos(1)),
                 Some(Duration::from_nanos(1)),
                 0,

crates/lib/examples/valve_protocol_query.rs

@@ -15,8 +15,15 @@ fn main() {
 
     let read_timeout = Duration::from_secs(2);
     let write_timeout = Duration::from_secs(3);
+    let connect_timeout = Duration::from_secs(4);
     let retries = 1; // does another request if the first one fails.
-    let timeout_settings = TimeoutSettings::new(Some(read_timeout), Some(write_timeout), retries).unwrap();
+    let timeout_settings = TimeoutSettings::new(
+        Some(read_timeout),
+        Some(write_timeout),
+        Some(connect_timeout),
+        retries,
+    )
+    .unwrap();
 
     let response = valve::query(
         address,

crates/lib/src/games/minecraft/protocol/bedrock.rs

@@ -21,8 +21,7 @@ pub struct Bedrock {
 
 impl Bedrock {
     fn new(address: &SocketAddr, timeout_settings: Option<TimeoutSettings>) -> GDResult<Self> {
-        let socket = UdpSocket::new(address)?;
-        socket.apply_timeout(&timeout_settings)?;
+        let socket = UdpSocket::new(address, &timeout_settings)?;
 
         let retry_count = TimeoutSettings::get_retries_or_default(&timeout_settings);
         Ok(Self {

crates/lib/src/games/minecraft/protocol/java.rs

@@ -24,8 +24,7 @@ impl Java {
         timeout_settings: Option<TimeoutSettings>,
         request_settings: Option<RequestSettings>,
     ) -> GDResult<Self> {
-        let socket = TcpSocket::new(address)?;
-        socket.apply_timeout(&timeout_settings)?;
+        let socket = TcpSocket::new(address, &timeout_settings)?;
 
         let retry_count = TimeoutSettings::get_retries_or_default(&timeout_settings);
         Ok(Self {

crates/lib/src/games/minecraft/protocol/legacy_v1_4.rs

@@ -19,8 +19,7 @@ pub struct LegacyV1_4 {
 
 impl LegacyV1_4 {
     fn new(address: &SocketAddr, timeout_settings: Option<TimeoutSettings>) -> GDResult<Self> {
-        let socket = TcpSocket::new(address)?;
-        socket.apply_timeout(&timeout_settings)?;
+        let socket = TcpSocket::new(address, &timeout_settings)?;
 
         let retry_count = TimeoutSettings::get_retries_or_default(&timeout_settings);
         Ok(Self {

crates/lib/src/games/minecraft/protocol/legacy_v1_6.rs

@@ -18,8 +18,7 @@ pub struct LegacyV1_6 {
 
 impl LegacyV1_6 {
     fn new(address: &SocketAddr, timeout_settings: Option<TimeoutSettings>) -> GDResult<Self> {
-        let socket = TcpSocket::new(address)?;
-        socket.apply_timeout(&timeout_settings)?;
+        let socket = TcpSocket::new(address, &timeout_settings)?;
 
         let retry_count = TimeoutSettings::get_retries_or_default(&timeout_settings);
         Ok(Self {

crates/lib/src/games/minecraft/protocol/legacy_vb1_8.rs

@@ -19,8 +19,7 @@ pub struct LegacyVB1_8 {
 
 impl LegacyVB1_8 {
     fn new(address: &SocketAddr, timeout_settings: Option<TimeoutSettings>) -> GDResult<Self> {
-        let socket = TcpSocket::new(address)?;
-        socket.apply_timeout(&timeout_settings)?;
+        let socket = TcpSocket::new(address, &timeout_settings)?;
 
         let retry_count = TimeoutSettings::get_retries_or_default(&timeout_settings);
         Ok(Self {

crates/lib/src/protocols/gamespy/protocols/one/protocol.rs

@@ -24,8 +24,7 @@ fn get_server_values(
     address: &SocketAddr,
     timeout_settings: &Option<TimeoutSettings>,
 ) -> GDResult<HashMap<String, String>> {
-    let mut socket = UdpSocket::new(address)?;
-    socket.apply_timeout(timeout_settings)?;
+    let mut socket = UdpSocket::new(address, timeout_settings)?;
     retry_on_timeout(
         TimeoutSettings::get_retries_or_default(timeout_settings),
         move || get_server_values_impl(&mut socket),

crates/lib/src/protocols/gamespy/protocols/three/protocol.rs

@@ -52,9 +52,8 @@ const DEFAULT_PAYLOAD: [u8; 4] = [0xFF, 0xFF, 0xFF, 0x01];
 
 impl GameSpy3 {
     fn new(address: &SocketAddr, timeout_settings: Option<TimeoutSettings>) -> GDResult<Self> {
-        let socket = UdpSocket::new(address)?;
+        let socket = UdpSocket::new(address, &timeout_settings)?;
         let retry_count = TimeoutSettings::get_retries_or_default(&timeout_settings);
-        socket.apply_timeout(&timeout_settings)?;
 
         Ok(Self {
             socket,
@@ -70,9 +69,8 @@ impl GameSpy3 {
         payload: [u8; 4],
         single_packets: bool,
     ) -> GDResult<Self> {
-        let socket = UdpSocket::new(address)?;
+        let socket = UdpSocket::new(address, &timeout_settings)?;
         let retry_count = TimeoutSettings::get_retries_or_default(&timeout_settings);
-        socket.apply_timeout(&timeout_settings)?;
 
         Ok(Self {
             socket,

crates/lib/src/protocols/gamespy/protocols/two/protocol.rs

@@ -79,9 +79,8 @@ fn data_as_table(data: &mut Buffer<BigEndian>) -> GDResult<(HashMap<String, Vec<
 
 impl GameSpy2 {
     fn new(address: &SocketAddr, timeout_settings: Option<TimeoutSettings>) -> GDResult<Self> {
-        let socket = UdpSocket::new(address)?;
+        let socket = UdpSocket::new(address, &timeout_settings)?;
         let retry_count = TimeoutSettings::get_retries_or_default(&timeout_settings);
-        socket.apply_timeout(&timeout_settings)?;
 
         Ok(Self {
             socket,

crates/lib/src/protocols/quake/client.rs

@@ -25,8 +25,7 @@ fn get_data<Client: QuakeClient>(
     address: &SocketAddr,
     timeout_settings: &Option<TimeoutSettings>,
 ) -> GDResult<Vec<u8>> {
-    let mut socket = UdpSocket::new(address)?;
-    socket.apply_timeout(timeout_settings)?;
+    let mut socket = UdpSocket::new(address, timeout_settings)?;
     retry_on_timeout(
         TimeoutSettings::get_retries_or_default(timeout_settings),
         move || get_data_impl::<Client>(&mut socket),

crates/lib/src/protocols/types.rs

@@ -150,6 +150,7 @@ pub struct CommonPlayerJson<'a> {
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, PartialOrd, Ord)]
 pub struct TimeoutSettings {
+    connect: Option<Duration>,
     read: Option<Duration>,
     write: Option<Duration>,
     retries: usize,
@@ -164,22 +165,34 @@ impl TimeoutSettings {
     /// "1" will try the request again once if it fails.
     /// The retry count is per-request so for multi-request queries (valve) if a
     /// single part fails that part can be retried up to `retries` times.
-    pub fn new(read: Option<Duration>, write: Option<Duration>, retries: usize) -> GDResult<Self> {
+    pub fn new(
+        read: Option<Duration>,
+        write: Option<Duration>,
+        connect: Option<Duration>,
+        retries: usize,
+    ) -> GDResult<Self> {
         if let Some(read_duration) = read {
-            if read_duration == Duration::new(0, 0) {
+            if read_duration.is_zero() {
                 return Err(InvalidInput.context("Read duration must not be 0"));
             }
         }
 
         if let Some(write_duration) = write {
-            if write_duration == Duration::new(0, 0) {
+            if write_duration.is_zero() {
                 return Err(InvalidInput.context("Write duration must not be 0"));
             }
         }
 
+        if let Some(connect_duration) = connect {
+            if connect_duration.is_zero() {
+                return Err(InvalidInput.context("Connect duration must not be 0"));
+            }
+        }
+
         Ok(Self {
             read,
             write,
+            connect,
             retries,
         })
     }
@@ -190,6 +203,9 @@ impl TimeoutSettings {
     /// Get the write timeout.
     pub const fn get_write(&self) -> Option<Duration> { self.write }
 
+    /// Get the connect timeout.
+    pub const fn get_connect(&self) -> Option<Duration> { self.connect }
+
     /// Get number of retries
     pub const fn get_retries(&self) -> usize { self.retries }
 
@@ -216,11 +232,21 @@ impl TimeoutSettings {
         }
     }
 
+    /// Get the connect duration given timeout settings or get the default.
+    pub const fn get_connect_or_default(timeout_settings: &Option<TimeoutSettings>) -> Option<Duration> {
+        if let Some(timeout_settings) = timeout_settings {
+            timeout_settings.get_connect()
+        } else {
+            TimeoutSettings::const_default().get_connect()
+        }
+    }
+
     /// Default values are 4 seconds for both read and write, no retries.
     pub const fn const_default() -> Self {
         Self {
             read: Some(Duration::from_secs(4)),
             write: Some(Duration::from_secs(4)),
+            connect: Some(Duration::from_secs(4)),
             retries: 0,
         }
     }
@@ -320,9 +346,15 @@ mod tests {
         // Define valid read and write durations
         let read_duration = Duration::from_secs(1);
         let write_duration = Duration::from_secs(2);
+        let connect_duration = Duration::from_secs(3);
 
         // Create new TimeoutSettings with the valid durations
-        let timeout_settings = TimeoutSettings::new(Some(read_duration), Some(write_duration), 0)?;
+        let timeout_settings = TimeoutSettings::new(
+            Some(read_duration),
+            Some(write_duration),
+            Some(connect_duration),
+            0,
+        )?;
 
         // Verify that the get_read and get_write methods return the expected values
         assert_eq!(timeout_settings.get_read(), Some(read_duration));
@@ -337,10 +369,16 @@ mod tests {
         // Define a zero read duration and a valid write duration
         let read_duration = Duration::new(0, 0);
         let write_duration = Duration::from_secs(2);
+        let connect_duration = Duration::from_secs(3);
 
         // Try to create new TimeoutSettings with the zero read duration (this should
         // fail)
-        let result = TimeoutSettings::new(Some(read_duration), Some(write_duration), 0);
+        let result = TimeoutSettings::new(
+            Some(read_duration),
+            Some(write_duration),
+            Some(connect_duration),
+            0,
+        );
 
         // Verify that the function returned an error and that the error type is
         // InvalidInput

crates/lib/src/protocols/unreal2/protocol.rs

@@ -32,12 +32,11 @@ pub(crate) struct Unreal2Protocol {
 
 impl Unreal2Protocol {
     pub fn new(address: &SocketAddr, timeout_settings: Option<TimeoutSettings>) -> GDResult<Self> {
-        let socket = UdpSocket::new(address)?;
+        let socket = UdpSocket::new(address, &timeout_settings)?;
         let retry_count = timeout_settings
             .as_ref()
             .map(|t| t.get_retries())
             .unwrap_or_else(|| TimeoutSettings::default().get_retries());
-        socket.apply_timeout(&timeout_settings)?;
 
         Ok(Self {
             socket,

crates/lib/src/protocols/valve/protocol.rs

@@ -126,12 +126,11 @@ static PACKET_SIZE: usize = 6144;
 
 impl ValveProtocol {
     pub fn new(address: &SocketAddr, timeout_settings: Option<TimeoutSettings>) -> GDResult<Self> {
-        let socket = UdpSocket::new(address)?;
+        let socket = UdpSocket::new(address, &timeout_settings)?;
         let retry_count = timeout_settings
             .as_ref()
             .map(|t| t.get_retries())
             .unwrap_or_else(|| TimeoutSettings::default().get_retries());
-        socket.apply_timeout(&timeout_settings)?;
 
         Ok(Self {
             socket,

crates/lib/src/services/valve_master_server/service.rs

@@ -49,8 +49,7 @@ pub struct ValveMasterServer {
 impl ValveMasterServer {
     /// Construct a new struct.
     pub fn new(master_address: &SocketAddr) -> GDResult<Self> {
-        let socket = UdpSocket::new(master_address)?;
-        socket.apply_timeout(&None)?;
+        let socket = UdpSocket::new(master_address, &None)?;
 
         Ok(Self { socket })
     }

crates/lib/src/socket.rs

@@ -13,7 +13,10 @@ use std::{
 const DEFAULT_PACKET_SIZE: usize = 1024;
 
 pub trait Socket {
-    fn new(address: &SocketAddr) -> GDResult<Self>
+    /// Create a new socket and connect to the remote address (if required).
+    ///
+    /// Calls [Self::apply_timeout] with the given timeout settings.
+    fn new(address: &SocketAddr, timeout_settings: &Option<TimeoutSettings>) -> GDResult<Self>
     where Self: Sized;
 
     fn apply_timeout(&self, timeout_settings: &Option<TimeoutSettings>) -> GDResult<()>;
@@ -30,11 +33,21 @@ pub struct TcpSocket {
 }
 
 impl Socket for TcpSocket {
-    fn new(address: &SocketAddr) -> GDResult<Self> {
-        Ok(Self {
-            socket: net::TcpStream::connect(address).map_err(|e| SocketConnect.context(e))?,
+    fn new(address: &SocketAddr, timeout_settings: &Option<TimeoutSettings>) -> GDResult<Self> {
+        let socket = if let Some(timeout) = TimeoutSettings::get_connect_or_default(timeout_settings) {
+            net::TcpStream::connect_timeout(address, timeout)
+        } else {
+            net::TcpStream::connect(address)
+        };
+
+        let socket = Self {
+            socket: socket.map_err(|e| SocketConnect.context(e))?,
             address: *address,
-        })
+        };
+
+        socket.apply_timeout(timeout_settings)?;
+
+        Ok(socket)
     }
 
     fn apply_timeout(&self, timeout_settings: &Option<TimeoutSettings>) -> GDResult<()> {
@@ -68,13 +81,17 @@ pub struct UdpSocket {
 }
 
 impl Socket for UdpSocket {
-    fn new(address: &SocketAddr) -> GDResult<Self> {
+    fn new(address: &SocketAddr, timeout_settings: &Option<TimeoutSettings>) -> GDResult<Self> {
         let socket = net::UdpSocket::bind("0.0.0.0:0").map_err(|e| SocketBind.context(e))?;
 
-        Ok(Self {
+        let socket = Self {
             socket,
             address: *address,
-        })
+        };
+
+        socket.apply_timeout(timeout_settings)?;
+
+        Ok(socket)
     }
 
     fn apply_timeout(&self, timeout_settings: &Option<TimeoutSettings>) -> GDResult<()> {
@@ -125,7 +142,7 @@ mod tests {
         });
 
         // Create a TCP socket and send a message to the server
-        let mut socket = TcpSocket::new(&bound_address).unwrap();
+        let mut socket = TcpSocket::new(&bound_address, &None).unwrap();
         let message = b"hello, world!";
         socket.send(message).unwrap();
 
@@ -156,7 +173,7 @@ mod tests {
         });
 
         // Create a UDP socket and send a message to the server
-        let mut socket = UdpSocket::new(&bound_address).unwrap();
+        let mut socket = UdpSocket::new(&bound_address, &None).unwrap();
         let message = b"hello, world!";
         socket.send(message).unwrap();