BevyError: Bevy's new catch-all error type (#18144)

## Objective

Fixes #18092

Bevy's current error type is a simple type alias for `Box<dyn Error +
Send + Sync + 'static>`. This largely works as a catch-all error, but it
is missing a critical feature: the ability to capture a backtrace at the
point that the error occurs. The best way to do this is `anyhow`-style
error handling: a new error type that takes advantage of the fact that
the `?` `From` conversion happens "inline" to capture the backtrace at
the point of the error.

## Solution

This PR adds a new `BevyError` type (replacing our old
`std::error::Error` type alias), which uses the "from conversion
backtrace capture" approach:

```rust
fn oh_no() -> Result<(), BevyError> {
    // this fails with Rust's built in ParseIntError, which
    // is converted into the catch-all BevyError type
    let number: usize = "hi".parse()?;
    println!("parsed {number}");
    Ok(())
}
```

This also updates our exported `Result` type alias to default to
`BevyError`, meaning you can write this instead:

```rust
fn oh_no() -> Result {
    let number: usize = "hi".parse()?;
    println!("parsed {number}");
    Ok(())
}
```

When a BevyError is encountered in a system, it will use Bevy's default
system error handler (which panics by default). BevyError does custom
"backtrace filtering" by default, meaning we can cut out the _massive_
amount of "rust internals", "async executor internals", and "bevy system
scheduler internals" that show up in backtraces. It also trims out the
first generally-unnecssary `From` conversion backtrace lines that make
it harder to locate the real error location. The result is a blissfully
simple backtrace by default:


![image](https://github.com/user-attachments/assets/7a5f5c9b-ea70-4176-af3b-d231da31c967)

The full backtrace can be shown by setting the `BEVY_BACKTRACE=full`
environment variable. Non-BevyError panics still use the default Rust
backtrace behavior.

One issue that prevented the truly noise-free backtrace during panics
that you see above is that Rust's default panic handler will print the
unfiltered (and largely unhelpful real-panic-point) backtrace by
default, in _addition_ to our filtered BevyError backtrace (with the
helpful backtrace origin) that we capture and print. To resolve this, I
have extended Bevy's existing PanicHandlerPlugin to wrap the default
panic handler. If we panic from the result of a BevyError, we will skip
the default "print full backtrace" panic handler. This behavior can be
enabled and disabled using the new `error_panic_hook` cargo feature in
`bevy_app` (which is enabled by default).

One downside to _not_ using `Box<dyn Error>` directly is that we can no
longer take advantage of the built-in `Into` impl for strings to errors.
To resolve this, I have added the following:

```rust
// Before
Err("some error")?

// After
Err(BevyError::message("some error"))?
```

We can discuss adding shorthand methods or macros for this (similar to
anyhow's `anyhow!("some error")` macro), but I'd prefer to discuss that
later.

I have also added the following extension method:

```rust
// Before
some_option.ok_or("some error")?;

// After
some_option.ok_or_message("some error")?;
```

I've also moved all of our existing error infrastructure from
`bevy_ecs::result` to `bevy_ecs::error`, as I think that is the better
home for it

## Why not anyhow (or eyre)?

The biggest reason is that `anyhow` needs to be a "generically useful
error type", whereas Bevy is a much narrower scope. By using our own
error, we can be significantly more opinionated. For example, anyhow
doesn't do the extensive (and invasive) backtrace filtering that
BevyError does because it can't operate on Bevy-specific context, and
needs to be generically useful.

Bevy also has a lot of operational context (ex: system info) that could
be useful to attach to errors. If we have control over the error type,
we can add whatever context we want to in a structured way. This could
be increasingly useful as we add more visual / interactive error
handling tools and editor integrations.

Additionally, the core approach used is simple and requires almost no
code. anyhow clocks in at ~2500 lines of code, but the impl here uses
160. We are able to boil this down to exactly what we need, and by doing
so we improve our compile times and the understandability of our code.

Author: cart

crates/bevy_app/Cargo.toml

@@ -9,7 +9,13 @@ license = "MIT OR Apache-2.0"
 keywords = ["bevy"]
 
 [features]
-default = ["std", "bevy_reflect", "bevy_tasks", "bevy_ecs/default"]
+default = [
+  "std",
+  "bevy_reflect",
+  "bevy_tasks",
+  "bevy_ecs/default",
+  "error_panic_hook",
+]
 
 # Functionality
 
@@ -36,6 +42,10 @@ trace = ["dep:tracing"]
 ## other debug operations which can help with diagnosing certain behaviors.
 bevy_debug_stepping = []
 
+## Will set the BevyError panic hook, which gives cleaner filtered backtraces when
+## a BevyError is hit.
+error_panic_hook = []
+
 # Platform Compatibility
 
 ## Allows access to the `std` crate. Enabling this feature will prevent compilation

crates/bevy_app/src/app.rs

@@ -10,10 +10,10 @@ use alloc::{
 pub use bevy_derive::AppLabel;
 use bevy_ecs::{
     component::RequiredComponentsError,
+    error::{BevyError, SystemErrorContext},
     event::{event_update_system, EventCursor},
     intern::Interned,
     prelude::*,
-    result::{Error, SystemErrorContext},
     schedule::{ScheduleBuildSettings, ScheduleLabel},
     system::{IntoObserverSystem, SystemId, SystemInput},
 };
@@ -1280,7 +1280,7 @@ impl App {
     /// for more information.
     pub fn set_system_error_handler(
         &mut self,
-        error_handler: fn(Error, SystemErrorContext),
+        error_handler: fn(BevyError, SystemErrorContext),
     ) -> &mut Self {
         self.main_mut().set_system_error_handler(error_handler);
         self

crates/bevy_app/src/panic_handler.rs

@@ -39,13 +39,28 @@ pub struct PanicHandlerPlugin;
 
 impl Plugin for PanicHandlerPlugin {
     fn build(&self, _app: &mut App) {
-        #[cfg(target_arch = "wasm32")]
+        #[cfg(feature = "std")]
         {
-            console_error_panic_hook::set_once();
-        }
-        #[cfg(not(target_arch = "wasm32"))]
-        {
-            // Use the default target panic hook - Do nothing.
+            static SET_HOOK: std::sync::Once = std::sync::Once::new();
+            SET_HOOK.call_once(|| {
+                #[cfg(target_arch = "wasm32")]
+                {
+                    // This provides better panic handling in JS engines (displays the panic message and improves the backtrace).
+                    std::panic::set_hook(alloc::boxed::Box::new(console_error_panic_hook::hook));
+                }
+                #[cfg(not(target_arch = "wasm32"))]
+                {
+                    #[cfg(feature = "error_panic_hook")]
+                    {
+                        let current_hook = std::panic::take_hook();
+                        std::panic::set_hook(alloc::boxed::Box::new(
+                            bevy_ecs::error::bevy_error_panic_hook(current_hook),
+                        ));
+                    }
+
+                    // Otherwise use the default target panic hook - Do nothing.
+                }
+            });
         }
     }
 }

crates/bevy_app/src/sub_app.rs

@@ -1,9 +1,9 @@
 use crate::{App, AppLabel, InternedAppLabel, Plugin, Plugins, PluginsState};
 use alloc::{boxed::Box, string::String, vec::Vec};
 use bevy_ecs::{
+    error::{DefaultSystemErrorHandler, SystemErrorContext},
     event::EventRegistry,
     prelude::*,
-    result::{DefaultSystemErrorHandler, SystemErrorContext},
     schedule::{InternedScheduleLabel, ScheduleBuildSettings, ScheduleLabel},
     system::{SystemId, SystemInput},
 };
@@ -342,7 +342,7 @@ impl SubApp {
     /// for more information.
     pub fn set_system_error_handler(
         &mut self,
-        error_handler: fn(Error, SystemErrorContext),
+        error_handler: fn(BevyError, SystemErrorContext),
     ) -> &mut Self {
         let mut default_handler = self
             .world_mut()

crates/bevy_ecs/Cargo.toml

@@ -11,7 +11,7 @@ categories = ["game-engines", "data-structures"]
 rust-version = "1.85.0"
 
 [features]
-default = ["std", "bevy_reflect", "async_executor"]
+default = ["std", "bevy_reflect", "async_executor", "backtrace"]
 
 # Functionality
 
@@ -36,6 +36,9 @@ reflect_functions = ["bevy_reflect", "bevy_reflect/functions"]
 ## Use the configurable global error handler as the default error handler
 configurable_error_handler = []
 
+## Enables automatic backtrace capturing in BevyError
+backtrace = []
+
 # Debugging Features
 
 ## Enables `tracing` integration, allowing spans and other metrics to be reported

crates/bevy_ecs/src/error/bevy_error.rs

@@ -0,0 +1,241 @@
+use alloc::boxed::Box;
+use core::{
+    error::Error,
+    fmt::{Debug, Display},
+};
+
+/// The built in "universal" Bevy error type. This has a blanket [`From`] impl for any type that implements Rust's [`Error`],
+/// meaning it can be used as a "catch all" error.
+///
+/// # Backtraces
+///
+/// When used with the `backtrace` Cargo feature, it will capture a backtrace when the error is constructed (generally in the [`From`] impl]).
+/// When printed, the backtrace will be displayed. By default, the backtrace will be trimmed down to filter out noise. To see the full backtrace,
+/// set the `BEVY_BACKTRACE=full` environment variable.
+///
+/// # Usage
+///
+/// ```
+/// # use bevy_ecs::prelude::*;
+///
+/// fn fallible_system() -> Result<(), BevyError> {
+///     // This will result in Rust's built-in ParseIntError, which will automatically
+///     // be converted into a BevyError.
+///     let parsed: usize = "I am not a number".parse()?;
+///     Ok(())
+/// }
+/// ```
+pub struct BevyError {
+    inner: Box<InnerBevyError>,
+}
+
+impl BevyError {
+    /// 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>()
+    }
+}
+
+/// This type exists (rather than having a `BevyError(Box<dyn InnerBevyError)`) to make [`BevyError`] use a "thin pointer" instead of
+/// a "fat pointer", which reduces the size of our Result by a usize. This does introduce an extra indirection, but error handling is a "cold path".
+/// We don't need to optimize it to that degree.
+/// PERF: We could probably have the best of both worlds with a "custom vtable" impl, but thats not a huge priority right now and the code simplicity
+/// of the current impl is nice.
+struct InnerBevyError {
+    error: Box<dyn Error + Send + Sync + 'static>,
+    #[cfg(feature = "backtrace")]
+    backtrace: std::backtrace::Backtrace,
+}
+
+// NOTE: writing the impl this way gives us From<&str> ... nice!
+impl<E> From<E> for BevyError
+where
+    Box<dyn Error + Send + Sync + 'static>: From<E>,
+{
+    #[cold]
+    fn from(error: E) -> Self {
+        BevyError {
+            inner: Box::new(InnerBevyError {
+                error: error.into(),
+                #[cfg(feature = "backtrace")]
+                backtrace: std::backtrace::Backtrace::capture(),
+            }),
+        }
+    }
+}
+
+impl Display for BevyError {
+    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
+        writeln!(f, "{}", self.inner.error)?;
+        Ok(())
+    }
+}
+
+impl Debug for BevyError {
+    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
+        writeln!(f, "{:?}", self.inner.error)?;
+        #[cfg(feature = "backtrace")]
+        {
+            let backtrace = &self.inner.backtrace;
+            if let std::backtrace::BacktraceStatus::Captured = backtrace.status() {
+                let full_backtrace = std::env::var("BEVY_BACKTRACE").is_ok_and(|val| val == "full");
+
+                let backtrace_str = alloc::string::ToString::to_string(backtrace);
+                let mut skip_next_location_line = false;
+                for line in backtrace_str.split('\n') {
+                    if !full_backtrace {
+                        if skip_next_location_line {
+                            if line.starts_with("             at") {
+                                continue;
+                            }
+                            skip_next_location_line = false;
+                        }
+                        if line.contains("std::backtrace_rs::backtrace::") {
+                            skip_next_location_line = true;
+                            continue;
+                        }
+                        if line.contains("std::backtrace::Backtrace::") {
+                            skip_next_location_line = true;
+                            continue;
+                        }
+                        if line.contains("<bevy_ecs::error::bevy_error::BevyError as core::convert::From<E>>::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") {
+                            skip_next_location_line = true;
+                            continue;
+                        }
+                        if line.contains("__rust_begin_short_backtrace") {
+                            break;
+                        }
+                        if line.contains("bevy_ecs::observer::Observers::invoke::{{closure}}") {
+                            break;
+                        }
+                    }
+                    writeln!(f, "{}", line)?;
+                }
+                if !full_backtrace {
+                    if std::thread::panicking() {
+                        SKIP_NORMAL_BACKTRACE.store(1, core::sync::atomic::Ordering::Relaxed);
+                    }
+                    writeln!(f, "{FILTER_MESSAGE}")?;
+                }
+            }
+        }
+
+        Ok(())
+    }
+}
+
+#[cfg(feature = "backtrace")]
+const FILTER_MESSAGE: &str = "note: Some \"noisy\" backtrace lines have been filtered out. Run with `BEVY_BACKTRACE=full` for a verbose backtrace.";
+
+#[cfg(feature = "backtrace")]
+static SKIP_NORMAL_BACKTRACE: core::sync::atomic::AtomicUsize =
+    core::sync::atomic::AtomicUsize::new(0);
+
+/// When called, this will skip the currently configured panic hook when a [`BevyError`] backtrace has already been printed.
+#[cfg(feature = "std")]
+pub fn bevy_error_panic_hook(
+    current_hook: impl Fn(&std::panic::PanicHookInfo),
+) -> impl Fn(&std::panic::PanicHookInfo) {
+    move |info| {
+        if SKIP_NORMAL_BACKTRACE.load(core::sync::atomic::Ordering::Relaxed) > 0 {
+            if let Some(payload) = info.payload().downcast_ref::<&str>() {
+                std::println!("{payload}");
+            } else if let Some(payload) = info.payload().downcast_ref::<alloc::string::String>() {
+                std::println!("{payload}");
+            }
+            SKIP_NORMAL_BACKTRACE.store(0, core::sync::atomic::Ordering::Relaxed);
+            return;
+        }
+
+        current_hook(info);
+    }
+}
+
+#[cfg(test)]
+mod tests {
+
+    #[test]
+    #[cfg(not(miri))] // miri backtraces are weird
+    #[cfg(not(windows))] // the windows backtrace in this context is ... unhelpful and not worth testing
+    fn filtered_backtrace_test() {
+        fn i_fail() -> crate::error::Result {
+            let _: usize = "I am not a number".parse()?;
+            Ok(())
+        }
+
+        // SAFETY: this is not safe ...  this test could run in parallel with another test
+        // that writes the environment variable. We either accept that so we can write this test,
+        // or we don't.
+
+        unsafe { std::env::set_var("RUST_BACKTRACE", "1") };
+
+        let error = i_fail().err().unwrap();
+        let debug_message = alloc::format!("{error:?}");
+        let mut lines = debug_message.lines().peekable();
+        assert_eq!(
+            "ParseIntError { kind: InvalidDigit }",
+            lines.next().unwrap()
+        );
+
+        // On mac backtraces can start with Backtrace::create
+        let mut skip = false;
+        if let Some(line) = lines.peek() {
+            if &line[6..] == "std::backtrace::Backtrace::create" {
+                skip = true;
+            }
+        }
+
+        if skip {
+            lines.next().unwrap();
+        }
+
+        let expected_lines = alloc::vec![
+            "bevy_ecs::error::bevy_error::tests::filtered_backtrace_test::i_fail",
+            "bevy_ecs::error::bevy_error::tests::filtered_backtrace_test",
+            "bevy_ecs::error::bevy_error::tests::filtered_backtrace_test::{{closure}}",
+            "core::ops::function::FnOnce::call_once",
+        ];
+
+        for expected in expected_lines {
+            let line = lines.next().unwrap();
+            assert_eq!(&line[6..], expected);
+            let mut skip = false;
+            if let Some(line) = lines.peek() {
+                if line.starts_with("             at") {
+                    skip = true;
+                }
+            }
+
+            if skip {
+                lines.next().unwrap();
+            }
+        }
+
+        // on linux there is a second call_once
+        let mut skip = false;
+        if let Some(line) = lines.peek() {
+            if &line[6..] == "core::ops::function::FnOnce::call_once" {
+                skip = true;
+            }
+        }
+        if skip {
+            lines.next().unwrap();
+        }
+        let mut skip = false;
+        if let Some(line) = lines.peek() {
+            if line.starts_with("             at") {
+                skip = true;
+            }
+        }
+
+        if skip {
+            lines.next().unwrap();
+        }
+        assert_eq!(super::FILTER_MESSAGE, lines.next().unwrap());
+        assert!(lines.next().is_none());
+    }
+}