Merge pull request #264 from SimonSapin/panic-msg

Include Rust panic messages in Python exception

Author: markbt

src/function.rs

@@ -224,7 +224,7 @@ where
     });
     match ret {
         Ok(r) => r,
-        Err(ref err) => {
+        Err(err) => {
             // Protect against panics in C::error_value() causing UB
             let guard = AbortOnDrop("handle_panic() / C::error_value()");
             handle_panic(Python::assume_gil_acquired(), err);
@@ -235,8 +235,29 @@ where
     }
 }
 
-fn handle_panic(_py: Python, _panic: &dyn any::Any) {
-    let msg = cstr!("Rust panic");
+// This only needs `&dyn Any`, but we keep a `Box` all the way to avoid the
+// risk of a subtle bug in the caller where `&Box<dyn Any>` is coerced to
+// `&dyn Any` by unsizing with another layer of vtable and wide pointer,
+// instead of the expected auto-deref.
+fn handle_panic(_py: Python, panic: Box<dyn any::Any>) {
+    let panic_str = if let Some(s) = panic.downcast_ref::<String>() {
+        Some(s.as_str())
+    } else if let Some(s) = panic.downcast_ref::<&'static str>() {
+        Some(*s)
+    } else {
+        None
+    };
+    let panic_cstring = panic_str.and_then(|s| {
+        let result = CString::new(format!("Rust panic: {}", s));
+        // Give up on representing the panic payload if it contains a null byte
+        // TODO: use PyErr_SetObject instead, so a `char*` string isn’t needed?
+        result.ok()
+    });
+    let msg = if let Some(s) = &panic_cstring {
+        s.as_c_str()
+    } else {
+        cstr!("Rust panic")
+    };
     unsafe {
         ffi::PyErr_SetString(ffi::PyExc_SystemError, msg.as_ptr());
     }

tests/test_function.rs

@@ -181,6 +181,30 @@ fn none_return() {
     assert_eq!(CALL_COUNT.load(Relaxed), 1);
 }
 
+/// When Python calls a Rust function, an unhandled Rust panic is turned into
+/// a Python `SystemError` exception. The exception’s value is a string that
+/// contains the panic’s payload, if that payload was a string.
+#[test]
+fn panicking() {
+    fn f(_py: Python) -> PyResult<PyNone> {
+        panic!("panicking because {}", "reasons")
+    }
+
+    let gil = Python::acquire_gil();
+    let py = gil.python();
+    let obj = py_fn!(py, f());
+
+    assert_eq!(
+        obj.call(py, NoArgs, None)
+            .unwrap_err() // Expect an exception
+            .instance(py)
+            .str(py)
+            .unwrap()
+            .to_string_lossy(py),
+        "Rust panic: panicking because reasons"
+    );
+}
+
 /* TODO: reimplement flexible sig support
 #[test]
 fn flexible_sig() {