From 4d1cb906f7830f62d1437107165b905a079d8a00 Mon Sep 17 00:00:00 2001 From: Jean Mertz Date: Sun, 9 Feb 2025 09:05:26 +0100 Subject: [PATCH 1/8] feat(ecs): configurable error handling for fallible systems You can now configure error handlers for fallible systems. These can be configured on several levels: - Globally via `App::set_systems_error_handler` - Per-schedule via `Schedule::set_error_handler` - Per-system via a piped system (this is existing functionality) The "fallible_systems" example demonstrates the new functionality. This builds on top of #17731, #16589, #17051. Signed-off-by: Jean Mertz --- crates/bevy_app/src/app.rs | 16 +++++-- crates/bevy_app/src/sub_app.rs | 17 ++++++- crates/bevy_ecs/src/result.rs | 5 ++ crates/bevy_ecs/src/schedule/executor/mod.rs | 3 +- .../src/schedule/executor/multi_threaded.rs | 27 +++++------ .../bevy_ecs/src/schedule/executor/simple.rs | 10 ++-- .../src/schedule/executor/single_threaded.rs | 17 ++----- crates/bevy_ecs/src/schedule/schedule.rs | 46 +++++++++++++++---- crates/bevy_ecs/src/world/mod.rs | 16 ++++++- examples/ecs/fallible_systems.rs | 41 +++++++++++++++-- 10 files changed, 144 insertions(+), 54 deletions(-) diff --git a/crates/bevy_app/src/app.rs b/crates/bevy_app/src/app.rs index 11602ff5ad719..7ee4bbcac418e 100644 --- a/crates/bevy_app/src/app.rs +++ b/crates/bevy_app/src/app.rs @@ -13,13 +13,13 @@ use bevy_ecs::{ event::{event_update_system, EventCursor}, intern::Interned, prelude::*, + result::Error, schedule::{ScheduleBuildSettings, ScheduleLabel}, - system::{IntoObserverSystem, SystemId, SystemInput}, + system::{IntoObserverSystem, ScheduleSystem, SystemId, SystemInput}, }; use bevy_platform_support::collections::HashMap; use core::{fmt::Debug, num::NonZero, panic::AssertUnwindSafe}; use log::debug; -use thiserror::Error; #[cfg(feature = "trace")] use tracing::info_span; @@ -44,7 +44,7 @@ pub use bevy_ecs::label::DynEq; /// A shorthand for `Interned`. pub type InternedAppLabel = Interned; -#[derive(Debug, Error)] +#[derive(Debug, thiserror::Error)] pub(crate) enum AppError { #[error("duplicate plugin {plugin_name:?}")] DuplicatePlugin { plugin_name: String }, @@ -1263,6 +1263,16 @@ impl App { self } + /// Set the global system error handler to use for systems that return a + /// [`Result`](crate::result::Result). + pub fn set_systems_error_handler( + &mut self, + error_handler: fn(Error, &ScheduleSystem), + ) -> &mut Self { + self.main_mut().set_systems_error_handler(error_handler); + self + } + /// Attempts to determine if an [`AppExit`] was raised since the last update. /// /// Will attempt to return the first [`Error`](AppExit::Error) it encounters. diff --git a/crates/bevy_app/src/sub_app.rs b/crates/bevy_app/src/sub_app.rs index 2e3f48f2ca1e6..28618c8089949 100644 --- a/crates/bevy_app/src/sub_app.rs +++ b/crates/bevy_app/src/sub_app.rs @@ -4,7 +4,7 @@ use bevy_ecs::{ event::EventRegistry, prelude::*, schedule::{InternedScheduleLabel, ScheduleBuildSettings, ScheduleLabel}, - system::{SystemId, SystemInput}, + system::{ScheduleSystem, SystemId, SystemInput}, }; use bevy_platform_support::collections::{HashMap, HashSet}; use core::fmt::Debug; @@ -335,6 +335,21 @@ impl SubApp { self } + /// Set the global error handler to use for systems that return a + /// [`Result`](crate::result::Result). + pub fn set_systems_error_handler( + &mut self, + error_handler: fn(Error, &ScheduleSystem), + ) -> &mut Self { + let mut schedules = self + .world_mut() + .remove_resource::() + .unwrap_or_default(); + schedules.error_handler = error_handler; + self.world_mut().insert_resource(schedules); + self + } + /// See [`App::add_event`]. pub fn add_event(&mut self) -> &mut Self where diff --git a/crates/bevy_ecs/src/result.rs b/crates/bevy_ecs/src/result.rs index 77e7fc00743ae..1d71bd9a4c5fb 100644 --- a/crates/bevy_ecs/src/result.rs +++ b/crates/bevy_ecs/src/result.rs @@ -7,3 +7,8 @@ pub type Error = Box; /// A result type for use in fallible systems. pub type Result = core::result::Result; + +pub(crate) fn default_error_handler(error: Error, system: &crate::system::ScheduleSystem) { + let name = system.name(); + panic!("Encountered an error in system `{name}`: {error:?}",); +} diff --git a/crates/bevy_ecs/src/schedule/executor/mod.rs b/crates/bevy_ecs/src/schedule/executor/mod.rs index 8b549a15ea6d3..83dfb6862efbf 100644 --- a/crates/bevy_ecs/src/schedule/executor/mod.rs +++ b/crates/bevy_ecs/src/schedule/executor/mod.rs @@ -18,7 +18,7 @@ use crate::{ component::{ComponentId, Tick}, prelude::{IntoSystemSet, SystemSet}, query::Access, - result::Result, + result::{Error, Result}, schedule::{BoxedCondition, InternedSystemSet, NodeId, SystemTypeSet}, system::{ScheduleSystem, System, SystemIn}, world::{unsafe_world_cell::UnsafeWorldCell, DeferredWorld, World}, @@ -33,6 +33,7 @@ pub(super) trait SystemExecutor: Send + Sync { schedule: &mut SystemSchedule, world: &mut World, skip_systems: Option<&FixedBitSet>, + error_handler: fn(Error, &ScheduleSystem), ); fn set_apply_final_deferred(&mut self, value: bool); } diff --git a/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs b/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs index cfa3dc8398431..d6cec8b5661c8 100644 --- a/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs +++ b/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs @@ -17,6 +17,7 @@ use crate::{ archetype::ArchetypeComponentId, prelude::Resource, query::Access, + result::{Error, Result}, schedule::{is_apply_deferred, BoxedCondition, ExecutorKind, SystemExecutor, SystemSchedule}, system::ScheduleSystem, world::{unsafe_world_cell::UnsafeWorldCell, World}, @@ -133,6 +134,7 @@ pub struct ExecutorState { struct Context<'scope, 'env, 'sys> { environment: &'env Environment<'env, 'sys>, scope: &'scope Scope<'scope, 'env, ()>, + error_handler: fn(Error, &ScheduleSystem), } impl Default for MultiThreadedExecutor { @@ -183,6 +185,7 @@ impl SystemExecutor for MultiThreadedExecutor { schedule: &mut SystemSchedule, world: &mut World, _skip_systems: Option<&FixedBitSet>, + error_handler: fn(Error, &ScheduleSystem), ) { let state = self.state.get_mut().unwrap(); // reset counts @@ -222,7 +225,11 @@ impl SystemExecutor for MultiThreadedExecutor { false, thread_executor, |scope| { - let context = Context { environment, scope }; + let context = Context { + environment, + scope, + error_handler, + }; // The first tick won't need to process finished systems, but we still need to run the loop in // tick_executor() in case a system completes while the first tick still holds the mutex. @@ -603,17 +610,12 @@ impl ExecutorState { // access the world data used by the system. // - `update_archetype_component_access` has been called. unsafe { - // TODO: implement an error-handling API instead of panicking. if let Err(err) = __rust_begin_short_backtrace::run_unsafe( system, context.environment.world_cell, ) { - panic!( - "Encountered an error in system `{}`: {:?}", - &*system.name(), - err - ); - }; + (context.error_handler)(err, &system); + } }; })); context.system_completed(system_index, res, system); @@ -657,14 +659,9 @@ impl ExecutorState { // that no other systems currently have access to the world. let world = unsafe { context.environment.world_cell.world_mut() }; let res = std::panic::catch_unwind(AssertUnwindSafe(|| { - // TODO: implement an error-handling API instead of panicking. if let Err(err) = __rust_begin_short_backtrace::run(system, world) { - panic!( - "Encountered an error in system `{}`: {:?}", - &*system.name(), - err - ); - }; + (context.error_handler)(err, &system); + } })); context.system_completed(system_index, res, system); }; diff --git a/crates/bevy_ecs/src/schedule/executor/simple.rs b/crates/bevy_ecs/src/schedule/executor/simple.rs index 81f7deab3a302..42e629e491532 100644 --- a/crates/bevy_ecs/src/schedule/executor/simple.rs +++ b/crates/bevy_ecs/src/schedule/executor/simple.rs @@ -8,9 +8,11 @@ use tracing::info_span; use std::eprintln; use crate::{ + result::Error, schedule::{ executor::is_apply_deferred, BoxedCondition, ExecutorKind, SystemExecutor, SystemSchedule, }, + system::ScheduleSystem, world::World, }; @@ -43,6 +45,7 @@ impl SystemExecutor for SimpleExecutor { schedule: &mut SystemSchedule, world: &mut World, _skip_systems: Option<&FixedBitSet>, + error_handler: fn(Error, &ScheduleSystem), ) { // If stepping is enabled, make sure we skip those systems that should // not be run. @@ -104,13 +107,8 @@ impl SystemExecutor for SimpleExecutor { } let f = AssertUnwindSafe(|| { - // TODO: implement an error-handling API instead of panicking. if let Err(err) = __rust_begin_short_backtrace::run(system, world) { - panic!( - "Encountered an error in system `{}`: {:?}", - &*system.name(), - err - ); + error_handler(err, &system); } }); diff --git a/crates/bevy_ecs/src/schedule/executor/single_threaded.rs b/crates/bevy_ecs/src/schedule/executor/single_threaded.rs index 8c5a7e0261bbe..2e79d82527e66 100644 --- a/crates/bevy_ecs/src/schedule/executor/single_threaded.rs +++ b/crates/bevy_ecs/src/schedule/executor/single_threaded.rs @@ -8,7 +8,9 @@ use tracing::info_span; use std::eprintln; use crate::{ + result::Error, schedule::{is_apply_deferred, BoxedCondition, ExecutorKind, SystemExecutor, SystemSchedule}, + system::ScheduleSystem, world::World, }; @@ -49,6 +51,7 @@ impl SystemExecutor for SingleThreadedExecutor { schedule: &mut SystemSchedule, world: &mut World, _skip_systems: Option<&FixedBitSet>, + error_handler: fn(Error, &ScheduleSystem), ) { // If stepping is enabled, make sure we skip those systems that should // not be run. @@ -112,13 +115,8 @@ impl SystemExecutor for SingleThreadedExecutor { let f = AssertUnwindSafe(|| { if system.is_exclusive() { - // TODO: implement an error-handling API instead of panicking. if let Err(err) = __rust_begin_short_backtrace::run(system, world) { - panic!( - "Encountered an error in system `{}`: {:?}", - &*system.name(), - err - ); + error_handler(err, &system); } } else { // Use run_unsafe to avoid immediately applying deferred buffers @@ -127,13 +125,8 @@ impl SystemExecutor for SingleThreadedExecutor { // SAFETY: We have exclusive, single-threaded access to the world and // update_archetype_component_access is being called immediately before this. unsafe { - // TODO: implement an error-handling API instead of panicking. if let Err(err) = __rust_begin_short_backtrace::run_unsafe(system, world) { - panic!( - "Encountered an error in system `{}`: {:?}", - &*system.name(), - err - ); + error_handler(err, &system); } }; } diff --git a/crates/bevy_ecs/src/schedule/schedule.rs b/crates/bevy_ecs/src/schedule/schedule.rs index 8c73d7cfe39f6..18377c4e3b2c3 100644 --- a/crates/bevy_ecs/src/schedule/schedule.rs +++ b/crates/bevy_ecs/src/schedule/schedule.rs @@ -29,7 +29,7 @@ use crate::{ component::{ComponentId, Components, Tick}, prelude::Component, resource::Resource, - result::Result, + result::{default_error_handler, Error}, schedule::*, system::ScheduleSystem, world::World, @@ -40,20 +40,30 @@ pub use stepping::Stepping; use Direction::{Incoming, Outgoing}; /// Resource that stores [`Schedule`]s mapped to [`ScheduleLabel`]s excluding the current running [`Schedule`]. -#[derive(Default, Resource)] +#[derive(Resource)] pub struct Schedules { inner: HashMap, /// List of [`ComponentId`]s to ignore when reporting system order ambiguity conflicts pub ignored_scheduling_ambiguities: BTreeSet, + + /// Error handler to use for systems that return a [`Result`](crate::result::Result). + pub error_handler: fn(Error, &ScheduleSystem), +} + +impl Default for Schedules { + fn default() -> Self { + Self { + inner: default(), + ignored_scheduling_ambiguities: default(), + error_handler: default_error_handler, + } + } } impl Schedules { /// Constructs an empty `Schedules` with zero initial capacity. pub fn new() -> Self { - Self { - inner: HashMap::default(), - ignored_scheduling_ambiguities: BTreeSet::new(), - } + Self::default() } /// Inserts a labeled schedule into the map. @@ -300,6 +310,7 @@ pub struct Schedule { executable: SystemSchedule, executor: Box, executor_initialized: bool, + error_handler: Option, } #[derive(ScheduleLabel, Hash, PartialEq, Eq, Debug, Clone)] @@ -324,6 +335,7 @@ impl Schedule { executable: SystemSchedule::new(), executor: make_executor(ExecutorKind::default()), executor_initialized: false, + error_handler: None, }; // Call `set_build_settings` to add any default build passes this.set_build_settings(Default::default()); @@ -401,6 +413,11 @@ impl Schedule { self } + /// Set the error handler to use for systems that return a [`Result`](crate::result::Result). + pub fn set_error_handler(&mut self, error_handler: fn(Error, &ScheduleSystem)) { + self.error_handler = Some(error_handler); + } + /// Returns the schedule's current `ScheduleBuildSettings`. pub fn get_build_settings(&self) -> ScheduleBuildSettings { self.graph.settings.clone() @@ -438,8 +455,11 @@ impl Schedule { self.initialize(world) .unwrap_or_else(|e| panic!("Error when initializing schedule {:?}: {e}", self.label)); + let error_handler = self.error_handler.expect("schedule initialized"); + #[cfg(not(feature = "bevy_debug_stepping"))] - self.executor.run(&mut self.executable, world, None); + self.executor + .run(&mut self.executable, world, None, error_handler); #[cfg(feature = "bevy_debug_stepping")] { @@ -448,8 +468,12 @@ impl Schedule { Some(mut stepping) => stepping.skipped_systems(self), }; - self.executor - .run(&mut self.executable, world, skip_systems.as_ref()); + self.executor.run( + &mut self.executable, + world, + skip_systems.as_ref(), + error_handler, + ); } } @@ -474,6 +498,10 @@ impl Schedule { self.executor_initialized = false; } + if self.error_handler.is_none() { + self.error_handler = Some(world.get_resource_or_init::().error_handler); + } + if !self.executor_initialized { self.executor.init(&self.executable); self.executor_initialized = true; diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index b7ecf5b6f3312..0af46f51ed01f 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -45,10 +45,10 @@ use crate::{ query::{DebugCheckedUnwrap, QueryData, QueryFilter, QueryState}, removal_detection::RemovedComponentEvents, resource::Resource, - result::Result, + result, schedule::{Schedule, ScheduleLabel, Schedules}, storage::{ResourceData, Storages}, - system::Commands, + system::{Commands, ScheduleSystem}, world::{ command_queue::RawCommandQueue, error::{EntityFetchError, TryDespawnError, TryInsertBatchError, TryRunScheduleError}, @@ -3666,6 +3666,18 @@ impl World { schedules.allow_ambiguous_resource::(self); self.insert_resource(schedules); } + + /// Set the error handler to use for systems that return a [`Result`](crate::result::Result) in + /// a specific schedule. + pub fn set_schedule_error_handler( + &mut self, + label: impl ScheduleLabel, + error_handler: fn(result::Error, &ScheduleSystem), + ) -> Result<(), TryRunScheduleError> { + self.try_schedule_scope(label, |_, schedule| { + schedule.set_error_handler(error_handler); + }) + } } impl fmt::Debug for World { diff --git a/examples/ecs/fallible_systems.rs b/examples/ecs/fallible_systems.rs index 740976dadfa6b..2cd27a9dc910d 100644 --- a/examples/ecs/fallible_systems.rs +++ b/examples/ecs/fallible_systems.rs @@ -6,13 +6,33 @@ use bevy::prelude::*; use rand::distributions::Distribution; fn main() { - App::new() - .add_plugins(DefaultPlugins) - .add_systems(Startup, setup) - .run(); + let mut app = App::new(); + + app.add_plugins(DefaultPlugins) + .add_systems(Startup, (setup, failing_system)) + .add_systems(PostStartup, failing_system) + // You can set a global error handler for fallible systems. + // + // By default, fallible systems that return an error will panic. + .set_systems_error_handler(|err, system| warn!("{:?} failed: {err}", system.name())) + // You can handle individual systems by piping the output result. + .add_systems( + PostStartup, + failing_system.pipe(|result: In| { + result.0.inspect_err(|err| info!("captured error: {err}")) + }), + ); + + // You can also set a custom error handler per `Schedule` (optionally at runtime through + // `World::try_schedule_scope`): + app.get_schedule_mut(PostStartup) + .unwrap() + .set_error_handler(|err, system| error!("{:?} failed: {err}", system.name())); + + app.run(); } -/// An example of a system that calls several fallible functions with the questionmark operator. +/// An example of a system that calls several fallible functions with the question mark operator. fn setup( mut commands: Commands, mut meshes: ResMut>, @@ -77,3 +97,14 @@ fn setup( // Indicate the system completed successfully: Ok(()) } + +#[derive(Resource)] +struct UninitializedResource; + +fn failing_system(world: &mut World) -> Result { + world + .get_resource::() + .ok_or("Resource not initialized")?; + + Ok(()) +} From 48e98eadbc94e4750aa8e7da5c52a71818e70429 Mon Sep 17 00:00:00 2001 From: Jean Mertz Date: Sun, 9 Feb 2025 09:23:05 +0100 Subject: [PATCH 2/8] fixup! feat(ecs): configurable error handling for fallible systems Signed-off-by: Jean Mertz --- crates/bevy_ecs/src/schedule/executor/multi_threaded.rs | 4 ++-- crates/bevy_ecs/src/schedule/executor/simple.rs | 2 +- crates/bevy_ecs/src/schedule/executor/single_threaded.rs | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs b/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs index d6cec8b5661c8..8d5a006bd72a8 100644 --- a/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs +++ b/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs @@ -614,7 +614,7 @@ impl ExecutorState { system, context.environment.world_cell, ) { - (context.error_handler)(err, &system); + (context.error_handler)(err, system); } }; })); @@ -660,7 +660,7 @@ impl ExecutorState { let world = unsafe { context.environment.world_cell.world_mut() }; let res = std::panic::catch_unwind(AssertUnwindSafe(|| { if let Err(err) = __rust_begin_short_backtrace::run(system, world) { - (context.error_handler)(err, &system); + (context.error_handler)(err, system); } })); context.system_completed(system_index, res, system); diff --git a/crates/bevy_ecs/src/schedule/executor/simple.rs b/crates/bevy_ecs/src/schedule/executor/simple.rs index 42e629e491532..075763840faf5 100644 --- a/crates/bevy_ecs/src/schedule/executor/simple.rs +++ b/crates/bevy_ecs/src/schedule/executor/simple.rs @@ -108,7 +108,7 @@ impl SystemExecutor for SimpleExecutor { let f = AssertUnwindSafe(|| { if let Err(err) = __rust_begin_short_backtrace::run(system, world) { - error_handler(err, &system); + error_handler(err, system); } }); diff --git a/crates/bevy_ecs/src/schedule/executor/single_threaded.rs b/crates/bevy_ecs/src/schedule/executor/single_threaded.rs index 2e79d82527e66..b34c44dc8bff7 100644 --- a/crates/bevy_ecs/src/schedule/executor/single_threaded.rs +++ b/crates/bevy_ecs/src/schedule/executor/single_threaded.rs @@ -116,7 +116,7 @@ impl SystemExecutor for SingleThreadedExecutor { let f = AssertUnwindSafe(|| { if system.is_exclusive() { if let Err(err) = __rust_begin_short_backtrace::run(system, world) { - error_handler(err, &system); + error_handler(err, system); } } else { // Use run_unsafe to avoid immediately applying deferred buffers @@ -126,7 +126,7 @@ impl SystemExecutor for SingleThreadedExecutor { // update_archetype_component_access is being called immediately before this. unsafe { if let Err(err) = __rust_begin_short_backtrace::run_unsafe(system, world) { - error_handler(err, &system); + error_handler(err, system); } }; } From a186813eb3715ef7b7183b2c3227d1d76550d83e Mon Sep 17 00:00:00 2001 From: Jean Mertz Date: Sun, 9 Feb 2025 17:08:29 +0100 Subject: [PATCH 3/8] fixup! feat(ecs): configurable error handling for fallible systems Signed-off-by: Jean Mertz --- crates/bevy_app/src/app.rs | 2 +- crates/bevy_app/src/sub_app.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/bevy_app/src/app.rs b/crates/bevy_app/src/app.rs index 7ee4bbcac418e..45585f351e167 100644 --- a/crates/bevy_app/src/app.rs +++ b/crates/bevy_app/src/app.rs @@ -1264,7 +1264,7 @@ impl App { } /// Set the global system error handler to use for systems that return a - /// [`Result`](crate::result::Result). + /// [`Result`](bevy_ecs::result::Result). pub fn set_systems_error_handler( &mut self, error_handler: fn(Error, &ScheduleSystem), diff --git a/crates/bevy_app/src/sub_app.rs b/crates/bevy_app/src/sub_app.rs index 28618c8089949..611dbdb62932a 100644 --- a/crates/bevy_app/src/sub_app.rs +++ b/crates/bevy_app/src/sub_app.rs @@ -336,7 +336,7 @@ impl SubApp { } /// Set the global error handler to use for systems that return a - /// [`Result`](crate::result::Result). + /// [`Result`](bevy_ecs::result::Result). pub fn set_systems_error_handler( &mut self, error_handler: fn(Error, &ScheduleSystem), From 9dac003c3b8d91c6ded5a8f166fa98568f216345 Mon Sep 17 00:00:00 2001 From: Jean Mertz Date: Sun, 9 Feb 2025 17:39:36 +0100 Subject: [PATCH 4/8] fixup! feat(ecs): configurable error handling for fallible systems Signed-off-by: Jean Mertz --- crates/bevy_app/src/app.rs | 2 +- crates/bevy_app/src/sub_app.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/bevy_app/src/app.rs b/crates/bevy_app/src/app.rs index 45585f351e167..fbb9f098eb24b 100644 --- a/crates/bevy_app/src/app.rs +++ b/crates/bevy_app/src/app.rs @@ -1264,7 +1264,7 @@ impl App { } /// Set the global system error handler to use for systems that return a - /// [`Result`](bevy_ecs::result::Result). + /// [`Result`]. pub fn set_systems_error_handler( &mut self, error_handler: fn(Error, &ScheduleSystem), diff --git a/crates/bevy_app/src/sub_app.rs b/crates/bevy_app/src/sub_app.rs index 611dbdb62932a..e381dfb8b5d99 100644 --- a/crates/bevy_app/src/sub_app.rs +++ b/crates/bevy_app/src/sub_app.rs @@ -336,7 +336,7 @@ impl SubApp { } /// Set the global error handler to use for systems that return a - /// [`Result`](bevy_ecs::result::Result). + /// [`Result`]. pub fn set_systems_error_handler( &mut self, error_handler: fn(Error, &ScheduleSystem), From 95b54bac8e63257562d3e5d6ef805455233e17bc Mon Sep 17 00:00:00 2001 From: Jean Mertz Date: Mon, 10 Feb 2025 09:59:46 +0100 Subject: [PATCH 5/8] resolve review feedback Signed-off-by: Jean Mertz --- crates/bevy_app/src/app.rs | 6 +- crates/bevy_app/src/sub_app.rs | 6 +- crates/bevy_ecs/src/result.rs | 133 ++++++++++++++++++++++- crates/bevy_ecs/src/schedule/schedule.rs | 2 + crates/bevy_ecs/src/world/mod.rs | 15 +-- examples/ecs/fallible_systems.rs | 63 +++++++---- 6 files changed, 183 insertions(+), 42 deletions(-) diff --git a/crates/bevy_app/src/app.rs b/crates/bevy_app/src/app.rs index fbb9f098eb24b..cc7f45c8fd6d3 100644 --- a/crates/bevy_app/src/app.rs +++ b/crates/bevy_app/src/app.rs @@ -1263,8 +1263,10 @@ impl App { self } - /// Set the global system error handler to use for systems that return a - /// [`Result`]. + /// Set the global system error handler to use for systems that return a [`Result`]. + /// + /// See the [`bevy_ecs::result` module-level documentation](../../bevy_ecs/result/index.html) + /// for more information. pub fn set_systems_error_handler( &mut self, error_handler: fn(Error, &ScheduleSystem), diff --git a/crates/bevy_app/src/sub_app.rs b/crates/bevy_app/src/sub_app.rs index e381dfb8b5d99..15467031ec9fd 100644 --- a/crates/bevy_app/src/sub_app.rs +++ b/crates/bevy_app/src/sub_app.rs @@ -335,8 +335,10 @@ impl SubApp { self } - /// Set the global error handler to use for systems that return a - /// [`Result`]. + /// Set the global error handler to use for systems that return a [`Result`]. + /// + /// See the [`bevy_ecs::result` module-level documentation](../../bevy_ecs/result/index.html) + /// for more information. pub fn set_systems_error_handler( &mut self, error_handler: fn(Error, &ScheduleSystem), diff --git a/crates/bevy_ecs/src/result.rs b/crates/bevy_ecs/src/result.rs index 1d71bd9a4c5fb..5365f420c3d78 100644 --- a/crates/bevy_ecs/src/result.rs +++ b/crates/bevy_ecs/src/result.rs @@ -1,5 +1,76 @@ -//! Contains error and result helpers for use in fallible systems. +//! Error handling for "fallible" systems. +//! +//! When a system is added to a [`Schedule`], and its return type is that of [`Result`], then Bevy +//! 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 +//! [`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 +//! per `Schedule`: +//! +//! - [`App::set_systems_error_handler`] sets the global error handler for all systems of the +//! current [`World`]. +//! - [`Schedule::set_error_handler`] sets the error handler for all systems of that schedule. +//! +//! Bevy provides a number of pre-built error-handlers for you to use: +//! +//! - [`panic`] – panics with the system error +//! - [`error`] – logs the system error at the `error` level +//! - [`warn`] – logs the system error at the `warn` level +//! - [`info`] – logs the system error at the `info` level +//! - [`debug`] – logs the system error at the `debug` level +//! - [`trace`] – logs the system error at the `trace` level +//! - [`ignore`] – ignores the system error +//! +//! However, you can use any custom error handler logic by providing your own function (or +//! non-capturing closure that coerces to the function signature) as long as it matches the +//! signature: +//! +//! ```rust,ignore +//! fn(Error, &ScheduleSystem) +//! ``` +//! +//! The reference to [`ScheduleSystem`] allows you to access any non-mutating methods from +//! [`System`] – such as the system's [`name`] – in your error messages. +//! +//! For example: +//! +//! ```rust +//! # use bevy_ecs::prelude::*; +//! # use bevy_ecs::schedule::ScheduleLabel; +//! # use log::trace; +//! # fn update() -> Result { Ok(()) } +//! # #[derive(ScheduleLabel, Hash, Debug, PartialEq, Eq, Clone, Copy)] +//! # struct MySchedule; +//! # fn main() { +//! let mut schedule = Schedule::new(MySchedule); +//! schedule.add_systems(update); +//! schedule.set_error_handler(|error, system| { +//! if system.name().ends_with("update") { +//! trace!("Nothing to see here, move along."); +//! return; +//! } +//! +//! bevy_ecs::result::error(error, system); +//! }); +//! # } +//! ``` +//! +//! If you need special handling of individual fallible systems, you can use Bevy's [`system piping +//! feature`] to capture the `Result` output of the system and handle it accordingly. +//! +//! [`Schedule`]: crate::schedule::Schedule +//! [`panic`]: panic() +//! [`World`]: crate::world::World +//! [`Schedule::set_error_handler`]: crate::schedule::Schedule::set_error_handler +//! [`System`]: crate::system::System +//! [`name`]: crate::system::System::name +//! [`App::set_systems_error_handler`]: ../../bevy_app/struct.App.html#method.set_systems_error_handler +//! [`system piping feature`]: crate::system::In +use crate::system::ScheduleSystem; use alloc::boxed::Box; /// A dynamic error type for use in fallible systems. @@ -8,7 +79,61 @@ pub type Error = Box; /// A result type for use in fallible systems. pub type Result = core::result::Result; -pub(crate) fn default_error_handler(error: Error, system: &crate::system::ScheduleSystem) { - let name = system.name(); - panic!("Encountered an error in system `{name}`: {error:?}",); +macro_rules! inner { + ($call:path, $e:ident, $s:ident) => { + $call!("Encountered an error in system `{}`: {:?}", $s.name(), $e); + }; +} + +/// Error handler that panics with the system error. +#[track_caller] +#[inline] +pub fn panic(error: Error, system: &ScheduleSystem) { + inner!(panic, error, system); +} + +/// Error handler that logs the system error at the `error` level. +#[track_caller] +#[inline] +pub fn error(error: Error, system: &ScheduleSystem) { + inner!(log::error, error, system); +} + +/// Error handler that logs the system error at the `warn` level. +#[track_caller] +#[inline] +pub fn warn(error: Error, system: &ScheduleSystem) { + inner!(log::warn, error, system); +} + +/// Error handler that logs the system error at the `info` level. +#[track_caller] +#[inline] +pub fn info(error: Error, system: &ScheduleSystem) { + inner!(log::info, error, system); +} + +/// Error handler that logs the system error at the `debug` level. +#[track_caller] +#[inline] +pub fn debug(error: Error, system: &ScheduleSystem) { + inner!(log::debug, error, system); +} + +/// Error handler that logs the system error at the `trace` level. +#[track_caller] +#[inline] +pub fn trace(error: Error, system: &ScheduleSystem) { + inner!(log::trace, error, system); +} + +/// Error handler that ignores the system error. +#[track_caller] +#[inline] +pub fn ignore(_: Error, _: &ScheduleSystem) {} + +#[track_caller] +#[inline] +pub(crate) fn default_error_handler(error: Error, system: &ScheduleSystem) { + panic(error, system); } diff --git a/crates/bevy_ecs/src/schedule/schedule.rs b/crates/bevy_ecs/src/schedule/schedule.rs index 18377c4e3b2c3..df43bfe7f523e 100644 --- a/crates/bevy_ecs/src/schedule/schedule.rs +++ b/crates/bevy_ecs/src/schedule/schedule.rs @@ -414,6 +414,8 @@ impl Schedule { } /// Set the error handler to use for systems that return a [`Result`](crate::result::Result). + /// + /// See the [`result` module-level documentation](crate::result) for more information. pub fn set_error_handler(&mut self, error_handler: fn(Error, &ScheduleSystem)) { self.error_handler = Some(error_handler); } diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index 0af46f51ed01f..84061f68ac251 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -45,10 +45,9 @@ use crate::{ query::{DebugCheckedUnwrap, QueryData, QueryFilter, QueryState}, removal_detection::RemovedComponentEvents, resource::Resource, - result, schedule::{Schedule, ScheduleLabel, Schedules}, storage::{ResourceData, Storages}, - system::{Commands, ScheduleSystem}, + system::Commands, world::{ command_queue::RawCommandQueue, error::{EntityFetchError, TryDespawnError, TryInsertBatchError, TryRunScheduleError}, @@ -3666,18 +3665,6 @@ impl World { schedules.allow_ambiguous_resource::(self); self.insert_resource(schedules); } - - /// Set the error handler to use for systems that return a [`Result`](crate::result::Result) in - /// a specific schedule. - pub fn set_schedule_error_handler( - &mut self, - label: impl ScheduleLabel, - error_handler: fn(result::Error, &ScheduleSystem), - ) -> Result<(), TryRunScheduleError> { - self.try_schedule_scope(label, |_, schedule| { - schedule.set_error_handler(error_handler); - }) - } } impl fmt::Debug for World { diff --git a/examples/ecs/fallible_systems.rs b/examples/ecs/fallible_systems.rs index 2cd27a9dc910d..1fbba6f95c514 100644 --- a/examples/ecs/fallible_systems.rs +++ b/examples/ecs/fallible_systems.rs @@ -8,27 +8,47 @@ use rand::distributions::Distribution; fn main() { let mut app = App::new(); - app.add_plugins(DefaultPlugins) - .add_systems(Startup, (setup, failing_system)) - .add_systems(PostStartup, failing_system) - // You can set a global error handler for fallible systems. - // - // By default, fallible systems that return an error will panic. - .set_systems_error_handler(|err, system| warn!("{:?} failed: {err}", system.name())) - // You can handle individual systems by piping the output result. - .add_systems( - PostStartup, - failing_system.pipe(|result: In| { - result.0.inspect_err(|err| info!("captured error: {err}")) - }), - ); - - // You can also set a custom error handler per `Schedule` (optionally at runtime through - // `World::try_schedule_scope`): - app.get_schedule_mut(PostStartup) + app.add_plugins(DefaultPlugins); + + // 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 + // types of systems the same way, except for the error handling. + app.add_systems(Startup, (setup, failing_system)); + + // By default, fallible systems that return an error will panic. + // + // We can change this by setting a custom error handler. This can be done globally for all + // 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_systems_error_handler(bevy::ecs::result::warn); + + // Additionally, you can set a custom error handler per `Schedule`. This will take precedence + // over the global error handler. + // + // 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::system::ScheduleSystem) + // + app.add_systems(PostStartup, failing_system) + .get_schedule_mut(PostStartup) .unwrap() - .set_error_handler(|err, system| error!("{:?} failed: {err}", system.name())); - + .set_error_handler(|err, system| error!("{} failed: {err}", system.name())); + + // Individual systems can also be handled by piping the output result: + app.add_systems( + PostStartup, + failing_system.pipe(|result: In| { + let _ = result.0.inspect_err(|err| info!("captured error: {err}")); + }), + ); + + // If we run the app, we'll see the following output at startup: + // + // WARN Encountered an error in system `fallible_systems::failing_system`: "Resource not initialized" + // ERROR fallible_systems::failing_system failed: Resource not initialized + // INFO captured error: Resource not initialized app.run(); } @@ -103,7 +123,10 @@ struct UninitializedResource; fn failing_system(world: &mut World) -> Result { world + // `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("Resource not initialized")?; Ok(()) From 5c30e11d3034a3d5e7cfe056e658ddcf874e9ba5 Mon Sep 17 00:00:00 2001 From: Jean Mertz Date: Mon, 10 Feb 2025 13:06:16 +0100 Subject: [PATCH 6/8] make error handler compatible with non-scheduled systems Signed-off-by: Jean Mertz --- crates/bevy_app/src/app.rs | 6 +- crates/bevy_app/src/sub_app.rs | 14 ++-- crates/bevy_ecs/src/result.rs | 68 ++++++++++++------- crates/bevy_ecs/src/schedule/executor/mod.rs | 4 +- .../src/schedule/executor/multi_threaded.rs | 22 ++++-- .../bevy_ecs/src/schedule/executor/simple.rs | 13 ++-- .../src/schedule/executor/single_threaded.rs | 21 ++++-- crates/bevy_ecs/src/schedule/schedule.rs | 8 +-- examples/ecs/fallible_systems.rs | 4 +- 9 files changed, 104 insertions(+), 56 deletions(-) diff --git a/crates/bevy_app/src/app.rs b/crates/bevy_app/src/app.rs index cc7f45c8fd6d3..bf49a64e5b9b1 100644 --- a/crates/bevy_app/src/app.rs +++ b/crates/bevy_app/src/app.rs @@ -13,9 +13,9 @@ use bevy_ecs::{ event::{event_update_system, EventCursor}, intern::Interned, prelude::*, - result::Error, + result::{Error, SystemErrorContext}, schedule::{ScheduleBuildSettings, ScheduleLabel}, - system::{IntoObserverSystem, ScheduleSystem, SystemId, SystemInput}, + system::{IntoObserverSystem, SystemId, SystemInput}, }; use bevy_platform_support::collections::HashMap; use core::{fmt::Debug, num::NonZero, panic::AssertUnwindSafe}; @@ -1269,7 +1269,7 @@ impl App { /// for more information. pub fn set_systems_error_handler( &mut self, - error_handler: fn(Error, &ScheduleSystem), + error_handler: fn(Error, SystemErrorContext), ) -> &mut Self { self.main_mut().set_systems_error_handler(error_handler); self diff --git a/crates/bevy_app/src/sub_app.rs b/crates/bevy_app/src/sub_app.rs index 15467031ec9fd..8f0ccc8c5d5e2 100644 --- a/crates/bevy_app/src/sub_app.rs +++ b/crates/bevy_app/src/sub_app.rs @@ -3,8 +3,9 @@ use alloc::{boxed::Box, string::String, vec::Vec}; use bevy_ecs::{ event::EventRegistry, prelude::*, + result::{DefaultSystemsErrorHandler, SystemErrorContext}, schedule::{InternedScheduleLabel, ScheduleBuildSettings, ScheduleLabel}, - system::{ScheduleSystem, SystemId, SystemInput}, + system::{SystemId, SystemInput}, }; use bevy_platform_support::collections::{HashMap, HashSet}; use core::fmt::Debug; @@ -341,14 +342,13 @@ impl SubApp { /// for more information. pub fn set_systems_error_handler( &mut self, - error_handler: fn(Error, &ScheduleSystem), + error_handler: fn(Error, SystemErrorContext), ) -> &mut Self { - let mut schedules = self + let mut default_handler = self .world_mut() - .remove_resource::() - .unwrap_or_default(); - schedules.error_handler = error_handler; - self.world_mut().insert_resource(schedules); + .get_resource_or_init::(); + + default_handler.0 = error_handler; self } diff --git a/crates/bevy_ecs/src/result.rs b/crates/bevy_ecs/src/result.rs index 5365f420c3d78..2a4acbe8c11b0 100644 --- a/crates/bevy_ecs/src/result.rs +++ b/crates/bevy_ecs/src/result.rs @@ -29,11 +29,11 @@ //! signature: //! //! ```rust,ignore -//! fn(Error, &ScheduleSystem) +//! fn(Error, SystemErrorContext) //! ``` //! -//! The reference to [`ScheduleSystem`] allows you to access any non-mutating methods from -//! [`System`] – such as the system's [`name`] – in your error messages. +//! The [`SystemErrorContext`] allows you to access additional details relevant to providing +//! context surrounding the system error – such as the system's [`name`] – in your error messages. //! //! For example: //! @@ -47,13 +47,13 @@ //! # fn main() { //! let mut schedule = Schedule::new(MySchedule); //! schedule.add_systems(update); -//! schedule.set_error_handler(|error, system| { -//! if system.name().ends_with("update") { +//! schedule.set_error_handler(|error, ctx| { +//! if ctx.name.ends_with("update") { //! trace!("Nothing to see here, move along."); //! return; //! } //! -//! bevy_ecs::result::error(error, system); +//! bevy_ecs::result::error(error, ctx); //! }); //! # } //! ``` @@ -70,8 +70,8 @@ //! [`App::set_systems_error_handler`]: ../../bevy_app/struct.App.html#method.set_systems_error_handler //! [`system piping feature`]: crate::system::In -use crate::system::ScheduleSystem; -use alloc::boxed::Box; +use crate::{component::Tick, resource::Resource}; +use alloc::{borrow::Cow, boxed::Box}; /// A dynamic error type for use in fallible systems. pub type Error = Box; @@ -79,61 +79,81 @@ pub type Error = Box; /// 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 DefaultSystemsErrorHandler(pub fn(Error, SystemErrorContext)); + +impl Resource for DefaultSystemsErrorHandler {} + +impl Default for DefaultSystemsErrorHandler { + fn default() -> Self { + Self(panic) + } +} + macro_rules! inner { - ($call:path, $e:ident, $s:ident) => { - $call!("Encountered an error in system `{}`: {:?}", $s.name(), $e); + ($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, system: &ScheduleSystem) { - inner!(panic, error, system); +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, system: &ScheduleSystem) { - inner!(log::error, error, system); +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, system: &ScheduleSystem) { - inner!(log::warn, error, system); +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, system: &ScheduleSystem) { - inner!(log::info, error, system); +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, system: &ScheduleSystem) { - inner!(log::debug, error, system); +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, system: &ScheduleSystem) { - inner!(log::trace, error, system); +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, _: &ScheduleSystem) {} +pub fn ignore(_: Error, _: SystemErrorContext) {} #[track_caller] #[inline] -pub(crate) fn default_error_handler(error: Error, system: &ScheduleSystem) { +pub(crate) fn default_error_handler(error: Error, system: SystemErrorContext) { panic(error, system); } diff --git a/crates/bevy_ecs/src/schedule/executor/mod.rs b/crates/bevy_ecs/src/schedule/executor/mod.rs index 83dfb6862efbf..6bf43058c4e8e 100644 --- a/crates/bevy_ecs/src/schedule/executor/mod.rs +++ b/crates/bevy_ecs/src/schedule/executor/mod.rs @@ -18,7 +18,7 @@ use crate::{ component::{ComponentId, Tick}, prelude::{IntoSystemSet, SystemSet}, query::Access, - result::{Error, Result}, + 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, &ScheduleSystem), + error_handler: fn(Error, SystemErrorContext), ); fn set_apply_final_deferred(&mut self, value: bool); } diff --git a/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs b/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs index 8d5a006bd72a8..b6dd3ae336d07 100644 --- a/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs +++ b/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs @@ -17,7 +17,7 @@ use crate::{ archetype::ArchetypeComponentId, prelude::Resource, query::Access, - result::{Error, Result}, + result::{Error, Result, SystemErrorContext}, schedule::{is_apply_deferred, BoxedCondition, ExecutorKind, SystemExecutor, SystemSchedule}, system::ScheduleSystem, world::{unsafe_world_cell::UnsafeWorldCell, World}, @@ -134,7 +134,7 @@ pub struct ExecutorState { struct Context<'scope, 'env, 'sys> { environment: &'env Environment<'env, 'sys>, scope: &'scope Scope<'scope, 'env, ()>, - error_handler: fn(Error, &ScheduleSystem), + error_handler: fn(Error, SystemErrorContext), } impl Default for MultiThreadedExecutor { @@ -185,7 +185,7 @@ impl SystemExecutor for MultiThreadedExecutor { schedule: &mut SystemSchedule, world: &mut World, _skip_systems: Option<&FixedBitSet>, - error_handler: fn(Error, &ScheduleSystem), + error_handler: fn(Error, SystemErrorContext), ) { let state = self.state.get_mut().unwrap(); // reset counts @@ -614,7 +614,13 @@ impl ExecutorState { system, context.environment.world_cell, ) { - (context.error_handler)(err, system); + (context.error_handler)( + err, + SystemErrorContext { + name: system.name(), + last_run: system.get_last_run(), + }, + ); } }; })); @@ -660,7 +666,13 @@ impl ExecutorState { let world = unsafe { context.environment.world_cell.world_mut() }; let res = std::panic::catch_unwind(AssertUnwindSafe(|| { if let Err(err) = __rust_begin_short_backtrace::run(system, world) { - (context.error_handler)(err, system); + (context.error_handler)( + err, + SystemErrorContext { + name: system.name(), + last_run: system.get_last_run(), + }, + ); } })); context.system_completed(system_index, res, system); diff --git a/crates/bevy_ecs/src/schedule/executor/simple.rs b/crates/bevy_ecs/src/schedule/executor/simple.rs index 075763840faf5..ad01c324e9ea5 100644 --- a/crates/bevy_ecs/src/schedule/executor/simple.rs +++ b/crates/bevy_ecs/src/schedule/executor/simple.rs @@ -8,11 +8,10 @@ use tracing::info_span; use std::eprintln; use crate::{ - result::Error, + result::{Error, SystemErrorContext}, schedule::{ executor::is_apply_deferred, BoxedCondition, ExecutorKind, SystemExecutor, SystemSchedule, }, - system::ScheduleSystem, world::World, }; @@ -45,7 +44,7 @@ impl SystemExecutor for SimpleExecutor { schedule: &mut SystemSchedule, world: &mut World, _skip_systems: Option<&FixedBitSet>, - error_handler: fn(Error, &ScheduleSystem), + error_handler: fn(Error, SystemErrorContext), ) { // If stepping is enabled, make sure we skip those systems that should // not be run. @@ -108,7 +107,13 @@ impl SystemExecutor for SimpleExecutor { let f = AssertUnwindSafe(|| { if let Err(err) = __rust_begin_short_backtrace::run(system, world) { - error_handler(err, system); + error_handler( + err, + SystemErrorContext { + name: system.name(), + last_run: system.get_last_run(), + }, + ); } }); diff --git a/crates/bevy_ecs/src/schedule/executor/single_threaded.rs b/crates/bevy_ecs/src/schedule/executor/single_threaded.rs index b34c44dc8bff7..b5c44085d5db3 100644 --- a/crates/bevy_ecs/src/schedule/executor/single_threaded.rs +++ b/crates/bevy_ecs/src/schedule/executor/single_threaded.rs @@ -8,9 +8,8 @@ use tracing::info_span; use std::eprintln; use crate::{ - result::Error, + result::{Error, SystemErrorContext}, schedule::{is_apply_deferred, BoxedCondition, ExecutorKind, SystemExecutor, SystemSchedule}, - system::ScheduleSystem, world::World, }; @@ -51,7 +50,7 @@ impl SystemExecutor for SingleThreadedExecutor { schedule: &mut SystemSchedule, world: &mut World, _skip_systems: Option<&FixedBitSet>, - error_handler: fn(Error, &ScheduleSystem), + error_handler: fn(Error, SystemErrorContext), ) { // If stepping is enabled, make sure we skip those systems that should // not be run. @@ -116,7 +115,13 @@ impl SystemExecutor for SingleThreadedExecutor { let f = AssertUnwindSafe(|| { if system.is_exclusive() { if let Err(err) = __rust_begin_short_backtrace::run(system, world) { - error_handler(err, system); + error_handler( + err, + SystemErrorContext { + name: system.name(), + last_run: system.get_last_run(), + }, + ); } } else { // Use run_unsafe to avoid immediately applying deferred buffers @@ -126,7 +131,13 @@ impl SystemExecutor for SingleThreadedExecutor { // update_archetype_component_access is being called immediately before this. unsafe { if let Err(err) = __rust_begin_short_backtrace::run_unsafe(system, world) { - error_handler(err, system); + error_handler( + err, + SystemErrorContext { + name: system.name(), + last_run: system.get_last_run(), + }, + ); } }; } diff --git a/crates/bevy_ecs/src/schedule/schedule.rs b/crates/bevy_ecs/src/schedule/schedule.rs index df43bfe7f523e..d4f831982cd22 100644 --- a/crates/bevy_ecs/src/schedule/schedule.rs +++ b/crates/bevy_ecs/src/schedule/schedule.rs @@ -29,7 +29,7 @@ use crate::{ component::{ComponentId, Components, Tick}, prelude::Component, resource::Resource, - result::{default_error_handler, Error}, + result::{default_error_handler, Error, SystemErrorContext}, schedule::*, system::ScheduleSystem, world::World, @@ -47,7 +47,7 @@ pub struct Schedules { pub ignored_scheduling_ambiguities: BTreeSet, /// Error handler to use for systems that return a [`Result`](crate::result::Result). - pub error_handler: fn(Error, &ScheduleSystem), + pub error_handler: fn(Error, SystemErrorContext), } impl Default for Schedules { @@ -310,7 +310,7 @@ pub struct Schedule { executable: SystemSchedule, executor: Box, executor_initialized: bool, - error_handler: Option, + error_handler: Option, } #[derive(ScheduleLabel, Hash, PartialEq, Eq, Debug, Clone)] @@ -416,7 +416,7 @@ impl Schedule { /// Set the error handler to use for systems that return a [`Result`](crate::result::Result). /// /// See the [`result` module-level documentation](crate::result) for more information. - pub fn set_error_handler(&mut self, error_handler: fn(Error, &ScheduleSystem)) { + pub fn set_error_handler(&mut self, error_handler: fn(Error, SystemErrorContext)) { self.error_handler = Some(error_handler); } diff --git a/examples/ecs/fallible_systems.rs b/examples/ecs/fallible_systems.rs index 1fbba6f95c514..5a74ce8c4f40d 100644 --- a/examples/ecs/fallible_systems.rs +++ b/examples/ecs/fallible_systems.rs @@ -29,12 +29,12 @@ 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::system::ScheduleSystem) + // fn(bevy_ecs::result::Error, bevy_ecs::result::SystemErrorContext) // app.add_systems(PostStartup, failing_system) .get_schedule_mut(PostStartup) .unwrap() - .set_error_handler(|err, system| error!("{} failed: {err}", system.name())); + .set_error_handler(|err, ctx| error!("{} failed: {err}", ctx.name)); // Individual systems can also be handled by piping the output result: app.add_systems( From 9198abe2229b836f98a2e7967a573cb587aebe3c Mon Sep 17 00:00:00 2001 From: Jean Mertz Date: Mon, 10 Feb 2025 13:12:46 +0100 Subject: [PATCH 7/8] fixup! make error handler compatible with non-scheduled systems Signed-off-by: Jean Mertz --- crates/bevy_ecs/src/result.rs | 6 ------ crates/bevy_ecs/src/schedule/schedule.rs | 19 +++---------------- 2 files changed, 3 insertions(+), 22 deletions(-) diff --git a/crates/bevy_ecs/src/result.rs b/crates/bevy_ecs/src/result.rs index 2a4acbe8c11b0..b11e5c6edd4ba 100644 --- a/crates/bevy_ecs/src/result.rs +++ b/crates/bevy_ecs/src/result.rs @@ -151,9 +151,3 @@ pub fn trace(error: Error, ctx: SystemErrorContext) { #[track_caller] #[inline] pub fn ignore(_: Error, _: SystemErrorContext) {} - -#[track_caller] -#[inline] -pub(crate) fn default_error_handler(error: Error, system: SystemErrorContext) { - panic(error, system); -} diff --git a/crates/bevy_ecs/src/schedule/schedule.rs b/crates/bevy_ecs/src/schedule/schedule.rs index d4f831982cd22..614c1456347b2 100644 --- a/crates/bevy_ecs/src/schedule/schedule.rs +++ b/crates/bevy_ecs/src/schedule/schedule.rs @@ -29,7 +29,7 @@ use crate::{ component::{ComponentId, Components, Tick}, prelude::Component, resource::Resource, - result::{default_error_handler, Error, SystemErrorContext}, + result::{DefaultSystemsErrorHandler, Error, SystemErrorContext}, schedule::*, system::ScheduleSystem, world::World, @@ -40,24 +40,11 @@ pub use stepping::Stepping; use Direction::{Incoming, Outgoing}; /// Resource that stores [`Schedule`]s mapped to [`ScheduleLabel`]s excluding the current running [`Schedule`]. -#[derive(Resource)] +#[derive(Default, Resource)] pub struct Schedules { inner: HashMap, /// List of [`ComponentId`]s to ignore when reporting system order ambiguity conflicts pub ignored_scheduling_ambiguities: BTreeSet, - - /// Error handler to use for systems that return a [`Result`](crate::result::Result). - pub error_handler: fn(Error, SystemErrorContext), -} - -impl Default for Schedules { - fn default() -> Self { - Self { - inner: default(), - ignored_scheduling_ambiguities: default(), - error_handler: default_error_handler, - } - } } impl Schedules { @@ -501,7 +488,7 @@ impl Schedule { } if self.error_handler.is_none() { - self.error_handler = Some(world.get_resource_or_init::().error_handler); + self.error_handler = Some(world.get_resource_or_init::().0); } if !self.executor_initialized { From 7ac274a3edda041444296e84f87e1bfbe6d783aa Mon Sep 17 00:00:00 2001 From: Jean Mertz Date: Tue, 11 Feb 2025 09:39:59 +0100 Subject: [PATCH 8/8] resolve review feedback Signed-off-by: Jean Mertz --- crates/bevy_app/src/app.rs | 4 ++-- crates/bevy_app/src/sub_app.rs | 6 +++--- crates/bevy_ecs/src/result.rs | 10 +++++----- crates/bevy_ecs/src/schedule/schedule.rs | 4 ++-- examples/ecs/fallible_systems.rs | 2 +- 5 files changed, 13 insertions(+), 13 deletions(-) diff --git a/crates/bevy_app/src/app.rs b/crates/bevy_app/src/app.rs index bf49a64e5b9b1..05cf651894e18 100644 --- a/crates/bevy_app/src/app.rs +++ b/crates/bevy_app/src/app.rs @@ -1267,11 +1267,11 @@ impl App { /// /// See the [`bevy_ecs::result` module-level documentation](../../bevy_ecs/result/index.html) /// for more information. - pub fn set_systems_error_handler( + pub fn set_system_error_handler( &mut self, error_handler: fn(Error, SystemErrorContext), ) -> &mut Self { - self.main_mut().set_systems_error_handler(error_handler); + self.main_mut().set_system_error_handler(error_handler); self } diff --git a/crates/bevy_app/src/sub_app.rs b/crates/bevy_app/src/sub_app.rs index 8f0ccc8c5d5e2..ead7c468c37a9 100644 --- a/crates/bevy_app/src/sub_app.rs +++ b/crates/bevy_app/src/sub_app.rs @@ -3,7 +3,7 @@ use alloc::{boxed::Box, string::String, vec::Vec}; use bevy_ecs::{ event::EventRegistry, prelude::*, - result::{DefaultSystemsErrorHandler, SystemErrorContext}, + result::{DefaultSystemErrorHandler, SystemErrorContext}, schedule::{InternedScheduleLabel, ScheduleBuildSettings, ScheduleLabel}, system::{SystemId, SystemInput}, }; @@ -340,13 +340,13 @@ impl SubApp { /// /// See the [`bevy_ecs::result` module-level documentation](../../bevy_ecs/result/index.html) /// for more information. - pub fn set_systems_error_handler( + pub fn set_system_error_handler( &mut self, error_handler: fn(Error, SystemErrorContext), ) -> &mut Self { let mut default_handler = self .world_mut() - .get_resource_or_init::(); + .get_resource_or_init::(); default_handler.0 = error_handler; self diff --git a/crates/bevy_ecs/src/result.rs b/crates/bevy_ecs/src/result.rs index b11e5c6edd4ba..ef5eace90b07e 100644 --- a/crates/bevy_ecs/src/result.rs +++ b/crates/bevy_ecs/src/result.rs @@ -10,7 +10,7 @@ //! You can change the default behavior by registering a custom error handler, either globally or //! per `Schedule`: //! -//! - [`App::set_systems_error_handler`] sets the global error handler for all systems of the +//! - [`App::set_system_error_handler`] sets the global error handler for all systems of the //! current [`World`]. //! - [`Schedule::set_error_handler`] sets the error handler for all systems of that schedule. //! @@ -67,7 +67,7 @@ //! [`Schedule::set_error_handler`]: crate::schedule::Schedule::set_error_handler //! [`System`]: crate::system::System //! [`name`]: crate::system::System::name -//! [`App::set_systems_error_handler`]: ../../bevy_app/struct.App.html#method.set_systems_error_handler +//! [`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}; @@ -89,11 +89,11 @@ pub struct SystemErrorContext { } /// The default systems error handler stored as a resource in the [`World`](crate::world::World). -pub struct DefaultSystemsErrorHandler(pub fn(Error, SystemErrorContext)); +pub struct DefaultSystemErrorHandler(pub fn(Error, SystemErrorContext)); -impl Resource for DefaultSystemsErrorHandler {} +impl Resource for DefaultSystemErrorHandler {} -impl Default for DefaultSystemsErrorHandler { +impl Default for DefaultSystemErrorHandler { fn default() -> Self { Self(panic) } diff --git a/crates/bevy_ecs/src/schedule/schedule.rs b/crates/bevy_ecs/src/schedule/schedule.rs index 614c1456347b2..1e5d5ed7e0d3a 100644 --- a/crates/bevy_ecs/src/schedule/schedule.rs +++ b/crates/bevy_ecs/src/schedule/schedule.rs @@ -29,7 +29,7 @@ use crate::{ component::{ComponentId, Components, Tick}, prelude::Component, resource::Resource, - result::{DefaultSystemsErrorHandler, Error, SystemErrorContext}, + result::{DefaultSystemErrorHandler, Error, SystemErrorContext}, schedule::*, system::ScheduleSystem, world::World, @@ -488,7 +488,7 @@ impl Schedule { } if self.error_handler.is_none() { - self.error_handler = Some(world.get_resource_or_init::().0); + self.error_handler = Some(world.get_resource_or_init::().0); } if !self.executor_initialized { diff --git a/examples/ecs/fallible_systems.rs b/examples/ecs/fallible_systems.rs index 5a74ce8c4f40d..34b772af57f2b 100644 --- a/examples/ecs/fallible_systems.rs +++ b/examples/ecs/fallible_systems.rs @@ -21,7 +21,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_systems_error_handler(bevy::ecs::result::warn); + app.set_system_error_handler(bevy::ecs::result::warn); // Additionally, you can set a custom error handler per `Schedule`. This will take precedence // over the global error handler.