Unify and simplify command and system error handling (#18351)

# Objective

- ECS error handling is a lovely flagship feature for Bevy 0.16, all in
the name of reducing panics and encouraging better error handling
(#14275).
- Currently though, command and system error handling are completely
disjoint and use different mechanisms.
- Additionally, there's a number of distinct ways to set the
default/fallback/global error handler that have limited value. As far as
I can tell, this will be cfg flagged to toggle between dev and
production builds in 99.9% of cases, with no real value in more granular
settings or helpers.
- Fixes #17272

## Solution

- Standardize error handling on the OnceLock global error mechanisms
ironed out in #17215
- As discussed there, there are serious performance concerns there,
especially for commands
- I also think this is a better fit for the use cases, as it's truly
global
- Move from `SystemErrorContext` to a more general purpose
`ErrorContext`, which can handle observers and commands more clearly
- Cut the superfluous setter methods on `App` and `SubApp`
- Rename the limited (and unhelpful) `fallible_systems` example to
`error_handling`, and add an example of command error handling

## Testing

Ran the `error_handling` example.

## Notes for reviewers

- Do you see a clear way to allow commands to retain &mut World access
in the per-command custom error handlers? IMO that's a key feature here
(allowing the ad-hoc creation of custom commands), but I'm not sure how
to get there without exploding complexity.
- I've removed the feature gate on the default_error_handler: contrary
to @cart's opinion in #17215 I think that virtually all apps will want
to use this. Can you think of a category of app that a) is extremely
performance sensitive b) is fine with shipping to production with the
panic error handler? If so, I can try to gather performance numbers
and/or reintroduce the feature flag. UPDATE: see benches at the end of
this message.
- ~~`OnceLock` is in `std`: @bushrat011899 what should we do here?~~
- Do you have ideas for more automated tests for this collection of
features?

## Benchmarks

I checked the impact of the feature flag introduced: benchmarks might
show regressions. This bears more investigation. I'm still skeptical
that there are users who are well-served by a fast always panicking
approach, but I'm going to re-add the feature flag here to avoid
stalling this out.


![image](https://github.com/user-attachments/assets/237f644a-b36d-4332-9b45-76fd5cbff4d0)

---------

Co-authored-by: Zachary Harrold <[email protected]>

Author: alice-i-cecile

Cargo.toml

@@ -280,6 +280,9 @@ bevy_log = ["bevy_internal/bevy_log"]
 # Enable input focus subsystem
 bevy_input_focus = ["bevy_internal/bevy_input_focus"]
 
+# Use the configurable global error handler as the default error handler.
+configurable_error_handler = ["bevy_internal/configurable_error_handler"]
+
 # Enable passthrough loading for SPIR-V shaders (Only supported on Vulkan, shader capabilities and extensions must agree with the platform implementation)
 spirv_shader_passthrough = ["bevy_internal/spirv_shader_passthrough"]
 
@@ -2208,12 +2211,12 @@ category = "ECS (Entity Component System)"
 wasm = false
 
 [[example]]
-name = "fallible_systems"
-path = "examples/ecs/fallible_systems.rs"
+name = "error_handling"
+path = "examples/ecs/error_handling.rs"
 doc-scrape-examples = true
-required-features = ["bevy_mesh_picking_backend"]
+required-features = ["bevy_mesh_picking_backend", "configurable_error_handler"]
 
-[package.metadata.example.fallible_systems]
+[package.metadata.example.error_handling]
 name = "Fallible Systems"
 description = "Systems that return results to handle errors"
 category = "ECS (Entity Component System)"

crates/bevy_app/src/app.rs

@@ -10,7 +10,6 @@ use alloc::{
 pub use bevy_derive::AppLabel;
 use bevy_ecs::{
     component::RequiredComponentsError,
-    error::{BevyError, SystemErrorContext},
     event::{event_update_system, EventCursor},
     intern::Interned,
     prelude::*,
@@ -1274,18 +1273,6 @@ impl App {
         self
     }
 
-    /// Set the global system error handler to use for systems that return a [`Result`].
-    ///
-    /// See the [`bevy_ecs::error` module-level documentation](bevy_ecs::error)
-    /// for more information.
-    pub fn set_system_error_handler(
-        &mut self,
-        error_handler: fn(BevyError, SystemErrorContext),
-    ) -> &mut Self {
-        self.main_mut().set_system_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.

crates/bevy_app/src/sub_app.rs

@@ -1,7 +1,6 @@
 use crate::{App, AppLabel, InternedAppLabel, Plugin, Plugins, PluginsState};
 use alloc::{boxed::Box, string::String, vec::Vec};
 use bevy_ecs::{
-    error::{DefaultSystemErrorHandler, SystemErrorContext},
     event::EventRegistry,
     prelude::*,
     schedule::{InternedScheduleLabel, InternedSystemSet, ScheduleBuildSettings, ScheduleLabel},
@@ -336,22 +335,6 @@ impl SubApp {
         self
     }
 
-    /// Set the global error handler to use for systems that return a [`Result`].
-    ///
-    /// See the [`bevy_ecs::error` module-level documentation](bevy_ecs::error)
-    /// for more information.
-    pub fn set_system_error_handler(
-        &mut self,
-        error_handler: fn(BevyError, SystemErrorContext),
-    ) -> &mut Self {
-        let mut default_handler = self
-            .world_mut()
-            .get_resource_or_init::<DefaultSystemErrorHandler>();
-
-        default_handler.0 = error_handler;
-        self
-    }
-
     /// See [`App::add_event`].
     pub fn add_event<T>(&mut self) -> &mut Self
     where

crates/bevy_ecs/Cargo.toml

@@ -33,7 +33,11 @@ bevy_reflect = ["dep:bevy_reflect"]
 ## Extends reflection support to functions.
 reflect_functions = ["bevy_reflect", "bevy_reflect/functions"]
 
-## Use the configurable global error handler as the default error handler
+## Use the configurable global error handler as the default error handler.
+## 
+## This is typically used to turn panics from the ECS into loggable errors.
+## This may be useful for production builds,
+## but can result in a measurable performance impact, especially for commands.
 configurable_error_handler = []
 
 ## Enables automatic backtrace capturing in BevyError

crates/bevy_ecs/src/error/command_handling.rs

@@ -0,0 +1,108 @@
+use core::{any::type_name, fmt};
+
+use crate::{
+    entity::Entity,
+    system::{entity_command::EntityCommandError, Command, EntityCommand},
+    world::{error::EntityMutableFetchError, World},
+};
+
+use super::{default_error_handler, BevyError, ErrorContext};
+
+/// 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 `()`.
+pub trait HandleError<Out = ()> {
+    /// Takes a [`Command`] that returns a Result and uses a given error handler function to convert it into
+    /// a [`Command`] that internally handles an error if it occurs and returns `()`.
+    fn handle_error_with(self, error_handler: fn(BevyError, ErrorContext)) -> impl Command;
+    /// Takes a [`Command`] that returns a Result and uses the default error handler function to convert it into
+    /// a [`Command`] that internally handles an error if it occurs and returns `()`.
+    fn handle_error(self) -> impl Command
+    where
+        Self: Sized,
+    {
+        self.handle_error_with(default_error_handler())
+    }
+}
+
+impl<C, T, E> HandleError<Result<T, E>> for C
+where
+    C: Command<Result<T, E>>,
+    E: Into<BevyError>,
+{
+    fn handle_error_with(self, error_handler: fn(BevyError, ErrorContext)) -> impl Command {
+        move |world: &mut World| match self.apply(world) {
+            Ok(_) => {}
+            Err(err) => (error_handler)(
+                err.into(),
+                ErrorContext::Command {
+                    name: type_name::<C>().into(),
+                },
+            ),
+        }
+    }
+}
+
+impl<C> HandleError for C
+where
+    C: Command,
+{
+    #[inline]
+    fn handle_error_with(self, _error_handler: fn(BevyError, ErrorContext)) -> impl Command {
+        self
+    }
+    #[inline]
+    fn handle_error(self) -> impl Command
+    where
+        Self: Sized,
+    {
+        self
+    }
+}
+
+/// Passes in a specific entity to an [`EntityCommand`], resulting in a [`Command`] that
+/// internally runs the [`EntityCommand`] on that entity.
+///
+// NOTE: This is a separate trait from `EntityCommand` because "result-returning entity commands" and
+// "non-result returning entity commands" require different implementations, so they cannot be automatically
+// implemented. And this isn't the type of implementation that we want to thrust on people implementing
+// EntityCommand.
+pub trait CommandWithEntity<Out> {
+    /// Passes in a specific entity to an [`EntityCommand`], resulting in a [`Command`] that
+    /// internally runs the [`EntityCommand`] on that entity.
+    fn with_entity(self, entity: Entity) -> impl Command<Out> + HandleError<Out>;
+}
+
+impl<C> CommandWithEntity<Result<(), EntityMutableFetchError>> for C
+where
+    C: EntityCommand,
+{
+    fn with_entity(
+        self,
+        entity: Entity,
+    ) -> impl Command<Result<(), EntityMutableFetchError>>
+           + HandleError<Result<(), EntityMutableFetchError>> {
+        move |world: &mut World| -> Result<(), EntityMutableFetchError> {
+            let entity = world.get_entity_mut(entity)?;
+            self.apply(entity);
+            Ok(())
+        }
+    }
+}
+
+impl<C, T, Err> CommandWithEntity<Result<T, EntityCommandError<Err>>> for C
+where
+    C: EntityCommand<Result<T, Err>>,
+    Err: fmt::Debug + fmt::Display + Send + Sync + 'static,
+{
+    fn with_entity(
+        self,
+        entity: Entity,
+    ) -> impl Command<Result<T, EntityCommandError<Err>>> + HandleError<Result<T, EntityCommandError<Err>>>
+    {
+        move |world: &mut World| {
+            let entity = world.get_entity_mut(entity)?;
+            self.apply(entity)
+                .map_err(EntityCommandError::CommandFailed)
+        }
+    }
+}