From fdd43d0685a8a674bf672c1f751947de4c40457b Mon Sep 17 00:00:00 2001 From: BlueGlassBlock Date: Sat, 4 Mar 2023 09:55:34 +0800 Subject: [PATCH] feat: unwrap dyn Err when inner is simple PyErr --- guide/src/migration.md | 37 +++++++++++++++++++++++ newsfragments/3004.changed.md | 1 + src/conversions/anyhow.rs | 39 ++++++++++++++++++++---- src/conversions/eyre.rs | 56 +++++++++++++++++++++++++++++++---- 4 files changed, 123 insertions(+), 10 deletions(-) create mode 100644 newsfragments/3004.changed.md diff --git a/guide/src/migration.md b/guide/src/migration.md index 71042fe122f..1b6b58439dc 100644 --- a/guide/src/migration.md +++ b/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 diff --git a/newsfragments/3004.changed.md b/newsfragments/3004.changed.md new file mode 100644 index 00000000000..216789e3ae1 --- /dev/null +++ b/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. \ No newline at end of file diff --git a/src/conversions/anyhow.rs b/src/conversions/anyhow.rs index 463e1265046..614a0cc75db 100644 --- a/src/conversions/anyhow.rs +++ b/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 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::() { + 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::(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::(py) + )) + } } diff --git a/src/conversions/eyre.rs b/src/conversions/eyre.rs index 546a7fbc230..6ea6143e05c 100644 --- a/src/conversions/eyre.rs +++ b/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 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::() { + 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::(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::(py) + )) + } }