Merge #3004

3004: Unwrap dynamic error types when inner is simple `PyErr` r=davidhewitt,adamreichold,mejrs a=BlueGlassBlock

This is the first part of suggested improvements in #2998.

This change will make bubbled `PyErr` wrapped in `eyre::Report` / `anyhow::Error` bubble up unchanged, instead of being wrapped in a `PyRuntimeError`.

Co-authored-by: BlueGlassBlock <blueglassblock@outlook.com>

guide/src/migration.md

@@ -3,6 +3,43 @@
 This guide can help you upgrade code through breaking changes from one PyO3 version to the next.
 For a detailed list of all changes, see the [CHANGELOG](changelog.md).
 
+## from 0.18.* to 0.19
+
+### Smarter `anyhow::Error` / `eyre::Report` conversion when inner error is "simple" `PyErr`
+
+When converting from `anyhow::Error` or `eyre::Report` to `PyErr`, if the inner error is a "simple" `PyErr` (with no source error), then the inner error will be used directly as the `PyErr` instead of wrapping it in a new `PyRuntimeError` with the original information converted into a string.
+
+```rust
+# #[cfg(feature = "anyhow")]
+# #[allow(dead_code)]
+# mod anyhow_only {
+# use pyo3::prelude::*;
+# use pyo3::exceptions::PyValueError;
+#[pyfunction]
+fn raise_err() -> anyhow::Result<()> {
+    Err(PyValueError::new_err("original error message").into())
+}
+
+fn main() {
+    Python::with_gil(|py| {
+        let rs_func = wrap_pyfunction!(raise_err, py).unwrap();
+        pyo3::py_run!(py, rs_func, r"
+        try:
+            rs_func()
+        except Exception as e:
+            print(repr(e))
+        ");
+    })
+}
+# }
+```
+
+Before, the above code would have printed `RuntimeError('ValueError: original error message')`, which might be confusing.
+
+After, the same code will print `ValueError: original error message`, which is more straightforward.
+
+However, if the `anyhow::Error` or `eyre::Report` has a source, then the original exception will still be wrapped in a `PyRuntimeError`.
+
 ## from 0.17.* to 0.18
 
 ### Required arguments after `Option<_>` arguments will no longer be automatically inferred

newsfragments/3004.changed.md

@@ -0,0 +1 @@
+`anyhow::Error`/`eyre::Report` containing a basic `PyErr` won't be wrapped in a `PyRuntimeError` on conversion, if it's not chained.

src/conversions/anyhow.rs

@@ -9,9 +9,10 @@
 //! want error handling to be easy. If you are writing a library or you need more control over your
 //! errors you might want to design your own error type instead.
 //!
-//! This implementation always creates a Python [`RuntimeError`]. You might find that you need to
-//! map the error from your Rust code into another Python exception. See [`PyErr::new`] for more
-//! information about that.
+//! When the inner error is a [`PyErr`] without source, it will be extracted out.
+//! Otherwise a Python [`RuntimeError`] will be created.
+//! You might find that you need to map the error from your Rust code into another Python exception.
+//! See [`PyErr::new`] for more information about that.
 //!
 //! For information about error handling in general, see the [Error handling] chapter of the Rust
 //! book.
@@ -111,13 +112,21 @@ use crate::exceptions::PyRuntimeError;
 use crate::PyErr;
 
 impl From<anyhow::Error> for PyErr {
-    fn from(err: anyhow::Error) -> Self {
-        PyRuntimeError::new_err(format!("{:?}", err))
+    fn from(mut error: anyhow::Error) -> Self {
+        // Errors containing a PyErr without chain or context are returned as the underlying error
+        if error.source().is_none() {
+            error = match error.downcast::<Self>() {
+                Ok(py_err) => return py_err,
+                Err(error) => error,
+            };
+        }
+        PyRuntimeError::new_err(format!("{:?}", error))
     }
 }
 
 #[cfg(test)]
 mod test_anyhow {
+    use crate::exceptions::{PyRuntimeError, PyValueError};
     use crate::prelude::*;
     use crate::types::IntoPyDict;
 
@@ -165,4 +174,24 @@ mod test_anyhow {
             assert_eq!(pyerr.value(py).to_string(), expected_contents);
         })
     }
+
+    #[test]
+    fn test_pyo3_unwrap_simple_err() {
+        let origin_exc = PyValueError::new_err("Value Error");
+        let err: anyhow::Error = origin_exc.into();
+        let converted: PyErr = err.into();
+        assert!(Python::with_gil(
+            |py| converted.is_instance_of::<PyValueError>(py)
+        ))
+    }
+    #[test]
+    fn test_pyo3_unwrap_complex_err() {
+        let origin_exc = PyValueError::new_err("Value Error");
+        let mut err: anyhow::Error = origin_exc.into();
+        err = err.context("Context");
+        let converted: PyErr = err.into();
+        assert!(Python::with_gil(
+            |py| converted.is_instance_of::<PyRuntimeError>(py)
+        ))
+    }
 }

src/conversions/eyre.rs

@@ -8,9 +8,10 @@
 //! want error handling to be easy. If you are writing a library or you need more control over your
 //! errors you might want to design your own error type instead.
 //!
-//! This implementation always creates a Python [`RuntimeError`]. You might find that you need to
-//! map the error from your Rust code into another Python exception. See [`PyErr::new`] for more
-//! information about that.
+//! When the inner error is a [`PyErr`] without source, it will be extracted out.
+//! Otherwise a Python [`RuntimeError`] will be created.
+//! You might find that you need to map the error from your Rust code into another Python exception.
+//! See [`PyErr::new`] for more information about that.
 //!
 //! For information about error handling in general, see the [Error handling] chapter of the Rust
 //! book.
@@ -113,17 +114,25 @@ use eyre::Report;
 /// If you want to raise a different Python exception you will have to do so manually. See
 /// [`PyErr::new`] for more information about that.
 impl From<eyre::Report> for PyErr {
-    fn from(error: Report) -> Self {
+    fn from(mut error: Report) -> Self {
+        // Errors containing a PyErr without chain or context are returned as the underlying error
+        if error.source().is_none() {
+            error = match error.downcast::<Self>() {
+                Ok(py_err) => return py_err,
+                Err(error) => error,
+            };
+        }
         PyRuntimeError::new_err(format!("{:?}", error))
     }
 }
 
 #[cfg(test)]
 mod tests {
+    use crate::exceptions::{PyRuntimeError, PyValueError};
     use crate::prelude::*;
     use crate::types::IntoPyDict;
 
-    use eyre::{bail, Result, WrapErr};
+    use eyre::{bail, eyre, Report, Result, WrapErr};
 
     fn f() -> Result<()> {
         use std::io;
@@ -150,4 +159,41 @@ mod tests {
             assert_eq!(pyerr.value(py).to_string(), expected_contents);
         })
     }
+
+    fn k() -> Result<()> {
+        Err(eyre!("Some sort of error"))
+    }
+
+    #[test]
+    fn test_pyo3_exception_contents2() {
+        let err = k().unwrap_err();
+        let expected_contents = format!("{:?}", err);
+        let pyerr = PyErr::from(err);
+
+        Python::with_gil(|py| {
+            let locals = [("err", pyerr)].into_py_dict(py);
+            let pyerr = py.run("raise err", None, Some(locals)).unwrap_err();
+            assert_eq!(pyerr.value(py).to_string(), expected_contents);
+        })
+    }
+
+    #[test]
+    fn test_pyo3_unwrap_simple_err() {
+        let origin_exc = PyValueError::new_err("Value Error");
+        let report: Report = origin_exc.into();
+        let converted: PyErr = report.into();
+        assert!(Python::with_gil(
+            |py| converted.is_instance_of::<PyValueError>(py)
+        ))
+    }
+    #[test]
+    fn test_pyo3_unwrap_complex_err() {
+        let origin_exc = PyValueError::new_err("Value Error");
+        let mut report: Report = origin_exc.into();
+        report = report.wrap_err("Wrapped");
+        let converted: PyErr = report.into();
+        assert!(Python::with_gil(
+            |py| converted.is_instance_of::<PyRuntimeError>(py)
+        ))
+    }
 }