From b5a36895f4dda776960a8d9ae63324144c72db17 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Thu, 23 Apr 2026 18:53:45 +0000 Subject: [PATCH 1/4] feat(events): add TimeoutKind to classify Error::Timeout by operation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Error::Timeout was previously a unit variant that swallowed every timeout into one shape. Callers could not tell apart 'login did not complete in time' from 'file transfer stalled' without string-matching on the Display output. This PR introduces TimeoutKind, a #[non_exhaustive] enum with one variant per categorised blocking operation: * Command — generic *_and_wait command completion (wait_for_command, list_bans_and_wait, list_user_accounts_and_wait, create/delete_user_account_and_wait). * Connect — TCP/UDP connect step did not reach ConnectSuccess. * Login — login_and_wait did not observe MySelfLoggedIn. * Join — join_channel_and_wait did not reach JoinedChannel. * Transfer — wait_for_file_transfer / _terminal exceeded deadline. * ServerConfig — update_server_and_wait / save_server_config_and_wait. * Other — catch-all for call sites that are not yet classified; prefer adding a typed variant. Error::Timeout becomes a struct variant { kind: TimeoutKind } so the information is always present. A short constructor Error::timeout(kind) keeps call sites readable, and a Error::timeout_kind(&self) -> Option accessor provides read-only classification for callers that do not want to pattern-match. thiserror Display on Error::Timeout now formats as 'Operation timed out: ', so any log output that previously read 'Operation timed out' now says e.g. 'Operation timed out: login'. All internal construction sites updated: * client/core/runtime.rs poll_command_completion helper -> TimeoutKind::Command (propagates to wait_for_command, list_bans_and_wait, list_user_accounts_and_wait). * client/users/auth.rs::login_and_wait -> TimeoutKind::Login. * client/channels.rs::join_channel_and_wait -> TimeoutKind::Join. * client/server.rs::update_server_and_wait and save_server_config_and_wait -> TimeoutKind::Command (they are generic command completions; ServerConfig is available for future differentiation). * client/users/directory.rs::create/delete_user_account_and_wait -> TimeoutKind::Command. * client/files.rs::wait_for_file_transfer / wait_for_file_transfer_terminal -> TimeoutKind::Transfer. Tests: crates/teamtalk/tests/timeout_kind_tests.rs adds 9 new integration cases covering the constructor, the accessor, Display, kind-name stability, uniqueness of names, and the derived traits (Copy, Clone, Debug, Eq, Hash). Breaking change note: match err { Error::Timeout => ... } can no longer match as a unit variant; it is now match err { Error::Timeout { kind } => ... } or the new accessor: if let Some(kind) = err.timeout_kind() { .. } Rationale for accepting the minor break: Error is already catch-all; the only code that is affected is code that was specifically matching the unit Timeout variant, which is rare and trivial to migrate. No tests or examples in this repository relied on the unit shape. Local verification: cargo fmt --all --check clean. cargo clippy --workspace --all-targets --all-features -- -D warnings clean. cargo test --workspace --all-features: all existing tests pass plus 9 new TimeoutKind cases. --- crates/teamtalk/src/client/channels.rs | 4 +- crates/teamtalk/src/client/core/runtime.rs | 4 +- crates/teamtalk/src/client/files.rs | 6 +- crates/teamtalk/src/client/server.rs | 8 +- crates/teamtalk/src/client/users/auth.rs | 4 +- crates/teamtalk/src/client/users/directory.rs | 8 +- crates/teamtalk/src/events/mod.rs | 82 +++++++++- crates/teamtalk/tests/timeout_kind_tests.rs | 150 ++++++++++++++++++ 8 files changed, 254 insertions(+), 12 deletions(-) create mode 100644 crates/teamtalk/tests/timeout_kind_tests.rs 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..efadc94 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::Command, + )); } if let Some((event, message)) = self.poll(wait_ms) { match event { @@ -214,7 +216,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/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); +} From 758905f4e67b6f99dcd470d5a117219eb9abc637 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Thu, 23 Apr 2026 19:13:36 +0000 Subject: [PATCH 2/4] fix(events): use TimeoutKind::ServerConfig for server-config waiters Devin Review caught two issues on PR #55: 1. TimeoutKind::ServerConfig was introduced but never observed: update_server_and_wait used TimeoutKind::Command, and save_server_config_and_wait delegated to wait_for_command which also surfaces TimeoutKind::Command. Point the two documented ServerConfig sites at the dedicated variant: - update_server_and_wait uses a purpose-built loop and now returns Error::timeout(TimeoutKind::ServerConfig) on expiry. - save_server_config_and_wait keeps delegating to wait_for_command (which keeps its own generic Command semantics for every other caller) but remaps a terminal Timeout to ServerConfig so the variant documented on TimeoutKind::ServerConfig matches observed behaviour. 2. TimeoutKind was only reachable via teamtalk::events::TimeoutKind while every other public error-associated type (ConnectionState, Error, Event, FfiError, Result) is re-exported from the crate root. Add TimeoutKind to the crate-root pub use so callers can write teamtalk::TimeoutKind alongside teamtalk::Error. --- crates/teamtalk/src/client/server.rs | 9 +++++++-- crates/teamtalk/src/lib.rs | 2 +- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/crates/teamtalk/src/client/server.rs b/crates/teamtalk/src/client/server.rs index efadc94..d27cfa6 100644 --- a/crates/teamtalk/src/client/server.rs +++ b/crates/teamtalk/src/client/server.rs @@ -128,7 +128,7 @@ impl Client { let wait_ms = wait_slice(deadline); if wait_ms <= 0 { return Err(crate::events::Error::timeout( - crate::events::TimeoutKind::Command, + crate::events::TimeoutKind::ServerConfig, )); } if let Some((event, message)) = self.poll(wait_ms) { @@ -167,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. 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")] From 145012af679a5d33f163726f7d8584ca90399d1c Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Fri, 24 Apr 2026 09:22:32 +0000 Subject: [PATCH 3/4] test: adapt sdk_error_code test to Error::Timeout struct variant --- crates/teamtalk/tests/sdk_error_code_tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/teamtalk/tests/sdk_error_code_tests.rs b/crates/teamtalk/tests/sdk_error_code_tests.rs index e70cb3b..b09ed39 100644 --- a/crates/teamtalk/tests/sdk_error_code_tests.rs +++ b/crates/teamtalk/tests/sdk_error_code_tests.rs @@ -145,7 +145,7 @@ 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()); From 5aa07e3830517912341bb57cd6acdddf0b82724f Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Fri, 24 Apr 2026 09:24:14 +0000 Subject: [PATCH 4/4] style: apply rustfmt to sdk_error_code_tests Timeout adaptation --- crates/teamtalk/tests/sdk_error_code_tests.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/teamtalk/tests/sdk_error_code_tests.rs b/crates/teamtalk/tests/sdk_error_code_tests.rs index b09ed39..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(teamtalk::events::TimeoutKind::Command).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());