Add type info to conversion errors.

PyO3 · Jul 18, 2020 · 6bda2f9 · 6bda2f9
1 parent 264e885
commit 6bda2f9
Show file tree

Hide file tree

Showing 4 changed files with 60 additions and 12 deletions.
diff --git a/src/conversion.rs b/src/conversion.rs
@@ -364,7 +364,10 @@ where
             if T::is_instance(value) {
                 Ok(Self::try_from_unchecked(value))
             } else {
-                Err(PyDowncastError)
+                Err(PyDowncastError::new(
+                    Some(value.get_type().name().as_ref()),
+                    Some(T::NAME),
+                ))
             }
         }
     }
@@ -375,7 +378,10 @@ where
             if T::is_exact_instance(value) {
                 Ok(Self::try_from_unchecked(value))
             } else {
-                Err(PyDowncastError)
+                Err(PyDowncastError::new(
+                    Some(value.get_type().name().as_ref()),
+                    Some(T::NAME),
+                ))
             }
         }
     }
@@ -396,7 +402,10 @@ where
             if T::is_instance(value) {
                 Ok(Self::try_from_unchecked(value))
             } else {
-                Err(PyDowncastError)
+                Err(PyDowncastError::new(
+                    Some(value.get_type().name().as_ref()),
+                    Some(T::NAME),
+                ))
             }
         }
     }
@@ -406,7 +415,10 @@ where
             if T::is_exact_instance(value) {
                 Ok(Self::try_from_unchecked(value))
             } else {
-                Err(PyDowncastError)
+                Err(PyDowncastError::new(
+                    Some(value.get_type().name().as_ref()),
+                    Some(T::NAME),
+                ))
             }
         }
     }

diff --git a/src/err.rs b/src/err.rs
@@ -4,7 +4,7 @@ use crate::gil::ensure_gil;
 use crate::panic::PanicException;
 use crate::type_object::PyTypeObject;
 use crate::types::PyType;
-use crate::{exceptions, ffi};
+use crate::{exceptions, ffi, PyTypeInfo};
 use crate::{
     AsPyPointer, FromPy, FromPyPointer, IntoPy, IntoPyPointer, Py, PyAny, PyNativeType, PyObject,
     Python, ToBorrowedObject, ToPyObject,
@@ -56,7 +56,29 @@ pub struct PyErr {
 pub type PyResult<T> = Result<T, PyErr>;
 
 /// Marker type that indicates an error while downcasting
-pub struct PyDowncastError;
+pub struct PyDowncastError {
+    from: Option<String>,
+    to: Option<String>,
+}
+
+impl PyDowncastError {
+    pub fn new(from: Option<&str>, to: Option<&str>) -> Self {
+        PyDowncastError {
+            from: from.map(Into::into),
+            to: to.map(Into::into),
+        }
+    }
+
+    pub fn from_types<T1, T2>() -> Self
+    where
+        T1: PyTypeInfo,
+        T2: PyTypeInfo,
+    {
+        let from = Some(T1::NAME.into());
+        let to = Some(T2::NAME.into());
+        PyDowncastError { from, to }
+    }
+}
 
 /// Helper conversion trait that allows to use custom arguments for exception constructor.
 pub trait PyErrArguments {
@@ -450,14 +472,25 @@ impl<'a> IntoPy<PyObject> for &'a PyErr {
 
 /// Convert `PyDowncastError` to Python `TypeError`.
 impl std::convert::From<PyDowncastError> for PyErr {
-    fn from(_err: PyDowncastError) -> PyErr {
-        exceptions::TypeError.into()
+    fn from(err: PyDowncastError) -> PyErr {
+        match (err.from, err.to) {
+            (Some(from), Some(to)) => {
+                exceptions::TypeError::py_err(format!("Failed to convert from {} to {}", from, to))
+            }
+            (Some(from), None) => {
+                exceptions::TypeError::py_err(format!("Failed to convert from {}", from))
+            }
+            (None, Some(to)) => {
+                exceptions::TypeError::py_err(format!("Failed to convert to {}", to))
+            }
+            (None, None) => exceptions::TypeError::py_err("Failed to convert"),
+        }
     }
 }
 
 impl<'p> std::fmt::Debug for PyDowncastError {
     fn fmt(&self, f: &mut std::fmt::Formatter) -> Result<(), std::fmt::Error> {
-        f.write_str("PyDowncastError")
+        write!(f, "PyDowncastError from {:?} to {:?}", self.from, self.to)
     }
 }
 

diff --git a/src/types/iterator.rs b/src/types/iterator.rs
@@ -38,7 +38,7 @@ impl<'p> PyIterator<'p> {
             // Returns NULL if an object cannot be iterated.
             if ptr.is_null() {
                 PyErr::fetch(py);
-                return Err(PyDowncastError);
+                return Err(PyDowncastError::new(None, Some("Iterator")));
             }
 
             if ffi::PyIter_Check(ptr) != 0 {
@@ -50,7 +50,7 @@ impl<'p> PyIterator<'p> {
                 // GILPool does not take ownership of the reference.
                 Ok(PyIterator(py.from_borrowed_ptr(ptr)))
             } else {
-                Err(PyDowncastError)
+                Err(PyDowncastError::new(None, Some("Iterator")))
             }
         }
     }

diff --git a/src/types/sequence.rs b/src/types/sequence.rs
@@ -378,7 +378,10 @@ impl<'v> PyTryFrom<'v> for PySequence {
             if ffi::PySequence_Check(value.as_ptr()) != 0 {
                 Ok(<PySequence as PyTryFrom>::try_from_unchecked(value))
             } else {
-                Err(PyDowncastError)
+                Err(PyDowncastError::new(
+                    Some(value.get_type().name().as_ref()),
+                    Some("Sequence"),
+                ))
             }
         }
     }