Skip to content

Commit

Permalink
task: include panic msg when printing JoinError (#6753)
Browse files Browse the repository at this point in the history
  • Loading branch information
abonander authored Aug 7, 2024
1 parent 1e798d2 commit 0ecf5f0
Show file tree
Hide file tree
Showing 3 changed files with 156 additions and 2 deletions.
37 changes: 35 additions & 2 deletions tokio/src/runtime/task/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
},
}
}
}
Expand All @@ -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),
},
}
}
}
Expand All @@ -167,3 +183,20 @@ impl From<JoinError> for io::Error {
)
}
}

fn panic_payload_as_str(payload: &SyncWrapper<Box<dyn Any + Send>>) -> 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::<String>() {
return Some(s);
}

if let Some(s) = payload.downcast_ref_sync::<&'static str>() {
return Some(s);
}

None
}
11 changes: 11 additions & 0 deletions tokio/src/util/sync_wrapper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
//!
//! A similar primitive is provided in the `sync_wrapper` crate.
use std::any::Any;

pub(crate) struct SyncWrapper<T> {
value: T,
}
Expand All @@ -24,3 +26,12 @@ impl<T> SyncWrapper<T> {
self.value
}
}

impl SyncWrapper<Box<dyn Any + Send>> {
/// Attempt to downcast using `Any::downcast_ref()` to a type that is known to be `Sync`.
pub(crate) fn downcast_ref_sync<T: Any + 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()
}
}
110 changes: 110 additions & 0 deletions tokio/tests/task_abort.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
);
});
}

0 comments on commit 0ecf5f0

Please sign in to comment.