Skip to content

Commit

Permalink
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
err: add PyErr::take
Browse files Browse the repository at this point in the history
davidhewitt committed Oct 31, 2021
1 parent 833f365 commit af37d7b
Showing 24 changed files with 124 additions and 126 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -28,10 +28,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Add `abi3-py310` feature. [#1889](https://github.com/PyO3/pyo3/pull/1889)
- Add `PyCFunction::new_closure` to create a Python function from a Rust closure. [#1901](https://github.com/PyO3/pyo3/pull/1901)
- Add support for positional-only arguments in `#[pyfunction]` [#1925](https://github.com/PyO3/pyo3/pull/1925)
- Add `PyErr::take` to attempt to fetch a Python exception if present. [#1957](https://github.com/PyO3/pyo3/pull/1957)

### Changed

- Change `PyErr::fetch` to return `Option<PyErr>`. [#1717](https://github.com/PyO3/pyo3/pull/1717)
- `PyList`, `PyTuple` and `PySequence`'s APIs now accepts only `usize` indices instead of `isize`.
[#1733](https://github.com/PyO3/pyo3/pull/1733), [#1802](https://github.com/PyO3/pyo3/pull/1802),
[#1803](https://github.com/PyO3/pyo3/pull/1803)
@@ -48,6 +48,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Remove function PyTuple_ClearFreeList from python 3.9 above. [#1887](https://github.com/PyO3/pyo3/pull/1887)
- Deprecate `#[call]` attribute in favor of using `fn __call__`. [#1929](https://github.com/PyO3/pyo3/pull/1929)
- Fix missing FFI definition `_PyImport_FindExtensionObject` on Python 3.10. [#1942](https://github.com/PyO3/pyo3/pull/1942)
- Change `PyErr::fetch` to panic in debug mode if no exception is present. [#1957](https://github.com/PyO3/pyo3/pull/1957)

### Fixed

4 changes: 2 additions & 2 deletions src/conversion.rs
Original file line number Diff line number Diff line change
@@ -494,7 +494,7 @@ pub unsafe trait FromPyPointer<'p>: Sized {
///
/// Relies on [`from_owned_ptr_or_opt`](#method.from_owned_ptr_or_opt).
unsafe fn from_owned_ptr_or_err(py: Python<'p>, ptr: *mut ffi::PyObject) -> PyResult<&'p Self> {
Self::from_owned_ptr_or_opt(py, ptr).ok_or_else(|| err::PyErr::api_call_failed(py))
Self::from_owned_ptr_or_opt(py, ptr).ok_or_else(|| err::PyErr::fetch(py))
}
/// Convert from an arbitrary borrowed `PyObject`.
///
@@ -528,7 +528,7 @@ pub unsafe trait FromPyPointer<'p>: Sized {
py: Python<'p>,
ptr: *mut ffi::PyObject,
) -> PyResult<&'p Self> {
Self::from_borrowed_ptr_or_opt(py, ptr).ok_or_else(|| err::PyErr::api_call_failed(py))
Self::from_borrowed_ptr_or_opt(py, ptr).ok_or_else(|| err::PyErr::fetch(py))
}
}

12 changes: 4 additions & 8 deletions src/conversions/num_bigint.rs
Original file line number Diff line number Diff line change
@@ -108,19 +108,15 @@ macro_rules! bigint_conversion {
fn extract(ob: &'source PyAny) -> PyResult<$rust_ty> {
let py = ob.py();
unsafe {
let num = ffi::PyNumber_Index(ob.as_ptr());
if num.is_null() {
return Err(PyErr::api_call_failed(py));
}
let n_bits = ffi::_PyLong_NumBits(num);
let n_bytes = if n_bits < 0 {
return Err(PyErr::api_call_failed(py));
let num: Py<PyLong> = Py::from_owned_ptr_or_err(py, ffi::PyNumber_Index(ob.as_ptr()))?;
let n_bits = ffi::_PyLong_NumBits(num.as_ptr());
let n_bytes = if n_bits == -1 {
return Err(PyErr::fetch(py));
} else if n_bits == 0 {
0
} else {
(n_bits as usize - 1 + $is_signed) / 8 + 1
};
let num: Py<PyLong> = Py::from_owned_ptr(py, num);
if n_bytes <= 128 {
let mut buffer = [0; 128];
extract(num.as_ref(py), &mut buffer[..n_bytes], $is_signed)?;
15 changes: 9 additions & 6 deletions src/conversions/num_complex.rs
Original file line number Diff line number Diff line change
@@ -143,19 +143,22 @@ macro_rules! complex_conversion {
#[cfg(not(any(Py_LIMITED_API, PyPy)))]
unsafe {
let val = ffi::PyComplex_AsCComplex(obj.as_ptr());
if val.real == -1.0 && PyErr::occurred(obj.py()) {
Err(PyErr::api_call_failed(obj.py()))
} else {
Ok(Complex::new(val.real as $float, val.imag as $float))
if val.real == -1.0 {
if let Some(err) = PyErr::take(obj.py()) {
return Err(err);
}
}
Ok(Complex::new(val.real as $float, val.imag as $float))
}

#[cfg(any(Py_LIMITED_API, PyPy))]
unsafe {
let ptr = obj.as_ptr();
let real = ffi::PyComplex_RealAsDouble(ptr);
if real == -1.0 && PyErr::occurred(obj.py()) {
return Err(PyErr::api_call_failed(obj.py()));
if real == -1.0 {
if let Some(err) = PyErr::take(obj.py()) {
return Err(err);
}
}
let imag = ffi::PyComplex_ImagAsDouble(ptr);
Ok(Complex::new(real as $float, imag as $float))
2 changes: 1 addition & 1 deletion src/conversions/osstr.rs
Original file line number Diff line number Diff line change
@@ -94,7 +94,7 @@ impl FromPyObject<'_> for OsString {
let size =
unsafe { ffi::PyUnicode_AsWideChar(pystring.as_ptr(), std::ptr::null_mut(), 0) };
if size == -1 {
return Err(PyErr::api_call_failed(ob.py()));
return Err(PyErr::fetch(ob.py()));
}

let mut buffer = vec![0; size as usize];
2 changes: 1 addition & 1 deletion src/err/err_state.rs
Original file line number Diff line number Diff line change
@@ -23,7 +23,7 @@ pub(crate) enum PyErrState {
pvalue: Box<dyn FnOnce(Python) -> PyObject + Send + Sync>,
},
FfiTuple {
ptype: Option<PyObject>,
ptype: PyObject,
pvalue: Option<PyObject>,
ptraceback: Option<PyObject>,
},
146 changes: 72 additions & 74 deletions src/err/mod.rs
Original file line number Diff line number Diff line change
@@ -7,9 +7,7 @@ use crate::{
exceptions::{self, PyBaseException},
ffi,
};
use crate::{
AsPyPointer, FromPyPointer, IntoPy, Py, PyAny, PyObject, Python, ToBorrowedObject, ToPyObject,
};
use crate::{AsPyPointer, IntoPy, Py, PyAny, PyObject, Python, ToBorrowedObject, ToPyObject};
use std::borrow::Cow;
use std::cell::UnsafeCell;
use std::ffi::CString;
@@ -137,15 +135,13 @@ impl PyErr {

let state = if unsafe { ffi::PyExceptionInstance_Check(ptr) } != 0 {
PyErrState::Normalized(PyErrStateNormalized {
ptype: unsafe {
Py::from_borrowed_ptr(obj.py(), ffi::PyExceptionInstance_Class(ptr))
},
ptype: obj.get_type().into(),
pvalue: unsafe { Py::from_borrowed_ptr(obj.py(), obj.as_ptr()) },
ptraceback: None,
})
} else if unsafe { ffi::PyExceptionClass_Check(obj.as_ptr()) } != 0 {
PyErrState::FfiTuple {
ptype: unsafe { Some(Py::from_borrowed_ptr(obj.py(), ptr)) },
ptype: obj.into(),
pvalue: None,
ptraceback: None,
}
@@ -218,61 +214,86 @@ impl PyErr {
unsafe { !ffi::PyErr_Occurred().is_null() }
}

/// Retrieves the current error from the Python interpreter's global state.
///
/// The error is cleared from the Python interpreter.
/// If no error is set, returns a `None`.
/// Takes the current error from the Python interpreter's global state and clears the global
/// state. If no error is set, returns `None`.
///
/// If the error fetched is a `PanicException` (which would have originated from a panic in a
/// pyo3 callback) then this function will resume the panic.
pub fn fetch(py: Python) -> Option<PyErr> {
unsafe {
/// If the error is a `PanicException` (which would have originated from a panic in a pyo3
/// callback) then this function will resume the panic.
pub fn take(py: Python) -> Option<PyErr> {
let (ptype, pvalue, ptraceback) = unsafe {
let mut ptype: *mut ffi::PyObject = std::ptr::null_mut();
let mut pvalue: *mut ffi::PyObject = std::ptr::null_mut();
let mut ptraceback: *mut ffi::PyObject = std::ptr::null_mut();
ffi::PyErr_Fetch(&mut ptype, &mut pvalue, &mut ptraceback);

// If the error indicator is not set, all three variables are set to NULL
if ptype.is_null() && pvalue.is_null() && ptraceback.is_null() {
return None;
}

let err = PyErr::new_from_ffi_tuple(py, ptype, pvalue, ptraceback);
// Convert to Py immediately so that any references are freed by early return.
let ptype = Py::from_owned_ptr_or_opt(py, ptype);
let pvalue = Py::from_owned_ptr_or_opt(py, pvalue);
let ptraceback = Py::from_owned_ptr_or_opt(py, ptraceback);

// A valid exception state should always have a non-null ptype, but the other two may be
// null.
let ptype = match ptype {
Some(ptype) => ptype,
None => {
debug_assert!(
pvalue.is_none(),
"Exception type was null but value was not null"
);
debug_assert!(
ptraceback.is_none(),
"Exception type was null but traceback was not null"
);
return None;
}
};

(ptype, pvalue, ptraceback)
};

if ptype == PanicException::type_object(py).as_ptr() {
let msg: String = PyAny::from_borrowed_ptr_or_opt(py, pvalue)
.and_then(|obj| obj.extract().ok())
.unwrap_or_else(|| String::from("Unwrapped panic from Python code"));
if ptype.as_ptr() == PanicException::type_object(py).as_ptr() {
let msg: String = pvalue
.as_ref()
.and_then(|obj| obj.extract(py).ok())
.unwrap_or_else(|| String::from("Unwrapped panic from Python code"));

eprintln!(
"--- PyO3 is resuming a panic after fetching a PanicException from Python. ---"
);
eprintln!("Python stack trace below:");
err.print(py);
eprintln!(
"--- PyO3 is resuming a panic after fetching a PanicException from Python. ---"
);
eprintln!("Python stack trace below:");

std::panic::resume_unwind(Box::new(msg))
unsafe {
use crate::conversion::IntoPyPointer;
ffi::PyErr_Restore(ptype.into_ptr(), pvalue.into_ptr(), ptraceback.into_ptr());
ffi::PyErr_PrintEx(0);
}

Some(err)
std::panic::resume_unwind(Box::new(msg))
}

Some(PyErr::from_state(PyErrState::FfiTuple {
ptype,
pvalue,
ptraceback,
}))
}

/// Retrieves the current error from the Python interpreter's global state.
/// Equivalent to [PyErr::take], but when no error is set:
/// - Panics in debug mode.
/// - Returns a `SystemError` in release mode.
///
/// The error is cleared from the Python interpreter.
/// If no error is set, returns a `SystemError` in release mode,
/// panics in debug mode.
pub(crate) fn api_call_failed(py: Python) -> PyErr {
#[cfg(debug_assertions)]
{
PyErr::fetch(py).expect("error return without exception set")
}
#[cfg(not(debug_assertions))]
{
use crate::exceptions::PySystemError;

PyErr::fetch(py)
.unwrap_or_else(|| PySystemError::new_err("error return without exception set"))
/// This behavior is consistent with Python's internal handling of what happens when a C return
/// value indicates an error occurred but the global error state is empty.
#[cfg_attr(all(debug_assertions, track_caller), track_caller)]
#[inline]
pub fn fetch(py: Python) -> PyErr {
const FAILED_TO_FETCH: &str = "attempted to fetch exception but none was set";
match PyErr::take(py) {
Some(err) => err,
#[cfg(debug_assertions)]
None => panic!("{}", FAILED_TO_FETCH),
#[cfg(not(debug_assertions))]
None => exceptions::PySystemError::new_err(FAILED_TO_FETCH),
}
}

@@ -309,27 +330,6 @@ impl PyErr {
}
}

/// Create a PyErr from an ffi tuple
///
/// # Safety
/// - `ptype` must be a pointer to valid Python exception type object.
/// - `pvalue` must be a pointer to a valid Python object, or NULL.
/// - `ptraceback` must be a pointer to a valid Python traceback object, or NULL.
unsafe fn new_from_ffi_tuple(
py: Python,
ptype: *mut ffi::PyObject,
pvalue: *mut ffi::PyObject,
ptraceback: *mut ffi::PyObject,
) -> PyErr {
// Note: must not panic to ensure all owned pointers get acquired correctly,
// and because we mustn't panic in normalize().
PyErr::from_state(PyErrState::FfiTuple {
ptype: Py::from_owned_ptr_or_opt(py, ptype),
pvalue: Py::from_owned_ptr_or_opt(py, pvalue),
ptraceback: Py::from_owned_ptr_or_opt(py, ptraceback),
})
}

/// Prints a standard traceback to `sys.stderr`.
pub fn print(&self, py: Python) {
self.clone_ref(py).restore(py);
@@ -579,7 +579,7 @@ pub fn error_on_minusone(py: Python, result: c_int) -> PyResult<()> {
if result != -1 {
Ok(())
} else {
Err(PyErr::api_call_failed(py))
Err(PyErr::fetch(py))
}
}

@@ -596,9 +596,7 @@ mod tests {

#[test]
fn no_error() {
Python::with_gil(|py| {
assert!(PyErr::fetch(py).is_none());
});
assert!(Python::with_gil(PyErr::take).is_none());
}

#[test]
@@ -608,7 +606,7 @@ mod tests {
assert!(err.is_instance::<exceptions::PyValueError>(py));
err.restore(py);
assert!(PyErr::occurred(py));
let err = PyErr::fetch(py).unwrap();
let err = PyErr::fetch(py);
assert!(err.is_instance::<exceptions::PyValueError>(py));
assert_eq!(err.to_string(), "ValueError: some exception message");
})
@@ -620,7 +618,7 @@ mod tests {
let err: PyErr = PyErr::new::<crate::types::PyString, _>(());
assert!(err.is_instance::<exceptions::PyTypeError>(py));
err.restore(py);
let err = PyErr::fetch(py).unwrap();
let err = PyErr::fetch(py);
assert!(err.is_instance::<exceptions::PyTypeError>(py));
assert_eq!(
err.to_string(),
2 changes: 1 addition & 1 deletion src/exceptions.rs
Original file line number Diff line number Diff line change
@@ -829,7 +829,7 @@ mod tests {
e.restore(py);

assert_eq!(
PyErr::api_call_failed(py).to_string(),
PyErr::fetch(py).to_string(),
"UnicodeDecodeError: \'utf-8\' codec can\'t decode byte 0xd8 in position 2: invalid utf-8"
);
});
6 changes: 3 additions & 3 deletions src/instance.rs
Original file line number Diff line number Diff line change
@@ -589,7 +589,7 @@ impl<T> Py<T> {
let kwargs = kwargs.into_ptr();
let ptr = ffi::PyObject_GetAttr(self.as_ptr(), name);
if ptr.is_null() {
return Err(PyErr::api_call_failed(py));
return Err(PyErr::fetch(py));
}
let result = PyObject::from_owned_ptr_or_err(py, ffi::PyObject_Call(ptr, args, kwargs));
ffi::Py_DECREF(ptr);
@@ -656,7 +656,7 @@ impl<T> Py<T> {
pub unsafe fn from_owned_ptr_or_err(py: Python, ptr: *mut ffi::PyObject) -> PyResult<Py<T>> {
match NonNull::new(ptr) {
Some(nonnull_ptr) => Ok(Py(nonnull_ptr, PhantomData)),
None => Err(PyErr::api_call_failed(py)),
None => Err(PyErr::fetch(py)),
}
}

@@ -694,7 +694,7 @@ impl<T> Py<T> {
/// `ptr` must be a pointer to a Python object of type T.
#[inline]
pub unsafe fn from_borrowed_ptr_or_err(py: Python, ptr: *mut ffi::PyObject) -> PyResult<Self> {
Self::from_borrowed_ptr_or_opt(py, ptr).ok_or_else(|| PyErr::api_call_failed(py))
Self::from_borrowed_ptr_or_opt(py, ptr).ok_or_else(|| PyErr::fetch(py))
}

/// Create a `Py<T>` instance by creating a new reference from the given FFI pointer.
2 changes: 1 addition & 1 deletion src/pyclass.rs
Original file line number Diff line number Diff line change
@@ -122,7 +122,7 @@ where

let type_object = unsafe { ffi::PyType_FromSpec(&mut spec) };
if type_object.is_null() {
Err(PyErr::api_call_failed(py))
Err(PyErr::fetch(py))
} else {
tp_init_additional::<T>(type_object as _);
Ok(type_object as _)
4 changes: 2 additions & 2 deletions src/pyclass_init.rs
Original file line number Diff line number Diff line change
@@ -45,7 +45,7 @@ impl<T: PyTypeInfo> PyObjectInit<T> for PyNativeTypeInitializer<T> {
let alloc = get_tp_alloc(subtype).unwrap_or(ffi::PyType_GenericAlloc);
let obj = alloc(subtype, 0);
return if obj.is_null() {
Err(PyErr::api_call_failed(py))
Err(PyErr::fetch(py))
} else {
Ok(obj)
};
@@ -61,7 +61,7 @@ impl<T: PyTypeInfo> PyObjectInit<T> for PyNativeTypeInitializer<T> {
Some(newfunc) => {
let obj = newfunc(subtype, std::ptr::null_mut(), std::ptr::null_mut());
if obj.is_null() {
Err(PyErr::api_call_failed(py))
Err(PyErr::fetch(py))
} else {
Ok(obj)
}
Loading

0 comments on commit af37d7b

Please sign in to comment.