BevyError: Bevy's new catch-all error type

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