Audit exception handling behavior (esp RuntimeError)

[freakboy3742]

There are potentially some inconsistencies in exception handling behavior, especially as a result of WebView handling, and the handling of the Winforms asyncio event loop integration. We need to audit these interactions to ensure consistency across platforms.

Originally posted by @rmartin16 in #2781 (comment)

My original thought was to go non-async, with app.loop.call_soon_threadsafe()... not sure if that will change anything by ensuring that you're getting a method on the app's specific loop, rather than potentially spawning a thread-specific loop in the .NET context.

This did print an exception to the console; however, the app continued running.

That's not entirely unexpected. The Winforms asyncio integration is a bit weird, because of the winforms STA apartment state. The app itself runs in a thread; exceptions are caught and the stack trace is manually unwound for printing purposes, but won't necessarily terminate the app itself.

I'm not 100% convinced this implementation is correct - the intention was that an exception in an event handler would be reported, but wouldn't kill the app; but it's possible this is being a little too enthusiastic with surpassing exit conditions, and actual errors in the app mainline aren't being surfaced.

So, my interest remain piqued since you said this.

To recap, I had put a task on the event loop that would raise a RuntimeError....and all that happened was the stacktrace was printed to the console.

I tracked down this behavior to BaseEventLoop.default_exception_handler(). If I add a default_exception_handler() to WinformsProactorEventLoop, I see it run for the RuntimeError.

    def default_exception_handler(self, context):
        print("this is your friendly exception handler")
        super().default_exception_handler(context)

[webview] Starting in dev mode...
===========================================================================
this is your friendly exception handler
Exception in callback WebView.winforms_initialization_completed.<locals>.raise_exception() at C:\Users\user\github\beeware\toga\winforms\src\toga_winforms\widgets\webview.py:72
handle: <Handle WebView.winforms_initialization_completed.<locals>.raise_exception() at C:\Users\user\github\beeware\toga\winforms\src\toga_winforms\widgets\webview.py:72 created at C:\Users\user\github\beeware\toga\winforms\src\toga_winforms\widgets\webview.py:75>
source_traceback: Object created at (most recent call last):
  File "C:\Users\user\github\beeware\toga\winforms\src\toga_winforms\app.py", line 167, in _run_app
    self.loop.run_forever(self)
  File "C:\Users\user\github\beeware\toga\winforms\src\toga_winforms\libs\proactor.py", line 76, in run_forever
    WinForms.Application.Run(self.app.app_context)
  File "C:\Users\user\github\beeware\toga\winforms\src\toga_winforms\libs\wrapper.py", line 22, in __call__
    return function(*args, **kwargs)
  File "C:\Users\user\github\beeware\toga\winforms\src\toga_winforms\widgets\webview.py", line 75, in winforms_initialization_completed
    self.interface.app.loop.call_soon_threadsafe(raise_exception)
Traceback (most recent call last):
  File "C:\Users\user\.pyenv\pyenv-win\versions\3.12.1\Lib\asyncio\events.py", line 84, in _run
    self._context.run(self._callback, *self._args)
  File "C:\Users\user\github\beeware\toga\winforms\src\toga_winforms\widgets\webview.py", line 73, in raise_exception
    raise RuntimeError("failed to load WebView")
RuntimeError: failed to load WebView

So, given this, what is the desired behavior when Toga itself raises an exception?

If I simulate raising the RuntimeError for WebView on Gtk, a console error is logged but the app window never renders.

[webview] Starting in dev mode...
===========================================================================
Traceback (most recent call last):
  File "/home/russell/github/beeware/toga/gtk/src/toga_gtk/app.py", line 45, in gtk_startup
    self.interface._startup()
  File "/home/russell/.pyenv/versions/briefcase-3.10/lib/python3.10/site-packages/toga/app.py", line 619, in _startup
    self.startup()
  File "/home/russell/github/beeware/toga/examples/webview/webview/app.py", line 86, in startup
    self.webview = toga.WebView(
  File "/home/russell/.pyenv/versions/briefcase-3.10/lib/python3.10/site-packages/toga/widgets/webview.py", line 47, in __init__
    self._impl = self.factory.WebView(interface=self)
  File "/home/russell/github/beeware/toga/gtk/src/toga_gtk/widgets/base.py", line 16, in __init__
    self.create()
  File "/home/russell/github/beeware/toga/gtk/src/toga_gtk/widgets/webview.py", line 15, in create
    raise RuntimeError(
RuntimeError: Unable to import WebKit2. Ensure that the system package providing WebKit2 and its GTK bindings have been installed. See https://toga.readthedocs.io/en/stable/reference/api/widgets/webview.html#system-requirements for details.

[freakboy3742]

There's a couple of closely related issues here:

1. What is the correct behavior when a runtime component (such as WebView) isn't available. Regarding WebView specifically, Winforms will successfully create a widget, but then raise a dialog to tell the user that the widget won't work. GTK crashes on import. MapView will have analogous issues (because it's a webview); there are some other places (especially on Android and GTK) where there are runtime checks of an import.
2. If an event handler raises an exception, what should happen?
3. Does the response to (2) change if the exception is a RuntimeError? Does it change if the exception is a SystemExit?
4. Does the answer to (2) change if the event handler is async?

Regarding (1), I think the recently added behavior on Winforms WebView is more desirable than the current "hard crash" GTK version. That's a potential enhancement to the GTK backend (and, for that matter, the Android backend as well). The one major complication will be that the Winforms Webview failure is a configuration error - the widget will exist, and take up the relevant space; it just won't display web content. In the GTK context, we might have to put in a dummy toga.Box (or a dummy widget that has enough of an implementation to satisfy, but not implement the WebView interface). An analog of unittest.Mock() which you can call any method you want, but it won't do anything might be one approach.

As for (2) and (3) - I'd argue that SystemExit is the only exception that should fully propagate and cause a termination. The rest of Toga's API is intentionally robust to programming error - clicking a button shouldn't ever cause the UI to crash - but it may cause log messages to be generated. In that context, SystemExit is the only exception that should actually kill the app, because that's literally what the exception is requesting. Anything else should be "soft reported".

The only qualifier I'd put on this is whether it would be worth having a debug mode that differentiates between surfacing those errors to the log, and surfacing them to the GUI as well. I'm not 100% convinced this is desirable; on the one hand, users need to be able to report errors they're seeing, and if they're in a non-console context, they won't see console errors; but on the other hand, it could be really distracting (and confusing) for users to get stack trace dialogs in the middle of "normal" app operation. I guess the counterargument is that "normal" app operation shouldn't generate exceptions, so if they're happening that's a bug that the developer should address...

I mentioned (4) specifically because I'm not completely convinced we've got exception handling for async handlers correct. Broadly, there shouldn't be any difference in error handling between sync and async handlers. However, in the recent work on Document API and Status Icons, I inevitably had some buggy code. There were a couple of occasions, especially in tests, where it looked like exceptions in asyncio handlers were being transparently swallowed. I didn't go down the rabbit hole to verify if we are missing a default exception handler on the asyncio event loop, or if the issue is the handler wrapper that is used to make handlers guaranteed async - or even to generate a replication case - but if we're taking a close look at exception handling, this would be worth looking into to ensure that there's no obvious gaps here.