Merge pull request #391 from signalapp/jrose/describe-panic

bridge: When converting panics to errors, check for String too

rust/bridge/jni/src/logging.rs

@@ -6,8 +6,7 @@
 use jni::objects::{GlobalRef, JClass, JObject, JValue};
 use jni::sys::jint;
 use jni::{JNIEnv, JavaVM};
-use libsignal_bridge::jni_signature;
-use std::any::Any;
+use libsignal_bridge::{describe_panic, jni_signature};
 use std::panic::{catch_unwind, AssertUnwindSafe};
 use std::process::abort;
 
@@ -118,17 +117,6 @@ impl log::Log for JniLogger {
     fn flush(&self) {}
 }
 
-// See https://github.com/rust-lang/rfcs/issues/1389
-fn describe_panic(any: &Box<dyn Any + Send>) -> String {
-    if let Some(msg) = any.downcast_ref::<&str>() {
-        msg.to_string()
-    } else if let Some(msg) = any.downcast_ref::<String>() {
-        msg.to_string()
-    } else {
-        "(break on rust_panic to debug)".to_string()
-    }
-}
-
 /// A low-level version of `run_ffi_safe` that just aborts on errors.
 ///
 /// This is important for logging failures because we might want to log during the normal

rust/bridge/shared/src/ffi/error.rs

@@ -10,6 +10,8 @@ use device_transfer::Error as DeviceTransferError;
 use libsignal_protocol::*;
 use signal_crypto::Error as SignalCryptoError;
 
+use crate::support::describe_panic;
+
 /// The top-level error type (opaquely) returned to C clients when something goes wrong.
 #[derive(Debug)]
 pub enum SignalFfiError {
@@ -39,11 +41,9 @@ impl fmt::Display for SignalFfiError {
             SignalFfiError::InsufficientOutputSize(n, h) => {
                 write!(f, "needed {} elements only {} provided", n, h)
             }
-
-            SignalFfiError::UnexpectedPanic(e) => match e.downcast_ref::<&'static str>() {
-                Some(s) => write!(f, "unexpected panic: {}", s),
-                None => write!(f, "unknown unexpected panic"),
-            },
+            SignalFfiError::UnexpectedPanic(e) => {
+                write!(f, "unexpected panic: {}", describe_panic(e))
+            }
         }
     }
 }

rust/bridge/shared/src/jni/error.rs

@@ -12,6 +12,8 @@ use hsm_enclave::Error as HsmEnclaveError;
 use libsignal_protocol::*;
 use signal_crypto::Error as SignalCryptoError;
 
+use crate::support::describe_panic;
+
 use super::*;
 
 /// The top-level error type for when something goes wrong.
@@ -47,10 +49,9 @@ impl fmt::Display for SignalJniError {
             SignalJniError::HsmEnclave(e) => {
                 write!(f, "{}", e)
             }
-            SignalJniError::UnexpectedPanic(e) => match e.downcast_ref::<&'static str>() {
-                Some(s) => write!(f, "unexpected panic: {}", s),
-                None => write!(f, "unknown unexpected panic"),
-            },
+            SignalJniError::UnexpectedPanic(e) => {
+                write!(f, "unexpected panic: {}", describe_panic(e))
+            }
         }
     }
 }

rust/bridge/shared/src/lib.rs

@@ -23,6 +23,7 @@ pub mod node;
 
 #[macro_use]
 mod support;
+pub use support::describe_panic;
 
 pub mod crypto;
 pub mod protocol;

rust/bridge/shared/src/support/mod.rs

@@ -8,6 +8,17 @@ pub(crate) use paste::paste;
 mod transform_helper;
 pub(crate) use transform_helper::*;
 
+// See https://github.com/rust-lang/rfcs/issues/1389
+pub fn describe_panic(any: &Box<dyn std::any::Any + Send>) -> String {
+    if let Some(msg) = any.downcast_ref::<&str>() {
+        msg.to_string()
+    } else if let Some(msg) = any.downcast_ref::<String>() {
+        msg.to_string()
+    } else {
+        "(break on rust_panic to debug)".to_string()
+    }
+}
+
 /// Exposes a Rust type to each of the bridges as a boxed value.
 ///
 /// Full form: