modify to work with reply retries

freedomofpress · Aug 21, 2019 · dbb2359 · dbb2359
1 parent c70b12a
commit dbb2359
Show file tree

Hide file tree

Showing 4 changed files with 7 additions and 8 deletions.
diff --git a/securedrop_client/api_jobs/base.py b/securedrop_client/api_jobs/base.py
@@ -70,15 +70,12 @@ def _do_call_api(self, api_client: API, session: Session) -> None:
                 self.remaining_attempts -= 1
                 result = self.call_api(api_client, session)
             except (AuthError, ApiInaccessibleError) as e:
-                logger.error('Client is not authenticated')
                 raise ApiInaccessibleError() from e
             except RequestTimeoutError as e:
                 if self.remaining_attempts == 0:
                     self.failure_signal.emit(e)
                     raise
             except Exception as e:
-                logger.error('Job {} raised an exception: {}: {}'.format(self, type(e).__name__, e))
-                logger.error('Skipping job')
                 self.failure_signal.emit(e)
                 raise
             else:

diff --git a/securedrop_client/logic.py b/securedrop_client/logic.py
@@ -35,7 +35,6 @@
     ReplyDownloadJob, DownloadChecksumMismatchException
 from securedrop_client.api_jobs.uploads import SendReplyJob, SendReplyJobError, \
     SendReplyJobTimeoutError
-from securedrop_client.api_jobs.base import PauseQueueJob
 from securedrop_client.api_jobs.updatestar import UpdateStarJob, UpdateStarJobException
 from securedrop_client.crypto import GpgHelper, CryptoError
 from securedrop_client.queue import ApiJobQueue

diff --git a/securedrop_client/queue.py b/securedrop_client/queue.py
@@ -69,16 +69,21 @@ def process(self) -> None:
             priority, job = self.queue.get(block=True)
 
             if isinstance(job, PauseQueueJob):
-                logger.info('Paused queue')
+                logger.debug('Paused queue')
                 return
 
             try:
                 session = self.session_maker()
                 job._do_call_api(self.api_client, session)
-            except (RequestTimeoutError, ApiInaccessibleError):
+            except (RequestTimeoutError, ApiInaccessibleError) as e:
+                logger.debug('Job {} raised an exception: {}: {}'.format(self, type(e).__name__, e))
+                logger.debug('Pausing queue')
                 self.pause.emit()
                 job.remaining_attempts = DEFAULT_NUM_ATTEMPTS
                 self.queue.put_nowait((priority, job))
+            except Exception as e:
+                logger.error('Job {} raised an exception: {}: {}'.format(self, type(e).__name__, e))
+                logger.error('Skipping job')
             finally:
                 session.close()
 

diff --git a/tests/test_queue.py b/tests/test_queue.py
@@ -202,7 +202,6 @@ def test_RunnableQueue_does_not_run_jobs_when_not_authed(mocker):
     '''
     queue = RunnableQueue(mocker.MagicMock(), mocker.MagicMock())
     queue.pause = mocker.MagicMock()
-    mock_logger = mocker.patch('securedrop_client.api_jobs.base.logger')
 
     # ApiInaccessibleError will cause the queue to pause, use our fake pause method instead
     def fake_pause() -> None:
@@ -217,7 +216,6 @@ def fake_pause() -> None:
     # attempt to process job1 knowing that it times out
     queue.process()
     assert queue.queue.qsize() == 1  # queue contains: job1
-    assert "Client is not authenticated" in mock_logger.error.call_args[0][0]
 
 
 def test_ApiJobQueue_enqueue(mocker):