diff --git a/CHANGELOG.md b/CHANGELOG.md index df6b97e3..07453a53 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - `#[pyclass(subclass)]` is now required for subclassing from Rust (was previously just required for subclassing from Python). [#1152](https://github.com/PyO3/pyo3/pull/1152) - Change `PyIterator` to be consistent with other native types: it is now used as `&PyIterator` instead of `PyIterator<'a>`. [#1176](https://github.com/PyO3/pyo3/pull/1176) - Change formatting of `PyDowncastError` messages to be closer to Python's builtin error messages. [#1212](https://github.com/PyO3/pyo3/pull/1212) +- Change `Debug` and `Display` impls for `PyException` to be consistent with `PyAny`. [#1275](https://github.com/PyO3/pyo3/pull/1275) +- Change `Debug` impl of `PyErr` to output more helpful information (acquiring the GIL if necessary). [#1275](https://github.com/PyO3/pyo3/pull/1275) ### Removed - Remove deprecated ffi definitions `PyUnicode_AsUnicodeCopy`, `PyUnicode_GetMax`, `_Py_CheckRecursionLimit`, `PyObject_AsCharBuffer`, `PyObject_AsReadBuffer`, `PyObject_CheckReadBuffer` and `PyObject_AsWriteBuffer`, which will be removed in Python 3.10. [#1217](https://github.com/PyO3/pyo3/pull/1217) diff --git a/src/err/mod.rs b/src/err/mod.rs index c1a384cf..bfbc1f9a 100644 --- a/src/err/mod.rs +++ b/src/err/mod.rs @@ -176,7 +176,8 @@ impl PyErr { /// use pyo3::{Python, PyErr, exceptions::PyTypeError, types::PyType}; /// Python::with_gil(|py| { /// let err = PyTypeError::new_err(("some type error",)); - /// assert_eq!(err.pvalue(py).to_string(), "TypeError: some type error"); + /// assert!(err.is_instance::(py)); + /// assert_eq!(err.pvalue(py).to_string(), "some type error"); /// }); /// ``` pub fn pvalue<'py>(&'py self, py: Python<'py>) -> &'py PyBaseException { @@ -447,13 +448,28 @@ impl PyErr { impl std::fmt::Debug for PyErr { fn fmt(&self, f: &mut std::fmt::Formatter) -> Result<(), std::fmt::Error> { - f.write_str(format!("PyErr {{ type: {:?} }}", self.ptype_ptr()).as_str()) + Python::with_gil(|py| { + f.debug_struct("PyErr") + .field("type", self.ptype(py)) + .field("value", self.pvalue(py)) + .field("traceback", &self.ptraceback(py)) + .finish() + }) } } impl std::fmt::Display for PyErr { - fn fmt(&self, f: &mut std::fmt::Formatter) -> Result<(), std::fmt::Error> { - Python::with_gil(|py| self.instance(py).fmt(f)) + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + Python::with_gil(|py| { + let instance = self.instance(py); + let type_name = instance.get_type().name().map_err(|_| std::fmt::Error)?; + write!(f, "{}", type_name)?; + if let Ok(s) = instance.str() { + write!(f, ": {}", &s.to_string_lossy()) + } else { + write!(f, ": ") + } + }) } } @@ -559,6 +575,55 @@ mod tests { let _ = PyErr::fetch(py); } + #[test] + fn err_debug() { + // Debug representation should be like the following (without the newlines): + // PyErr { + // type: , + // value: Exception('banana'), + // traceback: Some("); + #[cfg(not(Py_3_7))] // Python 3.6 and below formats the repr differently + assert_eq!(fields.next().unwrap(), ("value: Exception('banana',)")); + #[cfg(Py_3_7)] + assert_eq!(fields.next().unwrap(), "value: Exception('banana')"); + + let traceback = fields.next().unwrap(); + assert!(traceback.starts_with("traceback: Some()")); + + assert!(fields.next().is_none()); + } + + #[test] + fn err_display() { + let gil = Python::acquire_gil(); + let py = gil.python(); + let err = py + .run("raise Exception('banana')", None, None) + .expect_err("raising should have given us an error"); + assert_eq!(err.to_string(), "Exception: banana"); + } + #[test] fn test_pyerr_send_sync() { fn is_send() {} diff --git a/src/exceptions.rs b/src/exceptions.rs index 161501f3..9bbb2e63 100644 --- a/src/exceptions.rs +++ b/src/exceptions.rs @@ -12,6 +12,8 @@ use std::os::raw::c_char; #[macro_export] macro_rules! impl_exception_boilerplate { ($name: ident) => { + $crate::pyobject_native_type_fmt!($name); + impl std::convert::From<&$name> for $crate::PyErr { fn from(err: &$name) -> $crate::PyErr { $crate::PyErr::from_instance(err) @@ -27,27 +29,6 @@ macro_rules! impl_exception_boilerplate { } } - impl std::fmt::Debug for $name { - fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { - let type_name = self.get_type().name().map_err(|_| std::fmt::Error)?; - f.debug_struct(type_name) - // TODO: print out actual fields! - .finish() - } - } - - impl std::fmt::Display for $name { - fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { - let type_name = self.get_type().name().map_err(|_| std::fmt::Error)?; - write!(f, "{}", type_name)?; - if let Ok(s) = self.str() { - write!(f, ": {}", &s.to_string_lossy()) - } else { - write!(f, ": ") - } - } - } - impl std::error::Error for $name { fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { unsafe { @@ -493,7 +474,6 @@ mod test { use super::{PyException, PyUnicodeDecodeError}; use crate::types::{IntoPyDict, PyDict}; use crate::{PyErr, Python}; - use std::error::Error; import_exception!(socket, gaierror); import_exception!(email.errors, MessageError); @@ -580,8 +560,12 @@ mod test { let exc = py .run("raise Exception('banana')", None, None) .expect_err("raising should have given us an error") - .into_instance(py); - assert_eq!(format!("{:?}", exc.as_ref(py)), "Exception"); + .into_instance(py) + .into_ref(py); + assert_eq!( + format!("{:?}", exc), + exc.repr().unwrap().extract::().unwrap() + ); } #[test] @@ -591,12 +575,18 @@ mod test { let exc = py .run("raise Exception('banana')", None, None) .expect_err("raising should have given us an error") - .into_instance(py); - assert_eq!(exc.to_string(), "Exception: banana"); + .into_instance(py) + .into_ref(py); + assert_eq!( + exc.to_string(), + exc.str().unwrap().extract::().unwrap() + ); } #[test] fn native_exception_chain() { + use std::error::Error; + let gil = Python::acquire_gil(); let py = gil.python(); let exc = py @@ -606,10 +596,21 @@ mod test { None, ) .expect_err("raising should have given us an error") - .into_instance(py); - assert_eq!(exc.to_string(), "Exception: banana"); - let source = exc.as_ref(py).source().expect("cause should exist"); - assert_eq!(source.to_string(), "TypeError: peach"); + .into_instance(py) + .into_ref(py); + + #[cfg(Py_3_7)] + assert_eq!(format!("{:?}", exc), "Exception('banana')"); + #[cfg(not(Py_3_7))] + assert_eq!(format!("{:?}", exc), "Exception('banana',)"); + + let source = exc.source().expect("cause should exist"); + + #[cfg(Py_3_7)] + assert_eq!(format!("{:?}", source), "TypeError('peach')"); + #[cfg(not(Py_3_7))] + assert_eq!(format!("{:?}", source), "TypeError('peach',)"); + let source_source = source.source(); assert!(source_source.is_none(), "source_source should be None"); } @@ -621,8 +622,8 @@ mod test { Python::with_gil(|py| { let decode_err = PyUnicodeDecodeError::new_utf8(py, invalid_utf8, err).unwrap(); assert_eq!( - decode_err.to_string(), - "UnicodeDecodeError: \'utf-8\' codec can\'t decode byte 0xd8 in position 2: invalid utf-8" + format!("{:?}", decode_err), + "UnicodeDecodeError('utf-8', b'fo\\xd8o', 2, 3, 'invalid utf-8')" ); // Restoring should preserve the same error diff --git a/tests/test_inheritance.rs b/tests/test_inheritance.rs index da9ab298..d2d019a9 100644 --- a/tests/test_inheritance.rs +++ b/tests/test_inheritance.rs @@ -99,10 +99,7 @@ fn mutation_fails() { let e = py .run("obj.base_set(lambda: obj.sub_set_and_ret(1))", global, None) .unwrap_err(); - assert_eq!( - &e.instance(py).to_string(), - "RuntimeError: Already borrowed" - ) + assert_eq!(&e.to_string(), "RuntimeError: Already borrowed") } #[pyclass(subclass)]