task: include panic msg when printing JoinError (#6753)

Author: abonander

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<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
+}

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<T> {
     value: T,
 }
@@ -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()
+    }
+}

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
+        );
+    });
+}