app: use ServerConnectionError from SDK 0.0.13 (resolves failed sync causing AuthError, and KeyError during a failed download)

build-requirements.txt

@@ -9,7 +9,7 @@ pathlib2==2.3.2 --hash=sha256:460e67b14d0574b0529a0017b1eb05d10d9722681e303fec70
 python-dateutil==2.7.5 --hash=sha256:f6eb9c17acd5a6954e1a5f2f999a41de3e7e25b6bc41baf6344bd053ec25ceeb
 python-editor==1.0.3 --hash=sha256:e47dcec4ea883853b8196fbd425b875d7ec791d4ede2e20cfc70b9a25365c65b
 requests==2.20.0 --hash=sha256:d87b2085783d31d874ac7bc62660e287932aaee7059e80b41b76462eb18d35cc
-securedrop-sdk==0.0.12 --hash=sha256:d05bb78652c8771e6aa1aefcd76ade1fef08c563d2641acbc5ac8e1d635e6a53
+securedrop-sdk==0.0.13 --hash=sha256:c8d98208fb2074336c06be3fef0994a8a57fde7a765cead12bc36e9128d319e2
 six==1.11.0 --hash=sha256:aa4ad34049ddff178b533062797fd1db9f0038b7c5c2461a7cde2244300b9f3d
 sqlalchemy==1.3.3 --hash=sha256:bfb4cd0df5802a01acd738a080a19e60ff4700e030d497de273807f9a8bd4a0a
 urllib3==1.24.3 --hash=sha256:3d440cbb168e2c963d5099232bdb3f7390bf031b6270dad1bc79751698a1399a

dev-requirements.txt

@@ -176,8 +176,9 @@ python-editor==1.0.3 \
 requests==2.20.0 \
     --hash=sha256:99dcfdaaeb17caf6e526f32b6a7b780461512ab3f1d992187801694cba42770c \
     --hash=sha256:a84b8c9ab6239b578f22d1c21d51b696dcfe004032bb80ea832398d6909d7279
-securedrop-sdk==0.0.12 \
-    --hash=sha256:b5ddca26ce87d4007db5d64fe77d44b4086a902c3f79e69fb9a81343c81ce278
+securedrop-sdk==0.0.13 \
+    --hash=sha256:3d521e7a63cd7df55f9c6508010f968692eccdb222c57ed9c2d0390fbbbd6f99 \
+    --hash=sha256:7763bb44755bdfc387ab6c002cbe49eeec2611feb04a8787c3c9f2aa48a1ee5f
 sip==4.19.8 \
     --hash=sha256:09f9a4e6c28afd0bafedb26ffba43375b97fe7207bd1a0d3513f79b7d168b331 \
     --hash=sha256:105edaaa1c8aa486662226360bd3999b4b89dd56de3e314d82b83ed0587d8783 \

requirements.in

@@ -9,7 +9,7 @@ pathlib2==2.3.2
 python-dateutil==2.7.5
 python-editor==1.0.3
 requests==2.20.0
-securedrop-sdk==0.0.12
+securedrop-sdk==0.0.13
 six==1.11.0
 SQLAlchemy==1.3.3
 urllib3==1.24.3

requirements.txt

@@ -32,8 +32,9 @@ python-editor==1.0.3 \
 requests==2.20.0 \
     --hash=sha256:99dcfdaaeb17caf6e526f32b6a7b780461512ab3f1d992187801694cba42770c \
     --hash=sha256:a84b8c9ab6239b578f22d1c21d51b696dcfe004032bb80ea832398d6909d7279
-securedrop-sdk==0.0.12 \
-    --hash=sha256:b5ddca26ce87d4007db5d64fe77d44b4086a902c3f79e69fb9a81343c81ce278
+securedrop-sdk==0.0.13 \
+    --hash=sha256:3d521e7a63cd7df55f9c6508010f968692eccdb222c57ed9c2d0390fbbbd6f99 \
+    --hash=sha256:7763bb44755bdfc387ab6c002cbe49eeec2611feb04a8787c3c9f2aa48a1ee5f
 six==1.11.0 \
     --hash=sha256:70e8a77beed4562e7f14fe23a786b54f6296e34344c23bc42f07b15018ff98e9 \
     --hash=sha256:832dc0e10feb1aa2c68dcc57dbb658f1c7e65b9b61af69048abc87a2db00a0eb

securedrop_client/api_jobs/base.py

@@ -1,7 +1,7 @@
 import logging
 
 from PyQt5.QtCore import QObject, pyqtSignal
-from sdclientapi import API, AuthError, RequestTimeoutError
+from sdclientapi import API, AuthError, RequestTimeoutError, ServerConnectionError
 from sqlalchemy.orm.session import Session
 from typing import Any, Optional, TypeVar
 
@@ -71,7 +71,7 @@ def _do_call_api(self, api_client: API, session: Session) -> None:
                 result = self.call_api(api_client, session)
             except (AuthError, ApiInaccessibleError) as e:
                 raise ApiInaccessibleError() from e
-            except RequestTimeoutError as e:
+            except (RequestTimeoutError, ServerConnectionError) as e:
                 if self.remaining_attempts == 0:
                     self.failure_signal.emit(e)
                     raise

securedrop_client/api_jobs/uploads.py

@@ -1,7 +1,7 @@
 import logging
 import sdclientapi
 
-from sdclientapi import API, RequestTimeoutError
+from sdclientapi import API, RequestTimeoutError, ServerConnectionError
 from sqlalchemy.orm.session import Session
 
 from securedrop_client.api_jobs.base import ApiJob
@@ -65,7 +65,7 @@ def call_api(self, api_client: API, session: Session) -> str:
             session.commit()
 
             return reply_db_object.uuid
-        except RequestTimeoutError as e:
+        except (RequestTimeoutError, ServerConnectionError) as e:
             message = "Failed to send reply for source {id} due to Exception: {error}".format(
                 id=self.source_uuid, error=e)
 

securedrop_client/logic.py

@@ -28,7 +28,7 @@
 
 from gettext import gettext as _
 from PyQt5.QtCore import QObject, QThread, pyqtSignal, QTimer, QProcess, Qt
-from sdclientapi import RequestTimeoutError
+from sdclientapi import RequestTimeoutError, ServerConnectionError
 from sqlalchemy.orm.session import sessionmaker
 
 from securedrop_client import storage
@@ -98,7 +98,7 @@ def call_api(self):
         try:
             self.result = self.api_call(*self.args, **self.kwargs)
         except Exception as ex:
-            if isinstance(ex, RequestTimeoutError):
+            if isinstance(ex, (RequestTimeoutError, ServerConnectionError)):
                 self.call_timed_out.emit()
 
             logger.error(ex)

securedrop_client/queue.py

@@ -3,7 +3,7 @@
 
 from PyQt5.QtCore import QObject, QThread, pyqtSlot, pyqtSignal
 from queue import PriorityQueue
-from sdclientapi import API, RequestTimeoutError
+from sdclientapi import API, RequestTimeoutError, ServerConnectionError
 from sqlalchemy.orm import scoped_session
 from typing import Optional, Tuple  # noqa: F401
 
@@ -26,10 +26,10 @@ class RunnableQueue(QObject):
     job type. If multiple jobs of the same type are added to the queue then they are retrieved
     in FIFO order.
 
-    If a RequestTimeoutError is encountered while processing a job, the job will be added back to
-    the queue, the processing loop will stop, and the paused signal will be emitted. New jobs can
-    still be added, but the processing function will need to be called again in order to resume. The
-    processing loop is resumed when the resume signal is emitted.
+    If a RequestTimeoutError or ServerConnectionError is encountered while processing a job, the
+    job will be added back to the queue, the processing loop will stop, and the paused signal will
+    be emitted. New jobs can still be added, but the processing function will need to be called
+    again in order to resume. The processing loop is resumed when the resume signal is emitted.
 
     If an ApiInaccessibleError is encountered while processing a job, api_client will be set to
     None and the processing loop will stop. If the queue is resumed before the queue manager
@@ -97,7 +97,7 @@ def process(self) -> None:
         If the job is a PauseQueueJob, emit the paused signal and return from the processing loop so
         that no more jobs are processed until the queue resumes.
 
-        If the job raises RequestTimeoutError, then:
+        If the job raises RequestTimeoutError or ServerConnectionError, then:
         (1) Add a PauseQueuejob to the queue
         (2) Add the job back to the queue so that it can be reprocessed once the queue is resumed.
 
@@ -123,7 +123,7 @@ def process(self) -> None:
                 logger.debug('{}: {}'.format(type(e).__name__, e))
                 self.api_client = None
                 return
-            except RequestTimeoutError as e:
+            except (RequestTimeoutError, ServerConnectionError) as e:
                 logger.debug('{}: {}'.format(type(e).__name__, e))
                 self.add_job(PauseQueueJob())
                 self.re_add_job(job)

securedrop_client/sync.py

@@ -2,7 +2,7 @@
 
 from PyQt5.QtCore import pyqtSignal, QObject, QThread, QTimer, Qt
 from sqlalchemy.orm import scoped_session
-from sdclientapi import API, RequestTimeoutError
+from sdclientapi import API, RequestTimeoutError, ServerConnectionError
 
 from securedrop_client.api_jobs.base import ApiInaccessibleError
 from securedrop_client.api_jobs.sync import MetadataSyncJob
@@ -75,7 +75,7 @@ def on_sync_failure(self, result: Exception) -> None:
         Only start another sync on failure if the reason is a timeout request.
         '''
         self.sync_failure.emit(result)
-        if isinstance(result, RequestTimeoutError):
+        if isinstance(result, (RequestTimeoutError, ServerConnectionError)):
             QTimer.singleShot(self.TIME_BETWEEN_SYNCS_MS, self.api_sync_bg_task.sync)
 
 

tests/api_jobs/test_base.py

@@ -1,6 +1,6 @@
 import pytest
 
-from sdclientapi import AuthError, RequestTimeoutError
+from sdclientapi import AuthError, RequestTimeoutError, ServerConnectionError
 
 from securedrop_client.api_jobs.base import ApiInaccessibleError, ApiJob
 from tests.factory import dummy_job_factory
@@ -68,15 +68,18 @@ def test_ApiJob_auth_error(mocker):
     assert not api_job.failure_signal.emit.called
 
 
-def test_ApiJob_timeout_error(mocker):
-    return_value = RequestTimeoutError()
+@pytest.mark.parametrize("exception", [RequestTimeoutError, ServerConnectionError])
+def test_ApiJob_timeout_error(mocker, exception):
+    """If the server times out or is unreachable, the corresponding
+    exception should be raised"""
+    return_value = exception()
     api_job_cls = dummy_job_factory(mocker, return_value)
     api_job = api_job_cls()
 
     mock_api_client = mocker.MagicMock()
     mock_session = mocker.MagicMock()
 
-    with pytest.raises(RequestTimeoutError):
+    with pytest.raises(exception):
         api_job._do_call_api(mock_api_client, mock_session)
 
     assert not api_job.success_signal.emit.called
@@ -98,12 +101,13 @@ def test_ApiJob_other_error(mocker):
     api_job.failure_signal.emit.assert_called_once_with(return_value)
 
 
-def test_ApiJob_retry_suceeds_after_failed_attempt(mocker):
+@pytest.mark.parametrize("exception", [RequestTimeoutError, ServerConnectionError])
+def test_ApiJob_retry_suceeds_after_failed_attempt(mocker, exception):
     """Retry logic: after failed attempt should succeed"""
 
     number_of_attempts = 5
     success_return_value = 'now works'
-    return_values = [RequestTimeoutError(), success_return_value]
+    return_values = [exception(), success_return_value]
     api_job_cls = dummy_job_factory(mocker, return_values,
                                     remaining_attempts=number_of_attempts)
     api_job = api_job_cls()
@@ -119,12 +123,13 @@ def test_ApiJob_retry_suceeds_after_failed_attempt(mocker):
     api_job.success_signal.emit.assert_called_once_with(success_return_value)
 
 
-def test_ApiJob_retry_exactly_n_attempts_times(mocker):
+@pytest.mark.parametrize("exception", [RequestTimeoutError, ServerConnectionError])
+def test_ApiJob_retry_exactly_n_attempts_times(mocker, exception):
     """Retry logic: boundary value case - 5th attempt should succeed"""
 
     number_of_attempts = 5
     success_return_value = 'now works'
-    return_values = [RequestTimeoutError()] * (number_of_attempts - 1) + [success_return_value]
+    return_values = [exception()] * (number_of_attempts - 1) + [success_return_value]
     api_job_cls = dummy_job_factory(mocker, return_values,
                                     remaining_attempts=number_of_attempts)
     api_job = api_job_cls()
@@ -140,19 +145,20 @@ def test_ApiJob_retry_exactly_n_attempts_times(mocker):
     api_job.success_signal.emit.assert_called_once_with(success_return_value)
 
 
-def test_ApiJob_retry_timeout(mocker):
+@pytest.mark.parametrize("exception", [RequestTimeoutError, ServerConnectionError])
+def test_ApiJob_retry_timeout(mocker, exception):
     """Retry logic: If we exceed the number of attempts, the job will still fail"""
 
     number_of_attempts = 5
-    return_values = [RequestTimeoutError()] * (number_of_attempts + 1)
+    return_values = [exception()] * (number_of_attempts + 1)
     api_job_cls = dummy_job_factory(mocker, return_values,
                                     remaining_attempts=number_of_attempts)
     api_job = api_job_cls()
 
     mock_api_client = mocker.MagicMock()
     mock_session = mocker.MagicMock()
 
-    with pytest.raises(RequestTimeoutError):
+    with pytest.raises(exception):
         api_job._do_call_api(mock_api_client, mock_session)
 
     assert api_job.remaining_attempts == 0

tests/api_jobs/test_uploads.py

@@ -218,19 +218,22 @@ def test_send_reply_failure_unknown_error(homedir, mocker, session, session_make
     assert len(drafts) == 1
 
 
+@pytest.mark.parametrize("exception",
+                         [sdclientapi.RequestTimeoutError,
+                          sdclientapi.ServerConnectionError])
 def test_send_reply_failure_timeout_error(homedir, mocker, session, session_maker,
-                                          reply_status_codes):
+                                          reply_status_codes, exception):
     '''
-    Check that if the SendReplyJob api call fails because of a RequestTimeoutError that a
-    SendReplyJobTimeoutError is raised.
+    Check that if the SendReplyJob api call fails because of a RequestTimeoutError or
+    ServerConnectionError that a SendReplyJobTimeoutError is raised.
     '''
     source = factory.Source()
     session.add(source)
     draft_reply = factory.DraftReply(uuid='mock_reply_uuid')
     session.add(draft_reply)
     session.commit()
     api_client = mocker.MagicMock()
-    mocker.patch.object(api_client, 'reply_source', side_effect=sdclientapi.RequestTimeoutError)
+    mocker.patch.object(api_client, 'reply_source', side_effect=exception)
     gpg = GpgHelper(homedir, session_maker, is_qubes=False)
     encrypt_fn = mocker.patch.object(gpg, 'encrypt_to_source')
     job = SendReplyJob(source.uuid, 'mock_reply_uuid', 'mock_message', gpg)

tests/test_logic.py

@@ -7,7 +7,7 @@
 import pytest
 
 from PyQt5.QtCore import Qt
-from sdclientapi import RequestTimeoutError
+from sdclientapi import RequestTimeoutError, ServerConnectionError
 from tests import factory
 
 from securedrop_client import db
@@ -1430,13 +1430,14 @@ def test_Controller_resume_queues(homedir, mocker, session_maker):
     co.api_job_queue.resume_queues.assert_called_once_with()
 
 
-def test_APICallRunner_api_call_timeout(mocker):
+@pytest.mark.parametrize("exception", [RequestTimeoutError, ServerConnectionError])
+def test_APICallRunner_api_call_timeout(mocker, exception):
     """
-    Ensure that if a RequestTimeoutError is raised, both the failure and timeout signals are
-    emitted.
+    Ensure that if a RequestTimeoutError or ServerConnectionError is raised, both
+    the failure and timeout signals are emitted.
     """
     mock_api = mocker.MagicMock()
-    mock_api.fake_request = mocker.MagicMock(side_effect=RequestTimeoutError())
+    mock_api.fake_request = mocker.MagicMock(side_effect=exception())
 
     runner = APICallRunner(mock_api.fake_request)
 

tests/test_queue.py

@@ -1,8 +1,10 @@
 '''
 Testing for the ApiJobQueue and related classes.
 '''
+import pytest
+
 from queue import Queue
-from sdclientapi import RequestTimeoutError
+from sdclientapi import RequestTimeoutError, ServerConnectionError
 
 from securedrop_client.api_jobs.downloads import FileDownloadJob
 from securedrop_client.api_jobs.base import ApiInaccessibleError, PauseQueueJob
@@ -44,19 +46,21 @@ def test_RunnableQueue_happy_path(mocker):
     assert queue.queue.empty()
 
 
-def test_RunnableQueue_job_timeout(mocker):
+@pytest.mark.parametrize("exception", [RequestTimeoutError, ServerConnectionError])
+def test_RunnableQueue_job_timeout(mocker, exception):
     '''
     Add two jobs to the queue. The first times out, and then gets resubmitted for the next pass
     through the loop.
     '''
     queue = RunnableQueue(mocker.MagicMock(), mocker.MagicMock())
     queue.pause = mocker.MagicMock()
-    job_cls = factory.dummy_job_factory(mocker, RequestTimeoutError(), remaining_attempts=5)
+    job_cls = factory.dummy_job_factory(mocker, exception(), remaining_attempts=5)
     job1 = job_cls()
     job2 = job_cls()
     queue.JOB_PRIORITIES = {PauseQueueJob: 0, job_cls: 1}
 
-    # RequestTimeoutError will cause the queue to pause, use our fake pause method instead
+    # RequestTimeoutError or ServerConnectionError will cause the queue to pause,
+    # use our fake pause method instead
     def fake_pause() -> None:
         queue.add_job(PauseQueueJob())
     queue.pause.emit = fake_pause

tests/test_sync.py

@@ -1,4 +1,6 @@
-from sdclientapi import RequestTimeoutError
+import pytest
+
+from sdclientapi import RequestTimeoutError, ServerConnectionError
 
 from securedrop_client.api_jobs.base import ApiInaccessibleError
 from securedrop_client.sync import ApiSync
@@ -153,7 +155,7 @@ def test_ApiSync_on_sync_success(mocker, session_maker, homedir):
 def test_ApiSync_on_sync_failure(mocker, session_maker, homedir):
     '''
     Ensure failure handler emits failure signal that the Controller links to and does not fire
-    another sync for errors other than RequestTimeoutError
+    another sync for errors other than RequestTimeoutError or ServerConnectionError
     '''
     api_sync = ApiSync(mocker.MagicMock(), session_maker, mocker.MagicMock(), homedir)
     sync_failure = mocker.patch.object(api_sync, 'sync_failure')
@@ -167,15 +169,17 @@ def test_ApiSync_on_sync_failure(mocker, session_maker, homedir):
     singleShot_fn.assert_not_called()
 
 
-def test_ApiSync_on_sync_failure_because_of_timeout(mocker, session_maker, homedir):
+@pytest.mark.parametrize("exception", [RequestTimeoutError, ServerConnectionError])
+def test_ApiSync_on_sync_failure_because_of_timeout(mocker, session_maker, homedir, exception):
     '''
     Ensure failure handler emits failure signal that the Controller links to and sets up timer to
-    fire another sync after 15 seconds if the failure reason is a RequestTimeoutError.
+    fire another sync after 15 seconds if the failure reason is a RequestTimeoutError or
+    ServerConnectionError.
     '''
     api_sync = ApiSync(mocker.MagicMock(), session_maker, mocker.MagicMock(), homedir)
     sync_failure = mocker.patch.object(api_sync, 'sync_failure')
     singleShot_fn = mocker.patch('securedrop_client.sync.QTimer.singleShot')
-    error = RequestTimeoutError()
+    error = exception()
 
     api_sync.on_sync_failure(error)