Bevy: Result systems

[cart]

The only way to handle errors in systems right now is to result.unwrap() them. This is both less ergonomic and panic-ey. Ideally systems could optionally return an anyhow::Result<()>. It would also be great if developers could define their own error handlers. Maybe some devs want to print their errors whereas others want to panic.

I think there are two approaches we could adopt here:

1. Adapt legion to support return types
2. Add impls for system fns that return results and handle the Result inside of into_system()

I think option (2) is the least destructive / most friendly to upstream, but it means that only system fns can handle errors.

If this is implemented, it also makes sense to modify bevy libs to return error types instead of Options for common operations to improve erognomics.

fn some_system(a: Res<A>, b: Res<B>, x: Com<X>, y: Com<Y>) -> Result<()> {
  a.do_something_risky()?;
  // system logic here
  Ok(())
}

// inside into_system() impl for Fn(Res<A>, Com<X>) -> Result<()>
system_builder.with_resource::<ErrorHandler>()
// resources are (error_handler, a, b)
result = run_system(a, b, x, y);
error_handler.handle(result);

The biggest downside to implementing this I can think of is that this multiplies the number of system fn impls by 2 (which is already in the hundreds of impls). That would come at an estimated clean-compile time cost of 40 seconds on fast computers ... not ideal. The best way to mitigate that is to revert to non-flat system fn impls, which would then only require a single new impl:

Doing so would both remove the need for a macro for system fn impls and reduce clean compile times by 40 seconds (current) / 80 seconds (with Result impls). But its also not nearly as nice to look at / type

// pseudocode
impl IntoSystem for Fn(ResourceSet, View<'a>) -> Result<()> { /* impl here */ }

fn some_system((a, b): (Res<A>, Res<B>), (x, y): (Com<X>, Com<Y>)) -> Result<()> {
  a.do_something_risky()?;
  // system logic here
  Ok(())
}

[cart]

Now that we're using bevy_ecs i think we can make this story even nicer. The System trait could always require a Result<()> and we could implicitly return Ok() in system functions that dont do that.

[cart]

The additional compile time cost would also be much lower with bevy_ecs

[Ratysz]

What happens to the error returned by a system? What happens to systems after the one that produced the error, are they skipped? If not, what happens if several systems return errors?

In my opinion, if the error needs to be handled somewhere other than the system that produced it then a more direct way of transferring the necessary data would be more fitting (e.g. a channel).

[cart]

Skipping errors is certainly not desirable (at least while developing). The "error handler" that i mentioned above would probably still panic by default. But it could also be configured to just print the error or write it to a file.

In order for such an error handler to work, we would likely need to be able to send them from the system execution thread to some receiver. As you mentioned, a channel would make a lot of sense here.

[Ratysz]

I was insinuating that for all of the systems in a game there's unlikely to be a good one-size-fits-all error recovery strategy. Systems related to different mechanisms of the game are bound to have different recovery requirements, users shouldn't have to lump them all together into a single handler. This isn't something that the engine should anticipate.

If this isn't about error recovery but strictly about log and/or panic - still not convinced that returning results is going to win any ergonomics points, versus providing convenience utilities to do that on site of the error. Like a logging plugin; one can be easily built, for example, on top of log and any of it's compatible libraries (I like fern).

[mcobzarenco]

If this isn't about error recovery but strictly about log and/or panic - still not convinced that returning results is going to win any ergonomics points, versus providing convenience utilities to do that on site of the error.

Being able to use ? in a system would be more ergonomic compared to destructing the result and calling a function followed by an early return, no?

[memoryruins]

For systems where ? is preferred for many early returns, I have so far used a separate function for the system's implementation. The system can then handle the error and/or send it to an error handler via an event or another preferred method.

fn fallible_system(state: ResMut<State>, query: Query<Component>) {
    if let Err(e) = fallible(state, query) {
        // Handle the error here and/or forward error as an event,
        // e.g. error_events.send(e);
    }
}

fn fallible(mut state: ResMut<State>, mut query: Query<Component>) -> Result<ok, error> {
    // use `?` and friends
}

The second function can be placed inside the system to hide it. A closure (or nightly try blocks) can be used to avoid rewriting parameters on nightly. So far it has mostly been startup/setup systems where I have wanted early returns, but separating the implementation out works well in that effect.