ensure exceptions in reply job processing don't crash the application

securedrop_client/api_jobs/uploads.py

@@ -3,6 +3,7 @@
 
 from sdclientapi import API, RequestTimeoutError, ServerConnectionError
 from sqlalchemy.orm.session import Session
+from sqlalchemy.exc import SQLAlchemyError
 
 from securedrop_client.api_jobs.base import ApiJob
 from securedrop_client.crypto import GpgHelper
@@ -68,27 +69,28 @@ def call_api(self, api_client: API, session: Session) -> str:
         except (RequestTimeoutError, ServerConnectionError) as e:
             message = "Failed to send reply for source {id} due to Exception: {error}".format(
                 id=self.source_uuid, error=e)
-
-            # Update draft reply send status to FAILED
-            reply_status = session.query(ReplySendStatus).filter_by(
-                name=ReplySendStatusCodes.FAILED.value).one()
-            draft_reply_db_object.send_status_id = reply_status.id
-            session.add(draft_reply_db_object)
-            session.commit()
-
+            self._set_status_to_failed(session)
             raise SendReplyJobTimeoutError(message, self.reply_uuid)
         except Exception as e:
             message = "Failed to send reply for source {id} due to Exception: {error}".format(
                 id=self.source_uuid, error=e)
+            self._set_status_to_failed(session)
+            raise SendReplyJobError(message, self.reply_uuid)
 
-            # Update draft reply send status to FAILED
+    def _set_status_to_failed(self, session: Session) -> None:
+        try:  # If draft exists, we set it to failed.
+            draft_reply_db_object = session.query(DraftReply).filter_by(uuid=self.reply_uuid).one()
             reply_status = session.query(ReplySendStatus).filter_by(
                 name=ReplySendStatusCodes.FAILED.value).one()
             draft_reply_db_object.send_status_id = reply_status.id
             session.add(draft_reply_db_object)
             session.commit()
-
-            raise SendReplyJobError(message, self.reply_uuid)
+        except SQLAlchemyError as e:
+            logger.info('SQL error when setting reply {uuid} as failed, skipping: {e}'.format(
+                uuid=self.reply_uuid, e=e))
+        except Exception as e:
+            logger.error('unknown error when setting reply {uuid} as failed, skipping: {e}'.format(
+                uuid=self.reply_uuid, e=e))
 
     def _make_call(self, encrypted_reply: str, api_client: API) -> sdclientapi.Reply:
         sdk_source = sdclientapi.Source(uuid=self.source_uuid)

tests/api_jobs/test_uploads.py

@@ -189,6 +189,48 @@ def test_send_reply_failure_gpg_error(homedir, mocker, session, session_maker,
     assert len(drafts) == 1
 
 
+def test_send_reply_sql_exception_during_failure(homedir, mocker, session, session_maker,
+                                                 reply_status_codes):
+    '''
+    Check that we do not raise an unhandled exception when we set the draft reply
+    status to failed in the except block if there is a SQL exception.
+    '''
+    source = factory.Source()
+    session.add(source)
+
+    # Note that we do not add a DraftReply. An exception will occur when we try
+    # to set the reply status to 'FAILED' for a non-existent reply, which we
+    # expect to be handled.
+
+    gpg = GpgHelper(homedir, session_maker, is_qubes=False)
+    job = SendReplyJob(source.uuid, 'mock_reply_uuid', 'mock_message', gpg)
+
+    # This should not raise an exception
+    job._set_status_to_failed(session)
+
+
+def test_send_reply_unexpected_exception_during_failure(homedir, mocker, session,
+                                                        session_maker, reply_status_codes):
+    '''
+    Check that we do not raise an unhandled exception when we set the draft reply
+    status to failed in the except block if there is an unexpected exception.
+    '''
+    source = factory.Source()
+    session.add(source)
+    draft_reply = factory.DraftReply(uuid='mock_reply_uuid')
+    session.add(draft_reply)
+    session.commit()
+
+    # session.commit() is called when we try to set the status to failed.
+    session.commit = mocker.MagicMock(side_effect=Exception("BOOM"))
+
+    gpg = GpgHelper(homedir, session_maker, is_qubes=False)
+    job = SendReplyJob(source.uuid, 'mock_reply_uuid', 'mock_message', gpg)
+
+    # This should not raise an exception
+    job._set_status_to_failed(session)
+
+
 def test_send_reply_failure_unknown_error(homedir, mocker, session, session_maker,
                                           reply_status_codes):
     '''