diff --git a/crates/teamtalk/src/client/channels.rs b/crates/teamtalk/src/client/channels.rs index 74fc4f8..991a534 100644 --- a/crates/teamtalk/src/client/channels.rs +++ b/crates/teamtalk/src/client/channels.rs @@ -125,7 +125,9 @@ impl Client { loop { let wait_ms = wait_slice(deadline); if wait_ms <= 0 { - return Err(crate::events::Error::Timeout); + return Err(crate::events::Error::timeout( + crate::events::TimeoutKind::Join, + )); } if let Some((event, message)) = self.poll(wait_ms) { match event { diff --git a/crates/teamtalk/src/client/core/runtime.rs b/crates/teamtalk/src/client/core/runtime.rs index 7963a11..b466f90 100644 --- a/crates/teamtalk/src/client/core/runtime.rs +++ b/crates/teamtalk/src/client/core/runtime.rs @@ -153,7 +153,9 @@ impl Client { code: msg.source(), message: format!("{error_context} failed"), }), - None => Err(crate::events::Error::Timeout), + None => Err(crate::events::Error::timeout( + crate::events::TimeoutKind::Command, + )), Some((_, _)) => { unreachable!( "poll_until predicate restricts terminal event to Cmd{{Success,Error}}" diff --git a/crates/teamtalk/src/client/files.rs b/crates/teamtalk/src/client/files.rs index 0b0079a..c87ad07 100644 --- a/crates/teamtalk/src/client/files.rs +++ b/crates/teamtalk/src/client/files.rs @@ -1,7 +1,7 @@ //! File transfer APIs. use super::Client; use super::guards::can_issue_logged_in_command; -use crate::events::{Error, Event, Result}; +use crate::events::{Error, Event, Result, TimeoutKind}; use crate::types::{ChannelId, CommandId, FileId, RemoteFile, TransferId}; use std::time::{Duration, Instant}; use teamtalk_sys as ffi; @@ -157,7 +157,7 @@ impl Client { loop { let wait_ms = wait_slice(deadline); if wait_ms <= 0 { - return Err(Error::Timeout); + return Err(Error::timeout(TimeoutKind::Transfer)); } if let Some((event, message)) = self.poll(wait_ms) && matches!(event, Event::FileTransfer) @@ -188,7 +188,7 @@ impl Client { loop { let wait_ms = wait_slice(deadline); if wait_ms <= 0 { - return Err(Error::Timeout); + return Err(Error::timeout(TimeoutKind::Transfer)); } let transfer = self.wait_for_file_transfer(transfer_id, wait_ms)?; if transfer.is_terminal() { diff --git a/crates/teamtalk/src/client/server.rs b/crates/teamtalk/src/client/server.rs index cb2258d..d27cfa6 100644 --- a/crates/teamtalk/src/client/server.rs +++ b/crates/teamtalk/src/client/server.rs @@ -127,7 +127,9 @@ impl Client { loop { let wait_ms = wait_slice(deadline); if wait_ms <= 0 { - return Err(crate::events::Error::Timeout); + return Err(crate::events::Error::timeout( + crate::events::TimeoutKind::ServerConfig, + )); } if let Some((event, message)) = self.poll(wait_ms) { match event { @@ -165,7 +167,12 @@ impl Client { message: "save server config command rejected in current state".to_string(), }); } - self.wait_for_command(cmd_id, timeout_ms) + match self.wait_for_command(cmd_id, timeout_ms) { + Err(crate::events::Error::Timeout { .. }) => Err(crate::events::Error::timeout( + crate::events::TimeoutKind::ServerConfig, + )), + other => other, + } } /// Returns the root channel ID. @@ -214,7 +221,9 @@ impl Client { loop { let wait_ms = wait_slice(deadline); if wait_ms <= 0 { - return Err(crate::events::Error::Timeout); + return Err(crate::events::Error::timeout( + crate::events::TimeoutKind::Command, + )); } if let Some((event, message)) = self.poll(wait_ms) { match event { diff --git a/crates/teamtalk/src/client/users/auth.rs b/crates/teamtalk/src/client/users/auth.rs index 66c8b08..4b9fb43 100644 --- a/crates/teamtalk/src/client/users/auth.rs +++ b/crates/teamtalk/src/client/users/auth.rs @@ -97,7 +97,9 @@ impl Client { loop { let wait_ms = wait_slice(deadline); if wait_ms <= 0 { - return Err(crate::events::Error::Timeout); + return Err(crate::events::Error::timeout( + crate::events::TimeoutKind::Login, + )); } if let Some((event, message)) = self.poll(wait_ms) { match event { diff --git a/crates/teamtalk/src/client/users/directory.rs b/crates/teamtalk/src/client/users/directory.rs index 9755708..e949682 100644 --- a/crates/teamtalk/src/client/users/directory.rs +++ b/crates/teamtalk/src/client/users/directory.rs @@ -113,7 +113,9 @@ impl Client { loop { let wait_ms = wait_slice(deadline); if wait_ms <= 0 { - return Err(crate::events::Error::Timeout); + return Err(crate::events::Error::timeout( + crate::events::TimeoutKind::Command, + )); } if let Some((event, message)) = self.poll(wait_ms) { match event { @@ -185,7 +187,9 @@ impl Client { loop { let wait_ms = wait_slice(deadline); if wait_ms <= 0 { - return Err(crate::events::Error::Timeout); + return Err(crate::events::Error::timeout( + crate::events::TimeoutKind::Command, + )); } if let Some((event, message)) = self.poll(wait_ms) { match event { diff --git a/crates/teamtalk/src/events/mod.rs b/crates/teamtalk/src/events/mod.rs index f23e0cc..a40cbec 100644 --- a/crates/teamtalk/src/events/mod.rs +++ b/crates/teamtalk/src/events/mod.rs @@ -251,8 +251,13 @@ pub enum Error { }, #[error("IO error: {message}")] IoError { message: String }, - #[error("Operation timed out")] - Timeout, + #[error("Operation timed out: {kind}")] + Timeout { + /// Classification of which kind of operation hit the timeout, + /// so callers can branch on `Command` vs `Join` vs `Transfer` + /// etc. without string-matching on error messages. + kind: TimeoutKind, + }, #[error("FFI error: {0}")] Ffi(#[from] FfiError), } @@ -278,6 +283,79 @@ impl Error { _ => None, } } + + /// Constructs an [`Error::Timeout`] with the given classification. + /// + /// Shorthand for `Error::Timeout { kind }` that keeps the call + /// sites readable (`Error::timeout(TimeoutKind::Command)`). + #[must_use] + pub const fn timeout(kind: TimeoutKind) -> Self { + Self::Timeout { kind } + } + + /// Returns the [`TimeoutKind`] carried by an [`Error::Timeout`], + /// or [`None`] for any other variant. + #[must_use] + pub fn timeout_kind(&self) -> Option { + if let Self::Timeout { kind } = *self { + Some(kind) + } else { + None + } + } +} + +/// Categorises which blocking `*_and_wait` call hit its deadline. +/// +/// Produced by [`Error::Timeout`] so callers can differentiate +/// between, for example, a slow login from a slow file transfer +/// without scraping the error message. +/// +/// Marked `#[non_exhaustive]` so new kinds can be introduced in a +/// minor release; callers must always include a `_ =>` arm. +#[non_exhaustive] +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +pub enum TimeoutKind { + /// Generic command completion timeout (`wait_for_command`, + /// `list_*_and_wait`, `create_user_account_and_wait`, etc.). + Command, + /// TCP/UDP connect step timed out before `ConnectSuccess`. + Connect, + /// `login_and_wait` did not observe `MySelfLoggedIn` in time. + Login, + /// `join_channel_and_wait` did not reach `JoinedChannel`. + Join, + /// File transfer did not reach a terminal state in time. + Transfer, + /// `update_server_and_wait` / `save_server_config_and_wait`. + ServerConfig, + /// Timeout that does not fall into any of the categorised kinds + /// yet. Prefer adding a typed variant in a follow-up when the + /// call site is stable. + Other, +} + +impl TimeoutKind { + /// Returns a short, stable, lower-snake-case name for this + /// variant (useful for structured logging and metrics). + #[must_use] + pub const fn name(self) -> &'static str { + match self { + Self::Command => "command", + Self::Connect => "connect", + Self::Login => "login", + Self::Join => "join", + Self::Transfer => "transfer", + Self::ServerConfig => "server_config", + Self::Other => "other", + } + } +} + +impl std::fmt::Display for TimeoutKind { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(self.name()) + } } #[non_exhaustive] diff --git a/crates/teamtalk/src/lib.rs b/crates/teamtalk/src/lib.rs index ad0cdeb..3d58a2c 100644 --- a/crates/teamtalk/src/lib.rs +++ b/crates/teamtalk/src/lib.rs @@ -77,7 +77,7 @@ pub use dispatch::{ ClientConfig, ConnectParamsOwned, DispatchFlow, Dispatcher, EventContext as DispatchEventContext, ReconnectSettings, }; -pub use events::{ConnectionState, Error, Event, FfiError, Result, SdkErrorCode}; +pub use events::{ConnectionState, Error, Event, FfiError, Result, SdkErrorCode, TimeoutKind}; #[cfg(feature = "mock")] pub use mock::{MockClient, MockMessage, MockUserBuilder}; #[cfg(feature = "bot-macros")] diff --git a/crates/teamtalk/tests/sdk_error_code_tests.rs b/crates/teamtalk/tests/sdk_error_code_tests.rs index e70cb3b..ede5a67 100644 --- a/crates/teamtalk/tests/sdk_error_code_tests.rs +++ b/crates/teamtalk/tests/sdk_error_code_tests.rs @@ -145,7 +145,11 @@ fn error_sdk_code_on_ffi_sdk_error() { #[test] fn error_sdk_code_none_for_non_sdk_errors() { - assert!(Error::Timeout.sdk_code().is_none()); + assert!( + Error::timeout(teamtalk::events::TimeoutKind::Command) + .sdk_code() + .is_none() + ); assert!(Error::InitFailed.sdk_code().is_none()); assert!(Error::ConnectFailed.sdk_code().is_none()); assert!(Error::AuthFailed.sdk_code().is_none()); diff --git a/crates/teamtalk/tests/timeout_kind_tests.rs b/crates/teamtalk/tests/timeout_kind_tests.rs new file mode 100644 index 0000000..779e61f --- /dev/null +++ b/crates/teamtalk/tests/timeout_kind_tests.rs @@ -0,0 +1,150 @@ +//! Integration tests for the typed timeout classification on +//! [`teamtalk::events::Error::Timeout`]. + +use teamtalk::events::{Error, FfiError, TimeoutKind}; + +#[test] +fn timeout_constructor_produces_timeout_variant() { + let err = Error::timeout(TimeoutKind::Command); + assert!(matches!( + err, + Error::Timeout { + kind: TimeoutKind::Command + } + )); +} + +#[test] +fn timeout_kind_accessor_returns_kind_for_timeout() { + let kinds = [ + TimeoutKind::Command, + TimeoutKind::Connect, + TimeoutKind::Login, + TimeoutKind::Join, + TimeoutKind::Transfer, + TimeoutKind::ServerConfig, + TimeoutKind::Other, + ]; + for kind in kinds { + let err = Error::timeout(kind); + assert_eq!(err.timeout_kind(), Some(kind)); + } +} + +#[test] +fn timeout_kind_accessor_returns_none_for_non_timeout_errors() { + assert!(Error::InitFailed.timeout_kind().is_none()); + assert!(Error::ConnectFailed.timeout_kind().is_none()); + assert!(Error::AuthFailed.timeout_kind().is_none()); + assert!(Error::InvalidParam.timeout_kind().is_none()); + assert!(Error::MissingLoginParams.timeout_kind().is_none()); + assert!(Error::MissingReconnectParams.timeout_kind().is_none()); + assert!( + Error::IoError { + message: "x".into(), + } + .timeout_kind() + .is_none() + ); + assert!( + Error::CommandFailed { + code: 1001, + message: "x".into(), + } + .timeout_kind() + .is_none() + ); + assert!( + Error::ClientError { + code: 10000, + message: "x".into(), + } + .timeout_kind() + .is_none() + ); + assert!(Error::Ffi(FfiError::NullPointer).timeout_kind().is_none()); +} + +#[test] +fn display_includes_kind_name() { + let msg = format!("{}", Error::timeout(TimeoutKind::Login)); + assert!( + msg.contains("login"), + "Timeout Display should include kind name, got: {msg}" + ); + assert!( + msg.contains("timed out"), + "Timeout Display should include the base phrase, got: {msg}" + ); +} + +#[test] +fn display_distinguishes_each_kind() { + let login = format!("{}", Error::timeout(TimeoutKind::Login)); + let transfer = format!("{}", Error::timeout(TimeoutKind::Transfer)); + assert_ne!(login, transfer); +} + +#[test] +fn timeout_kind_name_is_snake_case_stable() { + assert_eq!(TimeoutKind::Command.name(), "command"); + assert_eq!(TimeoutKind::Connect.name(), "connect"); + assert_eq!(TimeoutKind::Login.name(), "login"); + assert_eq!(TimeoutKind::Join.name(), "join"); + assert_eq!(TimeoutKind::Transfer.name(), "transfer"); + assert_eq!(TimeoutKind::ServerConfig.name(), "server_config"); + assert_eq!(TimeoutKind::Other.name(), "other"); +} + +#[test] +fn timeout_kind_display_matches_name() { + let all = [ + TimeoutKind::Command, + TimeoutKind::Connect, + TimeoutKind::Login, + TimeoutKind::Join, + TimeoutKind::Transfer, + TimeoutKind::ServerConfig, + TimeoutKind::Other, + ]; + for kind in all { + assert_eq!(format!("{kind}"), kind.name()); + } +} + +#[test] +fn names_are_unique() { + let all = [ + TimeoutKind::Command, + TimeoutKind::Connect, + TimeoutKind::Login, + TimeoutKind::Join, + TimeoutKind::Transfer, + TimeoutKind::ServerConfig, + TimeoutKind::Other, + ]; + let mut names: Vec<_> = all.iter().map(|k| k.name()).collect(); + names.sort_unstable(); + let len = names.len(); + names.dedup(); + assert_eq!(len, names.len(), "TimeoutKind names must be unique"); +} + +#[test] +fn derives_are_available() { + // Compile-time assertions that the derived traits exist. + fn assert_impls() {} + assert_impls::(); + + // Equality and hash work. + let a = TimeoutKind::Command; + let b = TimeoutKind::Command; + let c = TimeoutKind::Login; + assert_eq!(a, b); + assert_ne!(a, c); + let mut set = std::collections::HashSet::new(); + set.insert(a); + set.insert(b); + set.insert(c); + assert_eq!(set.len(), 2); +}