Improve BevyError ergonomics

_release-content/release-notes/bevy_error_context.md

@@ -0,0 +1,50 @@
+---
+title: Bevy Error Context Messages
+authors: ["@cookie1170"]
+pull_requests: [24528]
+---
+
+Similar to the popular `anyhow` crate, `BevyError` now provides an ergonomic way to attach extra context to an error using the `context` method,
+which also allows creating a `Result<T, BevyError>` from an `Option<T>`.
+
+This makes it easier to trace back errors with human-readable messages without looking at verbose backtraces.
+
+```rs
+fn fallible() -> Result<(), BevyError> {
+    // This produces the error message `Failed to parse number: invalid digit found in string`
+    let parsed: usize = "I am not a number"
+        .parse()
+        .context("Failed to parse number")?;
+
+    Ok(())
+}
+```
+
+`with_context` may be used to produce the error string with a closure instead.
+
+If multiple `context`s were used on the same `BevyError`, they're enumerated below:
+
+```rs
+fn fallible() -> Result<Package, BevyError> {
+    let path = "package.json";
+    let package = std::fs::read_to_string(path)
+        .with_context(|| format!("Failed to read {path}"))?;
+
+    serde_json::parse(&package)?
+}
+
+fn uses_fallible() -> Result<(), BevyError> {
+    let package = fallible().context("Failed to parse package.json")?;
+    // Use `package`...
+}
+```
+
+Will produce the following error if `package.json` is missing:
+
+```rs
+Failed to parse package.json
+
+Caused by:
+    Failed to read package.json
+    No such file or directory (os error 2)
+```

crates/bevy_ecs/src/error/bevy_error.rs

@@ -1,4 +1,4 @@
-use alloc::boxed::Box;
+use alloc::{boxed::Box, string::ToString};
 use core::{
     error::Error,
     fmt::{Debug, Display},
@@ -184,12 +184,20 @@ impl BevyError {
 
     /// Checks if the internal error is of the given type.
     pub fn is<E: Error + 'static>(&self) -> bool {
-        self.inner.error.is::<E>()
+        if let Some(context) = self.inner.error.downcast_ref::<ContextError>() {
+            context.error.is::<E>()
+        } else {
+            self.inner.error.is::<E>()
+        }
     }
 
     /// Attempts to downcast the internal error to the given type.
     pub fn downcast_ref<E: Error + 'static>(&self) -> Option<&E> {
-        self.inner.error.downcast_ref::<E>()
+        if let Some(context) = self.inner.error.downcast_ref::<ContextError>() {
+            context.error.downcast_ref::<E>()
+        } else {
+            self.inner.error.downcast_ref::<E>()
+        }
     }
 
     fn format_backtrace(&self, _f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
@@ -201,7 +209,7 @@ impl BevyError {
                 // TODO: Cache
                 let full_backtrace = std::env::var("BEVY_BACKTRACE").is_ok_and(|val| val == "full");
 
-                let backtrace_str = alloc::string::ToString::to_string(backtrace);
+                let backtrace_str = backtrace.to_string();
                 let mut skip_next_location_line = false;
                 for line in backtrace_str.split('\n') {
                     if !full_backtrace {
@@ -219,18 +227,24 @@ impl BevyError {
                             skip_next_location_line = true;
                             continue;
                         }
-                        if line.contains("<bevy_ecs::error::bevy_error::BevyError as core::convert::From<E>>::from") {
+                        if line.contains(
+                            "bevy_ecs::error::bevy_error::BevyError as core::convert::From",
+                        ) {
                             skip_next_location_line = true;
                             continue;
                         }
-                        if line.contains("<core::result::Result<T,F> as core::ops::try_trait::FromResidual<core::result::Result<core::convert::Infallible,E>>>::from_residual") {
+                        if line.contains("core::ops::try_trait::FromResidual")
+                            && line.contains("::from_residual")
+                        {
                             skip_next_location_line = true;
                             continue;
                         }
                         if line.contains("__rust_begin_short_backtrace") {
                             break;
                         }
-                        if line.contains("bevy_ecs::observer::Observers::invoke::{{closure}}") {
+                        if line.contains("bevy_ecs::observer::Observers::invoke::")
+                            && line.contains("closure")
+                        {
                             break;
                         }
                     }
@@ -311,7 +325,7 @@ impl BevyError {
 }
 
 /// Extension methods for annotating errors with a [`Severity`].
-pub trait ResultSeverityExt<T, E> {
+pub trait ResultSeverityExt<T, E>: Sized {
     /// Overrides the [`Severity`] of the error if this result is `Err`.
     /// This does not change control flow; it only annotates the error.
     ///
@@ -362,6 +376,48 @@ pub trait ResultSeverityExt<T, E> {
     ///
     /// If you don't need to inspect the error, use [`Result::with_severity`](ResultSeverityExt::with_severity)
     fn map_severity(self, f: impl FnOnce(&E) -> Severity) -> Result<T, BevyError>;
+
+    /// Overrides the severity of the error with [`Severity::Ignore`]. See [`Result::with_severity`]
+    ///
+    /// This is shorthand for `self.with_severity(Severity::Ignore)`
+    fn ignore(self) -> Result<T, BevyError> {
+        self.with_severity(Severity::Ignore)
+    }
+
+    /// Overrides the severity of the error with [`Severity::Trace`]. See [`Result::with_severity`]
+    ///
+    /// This is shorthand for `self.with_severity(Severity::Trace)`
+    fn trace(self) -> Result<T, BevyError> {
+        self.with_severity(Severity::Trace)
+    }
+
+    /// Overrides the severity of the error with [`Severity::Info`]. See [`Result::with_severity`]
+    ///
+    /// This is shorthand for `self.with_severity(Severity::Info)`
+    fn info(self) -> Result<T, BevyError> {
+        self.with_severity(Severity::Info)
+    }
+
+    /// Overrides the severity of the error with [`Severity::Warning`]. See [`Result::with_severity`]
+    ///
+    /// This is shorthand for `self.with_severity(Severity::Warning)`
+    fn warn(self) -> Result<T, BevyError> {
+        self.with_severity(Severity::Warning)
+    }
+
+    /// Overrides the severity of the error with [`Severity::Error`]. See [`Result::with_severity`]
+    ///
+    /// This is shorthand for `self.with_severity(Severity::Error)`
+    fn error(self) -> Result<T, BevyError> {
+        self.with_severity(Severity::Error)
+    }
+
+    /// Overrides the severity of the error with [`Severity::Panic`]. See [`Result::with_severity`]
+    ///
+    /// This is shorthand for `self.with_severity(Severity::Panic)`
+    fn panic(self) -> Result<T, BevyError> {
+        self.with_severity(Severity::Panic)
+    }
 }
 
 impl<T, E> ResultSeverityExt<T, E> for Result<T, E>
@@ -380,6 +436,131 @@ where
     }
 }
 
+/// Extension methods for adding additional context messages to a [`BevyError`]
+pub trait ContextExt<T>: Sized {
+    /// Annotate the error with a context message.
+    ///
+    /// # Example
+    /// ```
+    /// # use bevy_ecs::error::{BevyError, ContextExt};
+    /// fn fallible() -> Result<(), BevyError> {
+    ///     // Produces a `BevyError` with the message
+    ///     // "failed to parse number: invalid digit found in string"
+    ///     let _parsed: usize = "I am not a number"
+    ///         .parse()
+    ///         .context("failed to parse number")?;
+    ///
+    ///     Ok(())
+    /// }
+    /// ```
+    fn context<C>(self, context: C) -> Result<T, BevyError>
+    where
+        C: Display,
+    {
+        self.with_context(move || context)
+    }
+
+    /// Annotate the error with a context message from a closure
+    ///
+    /// # Example
+    /// ```
+    /// # use bevy_ecs::error::{BevyError, ContextExt};
+    /// # use std::fs;
+    /// fn fallible() -> Result<(), BevyError> {
+    ///     let path = "some_file.txt";
+    ///     let _message = fs::read_to_string(path)
+    ///         .with_context(|| format!("failed to read {path}"))?;
+    ///
+    ///     Ok(())
+    /// }
+    /// ```
+    fn with_context<C>(self, context: impl FnOnce() -> C) -> Result<T, BevyError>
+    where
+        C: Display;
+}
+impl<T, E> ContextExt<T> for Result<T, E>
+where
+    E: Into<BevyError>,
+{
+    fn with_context<C>(self, context: impl FnOnce() -> C) -> Result<T, BevyError>
+    where
+        C: Display,
+    {
+        match self {
+            Ok(v) => Ok(v),
+            Err(error) => {
+                let mut error = error.into();
+                let msg = context().to_string();
+                if let Some(ctx) = error.inner.error.downcast_mut::<ContextError>() {
+                    ctx.messages.push(msg);
+                } else {
+                    error.inner.error = ContextError {
+                        messages: alloc::vec![error.inner.error.to_string(), msg],
+                        error: error.inner.error,
+                    }
+                    .into();
+                }
+                Err(error)
+            }
+        }
+    }
+}
+
+impl<T> ContextExt<T> for Option<T> {
+    fn with_context<C>(self, context: impl FnOnce() -> C) -> Result<T, BevyError>
+    where
+        C: Display,
+    {
+        match self {
+            Some(v) => Ok(v),
+            None => {
+                let message = context().to_string();
+                let error = ContextError {
+                    messages: alloc::vec![message.clone()],
+                    error: message.into(),
+                };
+
+                Err(error.into())
+            }
+        }
+    }
+}
+
+#[derive(Debug)]
+struct ContextError {
+    messages: alloc::vec::Vec<alloc::string::String>,
+    error: Box<dyn Error + Send + Sync + 'static>,
+}
+
+impl Display for ContextError {
+    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
+        match &self.messages {
+            messages if messages.len() == 2 => {
+                write!(f, "{}: {}", messages[1].trim(), messages[0].trim())?;
+            }
+            messages if messages.len() == 1 => write!(f, "{}", messages[0].trim())?,
+            messages if messages.is_empty() => {} // nop
+            messages => {
+                // The most recent message is the last one in the `Vec`
+                // so we need to reverse the iterator
+                let mut reversed = messages.iter().rev();
+                // `messages.len()` is at least 3
+                let first = reversed.next().unwrap().trim();
+
+                write!(f, "{first}\n\nCaused by:")?;
+                for message in reversed {
+                    let message = message.trim();
+                    write!(f, "\n\t{message}")?;
+                }
+            }
+        }
+
+        Ok(())
+    }
+}
+
+impl Error for ContextError {}
+
 // NOTE: writing the impl this way gives us From<&str> ... nice!
 impl<E> From<E> for BevyError
 where
@@ -510,6 +691,8 @@ macro_rules! bail {
 #[cfg(test)]
 mod tests {
     use crate::error::BevyError;
+    use crate::error::ContextExt;
+    use alloc::string::ToString;
 
     #[test]
     #[cfg(not(miri))] // miri backtraces are weird
@@ -573,7 +756,7 @@ mod tests {
         // on linux there is a second call_once
         let mut skip = false;
         if let Some(line) = lines.peek()
-            && &line[6..] == "core::ops::function::FnOnce::call_once"
+            && line.contains("core::ops::function::FnOnce")
         {
             skip = true;
         }
@@ -649,4 +832,64 @@ mod tests {
         });
         t(|| bail!("Format string {}", 1 + 2));
     }
+
+    #[test]
+    fn context() {
+        let empty = None::<i32>;
+        let as_result = empty.context("Didn't have anything!");
+        assert!(as_result
+            .unwrap_err()
+            .to_string()
+            .starts_with("Didn't have anything!\n"));
+
+        let err: Result<i32, BevyError> =
+            Err(BevyError::new(crate::error::Severity::Debug, "Oh no!"));
+        let mut with_context = err.context("Failed");
+
+        assert!(with_context
+            .as_ref()
+            .unwrap_err()
+            .to_string()
+            .starts_with("Failed: Oh no!\n"));
+
+        with_context = with_context.context("Something went wrong");
+        assert!(with_context.unwrap_err().to_string().starts_with(
+            "Something went wrong
+
+Caused by:
+\tFailed
+\tOh no!
+"
+        ));
+    }
+
+    #[test]
+    fn context_downcasting() {
+        #[derive(Debug, PartialEq)]
+        struct Fun(i32);
+
+        impl core::fmt::Display for Fun {
+            fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
+                core::fmt::Debug::fmt(&self, f)
+            }
+        }
+        impl core::error::Error for Fun {}
+
+        let fun: Result<i32, Fun> = Err(Fun(1));
+        let new_error = fun.context("Hello world!");
+
+        assert!(new_error.as_ref().unwrap_err().is::<Fun>());
+        assert_eq!(
+            new_error.as_ref().unwrap_err().downcast_ref::<Fun>(),
+            Some(&Fun(1))
+        );
+
+        let new_new_error = new_error.context("Hey there!");
+
+        assert!(new_new_error.as_ref().unwrap_err().is::<Fun>());
+        assert_eq!(
+            new_new_error.as_ref().unwrap_err().downcast_ref::<Fun>(),
+            Some(&Fun(1))
+        );
+    }
 }

crates/bevy_ecs/src/lib.rs

@@ -74,7 +74,7 @@ pub mod prelude {
         children,
         component::Component,
         entity::{ContainsEntity, Entity, EntityMapper},
-        error::{BevyError, Result, ResultSeverityExt, Severity},
+        error::{BevyError, ContextExt, Result, ResultSeverityExt, Severity},
         event::{EntityEvent, Event},
         hierarchy::{ChildOf, ChildSpawner, ChildSpawnerCommands, Children},
         lifecycle::{Add, Despawn, Discard, Insert, Remove, RemovedComponents},