chore: Fix background tasks blocking UI

ankihub/gui/errors.py

@@ -7,7 +7,6 @@
 import time
 import traceback
 import zipfile
-from concurrent.futures import Future
 from pathlib import Path
 from sqlite3 import OperationalError
 from textwrap import dedent
@@ -46,6 +45,7 @@
 )
 from .deck_updater import NotLoggedInError
 from .error_dialog import ErrorDialog
+from .operations import AddonQueryOp
 from .utils import (
     ask_user,
     check_and_prompt_for_updates_on_main_window,
@@ -92,7 +92,7 @@ def report_exception_and_upload_logs(
 
 
 def upload_logs_in_background(
-    on_done: Optional[Callable[[Future], None]] = None, hide_username=False
+    on_done: Optional[Callable[[str], None]] = None, hide_username=False
 ) -> str:
     """Upload the logs to S3 in the background.
     Returns the S3 key of the uploaded logs."""
@@ -103,19 +103,18 @@ def upload_logs_in_background(
     user_name = config.user() if not hide_username else checksum(config.user())[:5]
     key = f"ankihub_addon_logs_{user_name}_{int(time.time())}.log"
 
-    if on_done is not None:
-        aqt.mw.taskman.run_in_background(
-            task=lambda: _upload_logs(key), on_done=on_done
-        )
-    else:
-        aqt.mw.taskman.run_in_background(
-            task=lambda: _upload_logs(key), on_done=_on_upload_logs_done
-        )
+    AddonQueryOp(
+        parent=aqt.mw,
+        op=lambda _: _upload_logs(key),
+        success=on_done if on_done is not None else lambda _: None,
+    ).failure(_on_upload_logs_failure).without_collection().run_in_background()
 
     return key
 
 
-def upload_logs_and_data_in_background(on_done: Callable[[Future], None] = None) -> str:
+def upload_logs_and_data_in_background(
+    on_done: Optional[Callable[[str], None]] = None
+) -> str:
     """Upload the data dir and logs to S3 in the background.
     Returns the S3 key of the uploaded file."""
 
@@ -125,9 +124,11 @@ def upload_logs_and_data_in_background(on_done: Callable[[Future], None] = None)
     user_name_hash = checksum(config.user())[:5]
     key = f"ankihub_addon_debug_info_{user_name_hash}_{int(time.time())}.zip"
 
-    aqt.mw.taskman.run_in_background(
-        task=lambda: _upload_logs_and_data_in_background(key), on_done=on_done
-    )
+    AddonQueryOp(
+        parent=aqt.mw,
+        op=lambda _: _upload_logs_and_data_in_background(key),
+        success=on_done if on_done is not None else lambda _: None,
+    ).failure(_on_upload_logs_failure).without_collection().run_in_background()
 
     return key
 
@@ -577,20 +578,18 @@ def _upload_logs(key: str) -> str:
         raise e
 
 
-def _on_upload_logs_done(future: Future) -> None:
-    try:
-        future.result()
-    except AnkiHubHTTPError as e:
+def _on_upload_logs_failure(exc: Exception) -> None:
+    if isinstance(exc, AnkiHubHTTPError):
         from .errors import OUTDATED_CLIENT_ERROR_REASON
 
         # Don't report outdated client errors that happen when uploading logs,
         # because they are handled by the add-on when they happen in other places
         # and we don't want to see them in Sentry.
-        if e.response.status_code == 401 or (
-            e.response.status_code == 406
-            and e.response.reason == OUTDATED_CLIENT_ERROR_REASON
+        if exc.response.status_code == 401 or (
+            exc.response.status_code == 406
+            and exc.response.reason == OUTDATED_CLIENT_ERROR_REASON
         ):
             return
-        _report_exception(e)
-    except Exception as e:
-        _report_exception(e)
+        _report_exception(exc)
+    else:
+        _report_exception(exc)

ankihub/gui/media_sync.py

@@ -1,5 +1,4 @@
 import uuid
-from concurrent.futures import Future
 from datetime import datetime
 from functools import cached_property
 from pathlib import Path
@@ -13,6 +12,7 @@
 from ..ankihub_client.models import DeckMedia
 from ..db import ankihub_db
 from ..settings import config
+from .operations import AddonQueryOp
 
 
 class _AnkiHubMediaSync:
@@ -44,10 +44,15 @@ def start_media_download(self):
         self._download_in_progress = True
         self.refresh_sync_status_text()
 
-        aqt.mw.taskman.run_in_background(
-            self._update_deck_media_and_download_missing_media,
-            on_done=self._on_download_finished,
-        )
+        def on_failure(exception: Exception) -> None:
+            self._download_in_progress = False
+            raise exception
+
+        AddonQueryOp(
+            parent=aqt.mw,
+            op=lambda _: self._update_deck_media_and_download_missing_media(),
+            success=self._on_download_finished,
+        ).failure(on_failure).without_collection().run_in_background()
 
     def start_media_upload(
         self,
@@ -60,12 +65,18 @@ def start_media_upload(
         self.refresh_sync_status_text()
 
         media_paths = self._media_paths_for_media_names(media_names)
-        aqt.mw.taskman.run_in_background(
-            lambda: self._client.upload_media(media_paths, ankihub_did),
-            on_done=lambda future: self._on_upload_finished(
-                future, ankihub_deck_id=ankihub_did, on_success=on_success
+
+        def on_failure(exception: Exception) -> None:
+            self._amount_uploads_in_progress -= 1
+            raise exception
+
+        AddonQueryOp(
+            parent=aqt.mw,
+            op=lambda _: self._client.upload_media(media_paths, ankihub_did),
+            success=lambda _: self._on_upload_finished(
+                ankihub_deck_id=ankihub_did, on_success=on_success
             ),
-        )
+        ).failure(on_failure).without_collection().run_in_background()
 
     def stop_background_threads(self):
         """Stop all media sync operations."""
@@ -91,12 +102,10 @@ def _media_paths_for_media_names(self, media_names: Iterable[str]) -> Set[Path]:
 
     def _on_upload_finished(
         self,
-        future: Future,
         ankihub_deck_id: uuid.UUID,
         on_success: Optional[Callable[[], None]] = None,
     ):
         self._amount_uploads_in_progress -= 1
-        future.result()
         LOGGER.info("Uploaded media to AnkiHub.")
         self.refresh_sync_status_text()
 
@@ -155,9 +164,8 @@ def _missing_media_for_ah_deck(self, ah_did: uuid.UUID) -> List[str]:
         ]
         return result
 
-    def _on_download_finished(self, future: Future) -> None:
+    def _on_download_finished(self, _: None) -> None:
         self._download_in_progress = False
-        future.result()
         self.refresh_sync_status_text()
 
     def _refresh_media_download_status_inner(self):

ankihub/gui/menu.py

@@ -249,13 +249,12 @@ def _upload_logs_and_data_action() -> None:
     upload_logs_and_data_in_background(on_done=_on_logs_uploaded)
 
 
-def _on_logs_uploaded(future: Future):
+def _on_logs_uploaded(log_file_name: str) -> None:
     aqt.mw.progress.finish()
-    log_file_name = future.result()
     LogUploadResultDialog(log_file_name=log_file_name).exec()
 
 
-def _ankihub_login_setup(parent: QMenu):
+def _ankihub_login_setup(parent: QMenu) -> None:
     sign_in_button = QAction("🔑 Sign into AnkiHub", aqt.mw)
     qconnect(sign_in_button.triggered, AnkiHubLogin.display_login)
     parent.addAction(sign_in_button)

ankihub/gui/operations/__init__.py

@@ -0,0 +1,31 @@
+from aqt.operations import QueryOp
+
+from ...settings import ANKI_MINOR
+
+
+class AddonQueryOp(QueryOp):
+    """A subclass of aqt.operations.QueryOp that is tailored for the AnkiHub add-on."""
+
+    def __init__(self, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+
+        # The default behavior of the QueryOp class on a failure is to show the exception to the user.
+        # However we want to raise the exception so that our central error handler can handle it.
+        self._failure = _on_failure
+
+    def without_collection(self):
+        """The QueryOp class doesn't have a without_collection method on Anki versions before 23.10.
+        We are setting this to be a no-op for backwards compatibility.
+        It's fine for this to be a no-op, because without_collection is used to allow
+        background tasks to run in parallel. On previous Anki versions background tasks were
+        already running in parallel, so there is no need to do anything.
+        This way we can use the same code for all Anki versions.
+        """
+        if ANKI_MINOR < 231000:
+            return self
+
+        return super().without_collection()
+
+
+def _on_failure(exception: Exception) -> None:
+    raise exception

ankihub/gui/operations/deck_creation.py

@@ -2,7 +2,6 @@
 
 import aqt
 from aqt import QCheckBox, QMessageBox
-from aqt.operations import QueryOp
 from aqt.studydeck import StudyDeck
 from aqt.utils import showInfo, tooltip
 
@@ -14,6 +13,7 @@
 from ...main.subdecks import SUBDECK_TAG
 from ...settings import config, url_view_deck
 from ..media_sync import media_sync
+from ..operations import AddonQueryOp
 from ..utils import ask_user
 
 
@@ -132,13 +132,13 @@ def on_success(deck_creation_result: DeckCreationResult) -> None:
             f"<a href={deck_url}>{deck_url}</a>"
         )
 
-    def on_failure(exc: Exception):
+    def on_failure(exc: Exception) -> None:
         aqt.mw.progress.finish()
         raise exc
 
-    op = QueryOp(
+    op = AddonQueryOp(
         parent=aqt.mw,
-        op=lambda col: create_ankihub_deck(
+        op=lambda _: create_ankihub_deck(
             deck_name,
             private=private,
             add_subdeck_tags=add_subdeck_tags,

ankihub/gui/optional_tag_suggestion_dialog.py

@@ -1,6 +1,6 @@
 """Dialog for suggesting optional tags for notes."""
 from concurrent.futures import Future
-from typing import Sequence
+from typing import List, Sequence
 
 import aqt
 from anki.notes import NoteId
@@ -20,7 +20,9 @@
 
 from .. import LOGGER
 from ..ankihub_client import AnkiHubHTTPError
+from ..ankihub_client.models import TagGroupValidationResponse
 from ..main.optional_tag_suggestions import OptionalTagsSuggestionHelper
+from .operations import AddonQueryOp
 from .utils import show_error_dialog
 
 
@@ -144,19 +146,20 @@ def _setup_tag_group_list(self):
             )
             item.setToolTip("Validating...")
 
-    def _validate_tag_groups_and_update_ui(self):
-        aqt.mw.taskman.run_in_background(
-            task=self._validate_tag_groups_in_background,
-            on_done=self._on_validate_tag_groups_finished,
-        )
+    def _validate_tag_groups_and_update_ui(self) -> None:
+        AddonQueryOp(
+            parent=self,
+            op=lambda _: self._validate_tag_groups(),
+            success=self._on_validate_tag_groups_finished,
+        ).without_collection().run_in_background()
 
-    def _validate_tag_groups_in_background(self):
+    def _validate_tag_groups(self) -> List[TagGroupValidationResponse]:
         result = self._optional_tags_helper.prevalidate()
         return result
 
-    def _on_validate_tag_groups_finished(self, future: Future):
-        tag_group_validation_responses = future.result()
-
+    def _on_validate_tag_groups_finished(
+        self, tag_group_validation_responses: List[TagGroupValidationResponse]
+    ) -> None:
         self._valid_tag_groups = [
             response.tag_group_name
             for response in tag_group_validation_responses