From 0aa4f95a98c6c233252650de6377e7a21265be0c Mon Sep 17 00:00:00 2001 From: David Hewitt <1939362+davidhewitt@users.noreply.github.com> Date: Thu, 2 Jun 2022 11:10:59 +0100 Subject: [PATCH] frompyobject: improve error messages of derived impls --- pyo3-macros-backend/src/frompyobject.rs | 50 ++++++++++--------------- src/impl_/frompyobject.rs | 14 ++++++- tests/test_frompyobject.rs | 38 +++++++++---------- 3 files changed, 52 insertions(+), 50 deletions(-) diff --git a/pyo3-macros-backend/src/frompyobject.rs b/pyo3-macros-backend/src/frompyobject.rs index ffa4dc6d..9e5036a3 100644 --- a/pyo3-macros-backend/src/frompyobject.rs +++ b/pyo3-macros-backend/src/frompyobject.rs @@ -36,12 +36,7 @@ impl<'a> Enum<'a> { .map(|variant| { let attrs = ContainerOptions::from_attrs(&variant.attrs)?; let var_ident = &variant.ident; - Container::new( - &variant.fields, - parse_quote!(#ident::#var_ident), - attrs, - true, - ) + Container::new(&variant.fields, parse_quote!(#ident::#var_ident), attrs) }) .collect::>>()?; @@ -123,19 +118,13 @@ struct Container<'a> { path: syn::Path, ty: ContainerType<'a>, err_name: String, - is_enum_variant: bool, } impl<'a> Container<'a> { /// Construct a container based on fields, identifier and attributes. /// /// Fails if the variant has no fields or incompatible attributes. - fn new( - fields: &'a Fields, - path: syn::Path, - options: ContainerOptions, - is_enum_variant: bool, - ) -> Result { + fn new(fields: &'a Fields, path: syn::Path, options: ContainerOptions) -> Result { ensure_spanned!( !fields.is_empty(), fields.span() => "cannot derive FromPyObject for empty structs and variants" @@ -195,11 +184,21 @@ impl<'a> Container<'a> { path, ty: style, err_name, - is_enum_variant, }; Ok(v) } + fn name(&self) -> String { + let mut value = String::new(); + for segment in &self.path.segments { + if !value.is_empty() { + value.push_str("::"); + } + value.push_str(&segment.ident.to_string()); + } + value + } + /// Build derivation body for a struct. fn build(&self) -> TokenStream { match &self.ty { @@ -212,37 +211,28 @@ impl<'a> Container<'a> { fn build_newtype_struct(&self, field_ident: Option<&Ident>) -> TokenStream { let self_ty = &self.path; + let struct_name = self.name(); if let Some(ident) = field_ident { - let struct_name = quote!(#self_ty).to_string(); let field_name = ident.to_string(); quote!( ::std::result::Result::Ok(#self_ty{ #ident: _pyo3::impl_::frompyobject::extract_struct_field(obj, #struct_name, #field_name)? }) ) - } else if !self.is_enum_variant { - let error_msg = format!("failed to extract inner field of {}", quote!(#self_ty)); - quote!( - ::std::result::Result::Ok(#self_ty(obj.extract().map_err(|err| { - let py = _pyo3::PyNativeType::py(obj); - let err_msg = ::std::format!("{}: {}", - #error_msg, - err.value(py).str().unwrap()); - _pyo3::exceptions::PyTypeError::new_err(err_msg) - })?)) - ) } else { - quote!(obj.extract().map(#self_ty)) + quote!( + _pyo3::impl_::frompyobject::extract_tuple_struct_field(obj, #struct_name, 0).map(#self_ty) + ) } } fn build_tuple_struct(&self, tups: &[FieldPyO3Attributes]) -> TokenStream { let self_ty = &self.path; + let struct_name = &self.name(); let field_idents: Vec<_> = (0..tups.len()) .into_iter() .map(|i| format_ident!("arg{}", i)) .collect(); - let struct_name = "e!(#self_ty).to_string(); let fields = tups.iter().zip(&field_idents).enumerate().map(|(index, (attrs, ident))| { match &attrs.from_py_with { None => quote!( @@ -265,9 +255,9 @@ impl<'a> Container<'a> { fn build_struct(&self, tups: &[(&Ident, FieldPyO3Attributes)]) -> TokenStream { let self_ty = &self.path; + let struct_name = &self.name(); let mut fields: Punctuated = Punctuated::new(); for (ident, attrs) in tups { - let struct_name = quote!(#self_ty).to_string(); let field_name = ident.to_string(); let getter = match &attrs.getter { FieldGetter::GetAttr(Some(name)) => { @@ -523,7 +513,7 @@ pub fn build_derive_from_pyobject(tokens: &DeriveInput) -> Result { bail_spanned!(lit_str.span() => "`annotation` is unsupported for structs"); } let ident = &tokens.ident; - let st = Container::new(&st.fields, parse_quote!(#ident), options, false)?; + let st = Container::new(&st.fields, parse_quote!(#ident), options)?; st.build() } syn::Data::Union(_) => bail_spanned!( diff --git a/src/impl_/frompyobject.rs b/src/impl_/frompyobject.rs index 3d68ba55..022e4deb 100644 --- a/src/impl_/frompyobject.rs +++ b/src/impl_/frompyobject.rs @@ -8,6 +8,7 @@ pub fn failed_to_extract_enum( error_names: &[&str], errors: &[PyErr], ) -> PyErr { + // TODO maybe use ExceptionGroup on Python 3.11+ ? let mut err_msg = format!( "failed to extract enum {} ('{}')", type_name, @@ -19,12 +20,23 @@ pub fn failed_to_extract_enum( "- variant {variant_name} ({error_name}): {error_msg}", variant_name = variant_name, error_name = error_name, - error_msg = error.value(py).str().unwrap().to_str().unwrap(), + error_msg = extract_traceback(py, error.clone_ref(py)), )); } PyTypeError::new_err(err_msg) } +/// Flattens a chain of errors into a single string. +fn extract_traceback(py: Python<'_>, mut error: PyErr) -> String { + let mut error_msg = error.to_string(); + while let Some(cause) = error.cause(py) { + error_msg.push_str(", caused by "); + error_msg.push_str(&cause.to_string()); + error = cause + } + error_msg +} + pub fn extract_struct_field<'py, T>( obj: &'py PyAny, struct_name: &str, diff --git a/tests/test_frompyobject.rs b/tests/test_frompyobject.rs index d1aea8dd..454159d2 100644 --- a/tests/test_frompyobject.rs +++ b/tests/test_frompyobject.rs @@ -282,7 +282,7 @@ fn test_transparent_tuple_error_message() { assert!(tup.is_err()); assert_eq!( extract_traceback(py, tup.unwrap_err()), - "TypeError: failed to extract inner field of TransparentTuple: 'int' object \ + "TypeError: failed to extract field TransparentTuple.0: TypeError: 'int' object \ cannot be converted to 'PyString'", ); }); @@ -391,13 +391,13 @@ fn test_enum_error() { err.to_string(), "\ TypeError: failed to extract enum Foo ('TupleVar | StructVar | TransparentTuple | TransparentStructVar | StructVarGetAttrArg | StructWithGetItem | StructWithGetItemArg') -- variant TupleVar (TupleVar): 'dict' object cannot be converted to 'PyTuple' -- variant StructVar (StructVar): 'dict' object has no attribute 'test' -- variant TransparentTuple (TransparentTuple): 'dict' object cannot be interpreted as an integer -- variant TransparentStructVar (TransparentStructVar): failed to extract field Foo :: TransparentStructVar.a -- variant StructVarGetAttrArg (StructVarGetAttrArg): 'dict' object has no attribute 'bla' -- variant StructWithGetItem (StructWithGetItem): 'a' -- variant StructWithGetItemArg (StructWithGetItemArg): 'foo'" +- variant TupleVar (TupleVar): TypeError: 'dict' object cannot be converted to 'PyTuple' +- variant StructVar (StructVar): AttributeError: 'dict' object has no attribute 'test' +- variant TransparentTuple (TransparentTuple): TypeError: failed to extract field Foo::TransparentTuple.0, caused by TypeError: 'dict' object cannot be interpreted as an integer +- variant TransparentStructVar (TransparentStructVar): TypeError: failed to extract field Foo::TransparentStructVar.a, caused by TypeError: 'dict' object cannot be converted to 'PyString' +- variant StructVarGetAttrArg (StructVarGetAttrArg): AttributeError: 'dict' object has no attribute 'bla' +- variant StructWithGetItem (StructWithGetItem): KeyError: 'a' +- variant StructWithGetItemArg (StructWithGetItemArg): KeyError: 'foo'" ); let tup = PyTuple::empty(py); @@ -406,13 +406,13 @@ TypeError: failed to extract enum Foo ('TupleVar | StructVar | TransparentTuple err.to_string(), "\ TypeError: failed to extract enum Foo ('TupleVar | StructVar | TransparentTuple | TransparentStructVar | StructVarGetAttrArg | StructWithGetItem | StructWithGetItemArg') -- variant TupleVar (TupleVar): expected tuple of length 2, but got tuple of length 0 -- variant StructVar (StructVar): 'tuple' object has no attribute 'test' -- variant TransparentTuple (TransparentTuple): 'tuple' object cannot be interpreted as an integer -- variant TransparentStructVar (TransparentStructVar): failed to extract field Foo :: TransparentStructVar.a -- variant StructVarGetAttrArg (StructVarGetAttrArg): 'tuple' object has no attribute 'bla' -- variant StructWithGetItem (StructWithGetItem): tuple indices must be integers or slices, not str -- variant StructWithGetItemArg (StructWithGetItemArg): tuple indices must be integers or slices, not str" +- variant TupleVar (TupleVar): ValueError: expected tuple of length 2, but got tuple of length 0 +- variant StructVar (StructVar): AttributeError: 'tuple' object has no attribute 'test' +- variant TransparentTuple (TransparentTuple): TypeError: failed to extract field Foo::TransparentTuple.0, caused by TypeError: 'tuple' object cannot be interpreted as an integer +- variant TransparentStructVar (TransparentStructVar): TypeError: failed to extract field Foo::TransparentStructVar.a, caused by TypeError: 'tuple' object cannot be converted to 'PyString' +- variant StructVarGetAttrArg (StructVarGetAttrArg): AttributeError: 'tuple' object has no attribute 'bla' +- variant StructWithGetItem (StructWithGetItem): TypeError: tuple indices must be integers or slices, not str +- variant StructWithGetItemArg (StructWithGetItemArg): TypeError: tuple indices must be integers or slices, not str" ); }); } @@ -463,10 +463,10 @@ fn test_err_rename() { assert_eq!( f.unwrap_err().to_string(), "\ -TypeError: failed to extract enum Bar (\'str | uint | int\') -- variant A (str): \'dict\' object cannot be converted to \'PyString\' -- variant B (uint): \'dict\' object cannot be interpreted as an integer -- variant C (int): \'dict\' object cannot be interpreted as an integer" +TypeError: failed to extract enum Bar ('str | uint | int') +- variant A (str): TypeError: failed to extract field Bar::A.0, caused by TypeError: 'dict' object cannot be converted to 'PyString' +- variant B (uint): TypeError: failed to extract field Bar::B.0, caused by TypeError: 'dict' object cannot be interpreted as an integer +- variant C (int): TypeError: failed to extract field Bar::C.0, caused by TypeError: 'dict' object cannot be interpreted as an integer" ); }); }