Merge pull request #3455 from davidhewitt/normalized-exceptions

also use `PyErr::SetObject` on Python versions before 3.12

newsfragments/3455.changed.md

@@ -0,0 +1 @@
+`Err` returned from `#[pyfunction]` will now have a non-None `__context__` if called from inside a `catch` block.

pytests/src/pyclasses.rs

@@ -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(())
 }

pytests/tests/test_pyclasses.py

@@ -1,3 +1,5 @@
+from typing import Type
+
 import pytest
 from pyo3_pytests import pyclasses
 
@@ -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

src/err/err_state.rs

@@ -1,5 +1,3 @@
-#[cfg(not(Py_3_12))]
-use crate::types::PyString;
 use crate::{
     exceptions::{PyBaseException, PyTypeError},
     ffi,
@@ -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);
+    // To be consistent with 3.12 logic, go via raise_lazy, but also then normalize
-    let (mut ptype, mut pvalue) = if unsafe { ffi::PyExceptionClass_Check(ptype.as_ptr()) } == 0 {
+    // the resulting exception
-        (
+    raise_lazy(py, lazy);
-            PyTypeError::type_object_raw(py).cast(),
+    let mut ptype = std::ptr::null_mut();
-            PyString::new(py, "exceptions must derive from BaseException").into_ptr(),
+    let mut pvalue = std::ptr::null_mut();
-        )
-    } else {
-        (ptype.into_ptr(), pvalue.into_ptr())
-    };
     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)
@@ -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 {