Skip to content

Commit

Permalink
Change CommandPanic payload type to Option<String>
Browse files Browse the repository at this point in the history
Previously, it was Box<dyn Any + Send> - making the entire type not Sync. This wasn't intentional.

This is technically a breaking change, but since CommandPanic was only introduced a few days ago, and was itself a breaking change, this is basically just a quick hotfix reversal of a breaking change.
  • Loading branch information
kangalio committed Apr 23, 2023
1 parent f408cbf commit 7a29dfe
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 3 deletions.
23 changes: 21 additions & 2 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -472,10 +472,29 @@ pub type BoxFuture<'a, T> = std::pin::Pin<Box<dyn std::future::Future<Output = T
/// Internal wrapper function for catch_unwind that respects the `handle_panics` feature flag
async fn catch_unwind_maybe<T>(
fut: impl std::future::Future<Output = T>,
) -> Result<T, Box<dyn std::any::Any + Send + 'static>> {
) -> Result<T, Option<String>> {
#[cfg(feature = "handle_panics")]
let res = futures_util::FutureExt::catch_unwind(std::panic::AssertUnwindSafe(fut)).await;
let res = futures_util::FutureExt::catch_unwind(std::panic::AssertUnwindSafe(fut))
.await
.map_err(|e| {
if let Some(s) = e.downcast_ref::<&str>() {
Some(s.to_string())
} else if let Ok(s) = e.downcast::<String>() {
Some(*s)
} else {
None
}
});
#[cfg(not(feature = "handle_panics"))]
let res = Ok(fut.await);
res
}

#[cfg(test)]
mod tests {
fn assert_send_sync<T: Send + Sync>() {}

fn _test_framework_error_send_sync<'a, U: Send + Sync + 'static, E: Send + Sync + 'static>() {
assert_send_sync::<crate::FrameworkError<'a, U, E>>();
}
}
8 changes: 7 additions & 1 deletion src/structs/framework_error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,13 @@ pub enum FrameworkError<'a, U, E> {
/// normal errors!
CommandPanic {
/// Panic payload which was thrown in the command code
payload: Box<dyn std::any::Any + Send + 'static>,
///
/// If a panic was thrown via [`std::panic::panic_any()`] and the payload was neither &str,
/// nor String, the payload is `None`.
///
/// The reason the original [`Box<dyn Any + Send>`] payload isn't provided here is that it
/// would make [`FrameworkError`] not [`Sync`] anymore.
payload: Option<String>,
/// Command context
ctx: crate::Context<'a, U, E>,
},
Expand Down

0 comments on commit 7a29dfe

Please sign in to comment.