diff --git a/ankihub/gui/errors.py b/ankihub/gui/errors.py
index c2237b211..17ad4a2bb 100644
--- a/ankihub/gui/errors.py
+++ b/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)
diff --git a/ankihub/gui/media_sync.py b/ankihub/gui/media_sync.py
index 857108798..e09b3efbb 100644
--- a/ankihub/gui/media_sync.py
+++ b/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):
diff --git a/ankihub/gui/menu.py b/ankihub/gui/menu.py
index 50d2de8e6..93ac52cf1 100644
--- a/ankihub/gui/menu.py
+++ b/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)
diff --git a/ankihub/gui/operations/__init__.py b/ankihub/gui/operations/__init__.py
index e69de29bb..4815c5ea1 100644
--- a/ankihub/gui/operations/__init__.py
+++ b/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
diff --git a/ankihub/gui/operations/deck_creation.py b/ankihub/gui/operations/deck_creation.py
index 58864a084..95538e0bb 100644
--- a/ankihub/gui/operations/deck_creation.py
+++ b/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"{deck_url}"
)
- 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,
diff --git a/ankihub/gui/optional_tag_suggestion_dialog.py b/ankihub/gui/optional_tag_suggestion_dialog.py
index 212e925ae..ddcf6802e 100644
--- a/ankihub/gui/optional_tag_suggestion_dialog.py
+++ b/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
diff --git a/tests/addon/test_unit.py b/tests/addon/test_unit.py
index 90194b0f4..9d6544726 100644
--- a/tests/addon/test_unit.py
+++ b/tests/addon/test_unit.py
@@ -43,6 +43,7 @@
# has to be set before importing ankihub
os.environ["SKIP_INIT"] = "1"
+from ankihub.addon_ankihub_client import AddonAnkiHubClient
from ankihub.ankihub_client import (
AnkiHubClient,
AnkiHubHTTPError,
@@ -60,7 +61,9 @@
_contains_path_to_this_addon,
_normalize_url,
_try_handle_exception,
+ upload_logs_in_background,
)
+from ankihub.gui.media_sync import media_sync
from ankihub.gui.menu import AnkiHubLogin
from ankihub.gui.operations import deck_creation
from ankihub.gui.operations.deck_creation import (
@@ -108,7 +111,7 @@
mids_of_notes,
retain_nids_with_ah_note_type,
)
-from ankihub.settings import ANKIWEB_ID
+from ankihub.settings import ANKIWEB_ID, log_file_path
@pytest.fixture
@@ -162,6 +165,56 @@ def test_update_media_names_on_notes(
assert '
' in " ".join(notes[2].fields)
+class TestMediaSyncMediaDownload:
+ def test_with_exception(self, mock_function: MockFunction, qtbot: QtBot):
+ def raise_exception() -> None:
+ raise Exception("test")
+
+ update_and_download_mock = mock_function(
+ media_sync,
+ "_update_deck_media_and_download_missing_media",
+ side_effect=raise_exception,
+ )
+
+ with qtbot.captureExceptions() as exceptions:
+ media_sync.start_media_download()
+ qtbot.wait(500)
+
+ # Assert that _download_in_progress was set to False and the exception was raised
+ assert not media_sync._download_in_progress
+ assert len(exceptions) == 1
+ update_and_download_mock.assert_called_once() # sanity check
+
+
+class TestMediaSyncMediaUpload:
+ def test_with_exception(
+ self,
+ anki_session_with_addon_data: AnkiSession,
+ mock_function: MockFunction,
+ qtbot: QtBot,
+ next_deterministic_uuid,
+ ):
+ with anki_session_with_addon_data.profile_loaded():
+
+ def raise_exception() -> None:
+ raise Exception("test")
+
+ upload_media_mock = mock_function(
+ media_sync._client,
+ "upload_media",
+ side_effect=raise_exception,
+ )
+
+ with qtbot.captureExceptions() as exceptions:
+ media_sync.start_media_upload([], next_deterministic_uuid())
+ qtbot.wait(500)
+
+ # Assert that _amount_uploads_in_progress was was reset to 0 and the exception was raised
+ assert media_sync._amount_uploads_in_progress == 0
+ assert len(exceptions) == 1
+ upload_media_mock.assert_called_once() # sanity check
+
+
def test_lowest_level_common_ancestor_deck_name():
deck_names = [
@@ -1434,6 +1487,62 @@ def _mock_ask_user_to_return_false(
importlib.reload(errors)
+class TestUploadLogs:
+ def test_basic(
+ self,
+ qtbot: QtBot,
+ mock_function: MockFunction,
+ ):
+ on_done_mock = Mock()
+ upload_logs_mock = mock_function(AddonAnkiHubClient, "upload_logs")
+ upload_logs_in_background(on_done=on_done_mock)
+
+ qtbot.wait_until(lambda: on_done_mock.called)
+
+ upload_logs_mock.assert_called_once()
+ assert upload_logs_mock.call_args[1]["file"] == log_file_path()
+
+ @pytest.mark.parametrize(
+ "exception, expected_report_exception_called",
+ [
+ # The exception should not be reported for these two specific cases
+ (AnkiHubHTTPError(response=Mock(status_code=401)), False),
+ (
+ AnkiHubHTTPError(
+ response=Mock(status_code=406, reason=OUTDATED_CLIENT_ERROR_REASON)
+ ),
+ False,
+ ),
+ # The exception should be reported in all other cases
+ (AnkiHubHTTPError(response=Mock(status_code=500)), True),
+ (Exception("test"), True),
+ ],
+ )
+ def test_with_exception(
+ self,
+ qtbot: QtBot,
+ mock_function: MockFunction,
+ exception: Exception,
+ expected_report_exception_called: bool,
+ ):
+ def raise_exception(*args, **kwargs) -> None:
+ raise exception
+
+ on_done_mock = Mock()
+ upload_logs_mock = mock_function(
+ AddonAnkiHubClient, "upload_logs", side_effect=raise_exception
+ )
+ report_exception_mock = mock_function(errors, "_report_exception")
+ upload_logs_in_background(on_done=on_done_mock)
+
+ qtbot.wait(500)
+
+ upload_logs_mock.assert_called_once()
+ on_done_mock.assert_not_called()
+
+ assert report_exception_mock.called == expected_report_exception_called
+
+
class TestRateLimitedDecorator:
def test_rate_limited_decorator(self):
# Create a counter to keep track of how many times foo is executed