From 493e0034f85e362b1cabdc3bc3617dbfd0b17a1b Mon Sep 17 00:00:00 2001
From: Jakub Fidler <31575114+RisingOrange@users.noreply.github.com>
Date: Mon, 18 Dec 2023 15:23:11 +0100
Subject: [PATCH] fix: `JsonDecodeError` in error handler (#835)

* fix: `JsonDecodeError` in error handler

* Refactor

* Add test

* Refactor other  tests in `TestErrorHandling`
---
 ankihub/gui/errors.py    | 15 ++++++---
 tests/addon/test_unit.py | 71 +++++++++++++++++++---------------------
 2 files changed, 43 insertions(+), 43 deletions(-)

diff --git a/ankihub/gui/errors.py b/ankihub/gui/errors.py
index 17ec4b80f..9a43a9b7f 100644
--- a/ankihub/gui/errors.py
+++ b/ankihub/gui/errors.py
@@ -7,6 +7,7 @@
 import time
 import traceback
 import zipfile
+from json import JSONDecodeError
 from pathlib import Path
 from sqlite3 import OperationalError
 from textwrap import dedent
@@ -332,11 +333,15 @@ def _maybe_handle_ankihub_http_error(error: AnkiHubHTTPError) -> bool:
             check_and_prompt_for_updates_on_main_window()
         return True
     elif response.status_code == 403:
-        response_data = response.json()
-        error_message = response_data.get("detail")
-        if error_message:
-            show_error_dialog(error_message, title="Oh no!")
-            return True
+        try:
+            response_data = response.json()
+        except JSONDecodeError:
+            return False
+        else:
+            error_message = response_data.get("detail")
+            if error_message:
+                show_error_dialog(error_message, title="Oh no!")
+                return True
 
     return False
 
diff --git a/tests/addon/test_unit.py b/tests/addon/test_unit.py
index 745d7a9a7..ea9a79f3a 100644
--- a/tests/addon/test_unit.py
+++ b/tests/addon/test_unit.py
@@ -1,4 +1,4 @@
-import importlib
+import json
 import os
 import tempfile
 import time
@@ -17,7 +17,7 @@
 from anki.notes import Note, NoteId
 from aqt import utils
 from aqt.qt import QDialog, QDialogButtonBox, Qt, QTimer, QWidget
-from pytest import MonkeyPatch, fixture
+from pytest import MonkeyPatch
 from pytest_anki import AnkiSession
 from pytestqt.qtbot import QtBot  # type: ignore
 from requests import Response
@@ -1469,16 +1469,9 @@ def test_contains_path_to_this_addon(self):
             "\\addons21\\12345789\\src\\ankihub\\errors.py"
         )
 
-    def test_handle_ankihub_401(
-        self,
-        monkeypatch: MonkeyPatch,
-    ):
+    def test_handle_ankihub_401(self, mock_function: MockFunction):
         # Set up mock for AnkiHub login dialog.
-        display_login_mock = Mock()
-        monkeypatch.setattr(AnkiHubLogin, "display_login", display_login_mock)
-
-        # Mock _this_addon_is_involved to return True.
-        monkeypatch.setattr(errors, "_this_addon_mentioned_in_tb", lambda *args: True)
+        display_login_mock = mock_function(AnkiHubLogin, "display_login")
 
         handled = _try_handle_exception(
             exc_type=AnkiHubHTTPError,
@@ -1488,14 +1481,36 @@ def test_handle_ankihub_401(
         assert handled
         display_login_mock.assert_called_once()
 
-    def test_handle_ankihub_406(
-        self,
-        monkeypatch: MonkeyPatch,
-        _mock_ask_user_to_return_false: Mock,
+    @pytest.mark.parametrize(
+        "response_content, expected_handled",
+        [
+            # The exception should only be handled for responses with json content that
+            # contains the "detail" key.
+            ("", False),
+            ("{}", False),
+            ('{"detail": "test"}', True),
+        ],
+    )
+    def test_handle_ankihub_403(
+        self, mock_function: MockFunction, response_content: str, expected_handled: bool
     ):
-        # Mock _this_addon_is_involved to return True.
-        monkeypatch.setattr(errors, "_this_addon_mentioned_in_tb", lambda *args: True)
+        show_error_dialog_mock = mock_function(errors, "show_error_dialog")
 
+        response_mock = Mock()
+        response_mock.status_code = 403
+        response_mock.text = response_content
+        response_mock.json = lambda: json.loads(response_content)  # type: ignore
+
+        handled = _try_handle_exception(
+            exc_type=AnkiHubHTTPError,
+            exc_value=AnkiHubHTTPError(response=response_mock),
+            tb=None,
+        )
+        assert handled == expected_handled
+        assert show_error_dialog_mock.called == expected_handled
+
+    def test_handle_ankihub_406(self, mock_function: MockFunction):
+        ask_user_mock = mock_function(errors, "ask_user", return_value=False)
         handled = _try_handle_exception(
             exc_type=AnkiHubHTTPError,
             exc_value=AnkiHubHTTPError(
@@ -1504,27 +1519,7 @@ def test_handle_ankihub_406(
             tb=None,
         )
         assert handled
-        _mock_ask_user_to_return_false.assert_called_once()
-
-    @fixture
-    def _mock_ask_user_to_return_false(
-        self,
-        monkeypatch: MonkeyPatch,
-    ) -> Generator[Mock, None, None]:
-        # Simply monkeypatching ask_user to return False doesn't work because the errors module
-        # already imported the original askUser function when this fixture is called.
-        # So we need to reload the errors module after monkeypatching askUser.
-        try:
-            with monkeypatch.context() as m:
-                ask_user_mock = Mock(return_value=False)
-                m.setattr("ankihub.gui.utils.ask_user", ask_user_mock)
-                # Reload the errors module so that the monkeypatched askUser function is used.
-                importlib.reload(errors)
-
-                yield ask_user_mock
-        finally:
-            #  Reload the errors module again so that the original askUser function is used for other tests.
-            importlib.reload(errors)
+        ask_user_mock.assert_called_once()
 
 
 def test_show_error_dialog(