From 3cfc76ae937aab5926f7c59322096f9a75879fd5 Mon Sep 17 00:00:00 2001 From: R2D2 Date: Sat, 31 Jul 2021 23:21:07 +0200 Subject: [PATCH] Reworked struct error messages to use the of an error rather than concatenating all error messages --- pyo3-macros-backend/src/from_pyobject.rs | 34 ++++++++---------- tests/test_frompyobject.rs | 45 ++++++++++++++++++------ 2 files changed, 49 insertions(+), 30 deletions(-) diff --git a/pyo3-macros-backend/src/from_pyobject.rs b/pyo3-macros-backend/src/from_pyobject.rs index 1edb0e16..8764ffcd 100644 --- a/pyo3-macros-backend/src/from_pyobject.rs +++ b/pyo3-macros-backend/src/from_pyobject.rs @@ -205,7 +205,7 @@ impl<'a> Container<'a> { fn build_newtype_struct(&self, field_ident: Option<&Ident>) -> TokenStream { let self_ty = &self.path; if let Some(ident) = field_ident { - let err_msg = format!( + let error_msg = format!( "failed to extract field {}.{}", quote!(#self_ty), quote!(#ident) @@ -214,14 +214,13 @@ impl<'a> Container<'a> { Ok(#self_ty{#ident: obj.extract().map_err(|inner| { let gil = Python::acquire_gil(); let py = gil.python(); - let err_msg = format!("{}: {}", - #err_msg, - inner.instance(py).str().unwrap()); - pyo3::exceptions::PyTypeError::new_err(err_msg) + let new_err = pyo3::exceptions::PyTypeError::new_err(#error_msg); + new_err.set_cause(py, Some(inner)); + new_err })?}) ) } else { - let err_msg = if self.is_enum_variant { + let error_msg = if self.is_enum_variant { let variant_name = &self.path.segments.last().unwrap(); format!("- variant {} ({})", quote!(#variant_name), &self.err_name) } else { @@ -232,7 +231,7 @@ impl<'a> Container<'a> { let gil = Python::acquire_gil(); let py = gil.python(); let err_msg = format!("{}: {}", - #err_msg, + #error_msg, inner.instance(py).str().unwrap()); pyo3::exceptions::PyTypeError::new_err(err_msg) })?)) @@ -249,10 +248,9 @@ impl<'a> Container<'a> { s.get_item(#i).extract().map_err(|inner| { let gil = Python::acquire_gil(); let py = gil.python(); - let err_msg = format!("{}: {}", - #error_msg, - inner.instance(py).str().unwrap()); - pyo3::exceptions::PyTypeError::new_err(err_msg) + let new_err = pyo3::exceptions::PyTypeError::new_err(#error_msg); + new_err.set_cause(py, Some(inner)); + new_err })?)); } let msg = if self.is_enum_variant { @@ -291,19 +289,17 @@ impl<'a> Container<'a> { #get_field.extract().map_err(|inner| { let gil = Python::acquire_gil(); let py = gil.python(); - let err_msg = format!("{}: {}", - #conversion_error_msg, - inner.instance(py).str().unwrap()); - pyo3::exceptions::PyTypeError::new_err(err_msg) + let new_err = pyo3::exceptions::PyTypeError::new_err(#conversion_error_msg); + new_err.set_cause(py, Some(inner)); + new_err })?), Some(FromPyWithAttribute(expr_path)) => quote! ( #expr_path(#get_field).map_err(|inner| { let gil = Python::acquire_gil(); let py = gil.python(); - let err_msg = format!("{}: {}", - #conversion_error_msg, - inner.instance(py).str().unwrap()); - pyo3::exceptions::PyTypeError::new_err(err_msg) + let new_err = pyo3::exceptions::PyTypeError::new_err(#conversion_error_msg); + new_err.set_cause(py, Some(inner)); + new_err })? ), }; diff --git a/tests/test_frompyobject.rs b/tests/test_frompyobject.rs index b439ae7f..893c916b 100644 --- a/tests/test_frompyobject.rs +++ b/tests/test_frompyobject.rs @@ -6,6 +6,21 @@ use pyo3::PyMappingProtocol; #[macro_use] mod common; +/// Helper function that concatenates the error message from +/// each error in the traceback into a single string that can +/// be tested. +fn extract_traceback(mut error: PyErr) -> String { + let gil = Python::acquire_gil(); + let py = gil.python(); + let mut error_msg = error.to_string(); + while let Some(err) = error.cause(py) { + error_msg.push_str(": "); + error_msg.push_str(&err.to_string()); + error = err; + } + error_msg +} + #[derive(Debug, FromPyObject)] pub struct A<'a> { #[pyo3(attribute)] @@ -202,8 +217,9 @@ fn test_struct_nested_type_errors() { let test: PyResult> = FromPyObject::extract(pybaz.as_ref(py)); assert!(test.is_err()); assert_eq!( - test.unwrap_err().to_string(), - "TypeError: failed to extract field Baz.tup: failed to extract field Tuple.1: \'str\' object cannot be interpreted as an integer" + extract_traceback(test.unwrap_err()), + "TypeError: failed to extract field Baz.tup: TypeError: failed to extract field Tuple.1: \ + TypeError: \'str\' object cannot be interpreted as an integer" ); } @@ -224,8 +240,9 @@ fn test_struct_nested_type_errors_with_generics() { let test: PyResult> = FromPyObject::extract(pybaz.as_ref(py)); assert!(test.is_err()); assert_eq!( - test.unwrap_err().to_string(), - "TypeError: failed to extract field Baz.e: failed to extract field E.test: \'str\' object cannot be interpreted as an integer", + extract_traceback(test.unwrap_err()), + "TypeError: failed to extract field Baz.e: TypeError: failed to extract field E.test: \ + TypeError: \'str\' object cannot be interpreted as an integer", ); } @@ -237,8 +254,9 @@ fn test_transparent_struct_error_message() { let tup = B::extract(tup.as_ref(py)); assert!(tup.is_err()); assert_eq!( - "TypeError: failed to extract field B.test: \'int\' object cannot be converted to \'PyString\'", - tup.unwrap_err().to_string() + extract_traceback(tup.unwrap_err()), + "TypeError: failed to extract field B.test: TypeError: \'int\' object cannot be converted \ + to \'PyString\'" ); } @@ -250,8 +268,9 @@ fn test_tuple_struct_error_message() { let tup = Tuple::extract(tup.as_ref(py)); assert!(tup.is_err()); assert_eq!( - "TypeError: failed to extract field Tuple.0: \'int\' object cannot be converted to \'PyString\'", - tup.unwrap_err().to_string() + extract_traceback(tup.unwrap_err()), + "TypeError: failed to extract field Tuple.0: TypeError: \'int\' object cannot be \ + converted to \'PyString\'" ) } @@ -263,8 +282,9 @@ fn test_transparent_tuple_error_message() { let tup = TransparentTuple::extract(tup.as_ref(py)); assert!(tup.is_err()); assert_eq!( - "TypeError: failed to extract inner field of TransparentTuple: 'int' object cannot be converted to 'PyString'", - tup.unwrap_err().to_string() + extract_traceback(tup.unwrap_err()), + "TypeError: failed to extract inner field of TransparentTuple: 'int' object \ + cannot be converted to 'PyString'", ) } @@ -394,7 +414,10 @@ fn test_err_rename() { assert!(f.is_err()); assert_eq!( f.unwrap_err().to_string(), - "TypeError: failed to extract enum Bar (\'Union[str, uint, int]\')\n- variant A (str): \'dict\' object cannot be converted to \'PyString\'\n- variant B (uint): \'dict\' object cannot be interpreted as an integer\n- variant C (int): \'dict\' object cannot be interpreted as an integer\n" + "TypeError: failed to extract enum Bar (\'Union[str, uint, int]\')\n- variant A (str): \ + \'dict\' object cannot be converted to \'PyString\'\n- variant B (uint): \'dict\' object \ + cannot be interpreted as an integer\n- variant C (int): \'dict\' object cannot be \ + interpreted as an integer\n" ); }