From 0b44bf20efdd7c41639ab1e8e26c0824ed368c34 Mon Sep 17 00:00:00 2001 From: Harikeshav Rameshkumar Date: Fri, 17 Apr 2026 22:59:54 -0400 Subject: [PATCH 1/4] fix: use subclass check for AttributeError in getattr_opt on Python < 3.13 The `#[cfg(not(Py_3_13))]` branch of `getattr_opt` used `.is()` (type identity) to check for `AttributeError`, which missed subclasses. This replaces it with `.is_subclass_of::()` to match the behavior of `PyObject_GetOptionalAttr` on Python 3.13+. --- newsfragments/9999.fixed.md | 1 + src/types/any.rs | 42 ++++++++++++++++++++++++++++++++----- 2 files changed, 38 insertions(+), 5 deletions(-) create mode 100644 newsfragments/9999.fixed.md diff --git a/newsfragments/9999.fixed.md b/newsfragments/9999.fixed.md new file mode 100644 index 00000000000..052cec3ed7e --- /dev/null +++ b/newsfragments/9999.fixed.md @@ -0,0 +1 @@ +`getattr_opt` now correctly treats `AttributeError` subclasses as missing attributes on Python < 3.13. diff --git a/src/types/any.rs b/src/types/any.rs index 8923f429efa..d3c2878b49c 100644 --- a/src/types/any.rs +++ b/src/types/any.rs @@ -1048,12 +1048,13 @@ impl<'py> PyAnyMethods<'py> for Bound<'py, PyAny> { match any.getattr(attr_name) { Ok(bound) => Ok(Some(bound)), Err(err) => { - let err_type = err + if err .get_type(any.py()) - .is(PyType::new::(any.py())); - match err_type { - true => Ok(None), - false => Err(err), + .is_subclass_of::()? + { + Ok(None) + } else { + Err(err) } } } @@ -1789,6 +1790,37 @@ class Test: }); } + #[test] + fn test_getattr_opt_attribute_error_subclass() { + Python::attach(|py| { + let module = PyModule::from_code( + py, + cr#" +class CustomAttrError(AttributeError): + pass + +class Obj: + @property + def missing(self): + raise CustomAttrError("not here") + "#, + c"test.py", + &generate_unique_module_name("test"), + ) + .unwrap(); + + let obj = module + .getattr("Obj") + .unwrap() + .call0() + .unwrap(); + + // An AttributeError subclass should be treated as "attribute not found" + let result = obj.getattr_opt("missing").unwrap(); + assert!(result.is_none()); + }); + } + #[test] fn test_call_for_non_existing_method() { Python::attach(|py| { From 793044c8c2392c74434a112c77e7008d0071d24c Mon Sep 17 00:00:00 2001 From: Harikeshav Rameshkumar Date: Fri, 17 Apr 2026 23:04:32 -0400 Subject: [PATCH 2/4] chore: rename newsfragment to PR #5985 --- newsfragments/{9999.fixed.md => 5985.fixed.md} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename newsfragments/{9999.fixed.md => 5985.fixed.md} (100%) diff --git a/newsfragments/9999.fixed.md b/newsfragments/5985.fixed.md similarity index 100% rename from newsfragments/9999.fixed.md rename to newsfragments/5985.fixed.md From e6b0a8cffade23069d7d37be48957f5a24b549a2 Mon Sep 17 00:00:00 2001 From: Harikeshav Rameshkumar Date: Fri, 17 Apr 2026 23:09:35 -0400 Subject: [PATCH 3/4] fix: use infallible is_instance_of instead of fallible is_subclass_of Switch from `err.get_type().is_subclass_of::()?` to `err.is_instance_of::()` which is infallible and avoids masking the original error if the subclass check itself fails. This also matches the pattern used by `hasattr` in the same file. --- src/types/any.rs | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/types/any.rs b/src/types/any.rs index d3c2878b49c..b9d19bda084 100644 --- a/src/types/any.rs +++ b/src/types/any.rs @@ -1047,16 +1047,10 @@ impl<'py> PyAnyMethods<'py> for Bound<'py, PyAny> { { match any.getattr(attr_name) { Ok(bound) => Ok(Some(bound)), - Err(err) => { - if err - .get_type(any.py()) - .is_subclass_of::()? - { - Ok(None) - } else { - Err(err) - } + Err(err) if err.is_instance_of::(any.py()) => { + Ok(None) } + Err(err) => Err(err), } } } From fb457624e99347ab6e6c2863b37b728bccb0d5c7 Mon Sep 17 00:00:00 2001 From: Harikeshav Rameshkumar Date: Sat, 18 Apr 2026 15:53:05 -0400 Subject: [PATCH 4/4] fix: add PyObject_GetOptionalAttr compat shim and use it for all versions Add a compatibility shim for PyObject_GetOptionalAttr in pyo3-ffi/src/compat/py_3_13.rs that uses PyObject_GetAttr + PyErr_ExceptionMatches on Python < 3.13, matching the real C API behavior on 3.13+. Simplify getattr_opt to use the compat function unconditionally, removing the #[cfg] branching. --- pyo3-ffi/src/compat/py_3_13.rs | 25 ++++++++++++++++++ src/types/any.rs | 48 +++++++++++----------------------- 2 files changed, 40 insertions(+), 33 deletions(-) diff --git a/pyo3-ffi/src/compat/py_3_13.rs b/pyo3-ffi/src/compat/py_3_13.rs index 26b9f2698fb..0c766b0f86a 100644 --- a/pyo3-ffi/src/compat/py_3_13.rs +++ b/pyo3-ffi/src/compat/py_3_13.rs @@ -177,3 +177,28 @@ compat_function!( 0 } ); + +compat_function!( + originally_defined_for(Py_3_13); + + #[inline] + pub unsafe fn PyObject_GetOptionalAttr( + obj: *mut crate::PyObject, + name: *mut crate::PyObject, + result: *mut *mut crate::PyObject, + ) -> std::ffi::c_int { + use crate::{PyErr_Clear, PyErr_ExceptionMatches, PyExc_AttributeError, PyObject_GetAttr}; + + let attr = PyObject_GetAttr(obj, name); + if !attr.is_null() { + *result = attr; + return 1; // found + } + *result = std::ptr::null_mut(); + if PyErr_ExceptionMatches(PyExc_AttributeError) != 0 { + PyErr_Clear(); + return 0; // not found + } + -1 // other error + } +); diff --git a/src/types/any.rs b/src/types/any.rs index b9d19bda084..9ae17d35dbc 100644 --- a/src/types/any.rs +++ b/src/types/any.rs @@ -1024,34 +1024,20 @@ impl<'py> PyAnyMethods<'py> for Bound<'py, PyAny> { any: &Bound<'py, PyAny>, attr_name: Borrowed<'_, 'py, PyString>, ) -> PyResult>> { - #[cfg(Py_3_13)] - { - let mut resp_ptr: *mut ffi::PyObject = std::ptr::null_mut(); - match unsafe { - ffi::PyObject_GetOptionalAttr(any.as_ptr(), attr_name.as_ptr(), &mut resp_ptr) - } { - // Attribute found, result is a new strong reference - 1 => { - let bound = unsafe { Bound::from_owned_ptr(any.py(), resp_ptr) }; - Ok(Some(bound)) - } - // Attribute not found, result is NULL - 0 => Ok(None), - - // An error occurred (other than AttributeError) - _ => Err(PyErr::fetch(any.py())), - } - } - - #[cfg(not(Py_3_13))] - { - match any.getattr(attr_name) { - Ok(bound) => Ok(Some(bound)), - Err(err) if err.is_instance_of::(any.py()) => { - Ok(None) - } - Err(err) => Err(err), - } + let mut resp_ptr: *mut ffi::PyObject = std::ptr::null_mut(); + match unsafe { + ffi::compat::PyObject_GetOptionalAttr( + any.as_ptr(), + attr_name.as_ptr(), + &mut resp_ptr, + ) + } { + // Attribute found, result is a new strong reference + 1 => Ok(Some(unsafe { Bound::from_owned_ptr(any.py(), resp_ptr) })), + // Attribute not found + 0 => Ok(None), + // An error occurred (other than AttributeError) + _ => Err(PyErr::fetch(any.py())), } } @@ -1803,11 +1789,7 @@ class Obj: ) .unwrap(); - let obj = module - .getattr("Obj") - .unwrap() - .call0() - .unwrap(); + let obj = module.getattr("Obj").unwrap().call0().unwrap(); // An AttributeError subclass should be treated as "attribute not found" let result = obj.getattr_opt("missing").unwrap();