Skip to content

Commit

Permalink
panic on system error (#16979)
Browse files Browse the repository at this point in the history
# Objective

- First step for #16718 
- #16589 introduced an api that can only ignore errors, which is risky

## Solution

- Panic instead of just ignoring the errors

## Testing

- Changed the `fallible_systems` example to return an error
```
Encountered an error in system `fallible_systems::setup`: TooManyVertices { subdivisions: 300, number_of_resulting_points: 906012 }
Encountered a panic in system `fallible_systems::setup`!
Encountered a panic in system `bevy_app::main_schedule::Main::run_main`!
```
  • Loading branch information
mockersf authored Dec 26, 2024
1 parent c03e494 commit 394e82f
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 11 deletions.
22 changes: 17 additions & 5 deletions crates/bevy_ecs/src/schedule/executor/multi_threaded.rs
Original file line number Diff line number Diff line change
Expand Up @@ -605,11 +605,17 @@ 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 suppressing a possible failure.
let _ = __rust_begin_short_backtrace::run_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.system_completed(system_index, res, system);
Expand Down Expand Up @@ -653,8 +659,14 @@ 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 suppressing a possible failure.
let _ = __rust_begin_short_backtrace::run(system, world);
// 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.system_completed(system_index, res, system);
};
Expand Down
10 changes: 8 additions & 2 deletions crates/bevy_ecs/src/schedule/executor/simple.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,14 @@ impl SystemExecutor for SimpleExecutor {
}

let f = AssertUnwindSafe(|| {
// TODO: implement an error-handling API instead of suppressing a possible failure.
let _ = __rust_begin_short_backtrace::run(system, world);
// 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
);
}
});

#[cfg(feature = "std")]
Expand Down
20 changes: 16 additions & 4 deletions crates/bevy_ecs/src/schedule/executor/single_threaded.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,17 +109,29 @@ impl SystemExecutor for SingleThreadedExecutor {

let f = AssertUnwindSafe(|| {
if system.is_exclusive() {
// TODO: implement an error-handling API instead of suppressing a possible failure.
let _ = __rust_begin_short_backtrace::run(system, world);
// 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
);
}
} else {
// Use run_unsafe to avoid immediately applying deferred buffers
let world = world.as_unsafe_world_cell();
system.update_archetype_component_access(world);
// 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 suppressing a possible failure.
let _ = __rust_begin_short_backtrace::run_unsafe(system, world);
// 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
);
}
};
}
});
Expand Down
1 change: 1 addition & 0 deletions crates/bevy_ecs/src/system/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1766,6 +1766,7 @@ mod tests {
}

#[test]
#[should_panic]
fn simple_fallible_system() {
fn sys() -> Result {
Err("error")?;
Expand Down

0 comments on commit 394e82f

Please sign in to comment.