Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unwrap dynamic error types when inner is simple PyErr #3004

Merged
merged 1 commit into from
May 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
37 changes: 37 additions & 0 deletions guide/src/migration.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions newsfragments/3004.changed.md
Original file line number Diff line number Diff line change
@@ -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.
39 changes: 34 additions & 5 deletions src/conversions/anyhow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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>() {
davidhewitt marked this conversation as resolved.
Show resolved Hide resolved
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;

Expand Down Expand Up @@ -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)
))
}
}
56 changes: 51 additions & 5 deletions src/conversions/eyre.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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;
Expand All @@ -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)
))
}
}