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},
@@ -106,6 +106,7 @@ impl BevyError {
             inner: Box::new(InnerBevyError {
                 error: error.into(),
                 severity,
+                context: alloc::vec![],
                 #[cfg(feature = "backtrace")]
                 backtrace,
             }),
@@ -201,7 +202,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 {
@@ -255,6 +256,7 @@ impl BevyError {
 /// of the current impl is nice.
 struct InnerBevyError {
     error: Box<dyn Error + Send + Sync + 'static>,
+    context: alloc::vec::Vec<alloc::string::String>,
     severity: Severity,
     #[cfg(feature = "backtrace")]
     backtrace: std::backtrace::Backtrace,
@@ -311,7 +313,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 +364,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 +424,84 @@ 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 message = context().to_string();
+                error.inner.context.push(message);
+                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();
+
+                Err(message.into())
+            }
+        }
+    }
+}
+
 // NOTE: writing the impl this way gives us From<&str> ... nice!
 impl<E> From<E> for BevyError
 where
@@ -391,6 +513,7 @@ where
             inner: Box::new(InnerBevyError {
                 error: error.into(),
                 severity: Severity::Panic,
+                context: alloc::vec![],
                 #[cfg(feature = "backtrace")]
                 backtrace: std::backtrace::Backtrace::capture(),
             }),
@@ -400,7 +523,25 @@ where
 
 impl Display for BevyError {
     fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
-        writeln!(f, "{}", self.inner.error)?;
+        match &self.inner.context {
+            context if context.is_empty() => writeln!(f, "{}", self.inner.error)?,
+            context if context.len() == 1 => {
+                writeln!(f, "{}: {}", context[0].trim(), self.inner.error)?;
+            }
+            context => {
+                // The most recent message is the last one in the `Vec`
+                // so we need to reverse the iterator
+                let error = self.inner.error.to_string();
+                let mut reversed = context.iter().rev().chain(core::iter::once(&error));
+                let first = reversed.next().unwrap().trim();
+
+                writeln!(f, "{first}\n\nCaused by:")?;
+                for message in reversed {
+                    let message = message.trim();
+                    writeln!(f, "\t{message}")?;
+                }
+            }
+        }
         self.format_backtrace(f)?;
         Ok(())
     }
@@ -510,6 +651,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
@@ -649,4 +792,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},