diff --git a/crates/bevy_app/src/app.rs b/crates/bevy_app/src/app.rs index 0e13d9f5a6700..941935d4ea96f 100644 --- a/crates/bevy_app/src/app.rs +++ b/crates/bevy_app/src/app.rs @@ -1494,12 +1494,26 @@ impl App { /// # Example /// ``` /// # use bevy_app::*; - /// # use bevy_ecs::error::warn; - /// # fn MyPlugins(_: &mut App) {} - /// App::new() - /// .set_error_handler(warn) - /// .add_plugins(MyPlugins) - /// .run(); + /// # use bevy_ecs::{error::{BevyError, ErrorContext, Result}, system::Commands}; + /// fn exit_on_error<'w, 's>( + /// _: BevyError, + /// _: ErrorContext, + /// mut commands: Commands<'w, 's>, + /// ) -> Commands<'w, 's> { + /// commands.write_message(AppExit::error()); + /// commands + /// } + /// + /// fn fallible_system() -> Result { + /// Err("The system failed".into()) + /// } + /// + /// let mut app = App::new(); + /// app.set_error_handler(exit_on_error) + /// .add_systems(Update, fallible_system) + /// .update(); + /// + /// assert_eq!(app.should_exit(), Some(AppExit::error())); /// ``` /// /// [fallback error handler]: bevy_ecs::error::FallbackErrorHandler diff --git a/crates/bevy_ecs/src/error/handler.rs b/crates/bevy_ecs/src/error/handler.rs index 4541fe3ddbacd..fe4c001380042 100644 --- a/crates/bevy_ecs/src/error/handler.rs +++ b/crates/bevy_ecs/src/error/handler.rs @@ -1,6 +1,10 @@ use core::fmt::Display; -use crate::{change_detection::Tick, error::BevyError, prelude::Resource}; +use crate::{ + change_detection::Tick, + error::BevyError, + prelude::{Commands, Resource}, +}; use bevy_ecs::error::Severity; use bevy_utils::prelude::DebugName; use derive_more::derive::{Deref, DerefMut}; @@ -107,7 +111,11 @@ macro_rules! inner { /// consider setting [`PANIC_ORIGINATES_FROM_ERROR_HANDLER`]. /// This lets the executor know that a panic doesn't need to be /// converted back to a [`BevyError`] and passed to the [`FallbackErrorHandler`]. -pub type ErrorHandler = fn(BevyError, ErrorContext); +/// +/// Error handlers can queue deferred work using the provided [`Commands`]. +/// The [`Commands`] must be returned, even if they are not used. +pub type ErrorHandler = + for<'w, 's> fn(BevyError, ErrorContext, Commands<'w, 's>) -> Commands<'w, 's>; /// Fallback error handler to call when an error is not handled otherwise. /// Defaults to [`match_severity()`]. @@ -138,22 +146,30 @@ std::thread_local! { /// Error handler that defers to an error's [`Severity`]. #[track_caller] #[inline] -pub fn match_severity(err: BevyError, ctx: ErrorContext) { +pub fn match_severity<'w, 's>( + err: BevyError, + ctx: ErrorContext, + commands: Commands<'w, 's>, +) -> Commands<'w, 's> { match err.severity() { - Severity::Ignore => ignore(err, ctx), - Severity::Trace => trace(err, ctx), - Severity::Debug => debug(err, ctx), - Severity::Info => info(err, ctx), - Severity::Warning => warn(err, ctx), - Severity::Error => error(err, ctx), - Severity::Panic => panic(err, ctx), + Severity::Ignore => ignore(err, ctx, commands), + Severity::Trace => trace(err, ctx, commands), + Severity::Debug => debug(err, ctx, commands), + Severity::Info => info(err, ctx, commands), + Severity::Warning => warn(err, ctx, commands), + Severity::Error => error(err, ctx, commands), + Severity::Panic => panic(err, ctx, commands), } } /// Error handler that panics with the system error. #[track_caller] #[inline] -pub fn panic(error: BevyError, ctx: ErrorContext) { +pub fn panic<'w, 's>( + error: BevyError, + ctx: ErrorContext, + _commands: Commands<'w, 's>, +) -> Commands<'w, 's> { #[cfg(feature = "std")] PANIC_ORIGINATES_FROM_ERROR_HANDLER.set(true); inner!(panic, error, ctx); @@ -162,39 +178,70 @@ pub fn panic(error: BevyError, ctx: ErrorContext) { /// Error handler that logs the system error at the `error` level. #[track_caller] #[inline] -pub fn error(error: BevyError, ctx: ErrorContext) { +pub fn error<'w, 's>( + error: BevyError, + ctx: ErrorContext, + commands: Commands<'w, 's>, +) -> Commands<'w, 's> { inner!(log::error, error, ctx); + commands } /// Error handler that logs the system error at the `warn` level. #[track_caller] #[inline] -pub fn warn(error: BevyError, ctx: ErrorContext) { +pub fn warn<'w, 's>( + error: BevyError, + ctx: ErrorContext, + commands: Commands<'w, 's>, +) -> Commands<'w, 's> { inner!(log::warn, error, ctx); + commands } /// Error handler that logs the system error at the `info` level. #[track_caller] #[inline] -pub fn info(error: BevyError, ctx: ErrorContext) { +pub fn info<'w, 's>( + error: BevyError, + ctx: ErrorContext, + commands: Commands<'w, 's>, +) -> Commands<'w, 's> { inner!(log::info, error, ctx); + commands } /// Error handler that logs the system error at the `debug` level. #[track_caller] #[inline] -pub fn debug(error: BevyError, ctx: ErrorContext) { +pub fn debug<'w, 's>( + error: BevyError, + ctx: ErrorContext, + commands: Commands<'w, 's>, +) -> Commands<'w, 's> { inner!(log::debug, error, ctx); + commands } /// Error handler that logs the system error at the `trace` level. #[track_caller] #[inline] -pub fn trace(error: BevyError, ctx: ErrorContext) { +pub fn trace<'w, 's>( + error: BevyError, + ctx: ErrorContext, + commands: Commands<'w, 's>, +) -> Commands<'w, 's> { inner!(log::trace, error, ctx); + commands } /// Error handler that ignores the system error. #[track_caller] #[inline] -pub fn ignore(_: BevyError, _: ErrorContext) {} +pub fn ignore<'w, 's>( + _: BevyError, + _: ErrorContext, + commands: Commands<'w, 's>, +) -> Commands<'w, 's> { + commands +} diff --git a/crates/bevy_ecs/src/error/mod.rs b/crates/bevy_ecs/src/error/mod.rs index a9d167bfeafe0..9bd0d1c40b1a3 100644 --- a/crates/bevy_ecs/src/error/mod.rs +++ b/crates/bevy_ecs/src/error/mod.rs @@ -29,22 +29,29 @@ //! signature: //! //! ```rust,ignore -//! fn(BevyError, ErrorContext) +//! for<'w, 's> fn(BevyError, ErrorContext, Commands<'w, 's>) -> Commands<'w, 's> //! ``` //! //! The [`ErrorContext`] allows you to access additional details relevant to providing //! context surrounding the error – such as the system's [`name`] – in your error messages. //! //! ```rust, ignore -//! use bevy_ecs::error::{BevyError, ErrorContext, FallbackErrorHandler}; +//! use bevy_ecs::{ +//! error::{BevyError, ErrorContext, FallbackErrorHandler}, +//! system::Commands, +//! }; //! use log::trace; //! -//! fn my_error_handler(error: BevyError, ctx: ErrorContext) { +//! fn my_error_handler<'w, 's>( +//! error: BevyError, +//! ctx: ErrorContext, +//! commands: Commands<'w, 's>, +//! ) -> Commands<'w, 's> { //! if ctx.name().ends_with("plz_ignore") { //! trace!("Nothing to see here, move along."); -//! return; +//! return commands; //! } -//! bevy_ecs::error::error(error, ctx); +//! bevy_ecs::error::error(error, ctx, commands) //! } //! //! fn main() { diff --git a/crates/bevy_ecs/src/observer/distributed_storage.rs b/crates/bevy_ecs/src/observer/distributed_storage.rs index cb5a813461334..26dc1f1f4b013 100644 --- a/crates/bevy_ecs/src/observer/distributed_storage.rs +++ b/crates/bevy_ecs/src/observer/distributed_storage.rs @@ -15,7 +15,7 @@ use core::marker::PhantomData; use crate::{ component::{ComponentCloneBehavior, ComponentId, Mutable, StorageType}, - error::{ErrorContext, ErrorHandler}, + error::ErrorHandler, event::EventKey, lifecycle::{ComponentHook, HookContext}, observer::{ @@ -338,7 +338,7 @@ impl Observer { /// Sets the error handler to use for this observer. /// /// See the [`error` module-level documentation](crate::error) for more information. - pub fn with_error_handler(mut self, error_handler: fn(BevyError, ErrorContext)) -> Self { + pub fn with_error_handler(mut self, error_handler: ErrorHandler) -> Self { self.error_handler = Some(error_handler); self } diff --git a/crates/bevy_ecs/src/observer/runner.rs b/crates/bevy_ecs/src/observer/runner.rs index 90d6bd2add9a4..a2eba7f273710 100644 --- a/crates/bevy_ecs/src/observer/runner.rs +++ b/crates/bevy_ecs/src/observer/runner.rs @@ -111,12 +111,15 @@ pub(super) unsafe fn observer_system_runner( + _: BevyError, + ctx: ErrorContext, + commands: Commands<'w, 's>, + ) -> Commands<'w, 's> { let a = IntoSystem::into_system(system_a); let b = IntoSystem::into_system(system_b); assert!( matches!(ctx, ErrorContext::RunCondition { system, on_set, .. } if (on_set && system == b.name()) || (!on_set && system == a.name())) ); + commands } fn system_a() {} diff --git a/crates/bevy_ecs/src/schedule/executor/mod.rs b/crates/bevy_ecs/src/schedule/executor/mod.rs index 90b1d7afce9cb..b05127dcfba58 100644 --- a/crates/bevy_ecs/src/schedule/executor/mod.rs +++ b/crates/bevy_ecs/src/schedule/executor/mod.rs @@ -15,7 +15,7 @@ pub use fixedbitset::FixedBitSet; use crate::{ change_detection::{CheckChangeTicks, Tick}, - error::{BevyError, ErrorContext, Result}, + error::{ErrorHandler, Result}, prelude::{IntoSystemSet, SystemSet}, query::FilteredAccessSet, schedule::{ @@ -36,7 +36,7 @@ pub trait SystemExecutor: Send + Sync { schedule: &mut SystemSchedule, world: &mut World, skip_systems: Option<&FixedBitSet>, - error_handler: fn(BevyError, ErrorContext), + error_handler: ErrorHandler, ); /// Sets whether deferred system buffers should be applied after all systems have run. fn set_apply_final_deferred(&mut self, value: bool); @@ -242,6 +242,8 @@ impl IntoSystemSet<()> for ApplyDeferred { mod __rust_begin_short_backtrace { use core::hint::black_box; + #[cfg(feature = "std")] + use crate::system::Commands; #[cfg(feature = "std")] use crate::world::unsafe_world_cell::UnsafeWorldCell; use crate::{ @@ -314,13 +316,15 @@ mod __rust_begin_short_backtrace { #[inline(never)] #[cfg(feature = "std")] - pub(super) fn error_handler( + pub(super) fn error_handler<'w, 's>( error_handler: crate::error::ErrorHandler, err: crate::error::BevyError, err_context: crate::error::ErrorContext, - ) { - error_handler(err, err_context); + commands: Commands<'w, 's>, + ) -> Commands<'w, 's> { + let commands = error_handler(err, err_context, commands); black_box(()); + commands } } diff --git a/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs b/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs index 3b7d63f5ca7db..61e9effebdc8c 100644 --- a/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs +++ b/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs @@ -24,8 +24,8 @@ use crate::{ schedule::{ is_apply_deferred, ConditionWithAccess, SystemExecutor, SystemSchedule, SystemWithAccess, }, - system::{BoxedSystem, RunSystemError, ScheduleSystem}, - world::{unsafe_world_cell::UnsafeWorldCell, World}, + system::{BoxedSystem, Commands, RunSystemError, ScheduleSystem}, + world::{unsafe_world_cell::UnsafeWorldCell, CommandQueue, World}, }; #[cfg(feature = "hotpatching")] use crate::{prelude::DetectChanges, HotPatchChanges}; @@ -100,6 +100,8 @@ pub struct MultiThreadedExecutor { apply_final_deferred: bool, /// When set, tells the executor that a thread has panicked. panic_payload: Mutex>>, + /// Commands queued by the fallback error handler. + error_handler_command_queue: Mutex, starting_systems: FixedBitSet, /// Cached tracing span #[cfg(feature = "trace")] @@ -302,7 +304,13 @@ impl SystemExecutor for MultiThreadedExecutor { if self.apply_final_deferred { // Do one final apply buffers after all systems have completed // Commands should be applied while on the scope's thread, not the executor's thread - let res = apply_deferred(&state.unapplied_systems, systems, world, error_handler); + let res = apply_deferred( + &state.unapplied_systems, + systems, + world, + error_handler, + &self.error_handler_command_queue, + ); if let Err(payload) = res { let panic_payload = self.panic_payload.get_mut().unwrap(); *panic_payload = Some(payload); @@ -399,6 +407,7 @@ impl MultiThreadedExecutor { starting_systems: FixedBitSet::new(), apply_final_deferred: true, panic_payload: Mutex::new(None), + error_handler_command_queue: Mutex::new(CommandQueue::default()), #[cfg(feature = "trace")] executor_span: info_span!("multithreaded executor"), } @@ -512,6 +521,7 @@ impl ExecutorState { conditions, context.environment.world_cell, context.error_handler, + &context.environment.executor.error_handler_command_queue, ) } { self.skip_system_and_signal_dependents(system_index); @@ -596,6 +606,7 @@ impl ExecutorState { conditions: &mut Conditions, world: UnsafeWorldCell, error_handler: ErrorHandler, + error_handler_command_queue: &Mutex, ) -> bool { let mut should_run = !self.skipped_systems.contains(system_index); @@ -613,6 +624,7 @@ impl ExecutorState { &mut conditions.set_conditions[set_idx], world, error_handler, + error_handler_command_queue, system, true, ) @@ -636,6 +648,7 @@ impl ExecutorState { &mut conditions.system_conditions[system_index], world, error_handler, + error_handler_command_queue, system, false, ) @@ -678,7 +691,9 @@ impl ExecutorState { } }, system, + context.environment.world_cell, context.error_handler, + &context.environment.executor.error_handler_command_queue, "System panicked", ); context.system_completed(system_index, res, system); @@ -713,6 +728,7 @@ impl ExecutorState { context.environment.systems, world, context.error_handler, + &context.environment.executor.error_handler_command_queue, ); context.system_completed(system_index, res, system); }; @@ -726,7 +742,9 @@ impl ExecutorState { let res = handle_errors( |system| __rust_begin_short_backtrace::run(system, world), system, + context.environment.world_cell, context.error_handler, + &context.environment.executor.error_handler_command_queue, "Exclusive system panicked", ); context.system_completed(system_index, res, system); @@ -783,20 +801,31 @@ fn apply_deferred( systems: &[SyncUnsafeCell], world: &mut World, error_handler: ErrorHandler, + error_handler_command_queue: &Mutex, ) -> Result<(), Box> { for system_index in unapplied_systems.ones() { // SAFETY: none of these systems are running, no other references exist let system = &mut unsafe { &mut *systems[system_index].get() }.system; + let world_cell = world.as_unsafe_world_cell(); handle_errors( |system| { - system.apply_deferred(world); + // SAFETY: No systems are running while deferred buffers are applied. + system.apply_deferred(unsafe { world_cell.world_mut() }); Ok(()) }, system, + world_cell, error_handler, + error_handler_command_queue, "Encountered a panic while applying system buffers", )?; } + let mut error_handler_command_queue = error_handler_command_queue + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + if !error_handler_command_queue.is_empty() { + error_handler_command_queue.apply(world); + } Ok(()) } @@ -807,6 +836,7 @@ unsafe fn evaluate_and_fold_conditions( conditions: &mut [ConditionWithAccess], world: UnsafeWorldCell, error_handler: ErrorHandler, + error_handler_command_queue: &Mutex, for_system: &ScheduleSystem, on_set: bool, ) -> bool { @@ -830,7 +860,9 @@ unsafe fn evaluate_and_fold_conditions( Err(payload) if panic_originates_from_error_handler => std::panic::resume_unwind(payload), // Let the error handler handle the panic Err(_) => { - __rust_begin_short_backtrace::error_handler( + run_error_handler( + world, + error_handler_command_queue, error_handler, BevyError::new_with_backtrace( Severity::Panic, @@ -846,7 +878,9 @@ unsafe fn evaluate_and_fold_conditions( ); false}, // Condition returned an error, let the error handler handle it Ok(Err(RunSystemError::Failed(err))) => { - __rust_begin_short_backtrace::error_handler( + run_error_handler( + world, + error_handler_command_queue, error_handler, err, ErrorContext::RunCondition { @@ -869,7 +903,9 @@ unsafe fn evaluate_and_fold_conditions( fn handle_errors( f: impl FnOnce(&mut BoxedSystem) -> Result<(), RunSystemError>, system: &mut BoxedSystem, + world: UnsafeWorldCell, error_handler: ErrorHandler, + error_handler_command_queue: &Mutex, error_message: &str, ) -> Result<(), Box> { PANIC_ORIGINATES_FROM_ERROR_HANDLER.set(false); @@ -880,7 +916,9 @@ fn handle_errors( Err(payload) if panic_originates_from_error_handler => Err(payload), // Let the error handler handle the panic, passing on any panic it throws Err(_) => std::panic::catch_unwind(AssertUnwindSafe(|| { - __rust_begin_short_backtrace::error_handler( + run_error_handler( + world, + error_handler_command_queue, error_handler, BevyError::new_with_backtrace( Severity::Panic, @@ -895,7 +933,9 @@ fn handle_errors( })), // System returned an error, let the error handler handle it, passing on any panic it throws Ok(Err(RunSystemError::Failed(err))) => std::panic::catch_unwind(AssertUnwindSafe(|| { - __rust_begin_short_backtrace::error_handler( + run_error_handler( + world, + error_handler_command_queue, error_handler, err, ErrorContext::System { @@ -909,6 +949,24 @@ fn handle_errors( } } +fn run_error_handler( + world: UnsafeWorldCell, + error_handler_command_queue: &Mutex, + error_handler: ErrorHandler, + error: BevyError, + context: ErrorContext, +) { + let mut command_queue = error_handler_command_queue + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let commands = Commands::new_from_entities( + &mut command_queue, + world.entity_allocator(), + world.entities(), + ); + let _ = __rust_begin_short_backtrace::error_handler(error_handler, error, context, commands); +} + /// New-typed [`ThreadExecutor`] [`Resource`] that is used to run systems on the main thread #[derive(Resource, Clone)] pub struct MainThreadExecutor(pub Arc>); @@ -994,9 +1052,14 @@ mod tests { }); static HANDLER_CALLED: AtomicBool = AtomicBool::new(false); - fn handle(_: BevyError, ctx: ErrorContext) { + fn handle<'w, 's>( + _: BevyError, + ctx: ErrorContext, + commands: Commands<'w, 's>, + ) -> Commands<'w, 's> { assert!(matches!(ctx, ErrorContext::System { .. })); HANDLER_CALLED.store(true, Relaxed); + commands } world.insert_resource(FallbackErrorHandler(handle)); @@ -1010,7 +1073,11 @@ mod tests { assert!(HANDLER_CALLED.load(Relaxed)); const PANIC_PAYLOAD: &str = "UwU"; - fn panic(_: BevyError, ctx: ErrorContext) { + fn panic<'w, 's>( + _: BevyError, + ctx: ErrorContext, + _commands: Commands<'w, 's>, + ) -> Commands<'w, 's> { assert!(matches!(ctx, ErrorContext::System { .. })); PANIC_ORIGINATES_FROM_ERROR_HANDLER.set(true); panic!("{}", PANIC_PAYLOAD); diff --git a/crates/bevy_ecs/src/schedule/executor/single_threaded.rs b/crates/bevy_ecs/src/schedule/executor/single_threaded.rs index 58317f4355e27..ee27d4269b82d 100644 --- a/crates/bevy_ecs/src/schedule/executor/single_threaded.rs +++ b/crates/bevy_ecs/src/schedule/executor/single_threaded.rs @@ -10,18 +10,17 @@ use alloc::string::ToString as _; #[cfg(feature = "trace")] use tracing::info_span; -#[cfg(feature = "std")] use crate::{ - error::{BevyError, Severity, PANIC_ORIGINATES_FROM_ERROR_HANDLER}, - system::BoxedSystem, + error::{BevyError, ErrorContext, ErrorHandler}, + schedule::{is_apply_deferred, ConditionWithAccess, SystemExecutor, SystemSchedule}, + system::{Commands, RunSystemError, ScheduleSystem}, + world::{CommandQueue, World}, }; +#[cfg(feature = "std")] use crate::{ - error::{ErrorContext, ErrorHandler}, - schedule::{ - is_apply_deferred, BoxedCondition, ConditionWithAccess, SystemExecutor, SystemSchedule, - }, - system::{RunSystemError, ScheduleSystem}, - world::World, + error::{Severity, PANIC_ORIGINATES_FROM_ERROR_HANDLER}, + schedule::BoxedCondition, + system::BoxedSystem, }; #[cfg(feature = "hotpatching")] @@ -43,6 +42,8 @@ pub struct SingleThreadedExecutor { unapplied_systems: FixedBitSet, /// Setting when true applies deferred system buffers after all systems have run apply_final_deferred: bool, + /// Commands queued by the fallback error handler. + error_handler_command_queue: CommandQueue, } impl SystemExecutor for SingleThreadedExecutor { @@ -95,6 +96,7 @@ impl SystemExecutor for SingleThreadedExecutor { &mut schedule.set_conditions[set_idx], world, error_handler, + &mut self.error_handler_command_queue, system, true, ); @@ -113,6 +115,7 @@ impl SystemExecutor for SingleThreadedExecutor { &mut schedule.system_conditions[system_index], world, error_handler, + &mut self.error_handler_command_queue, system, false, ); @@ -139,11 +142,29 @@ impl SystemExecutor for SingleThreadedExecutor { continue; } - let f = |system: &mut _| { + #[cfg(feature = "std")] + { + handle_errors( + |system, world| { + __rust_begin_short_backtrace::run_without_applying_deferred(system, world) + }, + system, + world, + error_handler, + &mut self.error_handler_command_queue, + "System panicked", + ); + } + + #[cfg(not(feature = "std"))] + { if let Err(RunSystemError::Failed(err)) = __rust_begin_short_backtrace::run_without_applying_deferred(system, world) { - error_handler( + run_error_handler( + world, + &mut self.error_handler_command_queue, + error_handler, err, ErrorContext::System { name: system.name(), @@ -151,17 +172,6 @@ impl SystemExecutor for SingleThreadedExecutor { }, ); } - }; - - #[cfg(feature = "std")] - { - handle_unwind(f, system, error_handler, "System panicked"); - } - - #[cfg(not(feature = "std"))] - { - let mut f = f; - (f)(system); } self.unapplied_systems.insert(system_index); @@ -189,6 +199,7 @@ impl SingleThreadedExecutor { completed_systems: FixedBitSet::new(), unapplied_systems: FixedBitSet::new(), apply_final_deferred: true, + error_handler_command_queue: CommandQueue::new(), } } @@ -208,16 +219,24 @@ impl SingleThreadedExecutor { #[cfg(feature = "std")] { - handle_unwind( - |system| system.apply_deferred(world), + handle_errors( + |system, world| { + system.apply_deferred(world); + Ok(()) + }, system, + world, error_handler, + &mut self.error_handler_command_queue, "Encountered a panic while applying system buffers", ); } } self.unapplied_systems.clear(); + if !self.error_handler_command_queue.is_empty() { + self.error_handler_command_queue.apply(world); + } } } @@ -225,6 +244,7 @@ fn evaluate_and_fold_conditions( conditions: &mut [ConditionWithAccess], world: &mut World, error_handler: ErrorHandler, + error_handler_command_queue: &mut CommandQueue, for_system: &ScheduleSystem, on_set: bool, ) -> bool { @@ -245,58 +265,79 @@ fn evaluate_and_fold_conditions( if hotpatch_tick.is_newer_than(condition.get_last_run(), world.change_tick()) { condition.refresh_hotpatch(); } - let f = |condition: &mut BoxedCondition| { - __rust_begin_short_backtrace::readonly_run(&mut **condition, world).unwrap_or_else( - |err| { - if let RunSystemError::Failed(err) = err { - error_handler( - err, - ErrorContext::RunCondition { - name: condition.name(), - last_run: condition.get_last_run(), - system: for_system.name(), - on_set, - }, - ); - }; - false - }, - ) - }; #[cfg(not(feature = "std"))] - let result = { - let mut f = f; - f(condition) + let result = match __rust_begin_short_backtrace::readonly_run(&mut **condition, world) { + Ok(result) => result, + Err(RunSystemError::Failed(err)) => { + run_error_handler( + world, + error_handler_command_queue, + error_handler, + err, + ErrorContext::RunCondition { + name: condition.name(), + last_run: condition.get_last_run(), + system: for_system.name(), + on_set, + }, + ); + false + } + Err(RunSystemError::Skipped(_)) => false, }; #[cfg(feature = "std")] - let result = - handle_unwind_in_run_condition(f, condition, for_system, on_set, error_handler); + let result = handle_unwind_in_run_condition( + |condition, world| { + __rust_begin_short_backtrace::readonly_run(&mut **condition, world) + }, + condition, + world, + for_system, + on_set, + error_handler, + error_handler_command_queue, + ); result }) .fold(true, |acc, res| acc && res) } -/// Handle a potential panic by invoking the error handler +/// Handle a potential panic or failed system by invoking the error handler. #[cfg(feature = "std")] -fn handle_unwind( - f: impl FnOnce(&mut BoxedSystem), +fn handle_errors( + f: impl FnOnce(&mut BoxedSystem, &mut World) -> Result<(), RunSystemError>, system: &mut BoxedSystem, + world: &mut World, error_handler: ErrorHandler, + error_handler_command_queue: &mut CommandQueue, error_message: &str, ) { PANIC_ORIGINATES_FROM_ERROR_HANDLER.set(false); - let potential_unwind = std::panic::catch_unwind(AssertUnwindSafe(|| f(system))); + let potential_unwind = std::panic::catch_unwind(AssertUnwindSafe(|| { + if let Err(RunSystemError::Failed(err)) = f(system, world) { + run_error_handler( + world, + error_handler_command_queue, + error_handler, + err, + ErrorContext::System { + name: system.name(), + last_run: system.get_last_run(), + }, + ); + } + })); let panic_originates_from_error_handler = PANIC_ORIGINATES_FROM_ERROR_HANDLER.replace(false); if let Err(payload) = potential_unwind { if panic_originates_from_error_handler { std::panic::resume_unwind(payload); } - let err = - BevyError::new_with_backtrace(Severity::Panic, error_message, Backtrace::disabled()); - __rust_begin_short_backtrace::error_handler( + run_error_handler( + world, + error_handler_command_queue, error_handler, - err, + BevyError::new_with_backtrace(Severity::Panic, error_message, Backtrace::disabled()), ErrorContext::System { name: system.name(), last_run: system.get_last_run(), @@ -308,30 +349,49 @@ fn handle_unwind( /// Handle a potential panic by invoking the error handler #[cfg(feature = "std")] fn handle_unwind_in_run_condition( - f: impl FnOnce(&mut BoxedCondition) -> bool, + f: impl FnOnce(&mut BoxedCondition, &mut World) -> Result, condition: &mut BoxedCondition, + world: &mut World, for_system: &ScheduleSystem, on_set: bool, error_handler: ErrorHandler, + error_handler_command_queue: &mut CommandQueue, ) -> bool { PANIC_ORIGINATES_FROM_ERROR_HANDLER.set(false); - let potential_unwind = std::panic::catch_unwind(AssertUnwindSafe(|| f(condition))); + let potential_unwind = + std::panic::catch_unwind(AssertUnwindSafe(|| match f(condition, world) { + Ok(result) => result, + Err(RunSystemError::Failed(err)) => { + run_error_handler( + world, + error_handler_command_queue, + error_handler, + err, + ErrorContext::RunCondition { + name: condition.name(), + last_run: condition.get_last_run(), + system: for_system.name(), + on_set, + }, + ); + false + } + Err(RunSystemError::Skipped(_)) => false, + })); let panic_originates_from_error_handler = PANIC_ORIGINATES_FROM_ERROR_HANDLER.replace(false); match potential_unwind { - Ok(r) => r, - Err(payload) => { - if panic_originates_from_error_handler { - std::panic::resume_unwind(payload); - } - - let err = BevyError::new_with_backtrace( - Severity::Panic, - "Encountered panic", - Backtrace::disabled(), - ); - __rust_begin_short_backtrace::error_handler( + Ok(result) => result, + Err(payload) if panic_originates_from_error_handler => std::panic::resume_unwind(payload), + Err(_) => { + run_error_handler( + world, + error_handler_command_queue, error_handler, - err, + BevyError::new_with_backtrace( + Severity::Panic, + "Encountered panic", + Backtrace::disabled(), + ), ErrorContext::RunCondition { name: condition.name(), last_run: condition.get_last_run(), @@ -343,3 +403,17 @@ fn handle_unwind_in_run_condition( } } } + +fn run_error_handler( + world: &World, + error_handler_command_queue: &mut CommandQueue, + error_handler: ErrorHandler, + error: BevyError, + context: ErrorContext, +) { + let commands = Commands::new(error_handler_command_queue, world); + #[cfg(feature = "std")] + let _ = __rust_begin_short_backtrace::error_handler(error_handler, error, context, commands); + #[cfg(not(feature = "std"))] + let _ = error_handler(error, context, commands); +} diff --git a/crates/bevy_ecs/src/system/combinator.rs b/crates/bevy_ecs/src/system/combinator.rs index 58c9b0e931c1c..36749afb3d44c 100644 --- a/crates/bevy_ecs/src/system/combinator.rs +++ b/crates/bevy_ecs/src/system/combinator.rs @@ -8,8 +8,8 @@ use crate::{ prelude::World, query::FilteredAccessSet, schedule::InternedSystemSet, - system::{input::SystemInput, SystemIn}, - world::unsafe_world_cell::UnsafeWorldCell, + system::{input::SystemInput, Commands, SystemIn}, + world::{unsafe_world_cell::UnsafeWorldCell, CommandQueue}, }; use super::{IntoSystem, ReadOnlySystem, RunSystemError, System}; @@ -117,6 +117,7 @@ pub struct CombinatorSystem { a: A, b: B, name: DebugName, + error_handler_command_queue: CommandQueue, } impl CombinatorSystem { @@ -129,6 +130,7 @@ impl CombinatorSystem { a, b, name, + error_handler_command_queue: CommandQueue::default(), } } } @@ -156,7 +158,10 @@ where input: SystemIn<'_, Self>, world: UnsafeWorldCell, ) -> Result { - struct PrivateUnsafeWorldCell<'w>(UnsafeWorldCell<'w>); + struct PrivateUnsafeWorldCell<'w, 'q> { + world: UnsafeWorldCell<'w>, + error_handler_command_queue: &'q mut CommandQueue, + } // Since control over handling system run errors is passed on to the // implementation of `Func::combine`, which may run the two closures @@ -168,18 +173,24 @@ where world: &mut PrivateUnsafeWorldCell, ) -> Result { // SAFETY: see comment on `Func::combine` call - match unsafe { system.run_unsafe(input, world.0) } { + match unsafe { system.run_unsafe(input, world.world) } { // let the world's fallback error handler handle the error if `Failed(_)` Err(RunSystemError::Failed(err)) => { // SAFETY: We registered access to FallbackErrorHandler in `initialize`. - (unsafe { world.0.fallback_error_handler() })( + let error_handler = unsafe { world.world.fallback_error_handler() }; + let commands = Commands::new_from_entities( + world.error_handler_command_queue, + world.world.entity_allocator(), + world.world.entities(), + ); + let _commands = error_handler( err, ErrorContext::System { name: system.name(), last_run: system.get_last_run(), }, + commands, ); - // Since the error handler takes the error by value, create a new error: // The original error has already been handled, including // the reason for the failure here isn't important. @@ -194,7 +205,10 @@ where Func::combine( input, - &mut PrivateUnsafeWorldCell(world), + &mut PrivateUnsafeWorldCell { + world, + error_handler_command_queue: &mut self.error_handler_command_queue, + }, // SAFETY: The world accesses for both underlying systems have been registered, // so the caller will guarantee that no other systems will conflict with (`a` or `b`) and the `FallbackErrorHandler` resource. // If either system has `is_exclusive()`, then the combined system also has `is_exclusive`. @@ -222,12 +236,20 @@ where fn apply_deferred(&mut self, world: &mut World) { self.a.apply_deferred(world); self.b.apply_deferred(world); + if !self.error_handler_command_queue.is_empty() { + self.error_handler_command_queue.apply(world); + } } #[inline] fn queue_deferred(&mut self, mut world: crate::world::DeferredWorld) { self.a.queue_deferred(world.reborrow()); - self.b.queue_deferred(world); + self.b.queue_deferred(world.reborrow()); + if !self.error_handler_command_queue.is_empty() { + world + .commands() + .append(&mut self.error_handler_command_queue); + } } fn initialize(&mut self, world: &mut World) -> FilteredAccessSet { diff --git a/crates/bevy_ecs/src/system/commands/command.rs b/crates/bevy_ecs/src/system/commands/command.rs index 9d6cb84729dcc..01ae6907e3035 100644 --- a/crates/bevy_ecs/src/system/commands/command.rs +++ b/crates/bevy_ecs/src/system/commands/command.rs @@ -63,20 +63,26 @@ pub trait Command: Send + 'static { /// 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 `()`. #[inline] - fn handle_error_with( - self, - error_handler: impl FnOnce(BevyError, ErrorContext) + Send + 'static, - ) -> impl Command + fn handle_error_with(self, error_handler: F) -> impl Command where Self: Sized, + F: for<'w, 's> FnOnce( + BevyError, + ErrorContext, + crate::system::Commands<'w, 's>, + ) -> crate::system::Commands<'w, 's> + + Send + + 'static, { move |world: &mut World| { if let Some(error) = self.apply(world).to_err() { - error_handler( + let commands = world.commands(); + let _ = error_handler( error, ErrorContext::Command { name: DebugName::type_name::(), }, + commands, ); } } @@ -91,11 +97,14 @@ pub trait Command: Send + 'static { { move |world: &mut World| { if let Some(error) = self.apply(world).to_err() { - world.fallback_error_handler()( + let error_handler = world.fallback_error_handler(); + let commands = world.commands(); + let _ = error_handler( error, ErrorContext::Command { name: DebugName::type_name::(), }, + commands, ); } } diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index 915862c28c7ee..ecb560f17bad7 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -692,11 +692,16 @@ impl<'w, 's> Commands<'w, 's> { /// # bevy_ecs::system::assert_is_system(add_three_to_counter_system); /// # bevy_ecs::system::assert_is_system(add_twenty_five_to_counter_system); /// ``` - pub fn queue_handled( - &mut self, - command: impl Command, - error_handler: impl FnOnce(BevyError, ErrorContext) + Send + 'static, - ) { + pub fn queue_handled(&mut self, command: impl Command, error_handler: F) + where + F: for<'world, 'state> FnOnce( + BevyError, + ErrorContext, + Commands<'world, 'state>, + ) -> Commands<'world, 'state> + + Send + + 'static, + { self.queue_internal(command.handle_error_with(error_handler)); } @@ -2066,11 +2071,16 @@ impl<'a> EntityCommands<'a> { /// # } /// # bevy_ecs::system::assert_is_system(my_system); /// ``` - pub fn queue_handled( - &mut self, - command: impl EntityCommand, - error_handler: impl FnOnce(BevyError, ErrorContext) + Send + 'static, - ) -> &mut Self { + pub fn queue_handled(&mut self, command: impl EntityCommand, error_handler: F) -> &mut Self + where + F: for<'world, 'state> FnOnce( + BevyError, + ErrorContext, + Commands<'world, 'state>, + ) -> Commands<'world, 'state> + + Send + + 'static, + { self.commands .queue_handled(command.with_entity(self.entity), error_handler); self diff --git a/crates/bevy_ecs/src/world/command_queue.rs b/crates/bevy_ecs/src/world/command_queue.rs index 9ce7f7f0048b3..65cf2b7daa257 100644 --- a/crates/bevy_ecs/src/world/command_queue.rs +++ b/crates/bevy_ecs/src/world/command_queue.rs @@ -61,9 +61,16 @@ pub struct CommandQueue { impl Default for CommandQueue { #[track_caller] fn default() -> Self { + Self::new() + } +} + +impl CommandQueue { + #[track_caller] + pub(crate) const fn new() -> Self { Self { - bytes: Default::default(), - cursor: Default::default(), + bytes: Vec::new(), + cursor: 0, caller: MaybeLocation::caller(), warn_on_unapplied: true, } @@ -414,7 +421,10 @@ fn handle_panic_payload( }; let error = BevyError::new_with_backtrace(Severity::Panic, "Command panicked", Backtrace::disabled()); - world.fallback_error_handler()(error, ErrorContext::Command { name }); + let error_handler = world.fallback_error_handler(); + let commands = world.commands(); + let _ = error_handler(error, ErrorContext::Command { name }, commands); + world.flush(); } impl Drop for CommandQueueRunner<'_> { @@ -592,8 +602,13 @@ mod test { // handles the panicking command. queue.push(SpawnCommand); - fn record_last_error(error: BevyError, context: ErrorContext) { + fn record_last_error<'w, 's>( + error: BevyError, + context: ErrorContext, + commands: crate::system::Commands<'w, 's>, + ) -> crate::system::Commands<'w, 's> { *LAST_ERROR.lock().unwrap() = Some((error, context)); + commands } static LAST_ERROR: Mutex> = Mutex::new(None); *LAST_ERROR.lock().unwrap() = None; @@ -651,8 +666,13 @@ mod test { #[derive(Resource, Default)] struct Order(Vec); - fn record_last_error(error: BevyError, context: ErrorContext) { + fn record_last_error<'w, 's>( + error: BevyError, + context: ErrorContext, + commands: crate::system::Commands<'w, 's>, + ) -> crate::system::Commands<'w, 's> { *LAST_ERROR.lock().unwrap() = Some((error, context)); + commands } static LAST_ERROR: Mutex> = Mutex::new(None); *LAST_ERROR.lock().unwrap() = None; diff --git a/examples/ecs/custom_executor.rs b/examples/ecs/custom_executor.rs index ef7560f96b63b..b26437cac1e2d 100644 --- a/examples/ecs/custom_executor.rs +++ b/examples/ecs/custom_executor.rs @@ -2,7 +2,7 @@ use bevy::{ ecs::{ - error::{BevyError, ErrorContext}, + error::ErrorHandler, schedule::{FixedBitSet, SystemExecutor, SystemSchedule}, }, prelude::*, @@ -19,7 +19,7 @@ impl SystemExecutor for CustomExecutor { schedule: &mut SystemSchedule, world: &mut World, _skip_systems: Option<&FixedBitSet>, - _error_handler: fn(BevyError, ErrorContext), + _error_handler: ErrorHandler, ) { #[expect(unsafe_code, reason = "CustomExecutor's require unsafe")] // SAFETY: `run` is a trait method on `System` diff --git a/examples/ecs/error_handling.rs b/examples/ecs/error_handling.rs index ca52b4f6dbad8..a46b981a18149 100644 --- a/examples/ecs/error_handling.rs +++ b/examples/ecs/error_handling.rs @@ -191,8 +191,9 @@ fn failing_commands(mut commands: Commands) { Ok(()) }, - |error, context| { + |error, context, commands| { error!("{error}, {context}"); + commands }, ); }