Error handling on ApplicationHandler

[lylythechosenone]

Description

Introduction

Many of winit's functions return Results, including ActiveEventLoop::create_window. Despite this, all ApplicationHandler functions have no return type. It would be nice to be able to specify an Error type, and have the functions return Result<(), Self::Error> instead. If these functions were to fail, run_app would forward this error, potentially with a mechanism for resuming the loop after handling the error.

Current alternatives

The way I currently implement error handling is by having a result field on my handler, and setting it on every operation that returns a Result; then, if this is an Err, I exit the handler, and read the value outside of the handler. This is not very ergonomic to use.

Relevant platforms

Windows, macOS, Wayland, X11, Web, iOS, Android

[madsmtm]

run_app would forward this error

You could always just panic

potentially with a mechanism for resuming the loop after handling the error.

Hmm, how exactly do you envision this would work on iOS? Once we've started running the application in run_app, we cannot exit that function (outside of abort).

[kchibisov]

Many of winit's functions return Results, including ActiveEventLoop::create_window. Despite this, all ApplicationHandler functions have no return type. It would be nice to be able to specify an Error type, and have the functions return Result<(), Self::Error> instead. If these functions were to fail, run_app would forward this error, potentially with a mechanism for resuming the loop after handling the error.

You can forward the error yourself via the State if you really want to and dispatch it during the exiting function.

Having return Result for lots of trait functions is extremely noisy and a lot of errors are not fatal to the application in the first place.

[lylythechosenone]

forward the error yourself via the State

@kchibisov what does this mean? what State? The rest of your comment makes sense, I suppose. You're right that there isn't much point in returning fatal errors. I suppose I've developed an unhealthy aversion to panics.

[kchibisov]

You're passing the &mut ApplicationHandler, you can define a field holding Option<Error> and request exit or whatever you want to handle errors. For some errors you don't really nead to bail.

See https://github.com/alacritty/alacritty/blob/master/alacritty/src/event.rs#L76

[kchibisov]

We can return Result from the exiting though, but that's about it, I think.

[lylythechosenone]

you can define a field holding Option<Error>

@kchibisov ah, that's exactly what I'm already doing

[nerditation]

We can return Result from the exiting though

exiting is a notification callback, you still need to store the error in the app states when you encountered it and extract this information later, just as OP already did. the only difference is extracting inside the exiting callback rather than extracting outside the event loop.

I think a better alternative is to change exit() to add an argument of exit code (or status), e.g. ActiveEventLoop::exit(status: S) where type S can be a generic type, probably defaults to (), similar to UserEvent. along with the change to exit(), run_app() should be changed to return Result<S, EventLoopError>

a slight variant to this idea is to name it differently, e.g. exit_with(status: S), and provide a convenient exit() wrapper for cases where S: Default.

---

EDIT:

after some thinking, I found a major problem with my idea: it infests the ActiveEventLoop with application dependent generic parameter types, which resembles how the old ELWT design was back then.

in theory, we can erase the status type (Box<dyn Any>, for example), the trade off is that if application calls exit_with(wrong_status_type), it cannot be catched at compile time and will panic.

due to this problem, now I don't think my suggestion is a good fit anymore.

after all, this kind of ergonomic inconvenience is very subjective, maybe it can be better solved from the application side instead of the platform side, e.g., by automating (or "hiding") some of the error state management. the advantage for this approach is that it can be implemented as a library on top of winit, no need to change the winit core abstraction.

below is a sketch for one possible implementation of such "adapter/wrapper" library:

/// this is the trait application should implement instead of the winit one
/// it can use whatever protocol for return status propogation
/// for example, associated type as return type for event callbacks
trait ApplicationHandlerWithStatus<T: 'static = ()> {
	type Status;
	/// use `Result` may be convenient if the status is meant for error cases
	/// because it allows you touse question mark operator
	/// can also return other types, e.g. `Option<Self::Status>`
	fn foo_bar_event(
		&mut self,
		event_loop: &ActiveEventLoop,
		event: FooBarEvent<T>,
	) -> Result<(), Self::Status> {
		None
	}
}

/// add extension api to `EventLoop`
impl<T> EventLoopExt for EventLoop<T> {
	fn run_app_with_status(self, app: App) -> Result<App::Status, EventLoopError>
	where
		App: ApplicationHandlerWithStatus<T>,
	{
		let mut appx = AppWithStatus { app, status: None };
		let result = self.run_app_on_demand(&mut app);
		result.map(|()| {
			appx.status
				.take()
				.expect("app didn't exit with status properly")
		})
	}
}

// -----------------------------------------------------------------------------
//                                 glue logic                                   
// -----------------------------------------------------------------------------

struct AppWithStatus<A, S> {
	app: A,
	status: Result<S, EventLoopError>,
}

impl<A, E, S> ApplicationHandler<E> for AppWithStatus<A, S>
where
	E: 'static,
	A: ApplicationHandlerWithStatus<E, Status = S>,
{
	fn foo_bar_event(&mut self, event_loop: &ActiveEventLoop, event: FooBarEvent<E>) {
		// call app callback, and store the returned error status
		// and if it is `Err`, exit the event loop, the error will be extracted after
		self.status = self.app.foo_bar_event_with_status(event_loop, event).err();
		if self.status.is_some() {
			event_loop.exit();
		}
	}
}