Skip to content

Commit

Permalink
unify 3.12 and pre-3.12 exception handling pathways
Browse files Browse the repository at this point in the history
  • Loading branch information
davidhewitt committed Sep 30, 2023
1 parent f335f42 commit 2daddb4
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 13 deletions.
1 change: 1 addition & 0 deletions newsfragments/3455.changed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
`Err` returned from `#[pyfunction]` will now have a non-None `__context__` if called from inside a `catch` block.
4 changes: 4 additions & 0 deletions pytests/src/pyclasses.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,14 @@ impl AssertingBaseClass {
}
}

#[pyclass]
struct ClassWithoutConstructor;

#[pymodule]
pub fn pyclasses(_py: Python<'_>, m: &PyModule) -> PyResult<()> {
m.add_class::<EmptyClass>()?;
m.add_class::<PyClassIter>()?;
m.add_class::<AssertingBaseClass>()?;
m.add_class::<ClassWithoutConstructor>()?;
Ok(())
}
26 changes: 25 additions & 1 deletion pytests/tests/test_pyclasses.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from typing import Type

import pytest
from pyo3_pytests import pyclasses

Expand Down Expand Up @@ -32,7 +34,29 @@ class AssertingSubClass(pyclasses.AssertingBaseClass):


def test_new_classmethod():
# The `AssertingBaseClass` constructor errors if it is not passed the relevant subclass.
# The `AssertingBaseClass` constructor errors if it is not passed the
# relevant subclass.
_ = AssertingSubClass(expected_type=AssertingSubClass)
with pytest.raises(ValueError):
_ = AssertingSubClass(expected_type=str)


class ClassWithoutConstructorPy:
def __new__(cls):
raise TypeError("No constructor defined")


@pytest.mark.parametrize(
"cls", [pyclasses.ClassWithoutConstructor, ClassWithoutConstructorPy]
)
def test_no_constructor_defined_propagates_cause(cls: Type):
original_error = ValueError("Original message")
with pytest.raises(Exception) as exc_info:
try:
raise original_error
except Exception:
cls() # should raise TypeError("No constructor defined")

assert exc_info.type is TypeError
assert exc_info.value.args == ("No constructor defined",)
assert exc_info.value.__context__ is original_error
18 changes: 6 additions & 12 deletions src/err/err_state.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
#[cfg(not(Py_3_12))]
use crate::types::PyString;
use crate::{
exceptions::{PyBaseException, PyTypeError},
ffi,
Expand Down Expand Up @@ -195,17 +193,14 @@ fn lazy_into_normalized_ffi_tuple(
py: Python<'_>,
lazy: Box<PyErrStateLazyFn>,
) -> (*mut ffi::PyObject, *mut ffi::PyObject, *mut ffi::PyObject) {
let PyErrStateLazyFnOutput { ptype, pvalue } = lazy(py);
let (mut ptype, mut pvalue) = if unsafe { ffi::PyExceptionClass_Check(ptype.as_ptr()) } == 0 {
(
PyTypeError::type_object_raw(py).cast(),
PyString::new(py, "exceptions must derive from BaseException").into_ptr(),
)
} else {
(ptype.into_ptr(), pvalue.into_ptr())
};
// To be consistent with 3.12 logic, go via raise_lazy, but also then normalize
// the resulting exception
raise_lazy(py, lazy);
let mut ptype = std::ptr::null_mut();
let mut pvalue = std::ptr::null_mut();
let mut ptraceback = std::ptr::null_mut();
unsafe {
ffi::PyErr_Fetch(&mut ptype, &mut pvalue, &mut ptraceback);
ffi::PyErr_NormalizeException(&mut ptype, &mut pvalue, &mut ptraceback);
}
(ptype, pvalue, ptraceback)
Expand All @@ -218,7 +213,6 @@ fn lazy_into_normalized_ffi_tuple(
///
/// This would require either moving some logic from C to Rust, or requesting a new
/// API in CPython.
#[cfg(Py_3_12)]
fn raise_lazy(py: Python<'_>, lazy: Box<PyErrStateLazyFn>) {
let PyErrStateLazyFnOutput { ptype, pvalue } = lazy(py);
unsafe {
Expand Down

0 comments on commit 2daddb4

Please sign in to comment.