bevy_reflect: Contextual serialization error messages (bevyengine#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<Real>` 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<Real>`, 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_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

@@ -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

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.

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);
         }
 

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::<ReflectDeserialize>() {
-            let value = deserialize_reflect.deserialize(deserializer)?;
-            return Ok(value.into_partial_reflect());
-        }
+        let deserialize_internal = || -> Result<Self::Value, D::Error> {
+            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::<ReflectDeserialize>() {
+                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
     }
 }

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)

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<crate::type_info_stack::TypeInfoStack> = 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<E: 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);
+}

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)

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,
             ))?;