diff --git a/crates/irtt-cli/src/cmd/client/run.rs b/crates/irtt-cli/src/cmd/client/run.rs index 60f5386..3c78a8d 100644 --- a/crates/irtt-cli/src/cmd/client/run.rs +++ b/crates/irtt-cli/src/cmd/client/run.rs @@ -277,13 +277,13 @@ fn target_failure_messages(target: &ManagedTargetOutcome) -> Vec { let mut messages = Vec::with_capacity(2); if let ManagedTargetEndReason::Failed(failure) = &target.end_reason { messages.push(format!( - "irtt-rs: target {} failed ({:?} {:?}): {}", + "irtt-rs: target {} failed ({} {}): {}", target.target.id, failure.phase, failure.kind, failure.message )); } if let Some(failure) = &target.cleanup_failure { messages.push(format!( - "irtt-rs: target {} cleanup failed ({:?} {:?}): {}", + "irtt-rs: target {} cleanup failed ({} {}): {}", target.target.id, failure.phase, failure.kind, failure.message )); } @@ -480,8 +480,8 @@ mod tests { assert_eq!( target_failure_messages(&target), [ - "irtt-rs: target edge failed (Receiving Protocol): primary failure", - "irtt-rs: target edge cleanup failed (Closing Socket): cleanup failure", + "irtt-rs: target edge failed (receiving protocol): primary failure", + "irtt-rs: target edge cleanup failed (closing socket): cleanup failure", ] ); } diff --git a/crates/irtt-cli/src/cmd/tui/ui.rs b/crates/irtt-cli/src/cmd/tui/ui.rs index d468bac..b1742e8 100644 --- a/crates/irtt-cli/src/cmd/tui/ui.rs +++ b/crates/irtt-cli/src/cmd/tui/ui.rs @@ -194,7 +194,7 @@ impl TuiState { (TargetStatus::Replaced, "target replaced".to_owned(), None) } ManagedTargetEndReason::Failed(failure) => { - let message = format!("{:?}: {}", failure.kind, failure.message); + let message = format!("{}: {}", failure.kind, failure.message); ( TargetStatus::Failed, format!("target failed: {message}"), @@ -205,7 +205,7 @@ impl TuiState { let cleanup_warning = outcome.cleanup_failure.as_ref().map(|failure| { format!( - "cleanup failed ({:?} {:?}): {}", + "cleanup failed ({} {}): {}", failure.phase, failure.kind, failure.message ) }); @@ -2430,7 +2430,7 @@ mod tests { assert!(state.targets[1] .last_warning .as_deref() - .is_some_and(|warning| warning.contains("Timeout"))); + .is_some_and(|warning| warning.contains("timeout"))); } #[test] @@ -2474,11 +2474,11 @@ mod tests { assert!(state.targets[0] .last_warning .as_deref() - .is_some_and(|warning| warning.contains("cleanup failed (Closing Socket)"))); + .is_some_and(|warning| warning.contains("cleanup failed (closing socket)"))); assert!(state .recent_events .iter() - .any(|event| event.contains("cleanup failed (Closing Socket)"))); + .any(|event| event.contains("cleanup failed (closing socket)"))); } #[test] @@ -2509,11 +2509,11 @@ mod tests { assert!(state .recent_events .iter() - .any(|event| event.contains("target failed: Protocol: primary failure"))); + .any(|event| event.contains("target failed: protocol: primary failure"))); assert!(state .recent_events .iter() - .any(|event| event.contains("cleanup failed (Closing Socket): cleanup failure"))); + .any(|event| event.contains("cleanup failed (closing socket): cleanup failure"))); } #[test] diff --git a/crates/irtt-client/src/managed/model.rs b/crates/irtt-client/src/managed/model.rs index fac057f..7bb92d3 100644 --- a/crates/irtt-client/src/managed/model.rs +++ b/crates/irtt-client/src/managed/model.rs @@ -1,4 +1,4 @@ -use std::{net::SocketAddr, sync::Arc, time::Duration}; +use std::{fmt, net::SocketAddr, sync::Arc, time::Duration}; use thiserror::Error; @@ -235,6 +235,20 @@ pub enum ManagedTargetFailurePhase { Closing, } +impl fmt::Display for ManagedTargetFailurePhase { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let text = match self { + Self::Connecting => "connecting", + Self::Opening => "opening", + Self::Sending => "sending", + Self::Receiving => "receiving", + Self::Timing => "timing", + Self::Closing => "closing", + }; + f.write_str(text) + } +} + /// Stable category for a target-local failure. #[derive(Clone, Copy, Debug, Eq, PartialEq)] pub enum ManagedTargetFailureKind { @@ -246,9 +260,31 @@ pub enum ManagedTargetFailureKind { Configuration, ResourceExhausted, InvalidState, + /// A failure that deliberately has no more specific category. + /// + /// This is a classification decision, not a fallback: every current + /// [`ClientError`] variant is classified explicitly, so nothing reaches + /// this category by default. Other, } +impl fmt::Display for ManagedTargetFailureKind { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let text = match self { + Self::Resolve => "resolve", + Self::Socket => "socket", + Self::SocketOption => "socket option", + Self::Protocol => "protocol", + Self::Timeout => "timeout", + Self::Configuration => "configuration", + Self::ResourceExhausted => "resource exhausted", + Self::InvalidState => "invalid state", + Self::Other => "other", + }; + f.write_str(text) + } +} + /// Durable target-local failure details. #[derive(Clone, Debug, Eq, PartialEq)] pub struct ManagedTargetFailure { @@ -350,6 +386,12 @@ pub enum ManagedDriverFailure { Internal { message: Arc }, } +/// Map a [`ClientError`] onto its durable managed failure classification. +/// +/// The match is deliberately exhaustive over every [`ClientError`] variant. A +/// new variant must fail to compile here until its real classification is +/// chosen, rather than silently degrading to +/// [`ManagedTargetFailureKind::Other`]. pub(crate) fn classify_client_error( phase: ManagedTargetFailurePhase, error: &ClientError, @@ -375,13 +417,15 @@ pub(crate) fn classify_client_error( | ClientError::CounterOverflow { .. } | ClientError::DurationOverflow | ClientError::PendingLimitExceeded { .. } => ManagedTargetFailureKind::ResourceExhausted, + ClientError::DatagramLengthMismatch { .. } => ManagedTargetFailureKind::Socket, + #[cfg(feature = "tokio")] + ClientError::NoTokioRuntime => ManagedTargetFailureKind::InvalidState, ClientError::NotOpen | ClientError::AlreadyOpen | ClientError::AlreadyCompleted | ClientError::AlreadyClosed | ClientError::StalePreparedProbe { .. } | ClientError::PendingSequenceCollision { .. } => ManagedTargetFailureKind::InvalidState, - _ => ManagedTargetFailureKind::Other, }; ManagedTargetFailure { phase, @@ -389,3 +433,199 @@ pub(crate) fn classify_client_error( message: Arc::from(error.to_string()), } } + +#[cfg(test)] +mod tests { + use std::{collections::TryReserveError, io}; + + use super::*; + + fn io_error() -> io::Error { + io::Error::other("boom") + } + + fn try_reserve_error() -> TryReserveError { + Vec::::new().try_reserve(usize::MAX).unwrap_err() + } + + fn classification_cases() -> Vec<(ClientError, ManagedTargetFailureKind)> { + vec![ + ( + ClientError::Resolve { + addr: "example.invalid:2112".to_owned(), + }, + ManagedTargetFailureKind::Resolve, + ), + ( + ClientError::Socket(io_error()), + ManagedTargetFailureKind::Socket, + ), + ( + ClientError::DatagramLengthMismatch { + expected: 64, + actual: 32, + }, + ManagedTargetFailureKind::Socket, + ), + ( + ClientError::SocketOption { + operation: "set read timeout", + remote: "127.0.0.1:2112".parse().unwrap(), + source: io_error(), + }, + ManagedTargetFailureKind::SocketOption, + ), + ( + ClientError::ReadTimeoutRestore { source: io_error() }, + ManagedTargetFailureKind::SocketOption, + ), + ( + ClientError::Protocol(irtt_proto::ProtoError::BadMagic), + ManagedTargetFailureKind::Protocol, + ), + ( + ClientError::ProtocolVersionMismatch { + requested: 1, + received: 2, + }, + ManagedTargetFailureKind::Protocol, + ), + (ClientError::ZeroToken, ManagedTargetFailureKind::Protocol), + ( + ClientError::UnexpectedNoTestReply, + ManagedTargetFailureKind::Protocol, + ), + ( + ClientError::NonZeroNoTestToken { token: 7 }, + ManagedTargetFailureKind::Protocol, + ), + ( + ClientError::ServerRejected, + ManagedTargetFailureKind::Protocol, + ), + ( + ClientError::NegotiationRejected { + reason: "duration".to_owned(), + }, + ManagedTargetFailureKind::Protocol, + ), + (ClientError::OpenTimeout, ManagedTargetFailureKind::Timeout), + ( + ClientError::InvalidConfig { + reason: "interval".to_owned(), + }, + ManagedTargetFailureKind::Configuration, + ), + ( + ClientError::OpenTimeoutTooSmall { + timeout: Duration::from_millis(1), + minimum: Duration::from_millis(10), + }, + ManagedTargetFailureKind::Configuration, + ), + ( + ClientError::NoOpenTimeouts, + ManagedTargetFailureKind::Configuration, + ), + ( + ClientError::AllocationFailed { + operation: "pending probes", + source: try_reserve_error(), + }, + ManagedTargetFailureKind::ResourceExhausted, + ), + ( + ClientError::CounterOverflow { + counter: "packets_sent", + }, + ManagedTargetFailureKind::ResourceExhausted, + ), + ( + ClientError::DurationOverflow, + ManagedTargetFailureKind::ResourceExhausted, + ), + ( + ClientError::PendingLimitExceeded { limit: 8 }, + ManagedTargetFailureKind::ResourceExhausted, + ), + ( + ClientError::NoTokioRuntime, + ManagedTargetFailureKind::InvalidState, + ), + (ClientError::NotOpen, ManagedTargetFailureKind::InvalidState), + ( + ClientError::AlreadyOpen, + ManagedTargetFailureKind::InvalidState, + ), + ( + ClientError::AlreadyCompleted, + ManagedTargetFailureKind::InvalidState, + ), + ( + ClientError::AlreadyClosed, + ManagedTargetFailureKind::InvalidState, + ), + ( + ClientError::StalePreparedProbe { + prepared_seq: 3, + next_wire_seq: 4, + }, + ManagedTargetFailureKind::InvalidState, + ), + ( + ClientError::PendingSequenceCollision { seq: 9 }, + ManagedTargetFailureKind::InvalidState, + ), + ] + } + + #[test] + fn every_client_error_classifies_to_its_documented_kind() { + for (error, expected) in classification_cases() { + let failure = classify_client_error(ManagedTargetFailurePhase::Opening, &error); + assert_eq!( + failure.kind, expected, + "unexpected classification for {error:?}" + ); + assert_eq!(failure.phase, ManagedTargetFailurePhase::Opening); + assert_eq!(&*failure.message, error.to_string()); + } + } + + #[test] + fn classification_preserves_the_reported_phase() { + for phase in [ + ManagedTargetFailurePhase::Connecting, + ManagedTargetFailurePhase::Opening, + ManagedTargetFailurePhase::Sending, + ManagedTargetFailurePhase::Receiving, + ManagedTargetFailurePhase::Timing, + ManagedTargetFailurePhase::Closing, + ] { + let failure = classify_client_error(phase, &ClientError::OpenTimeout); + assert_eq!(failure.phase, phase); + } + } + + #[test] + fn phase_and_kind_display_without_debug_spelling() { + assert_eq!( + ManagedTargetFailurePhase::Connecting.to_string(), + "connecting" + ); + assert_eq!(ManagedTargetFailurePhase::Closing.to_string(), "closing"); + assert_eq!( + ManagedTargetFailureKind::SocketOption.to_string(), + "socket option" + ); + assert_eq!( + ManagedTargetFailureKind::ResourceExhausted.to_string(), + "resource exhausted" + ); + assert_eq!( + ManagedTargetFailureKind::InvalidState.to_string(), + "invalid state" + ); + assert_eq!(ManagedTargetFailureKind::Other.to_string(), "other"); + } +}