diff --git a/src/err.rs b/src/err.rs index fde8929547a..cc3cb231ccd 100644 --- a/src/err.rs +++ b/src/err.rs @@ -344,12 +344,13 @@ impl PyErr { /// /// This method takes `mut self` because the error might need /// to be normalized in order to create the exception instance. - fn instance(mut self, py: Python) -> PyObject { + fn instance(mut self, py: Python) -> Py { self.normalize(py); - match self.pvalue { - PyErrValue::Value(ref instance) => instance.clone_ref(py), - _ => py.None(), - } + let r = match self.pvalue { + PyErrValue::Value(ref instance) => instance.clone_ref(py).extract(py), + _ => Err(PyDowncastError.into()), + }; + r.expect("Normalized error instance should be a BaseException") } /// Writes the error back to the Python interpreter's global state. @@ -429,6 +430,12 @@ impl std::fmt::Debug for PyErr { } impl FromPy for PyObject { + fn from_py(other: PyErr, py: Python) -> Self { + other.instance(py).into() + } +} + +impl FromPy for Py { fn from_py(other: PyErr, py: Python) -> Self { other.instance(py) } @@ -437,14 +444,14 @@ impl FromPy for PyObject { impl ToPyObject for PyErr { fn to_object(&self, py: Python) -> PyObject { let err = self.clone_ref(py); - err.instance(py) + err.instance(py).into() } } impl<'a> IntoPy for &'a PyErr { fn into_py(self, py: Python) -> PyObject { let err = self.clone_ref(py); - err.instance(py) + err.instance(py).into() } } diff --git a/src/exceptions.rs b/src/exceptions.rs index a277189cec9..3c01dfe1a9b 100644 --- a/src/exceptions.rs +++ b/src/exceptions.rs @@ -8,6 +8,7 @@ use crate::type_object::PyTypeObject; use crate::types::{PyAny, PyTuple}; use crate::Python; use crate::{AsPyPointer, ToPyObject}; +use crate::{AsPyRef, Py, PyDowncastError, PyTryFrom}; use std::ffi::CStr; use std::ops; use std::os::raw::c_char; @@ -206,11 +207,13 @@ macro_rules! impl_native_exception ( PyErr::new::<$name, _>(()) } } + impl std::convert::Into<$crate::PyResult> for $name { fn into(self) -> $crate::PyResult { PyErr::new::<$name, _>(()).into() } } + impl $name { pub fn py_err(args: V) -> PyErr { PyErr::new::<$name, V>(args) @@ -219,11 +222,46 @@ macro_rules! impl_native_exception ( PyErr::new::<$name, V>(args).into() } } + unsafe impl PyTypeObject for $name { fn type_object(py: $crate::Python) -> &$crate::types::PyType { unsafe { py.from_borrowed_ptr(ffi::$exc_name) } } } + + impl<'v> PyTryFrom<'v> for $name { + fn try_from>(value: V) -> Result<&'v Self, PyDowncastError> { + unsafe { + let value = value.into(); + if ffi::PyObject_TypeCheck(value.as_ptr(), ffi::$exc_name as *mut _) != 0 { + Ok(PyTryFrom::try_from_unchecked(value)) + } else { + Err(PyDowncastError) + } + } + } + + fn try_from_exact>(value: V) -> Result<&'v Self, PyDowncastError> { + unsafe { + let value = value.into(); + if (*value.as_ptr()).ob_type == ffi::$exc_name as *mut _ { + Ok(PyTryFrom::try_from_unchecked(value)) + } else { + Err(PyDowncastError) + } + } + } + + unsafe fn try_from_unchecked>(value: V) -> &'v Self { + &*(value.into().as_ptr() as *const _) + } + } + + impl AsPyPointer for $name { + fn as_ptr(&self) -> *mut ffi::PyObject { + return self as *const _ as *const _ as *mut ffi::PyObject; + } + } ); ); @@ -338,6 +376,72 @@ impl StopIteration { } } +impl std::fmt::Debug for BaseException { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + // Sneaky: we don’t really need a GIL lock here as nothing should be able to just mutate + // the "type" of an object, right? RIGHT??? + // + // let gil = Python::acquire_gil(); + // let _py = gil.python(); + let py_type_name = unsafe { CStr::from_ptr((*(*self.as_ptr()).ob_type).tp_name) }; + let type_name = py_type_name.to_string_lossy(); + f.debug_struct(&*type_name) + // TODO: print out actual fields! + .finish() + } +} + +impl std::fmt::Display for BaseException { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + let py_type_name = unsafe { CStr::from_ptr((*(*self.as_ptr()).ob_type).tp_name) }; + let type_name = py_type_name.to_string_lossy(); + write!(f, "{}", type_name)?; + let py_self: Py = unsafe { Py::from_borrowed_ptr(self.as_ptr()) }; + + let gil = Python::acquire_gil(); + let py = gil.python(); + if let Ok(s) = crate::ObjectProtocol::str(&*py_self.as_ref(py)) { + write!(f, ": {}", &s.to_string_lossy()) + } else { + write!(f, ": ") + } + } +} + +impl std::error::Error for BaseException { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + unsafe { + // Returns either `None` or an instance of an exception. + let cause_object = ffi::PyException_GetCause(self.as_ptr()); + if cause_object == ffi::Py_None() { + None + } else { + // FIXME: PyException_GetCause returns a new reference to the cause object. + // + // While we know that `self` is "immutable" (`&self`!) it is also true that between + // now and when the return value is actually read the GIL could be unlocked and + // then concurrent threads could modify `self` to change its `__cause__`. + // + // If this was not a possibility, we could just `DECREF` here, instead, now, we + // must return a `&Py` instead… but we cannot do that because + // nothing is storing such a thing anywhere and thus we cannot take a reference to + // that… + // + // The only way to make this function to work sanely, without leaking, is to ensure + // that between a call to `Error::source` and drop of the reference there’s no + // possible way for for the object to be modified. Even if we had a way to prevent + // GIL from unlocking, people could modify the object through a different + // reference to the Exception. + // + // Sounds unsound, doesn’t it? + // + // ffi::Py_DECREF(cause_object); + Some(&*(cause_object as *const _ as *const BaseException)) + } + } + } +} + /// Exceptions defined in `asyncio` module pub mod asyncio { import_exception!(asyncio, CancelledError); @@ -360,7 +464,9 @@ pub mod socket { mod test { use crate::exceptions::Exception; use crate::types::{IntoPyDict, PyDict}; - use crate::{PyErr, Python}; + use crate::{AsPyPointer, FromPy, Py, PyErr, Python}; + use std::error::Error; + use std::fmt::Write; import_exception!(socket, gaierror); import_exception!(email.errors, MessageError); @@ -438,4 +544,42 @@ mod test { ) .unwrap(); } + + #[test] + fn native_exception_display() { + let mut out = String::new(); + let gil = Python::acquire_gil(); + let py = gil.python(); + let result = py + .run("raise Exception('banana')", None, None) + .expect_err("raising should have given us an error"); + let convert = Py::::from_py(result, py); + write!(&mut out, "{}", convert).expect("successful format"); + assert_eq!(out, "Exception: banana"); + } + + #[test] + fn native_exception_chain() { + let mut out = String::new(); + let gil = Python::acquire_gil(); + let py = gil.python(); + let result = py + .run( + "raise Exception('banana') from TypeError('peach')", + None, + None, + ) + .expect_err("raising should have given us an error"); + let convert = Py::::from_py(result, py); + write!(&mut out, "{}", convert).expect("successful format"); + assert_eq!(out, "Exception: banana"); + out.clear(); + let convert_ref: &super::BaseException = + unsafe { &*(convert.as_ptr() as *const _ as *const _) }; + let source = convert_ref.source().expect("cause should exist"); + write!(&mut out, "{}", source).expect("successful format"); + assert_eq!(out, "TypeError: peach"); + let source_source = source.source(); + assert!(source_source.is_none(), "source_source should be None"); + } } diff --git a/src/types/string.rs b/src/types/string.rs index 825cf49061c..bcee2ad2ab1 100644 --- a/src/types/string.rs +++ b/src/types/string.rs @@ -185,6 +185,10 @@ mod test { fn test_to_str_surrogate() { let gil = Python::acquire_gil(); let py = gil.python(); + assert!( + !crate::PyErr::occurred(py), + "test must begin without exceptions" + ); let obj: PyObject = py.eval(r#"'\ud800'"#, None, None).unwrap().into(); let py_string = ::try_from(obj.as_ref(py)).unwrap(); assert!(py_string.to_str().is_err()); @@ -204,6 +208,10 @@ mod test { fn test_to_string_lossy() { let gil = Python::acquire_gil(); let py = gil.python(); + assert!( + !crate::PyErr::occurred(py), + "test must begin without exceptions" + ); let obj: PyObject = py .eval(r#"'🐈 Hello \ud800World'"#, None, None) .unwrap()