Be defensive when updating ConversationView

Wrap some more uses of Source.collection when updating
ConversationView.

securedrop_client/gui/widgets.py

@@ -667,7 +667,10 @@ def refresh_source_conversations(self) -> None:
         source_widget = self.source_list.get_source_widget(source.uuid)
         if source_widget:
             source_widget.deletion_indicator.stop()
-        conversation_wrapper.conversation_view.update_conversation(source.collection)
+        try:
+            conversation_wrapper.conversation_view.update_conversation(source.collection)
+        except sqlalchemy.exc.InvalidRequestError as e:
+            logger.debug("Error refreshing source conversations: %s", e)
 
     def delete_conversation(self, source_uuid: str) -> None:
         """
@@ -3428,7 +3431,10 @@ def __init__(self, source_db_object: Source, controller: Controller):
 
         main_layout.addWidget(self.scroll)
 
-        self.update_conversation(self.source.collection)
+        try:
+            self.update_conversation(self.source.collection)
+        except sqlalchemy.exc.InvalidRequestError as e:
+            logger.debug("Error initializing ConversationView: %s", e)
 
     def update_deletion_markers(self, collection):
         if collection:
@@ -3628,7 +3634,15 @@ def on_reply_sent(self, source_uuid: str, reply_uuid: str, reply_text: str) -> N
         self.reply_flag = True
         if source_uuid == self.source.uuid:
             self.add_reply_from_reply_box(reply_uuid, reply_text)
-            self.update_deletion_markers(self.source.collection)
+            try:
+                self.update_deletion_markers(self.source.collection.copy())
+            except sqlalchemy.exc.InvalidRequestError as e:
+                # The only way we should get here is if
+                # source.collection can't be populated, presumably
+                # because it had never been loaded, and the source and
+                # its conversation items were deleted between adding
+                # the reply and updating deletion markers.
+                logger.debug("Error in ConversationView.on_reply_sent: %s", e)
 
 
 class SourceConversationWrapper(QWidget):

tests/gui/test_widgets.py

@@ -4,7 +4,7 @@
 import math
 import random
 from datetime import datetime
-from unittest.mock import Mock, patch
+from unittest.mock import Mock, PropertyMock, patch
 
 import arrow
 import pytest
@@ -905,6 +905,48 @@ def test_MainView_refresh_source_conversations(homedir, mocker, qtbot, session_m
     mv.refresh_source_conversations()
 
 
+def test_MainView_refresh_source_conversations_with_deleted(
+    homedir, mocker, session, session_maker
+):
+    mv = MainView(None)
+
+    mock_gui = mocker.MagicMock()
+    controller = logic.Controller("http://localhost", mock_gui, session_maker, homedir)
+    controller.api = mocker.MagicMock()
+
+    debug_logger = mocker.patch("securedrop_client.gui.widgets.logger.debug")
+    mv.setup(controller)
+
+    source1 = factory.Source(uuid="rsc-123")
+    session.add(source1)
+
+    source2 = factory.Source(uuid="rsc-456")
+    session.add(source2)
+
+    session.commit()
+
+    sources = [source1, source2]
+
+    mv.source_list.update(sources)
+    mv.show()
+
+    def collection_error():
+        raise sqlalchemy.exc.InvalidRequestError()
+
+    mocker.patch(
+        "securedrop_client.gui.widgets.SourceList.get_selected_source", return_value=source1
+    )
+    mv.on_source_changed()
+
+    with patch(
+        "securedrop_client.db.Source.collection", new_callable=PropertyMock
+    ) as mock_collection:
+        ire = sqlalchemy.exc.InvalidRequestError()
+        mock_collection.side_effect = ire
+        mv.refresh_source_conversations()
+        debug_logger.assert_any_call("Error refreshing source conversations: %s", ire)
+
+
 def test_MainView_set_conversation(mocker):
     """
     Ensure the passed-in widget is added to the layout of the main view holder
@@ -4417,6 +4459,29 @@ def test_ConversationView_init(mocker, homedir):
     assert isinstance(cv.scroll.conversation_layout, QVBoxLayout)
 
 
+def test_ConversationView_init_with_deleted_source(mocker, homedir, session):
+    """
+    Ensure the conversation view has a layout to add widgets to.
+    """
+    mocked_controller = mocker.MagicMock()
+    debug_logger = mocker.patch("securedrop_client.gui.widgets.logger.debug")
+
+    def collection_error():
+        raise sqlalchemy.exc.InvalidRequestError()
+
+    source = factory.Source(uuid="ire-123")
+    session.add(source)
+    session.commit()
+
+    with patch(
+        "securedrop_client.db.Source.collection", new_callable=PropertyMock
+    ) as mock_collection:
+        ire = sqlalchemy.exc.InvalidRequestError()
+        mock_collection.side_effect = ire
+        ConversationView(source, mocked_controller)
+        debug_logger.assert_any_call("Error initializing ConversationView: %s", ire)
+
+
 def test_ConversationView_ConversationScrollArea_resize(mocker):
     """
     Test that the resize event handler calls adjust_width on each conversation item, passing in the
@@ -4574,6 +4639,30 @@ def test_ConversationView_on_reply_sent_does_not_add_message_intended_for_differ
     assert not cv.add_reply.called
 
 
+def test_ConversationView_on_reply_sent_with_deleted_source(mocker):
+    """
+    Check that replying to a deleted source causes an error and is logged.
+    """
+    source = factory.Source()
+    controller = mocker.MagicMock()
+    cv = ConversationView(source, controller)
+    cv.add_reply_from_reply_box = mocker.MagicMock()
+
+    def collection_error():
+        raise sqlalchemy.exc.InvalidRequestError()
+
+    debug_logger = mocker.patch("securedrop_client.gui.widgets.logger.debug")
+    with patch(
+        "securedrop_client.db.Source.collection", new_callable=PropertyMock
+    ) as mock_collection:
+        ire = sqlalchemy.exc.InvalidRequestError()
+        mock_collection.side_effect = ire
+
+        cv.on_reply_sent(source.uuid, "mock", "mock")
+
+        debug_logger.assert_any_call("Error in ConversationView.on_reply_sent: %s", ire)
+
+
 def test_ConversationView_add_reply_from_reply_box(mocker, session, session_maker, homedir):
     """
     Adding a reply from reply box results in a new ReplyWidget added to the layout.
@@ -5054,15 +5143,15 @@ def test_ReplyBoxWidget_on_sync_source_deleted(mocker, source):
     controller = mocker.MagicMock()
     rb = ReplyBoxWidget(s, controller)
 
-    error_logger = mocker.patch("securedrop_client.gui.widgets.logger.debug")
+    debug_logger = mocker.patch("securedrop_client.gui.widgets.logger.debug")
 
     def pretend_source_was_deleted(self):
         raise sqlalchemy.orm.exc.ObjectDeletedError(attributes.instance_state(s), None)
 
     with patch.object(ReplyBoxWidget, "update_authentication_state") as uas:
         uas.side_effect = pretend_source_was_deleted
         rb._on_synced("syncing")
-        error_logger.assert_called_once_with(
+        debug_logger.assert_called_once_with(
             "During sync, ReplyBoxWidget found its source had been deleted."
         )
 
@@ -5129,14 +5218,14 @@ def test_ReplyBoxWidget_on_authentication_changed_source_deleted(mocker, source)
     co = mocker.MagicMock()
     rb = ReplyBoxWidget(s, co)
 
-    error_logger = mocker.patch("securedrop_client.gui.widgets.logger.debug")
+    debug_logger = mocker.patch("securedrop_client.gui.widgets.logger.debug")
 
     with mocker.patch(
         "securedrop_client.gui.widgets.ReplyBoxWidget.update_authentication_state",
         side_effect=sqlalchemy.orm.exc.ObjectDeletedError(attributes.instance_state(s), None),
     ):
         rb._on_authentication_changed(True)
-        error_logger.assert_called_once_with(
+        debug_logger.assert_called_once_with(
             "On authentication change, ReplyBoxWidget found its source had been deleted."
         )