diff --git a/tokio/src/runtime/task/error.rs b/tokio/src/runtime/task/error.rs index d0d731322..971690177 100644 --- a/tokio/src/runtime/task/error.rs +++ b/tokio/src/runtime/task/error.rs @@ -140,7 +140,18 @@ impl fmt::Display for JoinError { fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { match &self.repr { Repr::Cancelled => write!(fmt, "task {} was cancelled", self.id), - Repr::Panic(_) => write!(fmt, "task {} panicked", self.id), + Repr::Panic(p) => match panic_payload_as_str(p) { + Some(panic_str) => { + write!( + fmt, + "task {} panicked with message {:?}", + self.id, panic_str + ) + } + None => { + write!(fmt, "task {} panicked", self.id) + } + }, } } } @@ -149,7 +160,12 @@ impl fmt::Debug for JoinError { fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { match &self.repr { Repr::Cancelled => write!(fmt, "JoinError::Cancelled({:?})", self.id), - Repr::Panic(_) => write!(fmt, "JoinError::Panic({:?}, ...)", self.id), + Repr::Panic(p) => match panic_payload_as_str(p) { + Some(panic_str) => { + write!(fmt, "JoinError::Panic({:?}, {:?}, ...)", self.id, panic_str) + } + None => write!(fmt, "JoinError::Panic({:?}, ...)", self.id), + }, } } } @@ -167,3 +183,20 @@ impl From for io::Error { ) } } + +fn panic_payload_as_str(payload: &SyncWrapper>) -> Option<&str> { + // Panic payloads are almost always `String` (if invoked with formatting arguments) + // or `&'static str` (if invoked with a string literal). + // + // Non-string panic payloads have niche use-cases, + // so we don't really need to worry about those. + if let Some(s) = payload.downcast_ref_sync::() { + return Some(s); + } + + if let Some(s) = payload.downcast_ref_sync::<&'static str>() { + return Some(s); + } + + None +} diff --git a/tokio/src/util/sync_wrapper.rs b/tokio/src/util/sync_wrapper.rs index 5ffc8f96b..62738e4d0 100644 --- a/tokio/src/util/sync_wrapper.rs +++ b/tokio/src/util/sync_wrapper.rs @@ -3,6 +3,8 @@ //! //! A similar primitive is provided in the `sync_wrapper` crate. +use std::any::Any; + pub(crate) struct SyncWrapper { value: T, } @@ -24,3 +26,12 @@ impl SyncWrapper { self.value } } + +impl SyncWrapper> { + /// Attempt to downcast using `Any::downcast_ref()` to a type that is known to be `Sync`. + pub(crate) fn downcast_ref_sync(&self) -> Option<&T> { + // SAFETY: if the downcast fails, the inner value is not touched, + // so no thread-safety violation can occur. + self.value.downcast_ref() + } +} diff --git a/tokio/tests/task_abort.rs b/tokio/tests/task_abort.rs index 09414f094..0e742de13 100644 --- a/tokio/tests/task_abort.rs +++ b/tokio/tests/task_abort.rs @@ -220,3 +220,113 @@ fn test_abort_task_that_panics_on_drop_returned() { assert!(handle.await.unwrap_err().is_panic()); }); } + +// It's not clear where these tests belong. This was the place suggested by @Darksonn: +// https://github.com/tokio-rs/tokio/pull/6753#issuecomment-2271434176 +/// Checks that a `JoinError` with a panic payload prints the expected text. +#[test] +#[cfg(panic = "unwind")] +fn test_join_error_display() { + let rt = Builder::new_current_thread().build().unwrap(); + + rt.block_on(async move { + // `String` payload + let join_err = tokio::spawn(async move { + let value = 1234; + panic!("Format-args payload: {}", value) + }) + .await + .unwrap_err(); + + // We can't assert the full output because the task ID can change. + let join_err_str = join_err.to_string(); + + assert!( + join_err_str.starts_with("task ") + && join_err_str.ends_with(" panicked with message \"Format-args payload: 1234\""), + "Unexpected join_err_str {:?}", + join_err_str + ); + + // `&'static str` payload + let join_err = tokio::spawn(async move { panic!("Const payload") }) + .await + .unwrap_err(); + + let join_err_str = join_err.to_string(); + + assert!( + join_err_str.starts_with("task ") + && join_err_str.ends_with(" panicked with message \"Const payload\""), + "Unexpected join_err_str {:?}", + join_err_str + ); + + // Non-string payload + let join_err = tokio::spawn(async move { std::panic::panic_any(1234i32) }) + .await + .unwrap_err(); + + let join_err_str = join_err.to_string(); + + assert!( + join_err_str.starts_with("task ") && join_err_str.ends_with(" panicked"), + "Unexpected join_err_str {:?}", + join_err_str + ); + }); +} + +/// Checks that a `JoinError` with a panic payload prints the expected text from `Debug`. +#[test] +#[cfg(panic = "unwind")] +fn test_join_error_debug() { + let rt = Builder::new_current_thread().build().unwrap(); + + rt.block_on(async move { + // `String` payload + let join_err = tokio::spawn(async move { + let value = 1234; + panic!("Format-args payload: {}", value) + }) + .await + .unwrap_err(); + + // We can't assert the full output because the task ID can change. + let join_err_str = format!("{:?}", join_err); + + assert!( + join_err_str.starts_with("JoinError::Panic(Id(") + && join_err_str.ends_with("), \"Format-args payload: 1234\", ...)"), + "Unexpected join_err_str {:?}", + join_err_str + ); + + // `&'static str` payload + let join_err = tokio::spawn(async move { panic!("Const payload") }) + .await + .unwrap_err(); + + let join_err_str = format!("{:?}", join_err); + + assert!( + join_err_str.starts_with("JoinError::Panic(Id(") + && join_err_str.ends_with("), \"Const payload\", ...)"), + "Unexpected join_err_str {:?}", + join_err_str + ); + + // Non-string payload + let join_err = tokio::spawn(async move { std::panic::panic_any(1234i32) }) + .await + .unwrap_err(); + + let join_err_str = format!("{:?}", join_err); + + assert!( + join_err_str.starts_with("JoinError::Panic(Id(") && join_err_str.ends_with("), ...)"), + "Unexpected join_err_str {:?}", + join_err_str + ); + }); +}