From de2cb29147db3b14aa98d8952634da808c6fd890 Mon Sep 17 00:00:00 2001 From: Carter Anderson Date: Fri, 28 Feb 2025 15:02:50 -0800 Subject: [PATCH 01/18] BevyError: Bevy's new catch-all error type --- crates/bevy_app/Cargo.toml | 12 +- crates/bevy_app/src/app.rs | 4 +- crates/bevy_app/src/panic_handler.rs | 2 + crates/bevy_app/src/sub_app.rs | 4 +- crates/bevy_ecs/Cargo.toml | 5 +- crates/bevy_ecs/src/error/bevy_error.rs | 217 ++++++++++++++++++ crates/bevy_ecs/src/error/handler.rs | 75 ++++++ .../bevy_ecs/src/{result.rs => error/mod.rs} | 89 +------ crates/bevy_ecs/src/lib.rs | 4 +- crates/bevy_ecs/src/observer/runner.rs | 12 +- crates/bevy_ecs/src/schedule/config.rs | 2 +- crates/bevy_ecs/src/schedule/executor/mod.rs | 6 +- .../src/schedule/executor/multi_threaded.rs | 6 +- .../bevy_ecs/src/schedule/executor/simple.rs | 4 +- .../src/schedule/executor/single_threaded.rs | 4 +- crates/bevy_ecs/src/schedule/schedule.rs | 8 +- .../bevy_ecs/src/system/commands/command.rs | 10 +- .../src/system/commands/entity_command.rs | 2 +- .../src/system/commands/error_handler.rs | 16 +- crates/bevy_ecs/src/system/commands/mod.rs | 10 +- crates/bevy_ecs/src/system/mod.rs | 6 +- crates/bevy_ecs/src/system/observer_system.rs | 2 +- crates/bevy_ecs/src/system/schedule_system.rs | 2 +- examples/ecs/fallible_systems.rs | 6 +- 24 files changed, 371 insertions(+), 137 deletions(-) create mode 100644 crates/bevy_ecs/src/error/bevy_error.rs create mode 100644 crates/bevy_ecs/src/error/handler.rs rename crates/bevy_ecs/src/{result.rs => error/mod.rs} (54%) diff --git a/crates/bevy_app/Cargo.toml b/crates/bevy_app/Cargo.toml index c19a3329d3434..0f803367fcc94 100644 --- a/crates/bevy_app/Cargo.toml +++ b/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 diff --git a/crates/bevy_app/src/app.rs b/crates/bevy_app/src/app.rs index 9799693aff19f..83aeac84bc777 100644 --- a/crates/bevy_app/src/app.rs +++ b/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 diff --git a/crates/bevy_app/src/panic_handler.rs b/crates/bevy_app/src/panic_handler.rs index 5a2ae097ff053..c3c67ba1a64e8 100644 --- a/crates/bevy_app/src/panic_handler.rs +++ b/crates/bevy_app/src/panic_handler.rs @@ -47,5 +47,7 @@ impl Plugin for PanicHandlerPlugin { { // Use the default target panic hook - Do nothing. } + #[cfg(all(feature = "error_panic_hook", feature = "std"))] + bevy_ecs::error::set_bevy_error_panic_hook(); } } diff --git a/crates/bevy_app/src/sub_app.rs b/crates/bevy_app/src/sub_app.rs index ead7c468c37a9..1843143c2ed46 100644 --- a/crates/bevy_app/src/sub_app.rs +++ b/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() diff --git a/crates/bevy_ecs/Cargo.toml b/crates/bevy_ecs/Cargo.toml index e42b2b62a0650..f8adb9a92aae6 100644 --- a/crates/bevy_ecs/Cargo.toml +++ b/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 diff --git a/crates/bevy_ecs/src/error/bevy_error.rs b/crates/bevy_ecs/src/error/bevy_error.rs new file mode 100644 index 0000000000000..cca2b7672553b --- /dev/null +++ b/crates/bevy_ecs/src/error/bevy_error.rs @@ -0,0 +1,217 @@ +use alloc::boxed::Box; +use core::{ + error::Error, + fmt::{Debug, Display}, + sync::atomic::AtomicUsize, +}; + +#[cfg(feature = "backtrace")] +macro_rules! capture_backtrace { + () => { + Some(std::backtrace::Backtrace::capture()) + }; +} + +#[cfg(not(feature = "backtrace"))] +macro_rules! capture_backtrace { + () => { + None + }; +} + +/// The built in "universal" Bevy error type. This has a blanket [`From`] impl for an 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, +} + +impl BevyError { + /// Creates a new error with the given message. + pub fn message(message: M) -> Self { + BevyError { + inner: Box::new(ErrorImpl { + backtrace: capture_backtrace!(), + error: MessageError(message), + }), + } + } + + /// Attempts to downcast the internal error to the given type. + pub fn downcast_ref(&self) -> Option<&E> { + self.inner.error().downcast_ref::() + } +} + +trait InnerError: Send + Sync + 'static { + #[cfg(feature = "backtrace")] + fn backtrace(&self) -> Option<&std::backtrace::Backtrace>; + fn error(&self) -> &(dyn Error + Send + Sync + 'static); +} + +struct ErrorImpl { + error: E, + #[cfg(feature = "backtrace")] + backtrace: Option, +} + +impl InnerError for ErrorImpl { + #[cfg(feature = "backtrace")] + fn backtrace(&self) -> Option<&std::backtrace::Backtrace> { + self.backtrace.as_ref() + } + + fn error(&self) -> &(dyn Error + Send + Sync + 'static) { + &self.error + } +} + +impl From for BevyError +where + E: Error + Send + Sync + 'static, +{ + #[cold] + fn from(error: E) -> Self { + BevyError { + inner: Box::new(ErrorImpl { + error, + backtrace: capture_backtrace!(), + }), + } + } +} + +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")] + if let Some(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 = false; + for line in backtrace_str.split('\n') { + if skip_next { + skip_next = false; + continue; + } + if !full_backtrace { + if line.starts_with(" 0: >::from") { + skip_next = true; + continue; + } + if line.starts_with(" 1: as core::ops::try_trait::FromResidual>>::from_residual") { + skip_next = 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, "note: Some \"noisy\" backtrace lines have been filtered out. Run with `BEVY_BACKTRACE=full` for a verbose backtrace.")?; + } + } + } + + Ok(()) + } +} + +static SKIP_NORMAL_BACKTRACE: AtomicUsize = 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 set_bevy_error_panic_hook() { + let hook = std::panic::take_hook(); + std::panic::set_hook(Box::new(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::() { + std::println!("{payload}"); + } + SKIP_NORMAL_BACKTRACE.store(0, core::sync::atomic::Ordering::Relaxed); + return; + } + + hook(info); + })); +} + +/// An error containing a print-able message. +pub struct MessageError(pub(crate) M); + +impl Display for MessageError +where + M: Display + Debug, +{ + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + Display::fmt(&self.0, f) + } +} +impl Debug for MessageError +where + M: Display + Debug, +{ + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + Debug::fmt(&self.0, f) + } +} + +impl Error for MessageError where M: Display + Debug + 'static {} + +/// Returns a Result containing a given message if the given value does not exist. +pub trait OkOrMessage { + /// Returns a Result containing a given message if the given value does not exist. + fn ok_or_message( + self, + message: M, + ) -> Result>; +} + +impl OkOrMessage for Option { + fn ok_or_message( + self, + message: M, + ) -> Result> { + match self { + Some(value) => Ok(value), + None => Err(MessageError(message)), + } + } +} diff --git a/crates/bevy_ecs/src/error/handler.rs b/crates/bevy_ecs/src/error/handler.rs new file mode 100644 index 0000000000000..eb0d9809af789 --- /dev/null +++ b/crates/bevy_ecs/src/error/handler.rs @@ -0,0 +1,75 @@ +use crate::{component::Tick, error::BevyError, resource::Resource}; +use alloc::borrow::Cow; + +/// Additional context for a failed system run. +pub struct SystemErrorContext { + /// The name of the system that failed. + pub name: Cow<'static, str>, + + /// The last tick that the system was run. + pub last_run: Tick, +} + +/// The default systems error handler stored as a resource in the [`World`](crate::world::World). +pub struct DefaultSystemErrorHandler(pub fn(BevyError, SystemErrorContext)); + +impl Resource for DefaultSystemErrorHandler {} + +impl Default for DefaultSystemErrorHandler { + fn default() -> Self { + Self(panic) + } +} + +macro_rules! inner { + ($call:path, $e:ident, $c:ident) => { + $call!("Encountered an error in system `{}`: {:?}", $c.name, $e); + }; +} + +/// Error handler that panics with the system error. +#[track_caller] +#[inline] +pub fn panic(error: BevyError, ctx: SystemErrorContext) { + inner!(panic, error, ctx); +} + +/// Error handler that logs the system error at the `error` level. +#[track_caller] +#[inline] +pub fn error(error: BevyError, ctx: SystemErrorContext) { + inner!(log::error, error, ctx); +} + +/// Error handler that logs the system error at the `warn` level. +#[track_caller] +#[inline] +pub fn warn(error: BevyError, ctx: SystemErrorContext) { + inner!(log::warn, error, ctx); +} + +/// Error handler that logs the system error at the `info` level. +#[track_caller] +#[inline] +pub fn info(error: BevyError, ctx: SystemErrorContext) { + inner!(log::info, error, ctx); +} + +/// Error handler that logs the system error at the `debug` level. +#[track_caller] +#[inline] +pub fn debug(error: BevyError, ctx: SystemErrorContext) { + inner!(log::debug, error, ctx); +} + +/// Error handler that logs the system error at the `trace` level. +#[track_caller] +#[inline] +pub fn trace(error: BevyError, ctx: SystemErrorContext) { + inner!(log::trace, error, ctx); +} + +/// Error handler that ignores the system error. +#[track_caller] +#[inline] +pub fn ignore(_: BevyError, _: SystemErrorContext) {} diff --git a/crates/bevy_ecs/src/result.rs b/crates/bevy_ecs/src/error/mod.rs similarity index 54% rename from crates/bevy_ecs/src/result.rs rename to crates/bevy_ecs/src/error/mod.rs index ef5eace90b07e..307d93158f3e8 100644 --- a/crates/bevy_ecs/src/result.rs +++ b/crates/bevy_ecs/src/error/mod.rs @@ -4,7 +4,7 @@ //! considers those systems to be "fallible", and the ECS scheduler will special-case the [`Err`] //! variant of the returned `Result`. //! -//! All [`Error`]s returned by a system are handled by an "error handler". By default, the +//! All [`BevyError`]s returned by a system are handled by an "error handler". By default, the //! [`panic`] error handler function is used, resulting in a panic with the error message attached. //! //! You can change the default behavior by registering a custom error handler, either globally or @@ -29,7 +29,7 @@ //! signature: //! //! ```rust,ignore -//! fn(Error, SystemErrorContext) +//! fn(BevyError, SystemErrorContext) //! ``` //! //! The [`SystemErrorContext`] allows you to access additional details relevant to providing @@ -53,7 +53,7 @@ //! return; //! } //! -//! bevy_ecs::result::error(error, ctx); +//! bevy_ecs::error::error(error, ctx); //! }); //! # } //! ``` @@ -70,84 +70,11 @@ //! [`App::set_system_error_handler`]: ../../bevy_app/struct.App.html#method.set_system_error_handler //! [`system piping feature`]: crate::system::In -use crate::{component::Tick, resource::Resource}; -use alloc::{borrow::Cow, boxed::Box}; +mod bevy_error; +mod handler; -/// A dynamic error type for use in fallible systems. -pub type Error = Box; +pub use bevy_error::*; +pub use handler::*; /// A result type for use in fallible systems. -pub type Result = core::result::Result; - -/// Additional context for a failed system run. -pub struct SystemErrorContext { - /// The name of the system that failed. - pub name: Cow<'static, str>, - - /// The last tick that the system was run. - pub last_run: Tick, -} - -/// The default systems error handler stored as a resource in the [`World`](crate::world::World). -pub struct DefaultSystemErrorHandler(pub fn(Error, SystemErrorContext)); - -impl Resource for DefaultSystemErrorHandler {} - -impl Default for DefaultSystemErrorHandler { - fn default() -> Self { - Self(panic) - } -} - -macro_rules! inner { - ($call:path, $e:ident, $c:ident) => { - $call!("Encountered an error in system `{}`: {:?}", $c.name, $e); - }; -} - -/// Error handler that panics with the system error. -#[track_caller] -#[inline] -pub fn panic(error: Error, ctx: SystemErrorContext) { - inner!(panic, error, ctx); -} - -/// Error handler that logs the system error at the `error` level. -#[track_caller] -#[inline] -pub fn error(error: Error, ctx: SystemErrorContext) { - inner!(log::error, error, ctx); -} - -/// Error handler that logs the system error at the `warn` level. -#[track_caller] -#[inline] -pub fn warn(error: Error, ctx: SystemErrorContext) { - inner!(log::warn, error, ctx); -} - -/// Error handler that logs the system error at the `info` level. -#[track_caller] -#[inline] -pub fn info(error: Error, ctx: SystemErrorContext) { - inner!(log::info, error, ctx); -} - -/// Error handler that logs the system error at the `debug` level. -#[track_caller] -#[inline] -pub fn debug(error: Error, ctx: SystemErrorContext) { - inner!(log::debug, error, ctx); -} - -/// Error handler that logs the system error at the `trace` level. -#[track_caller] -#[inline] -pub fn trace(error: Error, ctx: SystemErrorContext) { - inner!(log::trace, error, ctx); -} - -/// Error handler that ignores the system error. -#[track_caller] -#[inline] -pub fn ignore(_: Error, _: SystemErrorContext) {} +pub type Result = core::result::Result; diff --git a/crates/bevy_ecs/src/lib.rs b/crates/bevy_ecs/src/lib.rs index 8d415d7469183..204e24a758492 100644 --- a/crates/bevy_ecs/src/lib.rs +++ b/crates/bevy_ecs/src/lib.rs @@ -36,6 +36,7 @@ pub mod change_detection; pub mod component; pub mod entity; pub mod entity_disabling; +pub mod error; pub mod event; pub mod hierarchy; pub mod identifier; @@ -49,7 +50,6 @@ pub mod reflect; pub mod relationship; pub mod removal_detection; pub mod resource; -pub mod result; pub mod schedule; pub mod spawn; pub mod storage; @@ -74,6 +74,7 @@ pub mod prelude { children, component::{require, Component}, entity::{Entity, EntityBorrow, EntityMapper}, + error::{BevyError, OkOrMessage, Result}, event::{Event, EventMutator, EventReader, EventWriter, Events}, hierarchy::{ChildOf, ChildSpawner, ChildSpawnerCommands, Children}, name::{Name, NameOrEntity}, @@ -83,7 +84,6 @@ pub mod prelude { relationship::RelationshipTarget, removal_detection::RemovedComponents, resource::Resource, - result::{Error, Result}, schedule::{ apply_deferred, common_conditions::*, ApplyDeferred, Condition, IntoSystemConfigs, IntoSystemSet, IntoSystemSetConfigs, Schedule, Schedules, SystemSet, diff --git a/crates/bevy_ecs/src/observer/runner.rs b/crates/bevy_ecs/src/observer/runner.rs index fdb3b4fa800ba..972f58889b19b 100644 --- a/crates/bevy_ecs/src/observer/runner.rs +++ b/crates/bevy_ecs/src/observer/runner.rs @@ -3,10 +3,10 @@ use core::any::Any; use crate::{ component::{ComponentHook, ComponentId, HookContext, Mutable, StorageType}, + error::{DefaultSystemErrorHandler, SystemErrorContext}, observer::{ObserverDescriptor, ObserverTrigger}, prelude::*, query::DebugCheckedUnwrap, - result::{DefaultSystemErrorHandler, SystemErrorContext}, system::{IntoObserverSystem, ObserverSystem}, world::DeferredWorld, }; @@ -273,7 +273,7 @@ pub struct Observer { system: Box, descriptor: ObserverDescriptor, hook_on_add: ComponentHook, - error_handler: Option, + error_handler: Option, } impl Observer { @@ -322,7 +322,7 @@ impl Observer { /// Set the error handler to use for this observer. /// /// See the [`result` module-level documentation](crate::result) for more information. - pub fn with_error_handler(mut self, error_handler: fn(Error, SystemErrorContext)) -> Self { + pub fn with_error_handler(mut self, error_handler: fn(BevyError, SystemErrorContext)) -> Self { self.error_handler = Some(error_handler); self } @@ -488,7 +488,7 @@ mod tests { #[should_panic(expected = "I failed!")] fn test_fallible_observer() { fn system(_: Trigger) -> Result { - Err("I failed!".into()) + Err(BevyError::message("I failed!")) } let mut world = World::default(); @@ -504,12 +504,12 @@ mod tests { fn system(_: Trigger, mut ran: ResMut) -> Result { ran.0 = true; - Err("I failed!".into()) + Err(BevyError::message("I failed!")) } let mut world = World::default(); world.init_resource::(); - let observer = Observer::new(system).with_error_handler(crate::result::ignore); + let observer = Observer::new(system).with_error_handler(crate::error::ignore); world.spawn(observer); Schedule::default().run(&mut world); world.trigger(TriggerEvent); diff --git a/crates/bevy_ecs/src/schedule/config.rs b/crates/bevy_ecs/src/schedule/config.rs index 898cf674245d2..f685bbecbdf51 100644 --- a/crates/bevy_ecs/src/schedule/config.rs +++ b/crates/bevy_ecs/src/schedule/config.rs @@ -2,7 +2,7 @@ use alloc::{boxed::Box, vec, vec::Vec}; use variadics_please::all_tuples; use crate::{ - result::Result, + error::Result, schedule::{ auto_insert_apply_deferred::IgnoreDeferred, condition::{BoxedCondition, Condition}, diff --git a/crates/bevy_ecs/src/schedule/executor/mod.rs b/crates/bevy_ecs/src/schedule/executor/mod.rs index c99d263e046df..983d9629f8690 100644 --- a/crates/bevy_ecs/src/schedule/executor/mod.rs +++ b/crates/bevy_ecs/src/schedule/executor/mod.rs @@ -16,9 +16,9 @@ use fixedbitset::FixedBitSet; use crate::{ archetype::ArchetypeComponentId, component::{ComponentId, Tick}, + error::{BevyError, Result, SystemErrorContext}, prelude::{IntoSystemSet, SystemSet}, query::Access, - result::{Error, Result, SystemErrorContext}, schedule::{BoxedCondition, InternedSystemSet, NodeId, SystemTypeSet}, system::{ScheduleSystem, System, SystemIn}, world::{unsafe_world_cell::UnsafeWorldCell, DeferredWorld, World}, @@ -33,7 +33,7 @@ pub(super) trait SystemExecutor: Send + Sync { schedule: &mut SystemSchedule, world: &mut World, skip_systems: Option<&FixedBitSet>, - error_handler: fn(Error, SystemErrorContext), + error_handler: fn(BevyError, SystemErrorContext), ); fn set_apply_final_deferred(&mut self, value: bool); } @@ -265,7 +265,7 @@ mod __rust_begin_short_backtrace { use core::hint::black_box; use crate::{ - result::Result, + error::Result, system::{ReadOnlySystem, ScheduleSystem}, world::{unsafe_world_cell::UnsafeWorldCell, World}, }; diff --git a/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs b/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs index e78cb666aedd4..7ab6f1a392c1d 100644 --- a/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs +++ b/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs @@ -15,9 +15,9 @@ use tracing::{info_span, Span}; use crate::{ archetype::ArchetypeComponentId, + error::{BevyError, Result, SystemErrorContext}, prelude::Resource, query::Access, - result::{Error, Result, SystemErrorContext}, schedule::{is_apply_deferred, BoxedCondition, ExecutorKind, SystemExecutor, SystemSchedule}, system::ScheduleSystem, world::{unsafe_world_cell::UnsafeWorldCell, World}, @@ -132,7 +132,7 @@ pub struct ExecutorState { struct Context<'scope, 'env, 'sys> { environment: &'env Environment<'env, 'sys>, scope: &'scope Scope<'scope, 'env, ()>, - error_handler: fn(Error, SystemErrorContext), + error_handler: fn(BevyError, SystemErrorContext), } impl Default for MultiThreadedExecutor { @@ -183,7 +183,7 @@ impl SystemExecutor for MultiThreadedExecutor { schedule: &mut SystemSchedule, world: &mut World, _skip_systems: Option<&FixedBitSet>, - error_handler: fn(Error, SystemErrorContext), + error_handler: fn(BevyError, SystemErrorContext), ) { let state = self.state.get_mut().unwrap(); // reset counts diff --git a/crates/bevy_ecs/src/schedule/executor/simple.rs b/crates/bevy_ecs/src/schedule/executor/simple.rs index ad01c324e9ea5..a295919690f94 100644 --- a/crates/bevy_ecs/src/schedule/executor/simple.rs +++ b/crates/bevy_ecs/src/schedule/executor/simple.rs @@ -8,7 +8,7 @@ use tracing::info_span; use std::eprintln; use crate::{ - result::{Error, SystemErrorContext}, + error::{BevyError, SystemErrorContext}, schedule::{ executor::is_apply_deferred, BoxedCondition, ExecutorKind, SystemExecutor, SystemSchedule, }, @@ -44,7 +44,7 @@ impl SystemExecutor for SimpleExecutor { schedule: &mut SystemSchedule, world: &mut World, _skip_systems: Option<&FixedBitSet>, - error_handler: fn(Error, SystemErrorContext), + error_handler: fn(BevyError, SystemErrorContext), ) { // If stepping is enabled, make sure we skip those systems that should // not be run. diff --git a/crates/bevy_ecs/src/schedule/executor/single_threaded.rs b/crates/bevy_ecs/src/schedule/executor/single_threaded.rs index b5c44085d5db3..277d41eb0bdc3 100644 --- a/crates/bevy_ecs/src/schedule/executor/single_threaded.rs +++ b/crates/bevy_ecs/src/schedule/executor/single_threaded.rs @@ -8,7 +8,7 @@ use tracing::info_span; use std::eprintln; use crate::{ - result::{Error, SystemErrorContext}, + error::{BevyError, SystemErrorContext}, schedule::{is_apply_deferred, BoxedCondition, ExecutorKind, SystemExecutor, SystemSchedule}, world::World, }; @@ -50,7 +50,7 @@ impl SystemExecutor for SingleThreadedExecutor { schedule: &mut SystemSchedule, world: &mut World, _skip_systems: Option<&FixedBitSet>, - error_handler: fn(Error, SystemErrorContext), + error_handler: fn(BevyError, SystemErrorContext), ) { // If stepping is enabled, make sure we skip those systems that should // not be run. diff --git a/crates/bevy_ecs/src/schedule/schedule.rs b/crates/bevy_ecs/src/schedule/schedule.rs index d67ca188e8172..2f86810f456f8 100644 --- a/crates/bevy_ecs/src/schedule/schedule.rs +++ b/crates/bevy_ecs/src/schedule/schedule.rs @@ -26,9 +26,9 @@ use tracing::info_span; use crate::{ component::{ComponentId, Components, Tick}, + error::{BevyError, DefaultSystemErrorHandler, SystemErrorContext}, prelude::Component, resource::Resource, - result::{DefaultSystemErrorHandler, Error, SystemErrorContext}, schedule::*, system::ScheduleSystem, world::World, @@ -296,7 +296,7 @@ pub struct Schedule { executable: SystemSchedule, executor: Box, executor_initialized: bool, - error_handler: Option, + error_handler: Option, } #[derive(ScheduleLabel, Hash, PartialEq, Eq, Debug, Clone)] @@ -399,10 +399,10 @@ impl Schedule { self } - /// Set the error handler to use for systems that return a [`Result`](crate::result::Result). + /// Set the error handler to use for systems that return a [`Result`](crate::error::Result). /// /// See the [`result` module-level documentation](crate::result) for more information. - pub fn set_error_handler(&mut self, error_handler: fn(Error, SystemErrorContext)) { + pub fn set_error_handler(&mut self, error_handler: fn(BevyError, SystemErrorContext)) { self.error_handler = Some(error_handler); } diff --git a/crates/bevy_ecs/src/system/commands/command.rs b/crates/bevy_ecs/src/system/commands/command.rs index 5344534f39568..7dd61946d6462 100644 --- a/crates/bevy_ecs/src/system/commands/command.rs +++ b/crates/bevy_ecs/src/system/commands/command.rs @@ -8,10 +8,10 @@ use crate::{ bundle::{Bundle, InsertMode, NoBundleEffect}, change_detection::MaybeLocation, entity::Entity, + error::{BevyError, Result}, event::{Event, Events}, observer::TriggerTargets, resource::Resource, - result::{Error, Result}, schedule::ScheduleLabel, system::{error_handler, IntoSystem, SystemId, SystemInput}, world::{FromWorld, SpawnBatchIter, World}, @@ -68,7 +68,7 @@ where pub trait HandleError { /// Takes a [`Command`] that returns a Result and uses a given error handler function to convert it into /// a [`Command`] that internally handles an error if it occurs and returns `()`. - fn handle_error_with(self, error_handler: fn(&mut World, Error)) -> impl Command; + fn handle_error_with(self, error_handler: fn(&mut World, BevyError)) -> impl Command; /// Takes a [`Command`] that returns a Result and uses the default error handler function to convert it into /// a [`Command`] that internally handles an error if it occurs and returns `()`. fn handle_error(self) -> impl Command @@ -82,9 +82,9 @@ pub trait HandleError { impl HandleError> for C where C: Command>, - E: Into, + E: Into, { - fn handle_error_with(self, error_handler: fn(&mut World, Error)) -> impl Command { + fn handle_error_with(self, error_handler: fn(&mut World, BevyError)) -> impl Command { move |world: &mut World| match self.apply(world) { Ok(_) => {} Err(err) => (error_handler)(world, err.into()), @@ -97,7 +97,7 @@ where C: Command, { #[inline] - fn handle_error_with(self, _error_handler: fn(&mut World, Error)) -> impl Command { + fn handle_error_with(self, _error_handler: fn(&mut World, BevyError)) -> impl Command { self } #[inline] diff --git a/crates/bevy_ecs/src/system/commands/entity_command.rs b/crates/bevy_ecs/src/system/commands/entity_command.rs index c73c76dc3adc4..87db3660b3952 100644 --- a/crates/bevy_ecs/src/system/commands/entity_command.rs +++ b/crates/bevy_ecs/src/system/commands/entity_command.rs @@ -13,8 +13,8 @@ use crate::{ change_detection::MaybeLocation, component::{Component, ComponentId, ComponentInfo}, entity::{Entity, EntityClonerBuilder}, + error::Result, event::Event, - result::Result, system::{command::HandleError, Command, IntoObserverSystem}, world::{error::EntityMutableFetchError, EntityWorldMut, FromWorld, World}, }; diff --git a/crates/bevy_ecs/src/system/commands/error_handler.rs b/crates/bevy_ecs/src/system/commands/error_handler.rs index 231df9ec7387e..e5895f91bb955 100644 --- a/crates/bevy_ecs/src/system/commands/error_handler.rs +++ b/crates/bevy_ecs/src/system/commands/error_handler.rs @@ -1,27 +1,27 @@ //! This module contains convenience functions that return simple error handlers //! for use with [`Commands::queue_handled`](super::Commands::queue_handled) and [`EntityCommands::queue_handled`](super::EntityCommands::queue_handled). -use crate::{result::Error, world::World}; +use crate::{error::BevyError, world::World}; use log::{error, warn}; /// An error handler that does nothing. -pub fn silent() -> fn(&mut World, Error) { +pub fn silent() -> fn(&mut World, BevyError) { |_, _| {} } /// An error handler that accepts an error and logs it with [`warn!`]. -pub fn warn() -> fn(&mut World, Error) { +pub fn warn() -> fn(&mut World, BevyError) { |_, error| warn!("{error}") } /// An error handler that accepts an error and logs it with [`error!`]. -pub fn error() -> fn(&mut World, Error) { +pub fn error() -> fn(&mut World, BevyError) { |_, error| error!("{error}") } /// An error handler that accepts an error and panics with the error in /// the panic message. -pub fn panic() -> fn(&mut World, Error) { +pub fn panic() -> fn(&mut World, BevyError) { |_, error| panic!("{error}") } @@ -30,7 +30,7 @@ pub fn panic() -> fn(&mut World, Error) { /// `GLOBAL_ERROR_HANDLER` will be used instead, enabling error handler customization. #[cfg(not(feature = "configurable_error_handler"))] #[inline] -pub fn default() -> fn(&mut World, Error) { +pub fn default() -> fn(&mut World, BevyError) { panic() } @@ -48,7 +48,7 @@ pub fn default() -> fn(&mut World, Error) { /// // initialize Bevy App here /// ``` #[cfg(feature = "configurable_error_handler")] -pub static GLOBAL_ERROR_HANDLER: std::sync::OnceLock = +pub static GLOBAL_ERROR_HANDLER: std::sync::OnceLock = std::sync::OnceLock::new(); /// The default error handler. This defaults to [`panic()`]. If the @@ -56,6 +56,6 @@ pub static GLOBAL_ERROR_HANDLER: std::sync::OnceLock = /// [`GLOBAL_ERROR_HANDLER`] will be used instead, enabling error handler customization. #[cfg(feature = "configurable_error_handler")] #[inline] -pub fn default() -> fn(&mut World, Error) { +pub fn default() -> fn(&mut World, BevyError) { *GLOBAL_ERROR_HANDLER.get_or_init(|| panic()) } diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index 442c4185dc464..46ff0d4b6c469 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -21,10 +21,10 @@ use crate::{ change_detection::{MaybeLocation, Mut}, component::{Component, ComponentId, Mutable}, entity::{Entities, Entity, EntityClonerBuilder, EntityDoesNotExistError}, + error::BevyError, event::Event, observer::{Observer, TriggerTargets}, resource::Resource, - result::Error, schedule::ScheduleLabel, system::{ command::HandleError, entity_command::CommandWithEntity, input::SystemInput, Deferred, @@ -88,7 +88,7 @@ use crate::{ /// /// # Error handling /// -/// A [`Command`] can return a [`Result`](crate::result::Result), +/// A [`Command`] can return a [`Result`](crate::error::Result), /// which will be passed to an error handler if the `Result` is an error. /// /// Error handlers are functions/closures of the form `fn(&mut World, Error)`. @@ -639,7 +639,7 @@ impl<'w, 's> Commands<'w, 's> { pub fn queue_handled + HandleError, T>( &mut self, command: C, - error_handler: fn(&mut World, Error), + error_handler: fn(&mut World, BevyError), ) { self.queue_internal(command.handle_error_with(error_handler)); } @@ -1152,7 +1152,7 @@ impl<'w, 's> Commands<'w, 's> { /// /// # Error handling /// -/// An [`EntityCommand`] can return a [`Result`](crate::result::Result), +/// An [`EntityCommand`] can return a [`Result`](crate::error::Result), /// which will be passed to an error handler if the `Result` is an error. /// /// Error handlers are functions/closures of the form `fn(&mut World, Error)`. @@ -1845,7 +1845,7 @@ impl<'a> EntityCommands<'a> { pub fn queue_handled + CommandWithEntity, T, M>( &mut self, command: C, - error_handler: fn(&mut World, Error), + error_handler: fn(&mut World, BevyError), ) -> &mut Self { self.commands .queue_handled(command.with_entity(self.entity), error_handler); diff --git a/crates/bevy_ecs/src/system/mod.rs b/crates/bevy_ecs/src/system/mod.rs index a8a3404537671..3154c6e6f70b9 100644 --- a/crates/bevy_ecs/src/system/mod.rs +++ b/crates/bevy_ecs/src/system/mod.rs @@ -82,7 +82,7 @@ //! # System return type //! //! Systems added to a schedule through [`add_systems`](crate::schedule::Schedule) may either return -//! empty `()` or a [`Result`](crate::result::Result). Other contexts (like one shot systems) allow +//! empty `()` or a [`Result`](crate::error::Result). Other contexts (like one shot systems) allow //! systems to return arbitrary values. //! //! # System parameter list @@ -335,11 +335,11 @@ mod tests { change_detection::DetectChanges, component::{Component, Components}, entity::{Entities, Entity}, + error::{BevyError, Result}, prelude::{AnyOf, EntityRef}, query::{Added, Changed, Or, With, Without}, removal_detection::RemovedComponents, resource::Resource, - result::Result, schedule::{ common_conditions::resource_exists, ApplyDeferred, Condition, IntoSystemConfigs, Schedule, @@ -1810,7 +1810,7 @@ mod tests { #[should_panic] fn simple_fallible_system() { fn sys() -> Result { - Err("error")?; + Err(BevyError::message("error"))?; Ok(()) } diff --git a/crates/bevy_ecs/src/system/observer_system.rs b/crates/bevy_ecs/src/system/observer_system.rs index 8a8f82d99e361..17dd4fb01784a 100644 --- a/crates/bevy_ecs/src/system/observer_system.rs +++ b/crates/bevy_ecs/src/system/observer_system.rs @@ -4,9 +4,9 @@ use core::marker::PhantomData; use crate::{ archetype::ArchetypeComponentId, component::{ComponentId, Tick}, + error::Result, prelude::{Bundle, Trigger}, query::Access, - result::Result, schedule::{Fallible, Infallible}, system::{input::SystemIn, System}, world::{unsafe_world_cell::UnsafeWorldCell, DeferredWorld, World}, diff --git a/crates/bevy_ecs/src/system/schedule_system.rs b/crates/bevy_ecs/src/system/schedule_system.rs index e0005f06f46d9..749060d2b968a 100644 --- a/crates/bevy_ecs/src/system/schedule_system.rs +++ b/crates/bevy_ecs/src/system/schedule_system.rs @@ -3,8 +3,8 @@ use alloc::{borrow::Cow, vec::Vec}; use crate::{ archetype::ArchetypeComponentId, component::{ComponentId, Tick}, + error::Result, query::Access, - result::Result, system::{input::SystemIn, BoxedSystem, System}, world::{unsafe_world_cell::UnsafeWorldCell, DeferredWorld, World}, }; diff --git a/examples/ecs/fallible_systems.rs b/examples/ecs/fallible_systems.rs index 39d29fbebdc05..b9592368c02ba 100644 --- a/examples/ecs/fallible_systems.rs +++ b/examples/ecs/fallible_systems.rs @@ -28,7 +28,7 @@ fn main() { // systems in a given `App`. Here we set the global error handler using one of the built-in // error handlers. Bevy provides built-in handlers for `panic`, `error`, `warn`, `info`, // `debug`, `trace` and `ignore`. - app.set_system_error_handler(bevy::ecs::result::warn); + app.set_system_error_handler(bevy::ecs::error::warn); // Additionally, you can set a custom error handler per `Schedule`. This will take precedence // over the global error handler. @@ -138,7 +138,7 @@ fn fallible_observer( ) -> Result { let mut transform = world .get_mut::(trigger.target) - .ok_or("No transform found.")?; + .ok_or_message("No transform found.")?; *step = if transform.translation.x > 3. { -0.1 @@ -162,7 +162,7 @@ fn failing_system(world: &mut World) -> Result { // which we can call `?` to propagate the error. .get_resource::() // We can provide a `str` here because `Box` implements `From<&str>`. - .ok_or("Resource not initialized")?; + .ok_or_message("Resource not initialized")?; Ok(()) } From d488edd15fd1b210eee6775ac723a9fcf57c7b13 Mon Sep 17 00:00:00 2001 From: Carter Anderson Date: Mon, 3 Mar 2025 11:57:49 -0800 Subject: [PATCH 02/18] typo --- crates/bevy_ecs/src/error/bevy_error.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/error/bevy_error.rs b/crates/bevy_ecs/src/error/bevy_error.rs index cca2b7672553b..68b90f45f960a 100644 --- a/crates/bevy_ecs/src/error/bevy_error.rs +++ b/crates/bevy_ecs/src/error/bevy_error.rs @@ -19,7 +19,7 @@ macro_rules! capture_backtrace { }; } -/// The built in "universal" Bevy error type. This has a blanket [`From`] impl for an type that implements Rust's [`Error`], +/// 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 From b5b84a51857e9efaf208870f901e08e041c8add7 Mon Sep 17 00:00:00 2001 From: Carter Anderson Date: Mon, 3 Mar 2025 12:12:00 -0800 Subject: [PATCH 03/18] Remove macro/option backtrace abstraction --- crates/bevy_ecs/src/error/bevy_error.rs | 36 +++++++++---------------- 1 file changed, 13 insertions(+), 23 deletions(-) diff --git a/crates/bevy_ecs/src/error/bevy_error.rs b/crates/bevy_ecs/src/error/bevy_error.rs index 68b90f45f960a..db644757fb9a8 100644 --- a/crates/bevy_ecs/src/error/bevy_error.rs +++ b/crates/bevy_ecs/src/error/bevy_error.rs @@ -2,23 +2,8 @@ use alloc::boxed::Box; use core::{ error::Error, fmt::{Debug, Display}, - sync::atomic::AtomicUsize, }; -#[cfg(feature = "backtrace")] -macro_rules! capture_backtrace { - () => { - Some(std::backtrace::Backtrace::capture()) - }; -} - -#[cfg(not(feature = "backtrace"))] -macro_rules! capture_backtrace { - () => { - None - }; -} - /// 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. /// @@ -49,7 +34,8 @@ impl BevyError { pub fn message(message: M) -> Self { BevyError { inner: Box::new(ErrorImpl { - backtrace: capture_backtrace!(), + #[cfg(feature = "backtrace")] + backtrace: std::backtrace::Backtrace::capture(), error: MessageError(message), }), } @@ -63,20 +49,20 @@ impl BevyError { trait InnerError: Send + Sync + 'static { #[cfg(feature = "backtrace")] - fn backtrace(&self) -> Option<&std::backtrace::Backtrace>; + fn backtrace(&self) -> &std::backtrace::Backtrace; fn error(&self) -> &(dyn Error + Send + Sync + 'static); } struct ErrorImpl { error: E, #[cfg(feature = "backtrace")] - backtrace: Option, + backtrace: std::backtrace::Backtrace, } impl InnerError for ErrorImpl { #[cfg(feature = "backtrace")] - fn backtrace(&self) -> Option<&std::backtrace::Backtrace> { - self.backtrace.as_ref() + fn backtrace(&self) -> &std::backtrace::Backtrace { + &self.backtrace } fn error(&self) -> &(dyn Error + Send + Sync + 'static) { @@ -93,7 +79,8 @@ where BevyError { inner: Box::new(ErrorImpl { error, - backtrace: capture_backtrace!(), + #[cfg(feature = "backtrace")] + backtrace: std::backtrace::Backtrace::capture(), }), } } @@ -110,7 +97,8 @@ impl Debug for BevyError { fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { writeln!(f, "{:?}", self.inner.error())?; #[cfg(feature = "backtrace")] - if let Some(backtrace) = self.inner.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"); @@ -152,7 +140,9 @@ impl Debug for BevyError { } } -static SKIP_NORMAL_BACKTRACE: AtomicUsize = AtomicUsize::new(0); +#[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")] From cc8db0221dc6cdb1b3edc9d1ce1c8bb029b0e381 Mon Sep 17 00:00:00 2001 From: Carter Anderson Date: Mon, 3 Mar 2025 12:13:20 -0800 Subject: [PATCH 04/18] less weird line break --- crates/bevy_app/Cargo.toml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/bevy_app/Cargo.toml b/crates/bevy_app/Cargo.toml index 0f803367fcc94..010e789fa5f92 100644 --- a/crates/bevy_app/Cargo.toml +++ b/crates/bevy_app/Cargo.toml @@ -42,8 +42,8 @@ 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. +## Will set the BevyError panic hook, which gives cleaner filtered backtraces when +## a BevyError is hit. error_panic_hook = [] # Platform Compatibility From a467275d24e832e1813ba2dba91478d75740ddd7 Mon Sep 17 00:00:00 2001 From: Carter Anderson Date: Mon, 3 Mar 2025 13:21:55 -0800 Subject: [PATCH 05/18] Only register panic handler once --- crates/bevy_app/src/panic_handler.rs | 28 ++++++++++++++++++------- crates/bevy_ecs/src/error/bevy_error.rs | 11 +++++----- 2 files changed, 26 insertions(+), 13 deletions(-) diff --git a/crates/bevy_app/src/panic_handler.rs b/crates/bevy_app/src/panic_handler.rs index c3c67ba1a64e8..7806c2e883d70 100644 --- a/crates/bevy_app/src/panic_handler.rs +++ b/crates/bevy_app/src/panic_handler.rs @@ -39,15 +39,27 @@ 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")] + { + 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. + } + }); } - #[cfg(all(feature = "error_panic_hook", feature = "std"))] - bevy_ecs::error::set_bevy_error_panic_hook(); } } diff --git a/crates/bevy_ecs/src/error/bevy_error.rs b/crates/bevy_ecs/src/error/bevy_error.rs index db644757fb9a8..5e278c1b5fd9f 100644 --- a/crates/bevy_ecs/src/error/bevy_error.rs +++ b/crates/bevy_ecs/src/error/bevy_error.rs @@ -146,9 +146,10 @@ static SKIP_NORMAL_BACKTRACE: core::sync::atomic::AtomicUsize = /// When called, this will skip the currently configured panic hook when a [`BevyError`] backtrace has already been printed. #[cfg(feature = "std")] -pub fn set_bevy_error_panic_hook() { - let hook = std::panic::take_hook(); - std::panic::set_hook(Box::new(move |info| { +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}"); @@ -159,8 +160,8 @@ pub fn set_bevy_error_panic_hook() { return; } - hook(info); - })); + current_hook(info); + } } /// An error containing a print-able message. From 3520b6902684b8a39e923382a18a3b610fc4559e Mon Sep 17 00:00:00 2001 From: Carter Anderson Date: Mon, 3 Mar 2025 13:27:12 -0800 Subject: [PATCH 06/18] Fix docs --- crates/bevy_ecs/src/observer/runner.rs | 2 +- crates/bevy_ecs/src/query/state.rs | 2 +- crates/bevy_ecs/src/schedule/schedule.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/bevy_ecs/src/observer/runner.rs b/crates/bevy_ecs/src/observer/runner.rs index 972f58889b19b..a6372df374d7d 100644 --- a/crates/bevy_ecs/src/observer/runner.rs +++ b/crates/bevy_ecs/src/observer/runner.rs @@ -321,7 +321,7 @@ impl Observer { /// Set the error handler to use for this observer. /// - /// See the [`result` module-level documentation](crate::result) for more information. + /// See the [`error` module-level documentation](crate::error) for more information. pub fn with_error_handler(mut self, error_handler: fn(BevyError, SystemErrorContext)) -> Self { self.error_handler = Some(error_handler); self diff --git a/crates/bevy_ecs/src/query/state.rs b/crates/bevy_ecs/src/query/state.rs index ce58ee9cf4296..c4d9ba000b802 100644 --- a/crates/bevy_ecs/src/query/state.rs +++ b/crates/bevy_ecs/src/query/state.rs @@ -1661,7 +1661,7 @@ impl QueryState { /// /// This allows you to globally control how errors are handled in your application, /// by setting up a custom error handler. - /// See the [`bevy_ecs::result`] module docs for more information! + /// See the [`bevy_ecs::error`] module docs for more information! /// Commonly, you might want to panic on an error during development, but log the error and continue /// execution in production. /// diff --git a/crates/bevy_ecs/src/schedule/schedule.rs b/crates/bevy_ecs/src/schedule/schedule.rs index 2f86810f456f8..0cb2db2de56cb 100644 --- a/crates/bevy_ecs/src/schedule/schedule.rs +++ b/crates/bevy_ecs/src/schedule/schedule.rs @@ -401,7 +401,7 @@ impl Schedule { /// Set the error handler to use for systems that return a [`Result`](crate::error::Result). /// - /// See the [`result` module-level documentation](crate::result) for more information. + /// See the [`error` module-level documentation](crate::error) for more information. pub fn set_error_handler(&mut self, error_handler: fn(BevyError, SystemErrorContext)) { self.error_handler = Some(error_handler); } From 4b793e514f69df038576697c000fc47419ba613a Mon Sep 17 00:00:00 2001 From: Carter Anderson Date: Tue, 4 Mar 2025 13:25:19 -0800 Subject: [PATCH 07/18] Filtered backtrace test --- crates/bevy_ecs/src/error/bevy_error.rs | 58 ++++++++++++++++++++++++- 1 file changed, 57 insertions(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/error/bevy_error.rs b/crates/bevy_ecs/src/error/bevy_error.rs index 5e278c1b5fd9f..1082251eea774 100644 --- a/crates/bevy_ecs/src/error/bevy_error.rs +++ b/crates/bevy_ecs/src/error/bevy_error.rs @@ -131,7 +131,7 @@ impl Debug for BevyError { if std::thread::panicking() { SKIP_NORMAL_BACKTRACE.store(1, core::sync::atomic::Ordering::Relaxed); } - writeln!(f, "note: Some \"noisy\" backtrace lines have been filtered out. Run with `BEVY_BACKTRACE=full` for a verbose backtrace.")?; + writeln!(f, "{FILTER_MESSAGE}")?; } } } @@ -140,6 +140,8 @@ impl Debug for BevyError { } } +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); @@ -206,3 +208,57 @@ impl OkOrMessage for Option { } } } + +#[cfg(test)] +mod tests { + use crate::error::{bevy_error::FILTER_MESSAGE, Result}; + use alloc::format; + + fn i_fail() -> Result { + let _: usize = "I am not a number".parse()?; + Ok(()) + } + + #[test] + fn filtered_backtrace_test() { + // 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 = format!("{error:?}"); + let mut lines = debug_message.lines(); + assert_eq!( + "ParseIntError { kind: InvalidDigit }", + lines.next().unwrap() + ); + assert_eq!( + " 2: bevy_ecs::error::bevy_error::tests::i_fail", + lines.next().unwrap() + ); + lines.next().unwrap(); + assert_eq!( + " 3: bevy_ecs::error::bevy_error::tests::filtered_backtrace_test", + lines.next().unwrap() + ); + lines.next().unwrap(); + assert_eq!( + " 4: bevy_ecs::error::bevy_error::tests::filtered_backtrace_test::{{closure}}", + lines.next().unwrap() + ); + lines.next().unwrap(); + assert_eq!( + " 5: core::ops::function::FnOnce::call_once", + lines.next().unwrap() + ); + lines.next().unwrap(); + assert_eq!( + " 6: core::ops::function::FnOnce::call_once", + lines.next().unwrap() + ); + lines.next().unwrap(); + assert_eq!(FILTER_MESSAGE, lines.next().unwrap()); + assert!(lines.next().is_none()); + } +} From a713476b0e5f02f7806ba19484b741e3940e871a Mon Sep 17 00:00:00 2001 From: Carter Anderson Date: Tue, 4 Mar 2025 13:27:37 -0800 Subject: [PATCH 08/18] explain console_error_panic_hook --- crates/bevy_app/src/panic_handler.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/bevy_app/src/panic_handler.rs b/crates/bevy_app/src/panic_handler.rs index 7806c2e883d70..e73c3d009845d 100644 --- a/crates/bevy_app/src/panic_handler.rs +++ b/crates/bevy_app/src/panic_handler.rs @@ -45,6 +45,7 @@ impl Plugin for PanicHandlerPlugin { 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"))] From eb0778d9d97082f4216540e0c3cff62d5cbd60dd Mon Sep 17 00:00:00 2001 From: Carter Anderson Date: Tue, 4 Mar 2025 13:37:31 -0800 Subject: [PATCH 09/18] Fix path --- crates/bevy_ecs/src/system/system_registry.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/system/system_registry.rs b/crates/bevy_ecs/src/system/system_registry.rs index cf42d0d875896..719a4885d0754 100644 --- a/crates/bevy_ecs/src/system/system_registry.rs +++ b/crates/bevy_ecs/src/system/system_registry.rs @@ -770,7 +770,7 @@ mod tests { #[test] fn cached_system_into_same_system_type() { - use crate::result::Result; + use crate::error::Result; struct Foo; impl IntoSystem<(), Result<()>, ()> for Foo { From dbdc9dffe9130fb29c8945e26438ae3e9c296b9f Mon Sep 17 00:00:00 2001 From: Carter Anderson Date: Tue, 4 Mar 2025 13:51:44 -0800 Subject: [PATCH 10/18] Temporary test change for debugging purposes --- crates/bevy_ecs/src/error/bevy_error.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/crates/bevy_ecs/src/error/bevy_error.rs b/crates/bevy_ecs/src/error/bevy_error.rs index 1082251eea774..ceedd3e5f6982 100644 --- a/crates/bevy_ecs/src/error/bevy_error.rs +++ b/crates/bevy_ecs/src/error/bevy_error.rs @@ -233,10 +233,13 @@ mod tests { "ParseIntError { kind: InvalidDigit }", lines.next().unwrap() ); - assert_eq!( - " 2: bevy_ecs::error::bevy_error::tests::i_fail", - lines.next().unwrap() - ); + if " 2: bevy_ecs::error::bevy_error::tests::i_fail" != lines.next().unwrap() { + panic!("{}", error.inner.backtrace()); + } + // assert_eq!( + // " 2: bevy_ecs::error::bevy_error::tests::i_fail", + // lines.next().unwrap(), + // ); lines.next().unwrap(); assert_eq!( " 3: bevy_ecs::error::bevy_error::tests::filtered_backtrace_test", From 41462ec1867197feba9a4b43fab1dbd4ef9d8f07 Mon Sep 17 00:00:00 2001 From: Carter Anderson Date: Tue, 4 Mar 2025 13:52:36 -0800 Subject: [PATCH 11/18] Put FILTER_MESSAGE behind "backtrace" feature --- crates/bevy_ecs/src/error/bevy_error.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/bevy_ecs/src/error/bevy_error.rs b/crates/bevy_ecs/src/error/bevy_error.rs index ceedd3e5f6982..6342fdfd011db 100644 --- a/crates/bevy_ecs/src/error/bevy_error.rs +++ b/crates/bevy_ecs/src/error/bevy_error.rs @@ -140,6 +140,7 @@ impl Debug for BevyError { } } +#[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")] From 5be3f5bf2478424cc05cd19bcc44f68c985c57f6 Mon Sep 17 00:00:00 2001 From: Carter Anderson Date: Wed, 5 Mar 2025 22:34:02 -0800 Subject: [PATCH 12/18] Make backtrace test more resilient --- crates/bevy_ecs/src/error/bevy_error.rs | 87 ++++++++++++++----------- 1 file changed, 49 insertions(+), 38 deletions(-) diff --git a/crates/bevy_ecs/src/error/bevy_error.rs b/crates/bevy_ecs/src/error/bevy_error.rs index 6342fdfd011db..1dadf26c6fa89 100644 --- a/crates/bevy_ecs/src/error/bevy_error.rs +++ b/crates/bevy_ecs/src/error/bevy_error.rs @@ -103,19 +103,25 @@ impl Debug for BevyError { 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 = false; + let mut skip_next_location_line = false; for line in backtrace_str.split('\n') { - if skip_next { - skip_next = false; - continue; - } if !full_backtrace { - if line.starts_with(" 0: >::from") { - skip_next = true; + if skip_next_location_line { + if line.starts_with(" at") { + continue; + } + skip_next_location_line = false; + } + if line.starts_with(" 0: std::backtrace::Backtrace::create") { + skip_next_location_line = true; + continue; + } + if (line.starts_with(" 0:") ||line.starts_with(" 1:")) && line.contains(">::from") { + skip_next_location_line = true; continue; } - if line.starts_with(" 1: as core::ops::try_trait::FromResidual>>::from_residual") { - skip_next = true; + if (line.starts_with(" 1:") ||line.starts_with(" 2:")) && line.contains(" as core::ops::try_trait::FromResidual>>::from_residual") { + skip_next_location_line = true; continue; } if line.contains("__rust_begin_short_backtrace") { @@ -221,6 +227,7 @@ mod tests { } #[test] + #[cfg(not(miri))] // miri backtraces are weird fn filtered_backtrace_test() { // 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, @@ -229,40 +236,44 @@ mod tests { let error = i_fail().err().unwrap(); let debug_message = format!("{error:?}"); - let mut lines = debug_message.lines(); + let mut lines = debug_message.lines().peekable(); assert_eq!( "ParseIntError { kind: InvalidDigit }", lines.next().unwrap() ); - if " 2: bevy_ecs::error::bevy_error::tests::i_fail" != lines.next().unwrap() { - panic!("{}", error.inner.backtrace()); + + let mut skip = false; + if let Some(line) = lines.peek() { + if &line[6..] == "std::backtrace::Backtrace::create" { + skip = true; + } } - // assert_eq!( - // " 2: bevy_ecs::error::bevy_error::tests::i_fail", - // lines.next().unwrap(), - // ); - lines.next().unwrap(); - assert_eq!( - " 3: bevy_ecs::error::bevy_error::tests::filtered_backtrace_test", - lines.next().unwrap() - ); - lines.next().unwrap(); - assert_eq!( - " 4: bevy_ecs::error::bevy_error::tests::filtered_backtrace_test::{{closure}}", - lines.next().unwrap() - ); - lines.next().unwrap(); - assert_eq!( - " 5: core::ops::function::FnOnce::call_once", - lines.next().unwrap() - ); - lines.next().unwrap(); - assert_eq!( - " 6: core::ops::function::FnOnce::call_once", - lines.next().unwrap() - ); - lines.next().unwrap(); - assert_eq!(FILTER_MESSAGE, lines.next().unwrap()); + + if skip { + lines.next().unwrap(); + } + + let expected_lines = alloc::vec![ + "bevy_ecs::error::bevy_error::tests::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", + "core::ops::function::FnOnce::call_once", + ]; + + for expected in expected_lines { + let mut line = lines.next().unwrap(); + if line.starts_with(" at") { + line = lines.next().unwrap(); + } + assert_eq!(&line[6..], expected); + } + + let mut line = lines.next().unwrap(); + if line.starts_with(" at") { + line = lines.next().unwrap(); + } + assert_eq!(FILTER_MESSAGE, line); assert!(lines.next().is_none()); } } From 260ad5fe76071d4282e2cfac253c97865d0424d0 Mon Sep 17 00:00:00 2001 From: Carter Anderson Date: Thu, 6 Mar 2025 00:04:54 -0800 Subject: [PATCH 13/18] More debugging --- crates/bevy_ecs/src/error/bevy_error.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/bevy_ecs/src/error/bevy_error.rs b/crates/bevy_ecs/src/error/bevy_error.rs index 1dadf26c6fa89..67daee8a4b967 100644 --- a/crates/bevy_ecs/src/error/bevy_error.rs +++ b/crates/bevy_ecs/src/error/bevy_error.rs @@ -232,10 +232,12 @@ mod tests { // 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 = format!("{error:?}"); + std::println!("{debug_message}"); let mut lines = debug_message.lines().peekable(); assert_eq!( "ParseIntError { kind: InvalidDigit }", From 17e0adec73774524ebc2009da20fbe95c606692a Mon Sep 17 00:00:00 2001 From: Carter Anderson Date: Thu, 6 Mar 2025 12:53:02 -0800 Subject: [PATCH 14/18] Alright this one for the win --- crates/bevy_ecs/src/error/bevy_error.rs | 40 +++++++++++++++++++------ 1 file changed, 31 insertions(+), 9 deletions(-) diff --git a/crates/bevy_ecs/src/error/bevy_error.rs b/crates/bevy_ecs/src/error/bevy_error.rs index 67daee8a4b967..d501d4c4ea955 100644 --- a/crates/bevy_ecs/src/error/bevy_error.rs +++ b/crates/bevy_ecs/src/error/bevy_error.rs @@ -237,13 +237,13 @@ mod tests { let error = i_fail().err().unwrap(); let debug_message = format!("{error:?}"); - std::println!("{debug_message}"); 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" { @@ -260,22 +260,44 @@ mod tests { "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", - "core::ops::function::FnOnce::call_once", ]; for expected in expected_lines { - let mut line = lines.next().unwrap(); + 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") { - line = lines.next().unwrap(); + skip = true; } - assert_eq!(&line[6..], expected); } - let mut line = lines.next().unwrap(); - if line.starts_with(" at") { - line = lines.next().unwrap(); + if skip { + lines.next().unwrap(); } - assert_eq!(FILTER_MESSAGE, line); + assert_eq!(FILTER_MESSAGE, lines.next().unwrap()); assert!(lines.next().is_none()); } } From c9e8be5a9945d8078557c72ba9400f58ef2c3cd3 Mon Sep 17 00:00:00 2001 From: Carter Anderson Date: Thu, 6 Mar 2025 13:16:34 -0800 Subject: [PATCH 15/18] Thin BevyError pointer, From<&str> impl replaces MessageError --- crates/bevy_ecs/src/error/bevy_error.rs | 97 ++++--------------------- crates/bevy_ecs/src/lib.rs | 2 +- crates/bevy_ecs/src/observer/runner.rs | 4 +- crates/bevy_ecs/src/system/mod.rs | 4 +- examples/ecs/fallible_systems.rs | 12 +-- 5 files changed, 27 insertions(+), 92 deletions(-) diff --git a/crates/bevy_ecs/src/error/bevy_error.rs b/crates/bevy_ecs/src/error/bevy_error.rs index d501d4c4ea955..1a78681732bf9 100644 --- a/crates/bevy_ecs/src/error/bevy_error.rs +++ b/crates/bevy_ecs/src/error/bevy_error.rs @@ -26,59 +26,37 @@ use core::{ /// } /// ``` pub struct BevyError { - inner: Box, + inner: Box, } impl BevyError { - /// Creates a new error with the given message. - pub fn message(message: M) -> Self { - BevyError { - inner: Box::new(ErrorImpl { - #[cfg(feature = "backtrace")] - backtrace: std::backtrace::Backtrace::capture(), - error: MessageError(message), - }), - } - } - /// Attempts to downcast the internal error to the given type. pub fn downcast_ref(&self) -> Option<&E> { - self.inner.error().downcast_ref::() + self.inner.error.downcast_ref::() } } -trait InnerError: Send + Sync + 'static { - #[cfg(feature = "backtrace")] - fn backtrace(&self) -> &std::backtrace::Backtrace; - fn error(&self) -> &(dyn Error + Send + Sync + 'static); -} - -struct ErrorImpl { - error: E, +/// This type exists (rather than having a `BevyError(Box, #[cfg(feature = "backtrace")] backtrace: std::backtrace::Backtrace, } -impl InnerError for ErrorImpl { - #[cfg(feature = "backtrace")] - fn backtrace(&self) -> &std::backtrace::Backtrace { - &self.backtrace - } - - fn error(&self) -> &(dyn Error + Send + Sync + 'static) { - &self.error - } -} - +// NOTE: writing the impl this way gives us From<&str> ... nice! impl From for BevyError where - E: Error + Send + Sync + 'static, + Box: From, { #[cold] fn from(error: E) -> Self { BevyError { - inner: Box::new(ErrorImpl { - error, + inner: Box::new(InnerBevyError { + error: error.into(), #[cfg(feature = "backtrace")] backtrace: std::backtrace::Backtrace::capture(), }), @@ -88,17 +66,17 @@ where impl Display for BevyError { fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { - writeln!(f, "{}", self.inner.error())?; + 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())?; + writeln!(f, "{:?}", self.inner.error)?; #[cfg(feature = "backtrace")] { - let backtrace = self.inner.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"); @@ -173,49 +151,6 @@ pub fn bevy_error_panic_hook( } } -/// An error containing a print-able message. -pub struct MessageError(pub(crate) M); - -impl Display for MessageError -where - M: Display + Debug, -{ - fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { - Display::fmt(&self.0, f) - } -} -impl Debug for MessageError -where - M: Display + Debug, -{ - fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { - Debug::fmt(&self.0, f) - } -} - -impl Error for MessageError where M: Display + Debug + 'static {} - -/// Returns a Result containing a given message if the given value does not exist. -pub trait OkOrMessage { - /// Returns a Result containing a given message if the given value does not exist. - fn ok_or_message( - self, - message: M, - ) -> Result>; -} - -impl OkOrMessage for Option { - fn ok_or_message( - self, - message: M, - ) -> Result> { - match self { - Some(value) => Ok(value), - None => Err(MessageError(message)), - } - } -} - #[cfg(test)] mod tests { use crate::error::{bevy_error::FILTER_MESSAGE, Result}; diff --git a/crates/bevy_ecs/src/lib.rs b/crates/bevy_ecs/src/lib.rs index 79496f7138141..d7988bcc996f8 100644 --- a/crates/bevy_ecs/src/lib.rs +++ b/crates/bevy_ecs/src/lib.rs @@ -74,7 +74,7 @@ pub mod prelude { children, component::Component, entity::{Entity, EntityBorrow, EntityMapper}, - error::{BevyError, OkOrMessage, Result}, + error::{BevyError, Result}, event::{Event, EventMutator, EventReader, EventWriter, Events}, hierarchy::{ChildOf, ChildSpawner, ChildSpawnerCommands, Children}, name::{Name, NameOrEntity}, diff --git a/crates/bevy_ecs/src/observer/runner.rs b/crates/bevy_ecs/src/observer/runner.rs index a6372df374d7d..7485af7f878b1 100644 --- a/crates/bevy_ecs/src/observer/runner.rs +++ b/crates/bevy_ecs/src/observer/runner.rs @@ -488,7 +488,7 @@ mod tests { #[should_panic(expected = "I failed!")] fn test_fallible_observer() { fn system(_: Trigger) -> Result { - Err(BevyError::message("I failed!")) + Err("I failed!".into()) } let mut world = World::default(); @@ -504,7 +504,7 @@ mod tests { fn system(_: Trigger, mut ran: ResMut) -> Result { ran.0 = true; - Err(BevyError::message("I failed!")) + Err("I failed!".into()) } let mut world = World::default(); diff --git a/crates/bevy_ecs/src/system/mod.rs b/crates/bevy_ecs/src/system/mod.rs index 3154c6e6f70b9..9be853bf12bd1 100644 --- a/crates/bevy_ecs/src/system/mod.rs +++ b/crates/bevy_ecs/src/system/mod.rs @@ -335,7 +335,7 @@ mod tests { change_detection::DetectChanges, component::{Component, Components}, entity::{Entities, Entity}, - error::{BevyError, Result}, + error::Result, prelude::{AnyOf, EntityRef}, query::{Added, Changed, Or, With, Without}, removal_detection::RemovedComponents, @@ -1810,7 +1810,7 @@ mod tests { #[should_panic] fn simple_fallible_system() { fn sys() -> Result { - Err(BevyError::message("error"))?; + Err("error")?; Ok(()) } diff --git a/examples/ecs/fallible_systems.rs b/examples/ecs/fallible_systems.rs index b9592368c02ba..bfd94b84d5d2c 100644 --- a/examples/ecs/fallible_systems.rs +++ b/examples/ecs/fallible_systems.rs @@ -18,9 +18,9 @@ fn main() { app.add_plugins(MeshPickingPlugin); // Fallible systems can be used the same way as regular systems. The only difference is they - // return a `Result<(), Box>` instead of a `()` (unit) type. Bevy will handle both + // return a `Result<(), BevyError>` instead of a `()` (unit) type. Bevy will handle both // types of systems the same way, except for the error handling. - app.add_systems(Startup, (setup, failing_system)); + app.add_systems(Startup, setup); // By default, fallible systems that return an error will panic. // @@ -36,7 +36,7 @@ fn main() { // In this instance we provide our own non-capturing closure that coerces to the expected error // handler function pointer: // - // fn(bevy_ecs::result::Error, bevy_ecs::result::SystemErrorContext) + // fn(bevy_ecs::error::BevyError, bevy_ecs::error::SystemErrorContext) // app.add_systems(PostStartup, failing_system) .get_schedule_mut(PostStartup) @@ -138,7 +138,7 @@ fn fallible_observer( ) -> Result { let mut transform = world .get_mut::(trigger.target) - .ok_or_message("No transform found.")?; + .ok_or("No transform found.")?; *step = if transform.translation.x > 3. { -0.1 @@ -161,8 +161,8 @@ fn failing_system(world: &mut World) -> Result { // `get_resource` returns an `Option`, so we use `ok_or` to convert it to a `Result` on // which we can call `?` to propagate the error. .get_resource::() - // We can provide a `str` here because `Box` implements `From<&str>`. - .ok_or_message("Resource not initialized")?; + // We can provide a `str` here because `BevyError` implements `From<&str>`. + .ok_or("Resource not initialized")?; Ok(()) } From 8b2269548dd84c712349fb8d571c02f17ecfbe2b Mon Sep 17 00:00:00 2001 From: Carter Anderson Date: Thu, 6 Mar 2025 13:17:22 -0800 Subject: [PATCH 16/18] And now, we debug windows stacktraces! --- crates/bevy_ecs/src/error/bevy_error.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/bevy_ecs/src/error/bevy_error.rs b/crates/bevy_ecs/src/error/bevy_error.rs index 1a78681732bf9..2feb8a0ea36ff 100644 --- a/crates/bevy_ecs/src/error/bevy_error.rs +++ b/crates/bevy_ecs/src/error/bevy_error.rs @@ -172,6 +172,7 @@ mod tests { let error = i_fail().err().unwrap(); let debug_message = format!("{error:?}"); + std::eprintln!("{debug_message}"); let mut lines = debug_message.lines().peekable(); assert_eq!( "ParseIntError { kind: InvalidDigit }", From ca43700cfd9ee78a9825f2b91543133c6e70affe Mon Sep 17 00:00:00 2001 From: Carter Anderson Date: Thu, 6 Mar 2025 13:43:16 -0800 Subject: [PATCH 17/18] Make windows backtraces nicer, but also don't run the test for them because they are deeply unhelpful --- crates/bevy_ecs/src/error/bevy_error.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/crates/bevy_ecs/src/error/bevy_error.rs b/crates/bevy_ecs/src/error/bevy_error.rs index 2feb8a0ea36ff..a9e5e8d94d9a7 100644 --- a/crates/bevy_ecs/src/error/bevy_error.rs +++ b/crates/bevy_ecs/src/error/bevy_error.rs @@ -90,15 +90,19 @@ impl Debug for BevyError { } skip_next_location_line = false; } - if line.starts_with(" 0: std::backtrace::Backtrace::create") { + if line.contains("std::backtrace_rs::backtrace::") { skip_next_location_line = true; continue; } - if (line.starts_with(" 0:") ||line.starts_with(" 1:")) && line.contains(">::from") { + if line.contains("std::backtrace::Backtrace::") { skip_next_location_line = true; continue; } - if (line.starts_with(" 1:") ||line.starts_with(" 2:")) && line.contains(" as core::ops::try_trait::FromResidual>>::from_residual") { + if line.contains(">::from") { + skip_next_location_line = true; + continue; + } + if line.contains(" as core::ops::try_trait::FromResidual>>::from_residual") { skip_next_location_line = true; continue; } @@ -163,6 +167,7 @@ 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() { // 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, From b79538f800bf223e4bb8cd7e2edff3b09885e501 Mon Sep 17 00:00:00 2001 From: Carter Anderson Date: Thu, 6 Mar 2025 14:03:01 -0800 Subject: [PATCH 18/18] Move test imports into test --- crates/bevy_ecs/src/error/bevy_error.rs | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/crates/bevy_ecs/src/error/bevy_error.rs b/crates/bevy_ecs/src/error/bevy_error.rs index a9e5e8d94d9a7..73777bad77faf 100644 --- a/crates/bevy_ecs/src/error/bevy_error.rs +++ b/crates/bevy_ecs/src/error/bevy_error.rs @@ -157,18 +157,16 @@ pub fn bevy_error_panic_hook( #[cfg(test)] mod tests { - use crate::error::{bevy_error::FILTER_MESSAGE, Result}; - use alloc::format; - - fn i_fail() -> Result { - let _: usize = "I am not a number".parse()?; - Ok(()) - } #[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. @@ -176,8 +174,7 @@ mod tests { unsafe { std::env::set_var("RUST_BACKTRACE", "1") }; let error = i_fail().err().unwrap(); - let debug_message = format!("{error:?}"); - std::eprintln!("{debug_message}"); + let debug_message = alloc::format!("{error:?}"); let mut lines = debug_message.lines().peekable(); assert_eq!( "ParseIntError { kind: InvalidDigit }", @@ -197,7 +194,7 @@ mod tests { } let expected_lines = alloc::vec![ - "bevy_ecs::error::bevy_error::tests::i_fail", + "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", @@ -238,7 +235,7 @@ mod tests { if skip { lines.next().unwrap(); } - assert_eq!(FILTER_MESSAGE, lines.next().unwrap()); + assert_eq!(super::FILTER_MESSAGE, lines.next().unwrap()); assert!(lines.next().is_none()); } }