Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
18 changes: 18 additions & 0 deletions src/panic.rs
Original file line number Diff line number Diff line change
@@ -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<dyn Any + Send> = Box::new("boom");
/// assert_eq!(format_panic(&payload), "Any { .. }");
/// ```
#[must_use]
pub fn format_panic(panic: &Box<dyn Any + Send>) -> String { format!("{panic:?}") }
84 changes: 68 additions & 16 deletions src/server/connection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,12 @@ pub(super) fn spawn_connection_task<F, T>(

if let Err(panic) = fut.await {
crate::metrics::inc_connection_panics();
let panic_msg = panic
.downcast_ref::<&str>()
.copied()
.or_else(|| panic.downcast_ref::<String>().map(String::as_str))
.unwrap_or("<non-string panic>");
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"
);
}
});
}
Expand Down Expand Up @@ -121,34 +121,86 @@ 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())
});
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}

/// 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]| {
lines
.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(|_| ())
Expand Down Expand Up @@ -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})"))
})
Comment thread
coderabbitai[bot] marked this conversation as resolved.
.map(|_| ())
Expand Down
8 changes: 2 additions & 6 deletions wireframe_testing/src/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,14 +71,10 @@ where
match result {
Ok(_) => Ok(()),
Err(panic) => {
let msg = panic
.downcast_ref::<&str>()
.copied()
.or_else(|| panic.downcast_ref::<String>().map(String::as_str))
.unwrap_or("<non-string panic>");
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}"),
))
}
}
Expand Down
Loading