diff --git a/securedrop_client/api_jobs/uploads.py b/securedrop_client/api_jobs/uploads.py index d5b451fb7..8616b7115 100644 --- a/securedrop_client/api_jobs/uploads.py +++ b/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) diff --git a/tests/api_jobs/test_uploads.py b/tests/api_jobs/test_uploads.py index 19f8c2b38..d607912cf 100644 --- a/tests/api_jobs/test_uploads.py +++ b/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): '''