From 1e1616eab7d77db192d0bda9c9410e9f06c09179 Mon Sep 17 00:00:00 2001 From: Leynos Date: Mon, 18 Aug 2025 08:23:35 +0100 Subject: [PATCH 1/5] Format panic payloads before logging --- src/server/connection.rs | 60 +++++++++++++++++++++++++++++--- wireframe_testing/src/helpers.rs | 12 ++++--- 2 files changed, 62 insertions(+), 10 deletions(-) diff --git a/src/server/connection.rs b/src/server/connection.rs index b0400a78..ab7c2022 100644 --- a/src/server/connection.rs +++ b/src/server/connection.rs @@ -39,11 +39,13 @@ 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(""); + let panic_msg = if let Some(s) = panic.downcast_ref::<&str>() { + (*s).to_string() + } else if let Some(s) = panic.downcast_ref::() { + s.clone() + } else { + format!("{panic:?}") + }; tracing::error!(panic = %panic_msg, ?peer_addr, "connection task panicked"); } }); @@ -156,6 +158,54 @@ mod tests { }); } + /// 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) }) + .unwrap() + }; + let tracker = TaskTracker::new(); + let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap(); + + let handle = tokio::spawn({ + let tracker = tracker.clone(); + async move { + let (stream, _) = listener.accept().await.unwrap(); + 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(); + drop(client); + + handle.await.unwrap(); + 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()) + }); + } + /// Ensure the server survives panicking connection tasks. #[rstest] #[traced_test] diff --git a/wireframe_testing/src/helpers.rs b/wireframe_testing/src/helpers.rs index 77988941..1f2e40f3 100644 --- a/wireframe_testing/src/helpers.rs +++ b/wireframe_testing/src/helpers.rs @@ -71,11 +71,13 @@ 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 msg = if let Some(s) = panic.downcast_ref::<&str>() { + (*s).to_string() + } else if let Some(s) = panic.downcast_ref::() { + s.clone() + } else { + format!("{panic:?}") + }; Err(io::Error::new( io::ErrorKind::Other, format!("server task failed: {msg}"), From 03bb1ce4aff059ae3b7591a19fcbd0b774472348 Mon Sep 17 00:00:00 2001 From: Leynos Date: Mon, 18 Aug 2025 18:50:47 +0100 Subject: [PATCH 2/5] Format non-string panic test --- src/lib.rs | 1 + src/panic.rs | 28 +++++++++++++++++++ src/server/connection.rs | 46 +++++++++++++++----------------- wireframe_testing/src/helpers.rs | 8 +----- 4 files changed, 52 insertions(+), 31 deletions(-) create mode 100644 src/panic.rs 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..3c6ba657 --- /dev/null +++ b/src/panic.rs @@ -0,0 +1,28 @@ +//! Utilities for working with panic payloads. +//! +//! These helpers aid in extracting useful information from `panic!` payloads +//! for logging and diagnostics. + +use std::any::Any; + +/// Format a panic payload into a human-readable message. +/// +/// The payload is downcast to `String` or `&'static str` if possible and falls +/// back to `Debug` formatting otherwise. +/// +/// ``` +/// use wireframe::panic::format_panic; +/// assert_eq!(format_panic(Box::new("boom")), "boom"); +/// assert_eq!(format_panic(Box::new(String::from("boom"))), "boom"); +/// assert_eq!(format_panic(Box::new(5_u32)), "Any"); +/// ``` +#[must_use] +pub fn format_panic(panic: Box) -> String { + match panic.downcast::() { + Ok(s) => *s, + Err(panic) => match panic.downcast::<&'static str>() { + Ok(s) => (*s).to_string(), + Err(panic) => format!("{panic:?}"), + }, + } +} diff --git a/src/server/connection.rs b/src/server/connection.rs index ab7c2022..33807f21 100644 --- a/src/server/connection.rs +++ b/src/server/connection.rs @@ -39,13 +39,7 @@ pub(super) fn spawn_connection_task( if let Err(panic) = fut.await { crate::metrics::inc_connection_panics(); - let panic_msg = if let Some(s) = panic.downcast_ref::<&str>() { - (*s).to_string() - } else if let Some(s) = panic.downcast_ref::() { - s.clone() - } else { - format!("{panic:?}") - }; + let panic_msg = crate::panic::format_panic(panic); tracing::error!(panic = %panic_msg, ?peer_addr, "connection task panicked"); } }); @@ -123,26 +117,28 @@ 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.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]| { @@ -168,29 +164,31 @@ mod tests { let app_factory = move || { factory() .on_connection_setup(|| async { std::panic::panic_any(5_u32) }) - .unwrap() + .expect("install panic setup callback") }; 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.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]| { diff --git a/wireframe_testing/src/helpers.rs b/wireframe_testing/src/helpers.rs index 1f2e40f3..b3dceae5 100644 --- a/wireframe_testing/src/helpers.rs +++ b/wireframe_testing/src/helpers.rs @@ -71,13 +71,7 @@ where match result { Ok(_) => Ok(()), Err(panic) => { - let msg = if let Some(s) = panic.downcast_ref::<&str>() { - (*s).to_string() - } else if let Some(s) = panic.downcast_ref::() { - s.clone() - } else { - format!("{panic:?}") - }; + let msg = wireframe::panic::format_panic(panic); Err(io::Error::new( io::ErrorKind::Other, format!("server task failed: {msg}"), From d80ab7e2ca40c335d11bb2bd74f1668294b14453 Mon Sep 17 00:00:00 2001 From: Leynos Date: Tue, 19 Aug 2025 00:58:33 +0100 Subject: [PATCH 3/5] Format panic payloads at log time --- src/panic.rs | 35 +++++++++++++++++++++----------- src/server/connection.rs | 7 +++++-- wireframe_testing/src/helpers.rs | 2 +- 3 files changed, 29 insertions(+), 15 deletions(-) diff --git a/src/panic.rs b/src/panic.rs index 3c6ba657..0dd39d45 100644 --- a/src/panic.rs +++ b/src/panic.rs @@ -3,26 +3,37 @@ //! These helpers aid in extracting useful information from `panic!` payloads //! for logging and diagnostics. -use std::any::Any; +use std::{any::Any, fmt}; -/// Format a panic payload into a human-readable message. +/// Wrapper that formats a panic payload when logged or displayed. /// /// The payload is downcast to `String` or `&'static str` if possible and falls /// back to `Debug` formatting otherwise. /// /// ``` /// use wireframe::panic::format_panic; -/// assert_eq!(format_panic(Box::new("boom")), "boom"); -/// assert_eq!(format_panic(Box::new(String::from("boom"))), "boom"); -/// assert_eq!(format_panic(Box::new(5_u32)), "Any"); +/// assert_eq!(format_panic(Box::new("boom")).to_string(), "boom"); +/// assert_eq!( +/// format_panic(Box::new(String::from("boom"))).to_string(), +/// "boom" +/// ); +/// assert!(format_panic(Box::new(5_u32)).to_string().contains("Any")); /// ``` +#[derive(Debug)] #[must_use] -pub fn format_panic(panic: Box) -> String { - match panic.downcast::() { - Ok(s) => *s, - Err(panic) => match panic.downcast::<&'static str>() { - Ok(s) => (*s).to_string(), - Err(panic) => format!("{panic:?}"), - }, +pub struct PanicMessage(Box); + +impl fmt::Display for PanicMessage { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + if let Some(s) = self.0.downcast_ref::() { + f.write_str(s) + } else if let Some(s) = self.0.downcast_ref::<&'static str>() { + f.write_str(s) + } else { + write!(f, "{:?}", self.0) + } } } + +/// Create a [`PanicMessage`] for the given payload. +pub fn format_panic(panic: Box) -> PanicMessage { PanicMessage(panic) } diff --git a/src/server/connection.rs b/src/server/connection.rs index 33807f21..88391356 100644 --- a/src/server/connection.rs +++ b/src/server/connection.rs @@ -39,8 +39,11 @@ pub(super) fn spawn_connection_task( if let Err(panic) = fut.await { crate::metrics::inc_connection_panics(); - let panic_msg = crate::panic::format_panic(panic); - tracing::error!(panic = %panic_msg, ?peer_addr, "connection task panicked"); + tracing::error!( + panic = %crate::panic::format_panic(panic), + ?peer_addr, + "connection task panicked" + ); } }); } diff --git a/wireframe_testing/src/helpers.rs b/wireframe_testing/src/helpers.rs index b3dceae5..d99fad97 100644 --- a/wireframe_testing/src/helpers.rs +++ b/wireframe_testing/src/helpers.rs @@ -71,7 +71,7 @@ where match result { Ok(_) => Ok(()), Err(panic) => { - let msg = wireframe::panic::format_panic(panic); + let msg = wireframe::panic::format_panic(panic).to_string(); Err(io::Error::new( io::ErrorKind::Other, format!("server task failed: {msg}"), From 4aa407123fef8b12b7b8dc5d89d40b1f60874f68 Mon Sep 17 00:00:00 2001 From: Leynos Date: Tue, 19 Aug 2025 13:49:28 +0100 Subject: [PATCH 4/5] Log panic payloads with Debug formatting --- src/lib.rs | 1 - src/panic.rs | 39 -------------------------------- src/server/connection.rs | 6 ++--- wireframe_testing/src/helpers.rs | 11 ++++----- 4 files changed, 7 insertions(+), 50 deletions(-) delete mode 100644 src/panic.rs diff --git a/src/lib.rs b/src/lib.rs index 2ae0d074..e4c19ec8 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -15,7 +15,6 @@ 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 deleted file mode 100644 index 0dd39d45..00000000 --- a/src/panic.rs +++ /dev/null @@ -1,39 +0,0 @@ -//! Utilities for working with panic payloads. -//! -//! These helpers aid in extracting useful information from `panic!` payloads -//! for logging and diagnostics. - -use std::{any::Any, fmt}; - -/// Wrapper that formats a panic payload when logged or displayed. -/// -/// The payload is downcast to `String` or `&'static str` if possible and falls -/// back to `Debug` formatting otherwise. -/// -/// ``` -/// use wireframe::panic::format_panic; -/// assert_eq!(format_panic(Box::new("boom")).to_string(), "boom"); -/// assert_eq!( -/// format_panic(Box::new(String::from("boom"))).to_string(), -/// "boom" -/// ); -/// assert!(format_panic(Box::new(5_u32)).to_string().contains("Any")); -/// ``` -#[derive(Debug)] -#[must_use] -pub struct PanicMessage(Box); - -impl fmt::Display for PanicMessage { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - if let Some(s) = self.0.downcast_ref::() { - f.write_str(s) - } else if let Some(s) = self.0.downcast_ref::<&'static str>() { - f.write_str(s) - } else { - write!(f, "{:?}", self.0) - } - } -} - -/// Create a [`PanicMessage`] for the given payload. -pub fn format_panic(panic: Box) -> PanicMessage { PanicMessage(panic) } diff --git a/src/server/connection.rs b/src/server/connection.rs index 88391356..d0e1d251 100644 --- a/src/server/connection.rs +++ b/src/server/connection.rs @@ -40,7 +40,7 @@ pub(super) fn spawn_connection_task( if let Err(panic) = fut.await { crate::metrics::inc_connection_panics(); tracing::error!( - panic = %crate::panic::format_panic(panic), + panic = ?panic, ?peer_addr, "connection task panicked" ); @@ -149,7 +149,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(|_| ()) @@ -260,7 +260,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 d99fad97..7c93598d 100644 --- a/wireframe_testing/src/helpers.rs +++ b/wireframe_testing/src/helpers.rs @@ -70,13 +70,10 @@ where .await; match result { Ok(_) => Ok(()), - Err(panic) => { - let msg = wireframe::panic::format_panic(panic).to_string(); - Err(io::Error::new( - io::ErrorKind::Other, - format!("server task failed: {msg}"), - )) - } + Err(panic) => Err(io::Error::new( + io::ErrorKind::Other, + format!("server task failed: {panic:?}"), + )), } }; From 19805572f810eb625ab09bff9942a93714906390 Mon Sep 17 00:00:00 2001 From: Leynos Date: Tue, 19 Aug 2025 15:05:39 +0100 Subject: [PATCH 5/5] Extract panic formatting into helper --- src/lib.rs | 1 + src/panic.rs | 18 ++++++++++++++++++ src/server/connection.rs | 3 ++- wireframe_testing/src/helpers.rs | 11 +++++++---- 4 files changed, 28 insertions(+), 5 deletions(-) create mode 100644 src/panic.rs 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 d0e1d251..f67eb598 100644 --- a/src/server/connection.rs +++ b/src/server/connection.rs @@ -39,8 +39,9 @@ pub(super) fn spawn_connection_task( if let Err(panic) = fut.await { crate::metrics::inc_connection_panics(); + let panic_msg = crate::panic::format_panic(&panic); tracing::error!( - panic = ?panic, + panic = %panic_msg, ?peer_addr, "connection task panicked" ); diff --git a/wireframe_testing/src/helpers.rs b/wireframe_testing/src/helpers.rs index 7c93598d..27f66fdc 100644 --- a/wireframe_testing/src/helpers.rs +++ b/wireframe_testing/src/helpers.rs @@ -70,10 +70,13 @@ where .await; match result { Ok(_) => Ok(()), - Err(panic) => Err(io::Error::new( - io::ErrorKind::Other, - format!("server task failed: {panic:?}"), - )), + Err(panic) => { + let panic_msg = wireframe::panic::format_panic(&panic); + Err(io::Error::new( + io::ErrorKind::Other, + format!("server task failed: {panic_msg}"), + )) + } } };