diff --git a/src/lib.rs b/src/lib.rs index e4c19ec8..2ae0d074 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -15,6 +15,7 @@ pub mod hooks; pub mod message; pub mod metrics; pub mod middleware; +pub mod panic; pub mod preamble; pub mod push; pub mod response; diff --git a/src/panic.rs b/src/panic.rs new file mode 100644 index 00000000..1950b40a --- /dev/null +++ b/src/panic.rs @@ -0,0 +1,18 @@ +//! Utilities for formatting panic payloads. +//! +//! This module centralises panic formatting so logs and errors can present +//! consistent messages. + +use std::any::Any; + +/// Format a panic payload into a `String` using `Debug` formatting. +/// +/// # Examples +/// ``` +/// # use std::any::Any; +/// # use wireframe::panic::format_panic; +/// let payload: Box = Box::new("boom"); +/// assert_eq!(format_panic(&payload), "Any { .. }"); +/// ``` +#[must_use] +pub fn format_panic(panic: &Box) -> String { format!("{panic:?}") } diff --git a/src/server/connection.rs b/src/server/connection.rs index b0400a78..f67eb598 100644 --- a/src/server/connection.rs +++ b/src/server/connection.rs @@ -39,12 +39,12 @@ pub(super) fn spawn_connection_task( if let Err(panic) = fut.await { crate::metrics::inc_connection_panics(); - let panic_msg = panic - .downcast_ref::<&str>() - .copied() - .or_else(|| panic.downcast_ref::().map(String::as_str)) - .unwrap_or(""); - tracing::error!(panic = %panic_msg, ?peer_addr, "connection task panicked"); + let panic_msg = crate::panic::format_panic(&panic); + tracing::error!( + panic = %panic_msg, + ?peer_addr, + "connection task panicked" + ); } }); } @@ -121,26 +121,78 @@ mod tests { .unwrap() }; let tracker = TaskTracker::new(); - let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); - let addr = listener.local_addr().unwrap(); + let listener = TcpListener::bind("127.0.0.1:0") + .await + .expect("bind listener"); + let addr = listener.local_addr().expect("listener.local_addr"); + + let handle = tokio::spawn({ + let tracker = tracker.clone(); + async move { + let (stream, _) = listener.accept().await.expect("accept"); + spawn_connection_task::<_, ()>(stream, app_factory, None, None, &tracker); + tracker.close(); + tracker.wait().await; + } + }); + + let client = TcpStream::connect(addr).await.expect("connect"); + let peer_addr = client.local_addr().expect("client.local_addr"); + client.writable().await.expect("client.writable"); + client.try_write(&[0; 8]).expect("client.try_write"); + drop(client); + + handle.await.expect("join connection task driver"); + tokio::task::yield_now().await; + + logs_assert(|lines: &[&str]| { + lines + .iter() + .find(|line| { + line.contains("connection task panicked") + && line.contains("panic=Any") + && line.contains(&format!("peer_addr=Some({peer_addr})")) + }) + .map(|_| ()) + .ok_or_else(|| "panic log not found".to_string()) + }); + } + + /// Non-string panic payloads are formatted with `Debug` in logs. + #[rstest] + #[traced_test] + #[tokio::test] + async fn spawn_connection_task_logs_non_string_panic( + factory: impl Fn() -> WireframeApp + Send + Sync + Clone + 'static, + ) { + let app_factory = move || { + factory() + .on_connection_setup(|| async { std::panic::panic_any(5_u32) }) + .expect("install panic setup callback") + }; + let tracker = TaskTracker::new(); + let listener = TcpListener::bind("127.0.0.1:0") + .await + .expect("bind listener"); + let addr = listener.local_addr().expect("listener.local_addr"); let handle = tokio::spawn({ let tracker = tracker.clone(); async move { - let (stream, _) = listener.accept().await.unwrap(); + let (stream, _) = listener.accept().await.expect("accept"); spawn_connection_task::<_, ()>(stream, app_factory, None, None, &tracker); tracker.close(); tracker.wait().await; } }); - let client = TcpStream::connect(addr).await.unwrap(); - let peer_addr = client.local_addr().unwrap(); - client.writable().await.unwrap(); - client.try_write(&[0; 8]).unwrap(); + let client = TcpStream::connect(addr).await.expect("connect"); + let peer_addr = client.local_addr().expect("client.local_addr"); + client.writable().await.expect("client.writable"); + client.try_write(&[0; 8]).expect("client.try_write"); drop(client); - handle.await.unwrap(); + handle.await.expect("join connection task driver"); tokio::task::yield_now().await; logs_assert(|lines: &[&str]| { @@ -148,7 +200,7 @@ mod tests { .iter() .find(|line| { line.contains("connection task panicked") - && line.contains("panic=boom") + && line.contains("panic=Any") && line.contains(&format!("peer_addr=Some({peer_addr})")) }) .map(|_| ()) @@ -209,7 +261,7 @@ mod tests { .iter() .find(|line| { line.contains("connection task panicked") - && line.contains("panic=boom") + && line.contains("panic=Any") && line.contains(&format!("peer_addr=Some({peer_addr})")) }) .map(|_| ()) diff --git a/wireframe_testing/src/helpers.rs b/wireframe_testing/src/helpers.rs index 77988941..27f66fdc 100644 --- a/wireframe_testing/src/helpers.rs +++ b/wireframe_testing/src/helpers.rs @@ -71,14 +71,10 @@ where match result { Ok(_) => Ok(()), Err(panic) => { - let msg = panic - .downcast_ref::<&str>() - .copied() - .or_else(|| panic.downcast_ref::().map(String::as_str)) - .unwrap_or(""); + let panic_msg = wireframe::panic::format_panic(&panic); Err(io::Error::new( io::ErrorKind::Other, - format!("server task failed: {msg}"), + format!("server task failed: {panic_msg}"), )) } }