Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add queue of ExceptionHandlerDialogs #1123

Merged
merged 1 commit into from
Jul 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 5 additions & 9 deletions GTG/gtk/browser/main_window.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
# -----------------------------------------------------------------------------

""" The main window for GTG, listing tags, and open and closed tasks """
from __future__ import annotations

import datetime
import logging
Expand Down Expand Up @@ -113,19 +114,14 @@ def __init__(self, app):
self.sidebar = Sidebar(app, app.ds, self)
self.sidebar_vbox.append(self.sidebar)

self.panes = {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

'active': None,
'workview': None,
'closed': None,
self.panes: dict[str, TaskPane] = {
'active': TaskPane(self, 'active'),
'workview': TaskPane(self, 'workview'),
'closed': TaskPane(self, 'closed')
}

self.panes['active'] = TaskPane(self, 'active')
self.open_pane.append(self.panes['active'])

self.panes['workview'] = TaskPane(self, 'workview')
self.actionable_pane.append(self.panes['workview'])

self.panes['closed'] = TaskPane(self, 'closed')
self.closed_pane.append(self.panes['closed'])

self._init_context_menus()
Expand Down
2 changes: 1 addition & 1 deletion GTG/gtk/browser/task_pane.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ class TaskBox(Gtk.Box):

def __init__(self, config, is_actionable=False):
self.config = config
super(TaskBox, self).__init__()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is from Python 2. Probably something that Ruff can help with :)

super().__init__()

self.expander = Gtk.TreeExpander()
self.expander.set_margin_end(6)
Expand Down
51 changes: 44 additions & 7 deletions GTG/gtk/errorhandler.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import functools
import enum
import logging
import queue

from GTG.core import info
from GTG.core.system_info import SystemInfo
Expand Down Expand Up @@ -144,17 +145,53 @@ def handle_response(dialog: ExceptionHandlerDialog, response: int):
log.info("Unhandled response: %r, interpreting as continue instead", response)
dialog.close()

# Discard dialog from the queue, now that it has been closed
exception_dialog_queue.get()

def do_error_dialog(exception, context: str = None, ignorable: bool = True, main_msg=None):
"""
Show (and return) the error dialog.
It does NOT block execution, but should lock the UI
(by being a modal dialog).
# Another exception happened while the dialog was opened
# Open the dialog for that new exception
if not exception_dialog_queue.empty():
next_dialog = exception_dialog_queue.get()
next_dialog.show()


# Queue of ExceptionHandlerDialogs to be shown
# Since GTG keeps running while the dialog is shown, it's possible that
# another exception will be thrown. Its dialog needs to be opened when the first
# one closes.
# Capping the size of the queue, to avoid trying to queue an infinite number of
# dialogs. This has happened in #1093.
exception_dialog_queue = queue.Queue(maxsize=5)


def do_error_dialog(exception, context: str = None, ignorable: bool = True, main_msg=None) -> None:
"""Show (and return) the error dialog.

It does NOT block execution, but should lock the UI (by being a modal dialog).

Only show one exception dialog at a time, to avoid creating an infinity of
them in a loop (see #1093)
If an exception happens while the dialog is active for a previous
exception, add the new one to the exception queue. Its dialog will open when
the previous one gets closed.
"""
# If the queue is empty, we just show the dialog
# 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)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

dialog.connect('response', handle_response)
dialog.show()
return dialog

try:
exception_dialog_queue.put_nowait(dialog)
except queue.Full:
log.warning("Caught %s (%s) but not showing dialog for it because too"
" many exceptions are happening.",
type(exception).__name__, exception)

if need_to_show_dialog:
dialog.show()


def errorhandler(func, context: str = None, ignorable: bool = True, reraise: bool = True):
Expand Down