From 3b183fb63c673494ab7f206ff70a2984e2bda2bd Mon Sep 17 00:00:00 2001
From: Kunal Mehta <legoktm@debian.org>
Date: Tue, 3 Sep 2024 15:01:46 -0700
Subject: [PATCH] Have BaseErrors trigger DownloadDecryptionException in
 download flow

DownloadDecryptionException is specially handled in
Controller.on_file_download_failure(), so if we raise a different
exception class, the download animation will never be stopped because
file_missing is never emitted.

It is worth a proper re-examination of our exception heirarchy and
reducing the amount of indirection in the download flow, but that can be
done outside of a hotfix.

More debug logging is added to trace when slots are triggered.

Refs #1633.
---
 client/securedrop_client/api_jobs/downloads.py | 4 +---
 client/securedrop_client/gui/widgets.py        | 6 ++++++
 client/tests/api_jobs/test_downloads.py        | 2 +-
 3 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/client/securedrop_client/api_jobs/downloads.py b/client/securedrop_client/api_jobs/downloads.py
index e6455de22..70802f102 100644
--- a/client/securedrop_client/api_jobs/downloads.py
+++ b/client/securedrop_client/api_jobs/downloads.py
@@ -164,9 +164,7 @@ def _download(self, api: API, db_object: File | Message | Reply, session: Sessio
             mark_as_downloaded(type(db_object), db_object.uuid, session)
             logger.info(f"File downloaded to {destination}")
             return destination
-        except BaseError as e:
-            raise e
-        except (ValueError, FileNotFoundError, RuntimeError) as e:
+        except (ValueError, FileNotFoundError, RuntimeError, BaseError) as e:
             logger.error("Download failed")
             logger.debug(f"Download failed: {e}")
             raise DownloadDecryptionException(
diff --git a/client/securedrop_client/gui/widgets.py b/client/securedrop_client/gui/widgets.py
index 52a016555..07c9949e1 100644
--- a/client/securedrop_client/gui/widgets.py
+++ b/client/securedrop_client/gui/widgets.py
@@ -2434,6 +2434,9 @@ def _on_file_download_started(self, id: state.FileId) -> None:
 
     @pyqtSlot(str, str, str)
     def _on_file_downloaded(self, source_uuid: str, file_uuid: str, filename: str) -> None:
+        logger.debug(
+            f"_on_file_downloaded: {source_uuid} / {file_uuid} ({filename}), expected {self.uuid}"
+        )
         if file_uuid == self.uuid:
             self.downloading = False
             QTimer.singleShot(
@@ -2442,6 +2445,9 @@ def _on_file_downloaded(self, source_uuid: str, file_uuid: str, filename: str) -
 
     @pyqtSlot(str, str, str)
     def _on_file_missing(self, source_uuid: str, file_uuid: str, filename: str) -> None:
+        logger.debug(
+            f"_on_file_missing: {source_uuid} / {file_uuid} ({filename}), expected {self.uuid}"
+        )
         if file_uuid == self.uuid:
             self.downloading = False
             QTimer.singleShot(
diff --git a/client/tests/api_jobs/test_downloads.py b/client/tests/api_jobs/test_downloads.py
index de7848c68..bd1c5cf77 100644
--- a/client/tests/api_jobs/test_downloads.py
+++ b/client/tests/api_jobs/test_downloads.py
@@ -262,7 +262,7 @@ def test_MessageDownloadJob_with_base_error(mocker, homedir, session, session_ma
     mocker.patch.object(api_client, "download_submission", side_effect=BaseError)
     decrypt_fn = mocker.patch.object(job.gpg, "decrypt_submission_or_reply")
 
-    with pytest.raises(BaseError):
+    with pytest.raises(DownloadDecryptionException):
         job.call_api(api_client, session)
 
     assert message.content is None