From 90bb1adeb28b82be96546e0c2cde81530241f7af Mon Sep 17 00:00:00 2001 From: Gino Valente <49806985+MrGVSV@users.noreply.github.com> Date: Mon, 9 Sep 2024 10:52:40 -0700 Subject: [PATCH] bevy_reflect: Contextual serialization error messages (#13888) # Objective Reflection serialization can be difficult to debug. A lot of times a type fails to be serialized and the user is left wondering where that type came from. This is most often encountered with Bevy's scenes. Attempting to serialize all resources in the world will fail because some resources can't be serialized. For example, users will often get complaints about `bevy_utils::Instant` not registering `ReflectSerialize`. Well, `Instant` can't be serialized, so the only other option is to exclude the resource that contains it. But what resource contains it? This is where reflection serialization can get a little tricky (it's `Time` btw). ## Solution Add the `debug_stack` feature to `bevy_reflect`. When enabled, the reflection serializers and deserializers will keep track of the current type stack. And this stack will be used in error messages to help with debugging. Now, if we unknowingly try to serialize `Time`, we'll get the following error: ``` type `bevy_utils::Instant` did not register the `ReflectSerialize` type data. For certain types, this may need to be registered manually using `register_type_data` (stack: `bevy_time::time::Time` -> `bevy_time::real::Real` -> `bevy_utils::Instant`) ``` ### Implementation This makes use of `thread_local!` to manage an internal `TypeInfoStack` which holds a stack of `&'static TypeInfo`. We push to the stack before a type is (de)serialized and pop from the stack afterwards. Using a thread-local should be fine since we know two (de)serializers can't be running at the same time (and if they're running on separate threads, then we're still good). The only potential issue would be if a user went through one of the sub-serializers, like `StructSerializer`. However, I don't think many users are going through these types (I don't even know if we necessarily want to keep those public either, but we'll save that for a different PR). Additionally, this is just a debug feature that only affects error messages, so it wouldn't have any drastically negative effect. It would just result in the stack not being cleared properly if there were any errors. Lastly, this is not the most performant implementation since we now fetch the `TypeInfo` an extra time. But I figured that for a debug tool, it wouldn't matter too much. ### Feature This also adds a `debug` feature, which enables the `debug_stack` feature. I added it because I think we may want to potentially add more debug tools in the future, and this gives us a good framework for adding those. Users who want all debug features, present and future, can just set `debug`. If they only want this feature, then they can just use `debug_stack`. I also made the `debug` feature default to help capture the widest audience (i.e. the users who want this feature but don't know they do). However, if we think it's better as a non-default feature, I can change it! And if there's any bikeshedding around the name `debug_stack`, let me know! ## Testing Run the following command: ``` cargo test --package bevy_reflect --features debug_stack ``` --- ## Changelog - Added the `debug` and `debug_stack` features to `bevy_reflect` - Updated the error messages returned by the reflection serializers and deserializers to include more contextual information when the `debug_stack` or `debug` feature is enabled --- crates/bevy_reflect/Cargo.toml | 6 +- crates/bevy_reflect/src/lib.rs | 16 +- crates/bevy_reflect/src/serde/de/arrays.rs | 7 +- .../bevy_reflect/src/serde/de/deserializer.rs | 196 ++++++++++-------- crates/bevy_reflect/src/serde/de/enums.rs | 12 +- .../bevy_reflect/src/serde/de/error_utils.rs | 26 +++ crates/bevy_reflect/src/serde/de/lists.rs | 7 +- crates/bevy_reflect/src/serde/de/maps.rs | 4 +- crates/bevy_reflect/src/serde/de/mod.rs | 51 ++++- crates/bevy_reflect/src/serde/de/options.rs | 5 +- .../src/serde/de/registration_utils.rs | 7 +- .../src/serde/de/registrations.rs | 3 +- crates/bevy_reflect/src/serde/de/sets.rs | 2 +- .../bevy_reflect/src/serde/de/struct_utils.rs | 26 ++- .../bevy_reflect/src/serde/de/tuple_utils.rs | 15 +- crates/bevy_reflect/src/serde/mod.rs | 2 +- crates/bevy_reflect/src/serde/ser/arrays.rs | 2 +- crates/bevy_reflect/src/serde/ser/enums.rs | 22 +- .../bevy_reflect/src/serde/ser/error_utils.rs | 26 +++ crates/bevy_reflect/src/serde/ser/lists.rs | 2 +- crates/bevy_reflect/src/serde/ser/maps.rs | 4 +- crates/bevy_reflect/src/serde/ser/mod.rs | 68 +++++- .../src/serde/ser/serializable.rs | 17 +- .../bevy_reflect/src/serde/ser/serializer.rs | 49 ++++- crates/bevy_reflect/src/serde/ser/sets.rs | 2 +- crates/bevy_reflect/src/serde/ser/structs.rs | 14 +- .../src/serde/ser/tuple_structs.rs | 11 +- crates/bevy_reflect/src/serde/ser/tuples.rs | 2 +- crates/bevy_reflect/src/type_info_stack.rs | 49 +++++ 29 files changed, 482 insertions(+), 171 deletions(-) create mode 100644 crates/bevy_reflect/src/serde/de/error_utils.rs create mode 100644 crates/bevy_reflect/src/serde/ser/error_utils.rs create mode 100644 crates/bevy_reflect/src/type_info_stack.rs diff --git a/crates/bevy_reflect/Cargo.toml b/crates/bevy_reflect/Cargo.toml index 65ded3f8deb6b..c86a92e852603 100644 --- a/crates/bevy_reflect/Cargo.toml +++ b/crates/bevy_reflect/Cargo.toml @@ -10,13 +10,17 @@ keywords = ["bevy"] rust-version = "1.76.0" [features] -default = ["smallvec"] +default = ["smallvec", "debug"] # When enabled, provides Bevy-related reflection implementations bevy = ["smallvec", "smol_str"] glam = ["dep:glam"] petgraph = ["dep:petgraph"] smallvec = ["dep:smallvec"] uuid = ["dep:uuid"] +# Enables features useful for debugging reflection +debug = ["debug_stack"] +# When enabled, keeps track of the current serialization/deserialization context for better error messages +debug_stack = [] # When enabled, allows documentation comments to be accessed via reflection documentation = ["bevy_reflect_derive/documentation"] # Enables function reflection diff --git a/crates/bevy_reflect/src/lib.rs b/crates/bevy_reflect/src/lib.rs index b226af4432402..dc820caaaa03e 100644 --- a/crates/bevy_reflect/src/lib.rs +++ b/crates/bevy_reflect/src/lib.rs @@ -473,7 +473,7 @@ //! //! | Default | Dependencies | //! | :-----: | :---------------------------------------: | -//! | ❌ | [`bevy_math`], [`glam`], [`smallvec`] | +//! | ❌ | [`bevy_math`], [`glam`], [`smallvec`] | //! //! This feature makes it so that the appropriate reflection traits are implemented on all the types //! necessary for the [Bevy] game engine. @@ -493,6 +493,18 @@ //! This can be useful for generating documentation for scripting language interop or //! for displaying tooltips in an editor. //! +//! ## `debug` +//! +//! | Default | Dependencies | +//! | :-----: | :-------------------------------------------: | +//! | ✅ | `debug_stack` | +//! +//! This feature enables useful debug features for reflection. +//! +//! This includes the `debug_stack` feature, +//! which enables capturing the type stack when serializing or deserializing a type +//! and displaying it in error messages. +//! //! [Reflection]: https://en.wikipedia.org/wiki/Reflective_programming //! [Bevy]: https://bevyengine.org/ //! [limitations]: #limitations @@ -562,6 +574,8 @@ pub mod attributes; mod enums; pub mod serde; pub mod std_traits; +#[cfg(feature = "debug_stack")] +mod type_info_stack; pub mod utility; /// The reflect prelude. diff --git a/crates/bevy_reflect/src/serde/de/arrays.rs b/crates/bevy_reflect/src/serde/de/arrays.rs index 6e2f331a658c2..2ffd2419a9217 100644 --- a/crates/bevy_reflect/src/serde/de/arrays.rs +++ b/crates/bevy_reflect/src/serde/de/arrays.rs @@ -35,9 +35,10 @@ impl<'a, 'de> Visitor<'de> for ArrayVisitor<'a> { { let mut vec = Vec::with_capacity(seq.size_hint().unwrap_or_default()); let registration = try_get_registration(self.array_info.item_ty(), self.registry)?; - while let Some(value) = - seq.next_element_seed(TypedReflectDeserializer::new(registration, self.registry))? - { + while let Some(value) = seq.next_element_seed(TypedReflectDeserializer::new_internal( + registration, + self.registry, + ))? { vec.push(value); } diff --git a/crates/bevy_reflect/src/serde/de/deserializer.rs b/crates/bevy_reflect/src/serde/de/deserializer.rs index a4496dc72d723..7af7aa09970e5 100644 --- a/crates/bevy_reflect/src/serde/de/deserializer.rs +++ b/crates/bevy_reflect/src/serde/de/deserializer.rs @@ -1,5 +1,8 @@ use crate::serde::de::arrays::ArrayVisitor; use crate::serde::de::enums::EnumVisitor; +use crate::serde::de::error_utils::make_custom_error; +#[cfg(feature = "debug_stack")] +use crate::serde::de::error_utils::TYPE_INFO_STACK; use crate::serde::de::lists::ListVisitor; use crate::serde::de::maps::MapVisitor; use crate::serde::de::options::OptionVisitor; @@ -231,6 +234,20 @@ pub struct TypedReflectDeserializer<'a> { impl<'a> TypedReflectDeserializer<'a> { pub fn new(registration: &'a TypeRegistration, registry: &'a TypeRegistry) -> Self { + #[cfg(feature = "debug_stack")] + TYPE_INFO_STACK.set(crate::type_info_stack::TypeInfoStack::new()); + + Self { + registration, + registry, + } + } + + /// An internal constructor for creating a deserializer without resetting the type info stack. + pub(super) fn new_internal( + registration: &'a TypeRegistration, + registry: &'a TypeRegistry, + ) -> Self { Self { registration, registry, @@ -245,89 +262,106 @@ impl<'a, 'de> DeserializeSeed<'de> for TypedReflectDeserializer<'a> { where D: serde::Deserializer<'de>, { - let type_path = self.registration.type_info().type_path(); - - // Handle both Value case and types that have a custom `ReflectDeserialize` - if let Some(deserialize_reflect) = self.registration.data::() { - let value = deserialize_reflect.deserialize(deserializer)?; - return Ok(value.into_partial_reflect()); - } + let deserialize_internal = || -> Result { + let type_path = self.registration.type_info().type_path(); - match self.registration.type_info() { - TypeInfo::Struct(struct_info) => { - let mut dynamic_struct = deserializer.deserialize_struct( - struct_info.type_path_table().ident().unwrap(), - struct_info.field_names(), - StructVisitor::new(struct_info, self.registration, self.registry), - )?; - dynamic_struct.set_represented_type(Some(self.registration.type_info())); - Ok(Box::new(dynamic_struct)) - } - TypeInfo::TupleStruct(tuple_struct_info) => { - let mut dynamic_tuple_struct = deserializer.deserialize_tuple_struct( - tuple_struct_info.type_path_table().ident().unwrap(), - tuple_struct_info.field_len(), - TupleStructVisitor::new(tuple_struct_info, self.registration, self.registry), - )?; - dynamic_tuple_struct.set_represented_type(Some(self.registration.type_info())); - Ok(Box::new(dynamic_tuple_struct)) - } - TypeInfo::List(list_info) => { - let mut dynamic_list = - deserializer.deserialize_seq(ListVisitor::new(list_info, self.registry))?; - dynamic_list.set_represented_type(Some(self.registration.type_info())); - Ok(Box::new(dynamic_list)) - } - TypeInfo::Array(array_info) => { - let mut dynamic_array = deserializer.deserialize_tuple( - array_info.capacity(), - ArrayVisitor::new(array_info, self.registry), - )?; - dynamic_array.set_represented_type(Some(self.registration.type_info())); - Ok(Box::new(dynamic_array)) - } - TypeInfo::Map(map_info) => { - let mut dynamic_map = - deserializer.deserialize_map(MapVisitor::new(map_info, self.registry))?; - dynamic_map.set_represented_type(Some(self.registration.type_info())); - Ok(Box::new(dynamic_map)) - } - TypeInfo::Set(set_info) => { - let mut dynamic_set = - deserializer.deserialize_seq(SetVisitor::new(set_info, self.registry))?; - dynamic_set.set_represented_type(Some(self.registration.type_info())); - Ok(Box::new(dynamic_set)) - } - TypeInfo::Tuple(tuple_info) => { - let mut dynamic_tuple = deserializer.deserialize_tuple( - tuple_info.field_len(), - TupleVisitor::new(tuple_info, self.registration, self.registry), - )?; - dynamic_tuple.set_represented_type(Some(self.registration.type_info())); - Ok(Box::new(dynamic_tuple)) - } - TypeInfo::Enum(enum_info) => { - let mut dynamic_enum = if enum_info.type_path_table().module_path() - == Some("core::option") - && enum_info.type_path_table().ident() == Some("Option") - { - deserializer.deserialize_option(OptionVisitor::new(enum_info, self.registry))? - } else { - deserializer.deserialize_enum( - enum_info.type_path_table().ident().unwrap(), - enum_info.variant_names(), - EnumVisitor::new(enum_info, self.registration, self.registry), - )? - }; - dynamic_enum.set_represented_type(Some(self.registration.type_info())); - Ok(Box::new(dynamic_enum)) + // Handle both Value case and types that have a custom `ReflectDeserialize` + if let Some(deserialize_reflect) = self.registration.data::() { + let value = deserialize_reflect.deserialize(deserializer)?; + return Ok(value.into_partial_reflect()); } - TypeInfo::Value(_) => { - // This case should already be handled - Err(Error::custom(format_args!( - "Type `{type_path}` did not register the `ReflectDeserialize` type data. For certain types, this may need to be registered manually using `register_type_data`", - ))) + + match self.registration.type_info() { + TypeInfo::Struct(struct_info) => { + let mut dynamic_struct = deserializer.deserialize_struct( + struct_info.type_path_table().ident().unwrap(), + struct_info.field_names(), + StructVisitor::new(struct_info, self.registration, self.registry), + )?; + dynamic_struct.set_represented_type(Some(self.registration.type_info())); + Ok(Box::new(dynamic_struct)) + } + TypeInfo::TupleStruct(tuple_struct_info) => { + let mut dynamic_tuple_struct = deserializer.deserialize_tuple_struct( + tuple_struct_info.type_path_table().ident().unwrap(), + tuple_struct_info.field_len(), + TupleStructVisitor::new( + tuple_struct_info, + self.registration, + self.registry, + ), + )?; + dynamic_tuple_struct.set_represented_type(Some(self.registration.type_info())); + Ok(Box::new(dynamic_tuple_struct)) + } + TypeInfo::List(list_info) => { + let mut dynamic_list = + deserializer.deserialize_seq(ListVisitor::new(list_info, self.registry))?; + dynamic_list.set_represented_type(Some(self.registration.type_info())); + Ok(Box::new(dynamic_list)) + } + TypeInfo::Array(array_info) => { + let mut dynamic_array = deserializer.deserialize_tuple( + array_info.capacity(), + ArrayVisitor::new(array_info, self.registry), + )?; + dynamic_array.set_represented_type(Some(self.registration.type_info())); + Ok(Box::new(dynamic_array)) + } + TypeInfo::Map(map_info) => { + let mut dynamic_map = + deserializer.deserialize_map(MapVisitor::new(map_info, self.registry))?; + dynamic_map.set_represented_type(Some(self.registration.type_info())); + Ok(Box::new(dynamic_map)) + } + TypeInfo::Set(set_info) => { + let mut dynamic_set = + deserializer.deserialize_seq(SetVisitor::new(set_info, self.registry))?; + dynamic_set.set_represented_type(Some(self.registration.type_info())); + Ok(Box::new(dynamic_set)) + } + TypeInfo::Tuple(tuple_info) => { + let mut dynamic_tuple = deserializer.deserialize_tuple( + tuple_info.field_len(), + TupleVisitor::new(tuple_info, self.registration, self.registry), + )?; + dynamic_tuple.set_represented_type(Some(self.registration.type_info())); + Ok(Box::new(dynamic_tuple)) + } + TypeInfo::Enum(enum_info) => { + let mut dynamic_enum = if enum_info.type_path_table().module_path() + == Some("core::option") + && enum_info.type_path_table().ident() == Some("Option") + { + deserializer + .deserialize_option(OptionVisitor::new(enum_info, self.registry))? + } else { + deserializer.deserialize_enum( + enum_info.type_path_table().ident().unwrap(), + enum_info.variant_names(), + EnumVisitor::new(enum_info, self.registration, self.registry), + )? + }; + dynamic_enum.set_represented_type(Some(self.registration.type_info())); + Ok(Box::new(dynamic_enum)) + } + TypeInfo::Value(_) => { + // This case should already be handled + Err(make_custom_error(format_args!( + "type `{type_path}` did not register the `ReflectDeserialize` type data. For certain types, this may need to be registered manually using `register_type_data`", + ))) + } } - } + }; + + #[cfg(feature = "debug_stack")] + TYPE_INFO_STACK.with_borrow_mut(|stack| stack.push(self.registration.type_info())); + + let output = deserialize_internal(); + + #[cfg(feature = "debug_stack")] + TYPE_INFO_STACK.with_borrow_mut(crate::type_info_stack::TypeInfoStack::pop); + + output } } diff --git a/crates/bevy_reflect/src/serde/de/enums.rs b/crates/bevy_reflect/src/serde/de/enums.rs index 9350ebce6fe1a..8f4a094e831e0 100644 --- a/crates/bevy_reflect/src/serde/de/enums.rs +++ b/crates/bevy_reflect/src/serde/de/enums.rs @@ -1,3 +1,4 @@ +use crate::serde::de::error_utils::make_custom_error; use crate::serde::de::helpers::ExpectedValues; use crate::serde::de::registration_utils::try_get_registration; use crate::serde::de::struct_utils::{visit_struct, visit_struct_seq}; @@ -67,10 +68,9 @@ impl<'a, 'de> Visitor<'de> for EnumVisitor<'a> { *TupleLikeInfo::field_at(tuple_info, 0)?.ty(), self.registry, )?; - let value = variant.newtype_variant_seed(TypedReflectDeserializer::new( - registration, - self.registry, - ))?; + let value = variant.newtype_variant_seed( + TypedReflectDeserializer::new_internal(registration, self.registry), + )?; let mut dynamic_tuple = DynamicTuple::default(); dynamic_tuple.insert_boxed(value); dynamic_tuple.into() @@ -121,7 +121,7 @@ impl<'de> DeserializeSeed<'de> for VariantDeserializer { E: Error, { self.0.variant_at(variant_index as usize).ok_or_else(|| { - Error::custom(format_args!( + make_custom_error(format_args!( "no variant found at index `{}` on enum `{}`", variant_index, self.0.type_path() @@ -135,7 +135,7 @@ impl<'de> DeserializeSeed<'de> for VariantDeserializer { { self.0.variant(variant_name).ok_or_else(|| { let names = self.0.iter().map(VariantInfo::name); - Error::custom(format_args!( + make_custom_error(format_args!( "unknown variant `{}`, expected one of {:?}", variant_name, ExpectedValues::from_iter(names) diff --git a/crates/bevy_reflect/src/serde/de/error_utils.rs b/crates/bevy_reflect/src/serde/de/error_utils.rs new file mode 100644 index 0000000000000..6a97c2518cbf7 --- /dev/null +++ b/crates/bevy_reflect/src/serde/de/error_utils.rs @@ -0,0 +1,26 @@ +use core::fmt::Display; +use serde::de::Error; + +#[cfg(feature = "debug_stack")] +thread_local! { + /// The thread-local [`TypeInfoStack`] used for debugging. + /// + /// [`TypeInfoStack`]: crate::type_info_stack::TypeInfoStack + pub(super) static TYPE_INFO_STACK: std::cell::RefCell = const { std::cell::RefCell::new( + crate::type_info_stack::TypeInfoStack::new() + ) }; +} + +/// A helper function for generating a custom deserialization error message. +/// +/// This function should be preferred over [`Error::custom`] as it will include +/// other useful information, such as the [type info stack]. +/// +/// [type info stack]: crate::type_info_stack::TypeInfoStack +pub(super) fn make_custom_error(msg: impl Display) -> E { + #[cfg(feature = "debug_stack")] + return TYPE_INFO_STACK + .with_borrow(|stack| E::custom(format_args!("{} (stack: {:?})", msg, stack))); + #[cfg(not(feature = "debug_stack"))] + return E::custom(msg); +} diff --git a/crates/bevy_reflect/src/serde/de/lists.rs b/crates/bevy_reflect/src/serde/de/lists.rs index 30e7d9282d426..d0387f8a0a854 100644 --- a/crates/bevy_reflect/src/serde/de/lists.rs +++ b/crates/bevy_reflect/src/serde/de/lists.rs @@ -35,9 +35,10 @@ impl<'a, 'de> Visitor<'de> for ListVisitor<'a> { { let mut list = DynamicList::default(); let registration = try_get_registration(self.list_info.item_ty(), self.registry)?; - while let Some(value) = - seq.next_element_seed(TypedReflectDeserializer::new(registration, self.registry))? - { + while let Some(value) = seq.next_element_seed(TypedReflectDeserializer::new_internal( + registration, + self.registry, + ))? { list.push_box(value); } Ok(list) diff --git a/crates/bevy_reflect/src/serde/de/maps.rs b/crates/bevy_reflect/src/serde/de/maps.rs index 0b185110d673e..1bda2e6837224 100644 --- a/crates/bevy_reflect/src/serde/de/maps.rs +++ b/crates/bevy_reflect/src/serde/de/maps.rs @@ -33,11 +33,11 @@ impl<'a, 'de> Visitor<'de> for MapVisitor<'a> { let mut dynamic_map = DynamicMap::default(); let key_registration = try_get_registration(self.map_info.key_ty(), self.registry)?; let value_registration = try_get_registration(self.map_info.value_ty(), self.registry)?; - while let Some(key) = map.next_key_seed(TypedReflectDeserializer::new( + while let Some(key) = map.next_key_seed(TypedReflectDeserializer::new_internal( key_registration, self.registry, ))? { - let value = map.next_value_seed(TypedReflectDeserializer::new( + let value = map.next_value_seed(TypedReflectDeserializer::new_internal( value_registration, self.registry, ))?; diff --git a/crates/bevy_reflect/src/serde/de/mod.rs b/crates/bevy_reflect/src/serde/de/mod.rs index 40bf92777482e..bc92d3ccf8e8e 100644 --- a/crates/bevy_reflect/src/serde/de/mod.rs +++ b/crates/bevy_reflect/src/serde/de/mod.rs @@ -4,6 +4,7 @@ pub use registrations::*; mod arrays; mod deserializer; mod enums; +mod error_utils; mod helpers; mod lists; mod maps; @@ -275,7 +276,7 @@ mod tests { let mut registry = get_registry(); registry.register::(); let registration = registry.get(TypeId::of::()).unwrap(); - let reflect_deserializer = TypedReflectDeserializer::new(registration, ®istry); + let reflect_deserializer = TypedReflectDeserializer::new_internal(registration, ®istry); let mut ron_deserializer = ron::de::Deserializer::from_str(input).unwrap(); let dynamic_output = reflect_deserializer .deserialize(&mut ron_deserializer) @@ -510,6 +511,52 @@ mod tests { let error = reflect_deserializer .deserialize(&mut deserializer) .unwrap_err(); - assert_eq!(error, ron::Error::Message("Type `core::ops::RangeInclusive` did not register the `ReflectDeserialize` type data. For certain types, this may need to be registered manually using `register_type_data`".to_string())); + #[cfg(feature = "debug_stack")] + assert_eq!(error, ron::Error::Message("type `core::ops::RangeInclusive` did not register the `ReflectDeserialize` type data. For certain types, this may need to be registered manually using `register_type_data` (stack: `core::ops::RangeInclusive`)".to_string())); + #[cfg(not(feature = "debug_stack"))] + assert_eq!(error, ron::Error::Message("type `core::ops::RangeInclusive` did not register the `ReflectDeserialize` type data. For certain types, this may need to be registered manually using `register_type_data`".to_string())); + } + + #[cfg(feature = "debug_stack")] + mod debug_stack { + use super::*; + + #[test] + fn should_report_context_in_errors() { + #[derive(Reflect)] + struct Foo { + bar: Bar, + } + + #[derive(Reflect)] + struct Bar { + some_other_field: Option, + baz: Baz, + } + + #[derive(Reflect)] + struct Baz { + value: Vec>, + } + + let mut registry = TypeRegistry::new(); + registry.register::(); + registry.register::(); + registry.register::(); + registry.register::>(); + + let input = r#"{"bevy_reflect::serde::de::tests::debug_stack::Foo":(bar:(some_other_field:Some(123),baz:(value:[(start:0.0,end:1.0)])))}"#; + let mut deserializer = ron::de::Deserializer::from_str(input).unwrap(); + let reflect_deserializer = ReflectDeserializer::new(®istry); + let error = reflect_deserializer + .deserialize(&mut deserializer) + .unwrap_err(); + assert_eq!( + error, + ron::Error::Message( + "type `core::ops::RangeInclusive` did not register the `ReflectDeserialize` type data. For certain types, this may need to be registered manually using `register_type_data` (stack: `bevy_reflect::serde::de::tests::debug_stack::Foo` -> `bevy_reflect::serde::de::tests::debug_stack::Bar` -> `bevy_reflect::serde::de::tests::debug_stack::Baz` -> `alloc::vec::Vec>` -> `core::ops::RangeInclusive`)".to_string() + ) + ); + } } } diff --git a/crates/bevy_reflect/src/serde/de/options.rs b/crates/bevy_reflect/src/serde/de/options.rs index 103121a4b1264..f4a5467c63d0c 100644 --- a/crates/bevy_reflect/src/serde/de/options.rs +++ b/crates/bevy_reflect/src/serde/de/options.rs @@ -1,3 +1,4 @@ +use crate::serde::de::error_utils::make_custom_error; use crate::serde::de::registration_utils::try_get_registration; use crate::serde::TypedReflectDeserializer; use crate::{DynamicEnum, DynamicTuple, EnumInfo, TypeRegistry, VariantInfo}; @@ -46,14 +47,14 @@ impl<'a, 'de> Visitor<'de> for OptionVisitor<'a> { VariantInfo::Tuple(tuple_info) if tuple_info.field_len() == 1 => { let field = tuple_info.field_at(0).unwrap(); let registration = try_get_registration(*field.ty(), self.registry)?; - let de = TypedReflectDeserializer::new(registration, self.registry); + let de = TypedReflectDeserializer::new_internal(registration, self.registry); let mut value = DynamicTuple::default(); value.insert_boxed(de.deserialize(deserializer)?); let mut option = DynamicEnum::default(); option.set_variant("Some", value); Ok(option) } - info => Err(Error::custom(format_args!( + info => Err(make_custom_error(format_args!( "invalid variant, expected `Some` but got `{}`", info.name() ))), diff --git a/crates/bevy_reflect/src/serde/de/registration_utils.rs b/crates/bevy_reflect/src/serde/de/registration_utils.rs index 8a170d173248e..2c88a73939d15 100644 --- a/crates/bevy_reflect/src/serde/de/registration_utils.rs +++ b/crates/bevy_reflect/src/serde/de/registration_utils.rs @@ -1,3 +1,4 @@ +use crate::serde::de::error_utils::make_custom_error; use crate::{Type, TypeRegistration, TypeRegistry}; use serde::de::Error; @@ -8,8 +9,8 @@ pub(super) fn try_get_registration( ty: Type, registry: &TypeRegistry, ) -> Result<&TypeRegistration, E> { - let registration = registry - .get(ty.id()) - .ok_or_else(|| Error::custom(format_args!("no registration found for type `{ty:?}`")))?; + let registration = registry.get(ty.id()).ok_or_else(|| { + make_custom_error(format_args!("no registration found for type `{ty:?}`")) + })?; Ok(registration) } diff --git a/crates/bevy_reflect/src/serde/de/registrations.rs b/crates/bevy_reflect/src/serde/de/registrations.rs index 9ed10628f7beb..8b34c348c0fad 100644 --- a/crates/bevy_reflect/src/serde/de/registrations.rs +++ b/crates/bevy_reflect/src/serde/de/registrations.rs @@ -1,3 +1,4 @@ +use crate::serde::de::error_utils::make_custom_error; use crate::{TypeRegistration, TypeRegistry}; use core::fmt::Formatter; use serde::de::{DeserializeSeed, Error, Visitor}; @@ -42,7 +43,7 @@ impl<'a, 'de> DeserializeSeed<'de> for TypeRegistrationDeserializer<'a> { E: Error, { self.0.get_with_type_path(type_path).ok_or_else(|| { - Error::custom(format_args!("No registration found for `{type_path}`")) + make_custom_error(format_args!("no registration found for `{type_path}`")) }) } } diff --git a/crates/bevy_reflect/src/serde/de/sets.rs b/crates/bevy_reflect/src/serde/de/sets.rs index fcb72a8f776a2..2127f84b48310 100644 --- a/crates/bevy_reflect/src/serde/de/sets.rs +++ b/crates/bevy_reflect/src/serde/de/sets.rs @@ -32,7 +32,7 @@ impl<'a, 'de> Visitor<'de> for SetVisitor<'a> { { let mut dynamic_set = DynamicSet::default(); let value_registration = try_get_registration(self.set_info.value_ty(), self.registry)?; - while let Some(value) = set.next_element_seed(TypedReflectDeserializer::new( + while let Some(value) = set.next_element_seed(TypedReflectDeserializer::new_internal( value_registration, self.registry, ))? { diff --git a/crates/bevy_reflect/src/serde/de/struct_utils.rs b/crates/bevy_reflect/src/serde/de/struct_utils.rs index 5035746a6055e..836d36c0bdba5 100644 --- a/crates/bevy_reflect/src/serde/de/struct_utils.rs +++ b/crates/bevy_reflect/src/serde/de/struct_utils.rs @@ -1,3 +1,4 @@ +use crate::serde::de::error_utils::make_custom_error; use crate::serde::de::helpers::{ExpectedValues, Ident}; use crate::serde::de::registration_utils::try_get_registration; use crate::serde::{SerializationData, TypedReflectDeserializer}; @@ -18,8 +19,8 @@ pub(super) trait StructLikeInfo { impl StructLikeInfo for StructInfo { fn field(&self, name: &str) -> Result<&NamedField, E> { Self::field(self, name).ok_or_else(|| { - Error::custom(format_args!( - "no field named {} on struct {}", + make_custom_error(format_args!( + "no field named `{}` on struct `{}`", name, self.type_path(), )) @@ -28,8 +29,8 @@ impl StructLikeInfo for StructInfo { fn field_at(&self, index: usize) -> Result<&NamedField, E> { Self::field_at(self, index).ok_or_else(|| { - Error::custom(format_args!( - "no field at index {} on struct {}", + make_custom_error(format_args!( + "no field at index `{}` on struct `{}`", index, self.type_path(), )) @@ -48,8 +49,8 @@ impl StructLikeInfo for StructInfo { impl StructLikeInfo for StructVariantInfo { fn field(&self, name: &str) -> Result<&NamedField, E> { Self::field(self, name).ok_or_else(|| { - Error::custom(format_args!( - "no field named {} on variant {}", + make_custom_error(format_args!( + "no field named `{}` on variant `{}`", name, self.name(), )) @@ -58,8 +59,8 @@ impl StructLikeInfo for StructVariantInfo { fn field_at(&self, index: usize) -> Result<&NamedField, E> { Self::field_at(self, index).ok_or_else(|| { - Error::custom(format_args!( - "no field at index {} on variant {}", + make_custom_error(format_args!( + "no field at index `{}` on variant `{}`", index, self.name(), )) @@ -92,14 +93,17 @@ where while let Some(Ident(key)) = map.next_key::()? { let field = info.field::(&key).map_err(|_| { let fields = info.iter_fields().map(NamedField::name); - Error::custom(format_args!( + make_custom_error(format_args!( "unknown field `{}`, expected one of {:?}", key, ExpectedValues::from_iter(fields) )) })?; let registration = try_get_registration(*field.ty(), registry)?; - let value = map.next_value_seed(TypedReflectDeserializer::new(registration, registry))?; + let value = map.next_value_seed(TypedReflectDeserializer::new_internal( + registration, + registry, + ))?; dynamic_struct.insert_boxed(&key, value); } @@ -156,7 +160,7 @@ where } let value = seq - .next_element_seed(TypedReflectDeserializer::new( + .next_element_seed(TypedReflectDeserializer::new_internal( try_get_registration(*info.field_at(index)?.ty(), registry)?, registry, ))? diff --git a/crates/bevy_reflect/src/serde/de/tuple_utils.rs b/crates/bevy_reflect/src/serde/de/tuple_utils.rs index 542b096bf5586..7dd8ad448185d 100644 --- a/crates/bevy_reflect/src/serde/de/tuple_utils.rs +++ b/crates/bevy_reflect/src/serde/de/tuple_utils.rs @@ -1,3 +1,4 @@ +use crate::serde::de::error_utils::make_custom_error; use crate::serde::de::registration_utils::try_get_registration; use crate::serde::{SerializationData, TypedReflectDeserializer}; use crate::{ @@ -18,8 +19,8 @@ impl TupleLikeInfo for TupleInfo { fn field_at(&self, index: usize) -> Result<&UnnamedField, E> { Self::field_at(self, index).ok_or_else(|| { - Error::custom(format_args!( - "no field at index {} on tuple {}", + make_custom_error(format_args!( + "no field at index `{}` on tuple `{}`", index, self.type_path(), )) @@ -34,8 +35,8 @@ impl TupleLikeInfo for TupleStructInfo { fn field_at(&self, index: usize) -> Result<&UnnamedField, E> { Self::field_at(self, index).ok_or_else(|| { - Error::custom(format_args!( - "no field at index {} on tuple struct {}", + make_custom_error(format_args!( + "no field at index `{}` on tuple struct `{}`", index, self.type_path(), )) @@ -50,8 +51,8 @@ impl TupleLikeInfo for TupleVariantInfo { fn field_at(&self, index: usize) -> Result<&UnnamedField, E> { Self::field_at(self, index).ok_or_else(|| { - Error::custom(format_args!( - "no field at index {} on tuple variant {}", + make_custom_error(format_args!( + "no field at index `{}` on tuple variant `{}`", index, self.name(), )) @@ -90,7 +91,7 @@ where } let value = seq - .next_element_seed(TypedReflectDeserializer::new( + .next_element_seed(TypedReflectDeserializer::new_internal( try_get_registration(*info.field_at(index)?.ty(), registry)?, registry, ))? diff --git a/crates/bevy_reflect/src/serde/mod.rs b/crates/bevy_reflect/src/serde/mod.rs index a962c36bd04bd..bc3137d0b2b55 100644 --- a/crates/bevy_reflect/src/serde/mod.rs +++ b/crates/bevy_reflect/src/serde/mod.rs @@ -142,7 +142,7 @@ mod tests { #[test] #[should_panic( - expected = "cannot serialize dynamic value without represented type: bevy_reflect::DynamicStruct" + expected = "cannot serialize dynamic value without represented type: `bevy_reflect::DynamicStruct`" )] fn should_not_serialize_unproxied_dynamic() { let registry = TypeRegistry::default(); diff --git a/crates/bevy_reflect/src/serde/ser/arrays.rs b/crates/bevy_reflect/src/serde/ser/arrays.rs index eb84919dfd714..b318047ee2887 100644 --- a/crates/bevy_reflect/src/serde/ser/arrays.rs +++ b/crates/bevy_reflect/src/serde/ser/arrays.rs @@ -22,7 +22,7 @@ impl<'a> Serialize for ArraySerializer<'a> { { let mut state = serializer.serialize_tuple(self.array.len())?; for value in self.array.iter() { - state.serialize_element(&TypedReflectSerializer::new(value, self.registry))?; + state.serialize_element(&TypedReflectSerializer::new_internal(value, self.registry))?; } state.end() } diff --git a/crates/bevy_reflect/src/serde/ser/enums.rs b/crates/bevy_reflect/src/serde/ser/enums.rs index da151cc894fce..6951617cb6243 100644 --- a/crates/bevy_reflect/src/serde/ser/enums.rs +++ b/crates/bevy_reflect/src/serde/ser/enums.rs @@ -1,6 +1,7 @@ +use crate::serde::ser::error_utils::make_custom_error; use crate::serde::TypedReflectSerializer; use crate::{Enum, TypeInfo, TypeRegistry, VariantInfo, VariantType}; -use serde::ser::{Error, SerializeStructVariant, SerializeTupleVariant}; +use serde::ser::{SerializeStructVariant, SerializeTupleVariant}; use serde::Serialize; /// A serializer for [`Enum`] values. @@ -24,8 +25,8 @@ impl<'a> Serialize for EnumSerializer<'a> { S: serde::Serializer, { let type_info = self.enum_value.get_represented_type_info().ok_or_else(|| { - Error::custom(format_args!( - "cannot get type info for {}", + make_custom_error(format_args!( + "cannot get type info for `{}`", self.enum_value.reflect_type_path() )) })?; @@ -33,7 +34,7 @@ impl<'a> Serialize for EnumSerializer<'a> { let enum_info = match type_info { TypeInfo::Enum(enum_info) => enum_info, info => { - return Err(Error::custom(format_args!( + return Err(make_custom_error(format_args!( "expected enum type but received {info:?}" ))); } @@ -44,7 +45,7 @@ impl<'a> Serialize for EnumSerializer<'a> { let variant_info = enum_info .variant_at(variant_index as usize) .ok_or_else(|| { - Error::custom(format_args!( + make_custom_error(format_args!( "variant at index `{variant_index}` does not exist", )) })?; @@ -66,7 +67,7 @@ impl<'a> Serialize for EnumSerializer<'a> { let struct_info = match variant_info { VariantInfo::Struct(struct_info) => struct_info, info => { - return Err(Error::custom(format_args!( + return Err(make_custom_error(format_args!( "expected struct variant type but received {info:?}", ))); } @@ -82,7 +83,7 @@ impl<'a> Serialize for EnumSerializer<'a> { let field_info = struct_info.field_at(index).unwrap(); state.serialize_field( field_info.name(), - &TypedReflectSerializer::new(field.value(), self.registry), + &TypedReflectSerializer::new_internal(field.value(), self.registry), )?; } state.end() @@ -93,13 +94,14 @@ impl<'a> Serialize for EnumSerializer<'a> { if type_info.type_path_table().module_path() == Some("core::option") && type_info.type_path_table().ident() == Some("Option") { - serializer.serialize_some(&TypedReflectSerializer::new(field, self.registry)) + serializer + .serialize_some(&TypedReflectSerializer::new_internal(field, self.registry)) } else { serializer.serialize_newtype_variant( enum_name, variant_index, variant_name, - &TypedReflectSerializer::new(field, self.registry), + &TypedReflectSerializer::new_internal(field, self.registry), ) } } @@ -111,7 +113,7 @@ impl<'a> Serialize for EnumSerializer<'a> { field_len, )?; for field in self.enum_value.iter_fields() { - state.serialize_field(&TypedReflectSerializer::new( + state.serialize_field(&TypedReflectSerializer::new_internal( field.value(), self.registry, ))?; diff --git a/crates/bevy_reflect/src/serde/ser/error_utils.rs b/crates/bevy_reflect/src/serde/ser/error_utils.rs new file mode 100644 index 0000000000000..58a029f986588 --- /dev/null +++ b/crates/bevy_reflect/src/serde/ser/error_utils.rs @@ -0,0 +1,26 @@ +use core::fmt::Display; +use serde::ser::Error; + +#[cfg(feature = "debug_stack")] +thread_local! { + /// The thread-local [`TypeInfoStack`] used for debugging. + /// + /// [`TypeInfoStack`]: crate::type_info_stack::TypeInfoStack + pub(super) static TYPE_INFO_STACK: std::cell::RefCell = const { std::cell::RefCell::new( + crate::type_info_stack::TypeInfoStack::new() + ) }; +} + +/// A helper function for generating a custom serialization error message. +/// +/// This function should be preferred over [`Error::custom`] as it will include +/// other useful information, such as the [type info stack]. +/// +/// [type info stack]: crate::type_info_stack::TypeInfoStack +pub(super) fn make_custom_error(msg: impl Display) -> E { + #[cfg(feature = "debug_stack")] + return TYPE_INFO_STACK + .with_borrow(|stack| E::custom(format_args!("{} (stack: {:?})", msg, stack))); + #[cfg(not(feature = "debug_stack"))] + return E::custom(msg); +} diff --git a/crates/bevy_reflect/src/serde/ser/lists.rs b/crates/bevy_reflect/src/serde/ser/lists.rs index 6e6b98958bb2c..ad3bb1a040a47 100644 --- a/crates/bevy_reflect/src/serde/ser/lists.rs +++ b/crates/bevy_reflect/src/serde/ser/lists.rs @@ -22,7 +22,7 @@ impl<'a> Serialize for ListSerializer<'a> { { let mut state = serializer.serialize_seq(Some(self.list.len()))?; for value in self.list.iter() { - state.serialize_element(&TypedReflectSerializer::new(value, self.registry))?; + state.serialize_element(&TypedReflectSerializer::new_internal(value, self.registry))?; } state.end() } diff --git a/crates/bevy_reflect/src/serde/ser/maps.rs b/crates/bevy_reflect/src/serde/ser/maps.rs index 589cfae3d163b..d5493cd35e523 100644 --- a/crates/bevy_reflect/src/serde/ser/maps.rs +++ b/crates/bevy_reflect/src/serde/ser/maps.rs @@ -23,8 +23,8 @@ impl<'a> Serialize for MapSerializer<'a> { let mut state = serializer.serialize_map(Some(self.map.len()))?; for (key, value) in self.map.iter() { state.serialize_entry( - &TypedReflectSerializer::new(key, self.registry), - &TypedReflectSerializer::new(value, self.registry), + &TypedReflectSerializer::new_internal(key, self.registry), + &TypedReflectSerializer::new_internal(value, self.registry), )?; } state.end() diff --git a/crates/bevy_reflect/src/serde/ser/mod.rs b/crates/bevy_reflect/src/serde/ser/mod.rs index 85876fabf2e89..ee2a283e82ecb 100644 --- a/crates/bevy_reflect/src/serde/ser/mod.rs +++ b/crates/bevy_reflect/src/serde/ser/mod.rs @@ -3,6 +3,7 @@ pub use serializer::*; mod arrays; mod enums; +mod error_utils; mod lists; mod maps; mod serializable; @@ -429,11 +430,20 @@ mod tests { let serializer = ReflectSerializer::new(&value, ®istry); let error = ron::ser::to_string(&serializer).unwrap_err(); + #[cfg(feature = "debug_stack")] assert_eq!( error, ron::Error::Message( - "Type `core::ops::RangeInclusive` is not registered in the type registry" - .to_string() + "type `core::ops::RangeInclusive` is not registered in the type registry (stack: `core::ops::RangeInclusive`)" + .to_string(), + ) + ); + #[cfg(not(feature = "debug_stack"))] + assert_eq!( + error, + ron::Error::Message( + "type `core::ops::RangeInclusive` is not registered in the type registry" + .to_string(), ) ); } @@ -446,11 +456,63 @@ mod tests { let serializer = ReflectSerializer::new(&value, ®istry); let error = ron::ser::to_string(&serializer).unwrap_err(); + #[cfg(feature = "debug_stack")] assert_eq!( error, ron::Error::Message( - "Type `core::ops::RangeInclusive` did not register the `ReflectSerialize` type data. For certain types, this may need to be registered manually using `register_type_data`".to_string() + "type `core::ops::RangeInclusive` did not register the `ReflectSerialize` type data. For certain types, this may need to be registered manually using `register_type_data` (stack: `core::ops::RangeInclusive`)".to_string() ) ); + #[cfg(not(feature = "debug_stack"))] + assert_eq!( + error, + ron::Error::Message( + "type `core::ops::RangeInclusive` did not register the `ReflectSerialize` type data. For certain types, this may need to be registered manually using `register_type_data`".to_string() + ) + ); + } + + #[cfg(feature = "debug_stack")] + mod debug_stack { + use super::*; + + #[test] + fn should_report_context_in_errors() { + #[derive(Reflect)] + struct Foo { + bar: Bar, + } + + #[derive(Reflect)] + struct Bar { + some_other_field: Option, + baz: Baz, + } + + #[derive(Reflect)] + struct Baz { + value: Vec>, + } + + let value = Foo { + bar: Bar { + some_other_field: Some(123), + baz: Baz { + value: vec![0.0..=1.0], + }, + }, + }; + + let registry = TypeRegistry::new(); + let serializer = ReflectSerializer::new(&value, ®istry); + + let error = ron::ser::to_string(&serializer).unwrap_err(); + assert_eq!( + error, + ron::Error::Message( + "type `core::ops::RangeInclusive` is not registered in the type registry (stack: `bevy_reflect::serde::ser::tests::debug_stack::Foo` -> `bevy_reflect::serde::ser::tests::debug_stack::Bar` -> `bevy_reflect::serde::ser::tests::debug_stack::Baz` -> `alloc::vec::Vec>` -> `core::ops::RangeInclusive`)".to_string() + ) + ); + } } } diff --git a/crates/bevy_reflect/src/serde/ser/serializable.rs b/crates/bevy_reflect/src/serde/ser/serializable.rs index 9c285816f252e..48d3b2968368d 100644 --- a/crates/bevy_reflect/src/serde/ser/serializable.rs +++ b/crates/bevy_reflect/src/serde/ser/serializable.rs @@ -1,3 +1,4 @@ +use crate::serde::ser::error_utils::make_custom_error; use crate::{PartialReflect, ReflectSerialize, TypeRegistry}; use serde::ser::Error; use std::ops::Deref; @@ -23,29 +24,29 @@ impl<'a> Serializable<'a> { type_registry: &TypeRegistry, ) -> Result, E> { let value = value.try_as_reflect().ok_or_else(|| { - Error::custom(format_args!( - "Type '{}' does not implement `Reflect`", + make_custom_error(format_args!( + "type `{}` does not implement `Reflect`", value.reflect_type_path() )) })?; let info = value.get_represented_type_info().ok_or_else(|| { - Error::custom(format_args!( - "Type '{}' does not represent any type", + make_custom_error(format_args!( + "type `{}` does not represent any type", value.reflect_type_path(), )) })?; let registration = type_registry.get(info.type_id()).ok_or_else(|| { - Error::custom(format_args!( - "Type `{}` is not registered in the type registry", + make_custom_error(format_args!( + "type `{}` is not registered in the type registry", info.type_path(), )) })?; let reflect_serialize = registration.data::().ok_or_else(|| { - Error::custom(format_args!( - "Type `{}` did not register the `ReflectSerialize` type data. For certain types, this may need to be registered manually using `register_type_data`", + make_custom_error(format_args!( + "type `{}` did not register the `ReflectSerialize` type data. For certain types, this may need to be registered manually using `register_type_data`", info.type_path(), )) })?; diff --git a/crates/bevy_reflect/src/serde/ser/serializer.rs b/crates/bevy_reflect/src/serde/ser/serializer.rs index eb3afe96f84e1..cdc00452aadee 100644 --- a/crates/bevy_reflect/src/serde/ser/serializer.rs +++ b/crates/bevy_reflect/src/serde/ser/serializer.rs @@ -1,5 +1,8 @@ use crate::serde::ser::arrays::ArraySerializer; use crate::serde::ser::enums::EnumSerializer; +use crate::serde::ser::error_utils::make_custom_error; +#[cfg(feature = "debug_stack")] +use crate::serde::ser::error_utils::TYPE_INFO_STACK; use crate::serde::ser::lists::ListSerializer; use crate::serde::ser::maps::MapSerializer; use crate::serde::ser::sets::SetSerializer; @@ -8,7 +11,7 @@ use crate::serde::ser::tuple_structs::TupleStructSerializer; use crate::serde::ser::tuples::TupleSerializer; use crate::serde::Serializable; use crate::{PartialReflect, ReflectRef, TypeRegistry}; -use serde::ser::{Error, SerializeMap}; +use serde::ser::SerializeMap; use serde::Serialize; /// A general purpose serializer for reflected types. @@ -54,7 +57,7 @@ pub struct ReflectSerializer<'a> { impl<'a> ReflectSerializer<'a> { pub fn new(value: &'a dyn PartialReflect, registry: &'a TypeRegistry) -> Self { - ReflectSerializer { value, registry } + Self { value, registry } } } @@ -69,13 +72,13 @@ impl<'a> Serialize for ReflectSerializer<'a> { .get_represented_type_info() .ok_or_else(|| { if self.value.is_dynamic() { - Error::custom(format_args!( - "cannot serialize dynamic value without represented type: {}", + make_custom_error(format_args!( + "cannot serialize dynamic value without represented type: `{}`", self.value.reflect_type_path() )) } else { - Error::custom(format_args!( - "cannot get type info for {}", + make_custom_error(format_args!( + "cannot get type info for `{}`", self.value.reflect_type_path() )) } @@ -132,7 +135,15 @@ pub struct TypedReflectSerializer<'a> { impl<'a> TypedReflectSerializer<'a> { pub fn new(value: &'a dyn PartialReflect, registry: &'a TypeRegistry) -> Self { - TypedReflectSerializer { value, registry } + #[cfg(feature = "debug_stack")] + TYPE_INFO_STACK.set(crate::type_info_stack::TypeInfoStack::new()); + + Self { value, registry } + } + + /// An internal constructor for creating a serializer without resetting the type info stack. + pub(super) fn new_internal(value: &'a dyn PartialReflect, registry: &'a TypeRegistry) -> Self { + Self { value, registry } } } @@ -141,14 +152,29 @@ impl<'a> Serialize for TypedReflectSerializer<'a> { where S: serde::Serializer, { + #[cfg(feature = "debug_stack")] + { + let info = self.value.get_represented_type_info().ok_or_else(|| { + make_custom_error(format_args!( + "type `{}` does not represent any type", + self.value.reflect_type_path(), + )) + })?; + + TYPE_INFO_STACK.with_borrow_mut(|stack| stack.push(info)); + } + // Handle both Value case and types that have a custom `Serialize` let serializable = Serializable::try_from_reflect_value::(self.value, self.registry); if let Ok(serializable) = serializable { + #[cfg(feature = "debug_stack")] + TYPE_INFO_STACK.with_borrow_mut(crate::type_info_stack::TypeInfoStack::pop); + return serializable.serialize(serializer); } - match self.value.reflect_ref() { + let output = match self.value.reflect_ref() { ReflectRef::Struct(value) => { StructSerializer::new(value, self.registry).serialize(serializer) } @@ -174,6 +200,11 @@ impl<'a> Serialize for TypedReflectSerializer<'a> { EnumSerializer::new(value, self.registry).serialize(serializer) } ReflectRef::Value(_) => Err(serializable.err().unwrap()), - } + }; + + #[cfg(feature = "debug_stack")] + TYPE_INFO_STACK.with_borrow_mut(crate::type_info_stack::TypeInfoStack::pop); + + output } } diff --git a/crates/bevy_reflect/src/serde/ser/sets.rs b/crates/bevy_reflect/src/serde/ser/sets.rs index 9a5abab57242d..846f9e4f84fd3 100644 --- a/crates/bevy_reflect/src/serde/ser/sets.rs +++ b/crates/bevy_reflect/src/serde/ser/sets.rs @@ -22,7 +22,7 @@ impl<'a> Serialize for SetSerializer<'a> { { let mut state = serializer.serialize_seq(Some(self.set.len()))?; for value in self.set.iter() { - state.serialize_element(&TypedReflectSerializer::new(value, self.registry))?; + state.serialize_element(&TypedReflectSerializer::new_internal(value, self.registry))?; } state.end() } diff --git a/crates/bevy_reflect/src/serde/ser/structs.rs b/crates/bevy_reflect/src/serde/ser/structs.rs index 2c31280f05b77..7763e297feaa8 100644 --- a/crates/bevy_reflect/src/serde/ser/structs.rs +++ b/crates/bevy_reflect/src/serde/ser/structs.rs @@ -1,6 +1,7 @@ +use crate::serde::ser::error_utils::make_custom_error; use crate::serde::{SerializationData, TypedReflectSerializer}; use crate::{Struct, TypeInfo, TypeRegistry}; -use serde::ser::{Error, SerializeStruct}; +use serde::ser::SerializeStruct; use serde::Serialize; /// A serializer for [`Struct`] values. @@ -27,8 +28,8 @@ impl<'a> Serialize for StructSerializer<'a> { .struct_value .get_represented_type_info() .ok_or_else(|| { - Error::custom(format_args!( - "cannot get type info for {}", + make_custom_error(format_args!( + "cannot get type info for `{}`", self.struct_value.reflect_type_path() )) })?; @@ -36,7 +37,7 @@ impl<'a> Serialize for StructSerializer<'a> { let struct_info = match type_info { TypeInfo::Struct(struct_info) => struct_info, info => { - return Err(Error::custom(format_args!( + return Err(make_custom_error(format_args!( "expected struct type but received {info:?}" ))); } @@ -60,7 +61,10 @@ impl<'a> Serialize for StructSerializer<'a> { continue; } let key = struct_info.field_at(index).unwrap().name(); - state.serialize_field(key, &TypedReflectSerializer::new(value, self.registry))?; + state.serialize_field( + key, + &TypedReflectSerializer::new_internal(value, self.registry), + )?; } state.end() } diff --git a/crates/bevy_reflect/src/serde/ser/tuple_structs.rs b/crates/bevy_reflect/src/serde/ser/tuple_structs.rs index e60c9ad66ca78..625e41e116a1c 100644 --- a/crates/bevy_reflect/src/serde/ser/tuple_structs.rs +++ b/crates/bevy_reflect/src/serde/ser/tuple_structs.rs @@ -1,6 +1,7 @@ +use crate::serde::ser::error_utils::make_custom_error; use crate::serde::{SerializationData, TypedReflectSerializer}; use crate::{TupleStruct, TypeInfo, TypeRegistry}; -use serde::ser::{Error, SerializeTupleStruct}; +use serde::ser::SerializeTupleStruct; use serde::Serialize; /// A serializer for [`TupleStruct`] values. @@ -27,8 +28,8 @@ impl<'a> Serialize for TupleStructSerializer<'a> { .tuple_struct .get_represented_type_info() .ok_or_else(|| { - Error::custom(format_args!( - "cannot get type info for {}", + make_custom_error(format_args!( + "cannot get type info for `{}`", self.tuple_struct.reflect_type_path() )) })?; @@ -36,7 +37,7 @@ impl<'a> Serialize for TupleStructSerializer<'a> { let tuple_struct_info = match type_info { TypeInfo::TupleStruct(tuple_struct_info) => tuple_struct_info, info => { - return Err(Error::custom(format_args!( + return Err(make_custom_error(format_args!( "expected tuple struct type but received {info:?}" ))); } @@ -59,7 +60,7 @@ impl<'a> Serialize for TupleStructSerializer<'a> { { continue; } - state.serialize_field(&TypedReflectSerializer::new(value, self.registry))?; + state.serialize_field(&TypedReflectSerializer::new_internal(value, self.registry))?; } state.end() } diff --git a/crates/bevy_reflect/src/serde/ser/tuples.rs b/crates/bevy_reflect/src/serde/ser/tuples.rs index c3b90d5f5663e..e106149613092 100644 --- a/crates/bevy_reflect/src/serde/ser/tuples.rs +++ b/crates/bevy_reflect/src/serde/ser/tuples.rs @@ -23,7 +23,7 @@ impl<'a> Serialize for TupleSerializer<'a> { let mut state = serializer.serialize_tuple(self.tuple.field_len())?; for value in self.tuple.iter_fields() { - state.serialize_element(&TypedReflectSerializer::new(value, self.registry))?; + state.serialize_element(&TypedReflectSerializer::new_internal(value, self.registry))?; } state.end() } diff --git a/crates/bevy_reflect/src/type_info_stack.rs b/crates/bevy_reflect/src/type_info_stack.rs new file mode 100644 index 0000000000000..f7f22a54ac44d --- /dev/null +++ b/crates/bevy_reflect/src/type_info_stack.rs @@ -0,0 +1,49 @@ +use crate::TypeInfo; +use core::fmt::{Debug, Formatter}; +use core::slice::Iter; + +/// Helper struct for managing a stack of [`TypeInfo`] instances. +/// +/// This is useful for tracking the type hierarchy when serializing and deserializing types. +#[derive(Default, Clone)] +pub(crate) struct TypeInfoStack { + stack: Vec<&'static TypeInfo>, +} + +impl TypeInfoStack { + /// Create a new empty [`TypeInfoStack`]. + pub const fn new() -> Self { + Self { stack: Vec::new() } + } + + /// Push a new [`TypeInfo`] onto the stack. + pub fn push(&mut self, type_info: &'static TypeInfo) { + self.stack.push(type_info); + } + + /// Pop the last [`TypeInfo`] off the stack. + pub fn pop(&mut self) { + self.stack.pop(); + } + + /// Get an iterator over the stack in the order they were pushed. + pub fn iter(&self) -> Iter<&'static TypeInfo> { + self.stack.iter() + } +} + +impl Debug for TypeInfoStack { + fn fmt(&self, f: &mut Formatter<'_>) -> core::fmt::Result { + let mut iter = self.iter(); + + if let Some(first) = iter.next() { + write!(f, "`{}`", first.type_path())?; + } + + for info in iter { + write!(f, " -> `{}`", info.type_path())?; + } + + Ok(()) + } +}