Add queue of ExceptionHandlerDialogs #1123
Conversation
Fixes getting-things-gnome#1093. This commit adds a queue of ExceptionHandlerDialogs, so we only show one at a time. This avoids the scenario where GTG creates dialogs in a loop, making them impossible to close. If an exception happens while the dialog is opened, its own dialog will pop up when the current one gets closed. First test with a repro of getting-things-gnome#1093, raising an exception in the TaskBox constructor. 3 dialogs pop up consecutively, and it's always possible to click on "Continue" to keep GTG running. After that, clicking on a different pane also makes 3 error dialogs pop up in sequence. Interestingly, GTG hangs on the click on the third Continue. I'm not too bothered about it because this emulates a rare case of the system being in a really bad state and it's probably a good idea to exit anyway. We could consider setting `ignorable` to False when there is already an exception in the queue, forcing an exit in the "exception while handling an exception" scenario. I'm also not sure why there are 3 dialogs and not 5 (the size of the queue) or however many times the exception is raised (it seems to stop at some point). But I don't think it matters much. I also confirmed that I'm seeing "Caught AttributeError ('ListItem' object has no attribute 'bindings') but not showing dialog for it because too many exceptions are happening." in the logs. Second test with the more common case of a single exception. I added an exception inside `on_search_toggled()`. GTG was working fine until I clicked on the magnifying glass, at which point the dialog popped up. I could click on Continue and keep using GTG.
| @@ -113,19 +114,14 @@ def __init__(self, app): | |||
| self.sidebar = Sidebar(app, app.ds, self) | |||
| self.sidebar_vbox.append(self.sidebar) | |||
|
|
|||
| self.panes = { | |||
There was a problem hiding this comment.
Not necessary, just something I noticed while I was going through the code. I left it as part of the commit because it's not mixed up with other changes.
| @@ -35,7 +35,7 @@ class TaskBox(Gtk.Box): | |||
|
|
|||
| def __init__(self, config, is_actionable=False): | |||
| self.config = config | |||
| super(TaskBox, self).__init__() | |||
There was a problem hiding this comment.
This is from Python 2. Probably something that Ruff can help with :)
| # It it's not, it means that a dialog is already being shown. In that case, | ||
| # the response handler of the current dialog will show the next one. | ||
| need_to_show_dialog = exception_dialog_queue.empty() | ||
|
|
||
| dialog = ExceptionHandlerDialog(exception, main_msg, ignorable, context) |
There was a problem hiding this comment.
We could also do something like
already_showing_other_dialog = not exception_dialog_queue.empty()
if already_showing_other_dialog:
# The current exception happened while another exception is being processed
# Show it to the user but force exit because GTG is in a bad state
ignorable = False
and that removes the problem of the infinite queue (there will never be more than 2 consecutive dialogs). We can even remove the queue entirely, and use a currently_shown_exception_dialog: Optional[ExceptionHandlerDialog] = None.
I'm pretty 50/50 about it.
|
A thing to consider is when a fatal error occurs ( |
Ah, excellent point. I think this is a good reason to consider the second exception as fatal and close GTG. That avoids the complexity of the queue while still fixing the problem of the infinite dialogs. It should be a very rare case anyway. If we do run into issues because of that, we can revisit. |
diegogangl
added this to the 0.7: "Getting things Gradius — Destroy the core!" milestone
|
LGTM, thanks! We can put this on master for now and tweak/revisit as we go |
Fixes #1093.
This commit adds a queue of ExceptionHandlerDialogs, so we only show one at a time. This avoids the scenario where GTG creates dialogs in a loop, making them impossible to close.
If an exception happens while the dialog is opened, its own dialog will pop up when the current one gets closed.
First test with a repro of #1093, raising an exception in the TaskBox constructor. 3 dialogs pop up consecutively, and it's always possible to click on "Continue" to keep GTG running. After that, clicking on a different pane also makes 3 error dialogs pop up in sequence. Interestingly, GTG hangs on the click on the third Continue. I'm not too bothered about it because this emulates a rare case of the system being in a really bad state and it's probably a good idea to exit anyway.
We could consider setting
ignorableto False when there is already an exception in the queue, forcing an exit in the "exception while handling an exception" scenario.I'm also not sure why there are 3 dialogs and not 5 (the size of the queue) or however many times the exception is raised (it seems to stop at some point). But I don't think it matters much.
I also confirmed that I'm seeing "Caught AttributeError ('ListItem' object has no attribute 'bindings') but not showing dialog for it because too many exceptions are happening." in the logs.
Second test with the more common case of a single exception. I added an exception inside
on_search_toggled(). GTG was working fine until I clicked on the magnifying glass, at which point the dialog popped up. I could click on Continue and keep using GTG.