Merge pull request #823 from freedomofpress/standardize-connection-er…

…rors

Standardize connection errors

securedrop_client/gui/main.py

@@ -179,12 +179,12 @@ def update_activity_status(self, message: str, duration=0):
         """
         self.top_pane.update_activity_status(message, duration)
 
-    def update_error_status(self, message: str, duration=10000, retry=False) -> None:
+    def update_error_status(self, message: str, duration=10000) -> None:
         """
         Display an error status message to the user. Optionally, supply a duration
         (in milliseconds), the default will continuously show the message.
         """
-        self.top_pane.update_error_status(message, duration, retry)
+        self.top_pane.update_error_status(message, duration)
 
     def clear_error_status(self):
         """

securedrop_client/gui/widgets.py

@@ -124,8 +124,8 @@ def set_logged_out(self):
     def update_activity_status(self, message: str, duration: int):
         self.activity_status_bar.update_message(message, duration)
 
-    def update_error_status(self, message: str, duration: int, retry: bool):
-        self.error_status_bar.update_message(message, duration, retry)
+    def update_error_status(self, message: str, duration: int):
+        self.error_status_bar.update_message(message, duration)
 
     def clear_error_status(self):
         self.error_status_bar.clear_message()
@@ -313,15 +313,6 @@ class ErrorStatusBar(QWidget):
         font-size: 14px;
         color: #0c3e75;
     }
-    QPushButton#retry_button {
-        border: none;
-        padding-right: 30px;
-        background-color: #fff;
-        color: #0065db;
-        font-family: 'Source Sans Pro';
-        font-weight: 600;
-        font-size: 12px;
-    }
     '''
 
     def __init__(self):
@@ -353,22 +344,15 @@ def __init__(self):
         self.status_bar.setObjectName('error_status_bar')  # Set css id
         self.status_bar.setSizeGripEnabled(False)
 
-        # Retry button
-        self.retry_button = QPushButton('RETRY')
-        self.retry_button.setObjectName('retry_button')
-        self.retry_button.setFixedHeight(42)
-
         # Add widgets to layout
         layout.addWidget(self.vertical_bar)
         layout.addWidget(self.label)
         layout.addWidget(self.status_bar)
-        layout.addWidget(self.retry_button)
 
         # Hide until a message needs to be displayed
         self.vertical_bar.hide()
         self.label.hide()
         self.status_bar.hide()
-        self.retry_button.hide()
 
         # Only show errors for a set duration
         self.status_timer = QTimer()
@@ -378,7 +362,6 @@ def _hide(self):
         self.vertical_bar.hide()
         self.label.hide()
         self.status_bar.hide()
-        self.retry_button.hide()
 
     def _show(self):
         self.vertical_bar.show()
@@ -390,21 +373,12 @@ def _on_status_timeout(self):
 
     def setup(self, controller):
         self.controller = controller
-        self.retry_button.clicked.connect(self._on_retry_clicked)
 
-    def _on_retry_clicked(self) -> None:
-        self.clear_message()
-        self._hide()
-        self.controller.resume_queues()
-
-    def update_message(self, message: str, duration: int, retry: bool) -> None:
+    def update_message(self, message: str, duration: int) -> None:
         """
         Display a status message to the user for a given duration. If the duration is zero,
         continuously show message.
         """
-        if retry:
-            self.retry_button.show()
-
         self.status_bar.showMessage(message, duration)
 
         if duration != 0:

securedrop_client/logic.py

@@ -322,17 +322,14 @@ def call_api(self,
 
     def on_queue_paused(self) -> None:
         self.gui.update_error_status(
-            _('The SecureDrop server cannot be reached.'),
-            duration=0,
-            retry=True)
+            _('The SecureDrop server cannot be reached. Trying to reconnect...'), duration=0)
         self.show_last_sync_timer.start(TIME_BETWEEN_SHOWING_LAST_SYNC_MS)
 
     def resume_queues(self) -> None:
         self.api_job_queue.resume_queues()
         self.show_last_sync_timer.stop()
 
-        # clear error status in case queue was paused resulting in a permanent error message with
-        # retry link
+        # clear error status in case queue was paused resulting in a permanent error message
         self.gui.clear_error_status()
 
     def completed_api_call(self, thread_id, user_callback):
@@ -465,6 +462,9 @@ def on_sync_failure(self, result: Exception) -> None:
             self.invalidate_token()
             self.logout()
             self.gui.show_login(error=_('Your session expired. Please log in again.'))
+        elif isinstance(result, (RequestTimeoutError, ServerConnectionError)):
+            self.gui.update_error_status(
+                _('The SecureDrop server cannot be reached. Trying to reconnect...'), duration=0)
 
     def show_last_sync(self):
         """
@@ -507,8 +507,7 @@ def logout(self):
         out in the GUI.
         """
 
-        # clear error status in case queue was paused resulting in a permanent error message with
-        # retry link
+        # clear error status in case queue was paused resulting in a permanent error message
         self.gui.clear_error_status()
 
         if self.api is not None:

tests/gui/test_main.py

@@ -170,12 +170,12 @@ def test_show_sources(mocker):
 def test_update_error_status_default(mocker):
     """
     Ensure that the error to be shown in the error status bar will be passed to the top pane with a
-    default duration of 10 seconds and no retry link.
+    default duration of 10 seconds.
     """
     w = Window()
     w.top_pane = mocker.MagicMock()
     w.update_error_status(message='test error message')
-    w.top_pane.update_error_status.assert_called_once_with('test error message', 10000, False)
+    w.top_pane.update_error_status.assert_called_once_with('test error message', 10000)
 
 
 def test_update_error_status(mocker):
@@ -186,7 +186,7 @@ def test_update_error_status(mocker):
     w = Window()
     w.top_pane = mocker.MagicMock()
     w.update_error_status(message='test error message', duration=123)
-    w.top_pane.update_error_status.assert_called_once_with('test error message', 123, False)
+    w.top_pane.update_error_status.assert_called_once_with('test error message', 123)
 
 
 def test_update_activity_status_default(mocker):

tests/gui/test_widgets.py

@@ -93,9 +93,9 @@ def test_TopPane_update_error_status(mocker):
     tp = TopPane()
     tp.error_status_bar = mocker.MagicMock()
 
-    tp.update_error_status(message='test message', duration=5, retry=True)
+    tp.update_error_status(message='test message', duration=5)
 
-    tp.error_status_bar.update_message.assert_called_once_with('test message', 5, True)
+    tp.error_status_bar.update_message.assert_called_once_with('test message', 5)
 
 
 def test_TopPane_clear_error_status(mocker):
@@ -283,7 +283,7 @@ def test_ErrorStatusBar_update_message(mocker):
     esb.status_bar = mocker.MagicMock()
     esb.status_timer = mocker.MagicMock()
 
-    esb.update_message(message='test message', duration=123, retry=True)
+    esb.update_message(message='test message', duration=123)
 
     esb.status_bar.showMessage.assert_called_once_with('test message', 123)
     esb.status_timer.start.assert_called_once_with(123)
@@ -321,17 +321,6 @@ def test_ErrorStatusBar_on_status_timeout(mocker):
     assert esb.isHidden()
 
 
-def test_ErrorStatusBar_on_retry_clicked(mocker):
-    controller = mocker.MagicMock()
-    esb = ErrorStatusBar()
-    esb.setup(controller)
-
-    esb._on_retry_clicked()
-
-    assert esb.isHidden()
-    controller.resume_queues.assert_called_once_with()
-
-
 def test_ActivityStatusBar_update_message(mocker):
     """
     Calling update_message updates the message of the QStatusBar.

tests/test_logic.py

@@ -427,6 +427,23 @@ def test_Controller_on_sync_failure_due_to_invalid_token(homedir, config, mocker
     co.gui.show_login.assert_called_once_with(error='Your session expired. Please log in again.')
 
 
+@pytest.mark.parametrize("exception", [RequestTimeoutError, ServerConnectionError])
+def test_Controller_on_sync_failure_due_to_timeout(homedir, mocker, exception):
+    """
+    If the sync fails because of a timeout, make sure to show an error message.
+    """
+    gui = mocker.MagicMock()
+    co = Controller('http://localhost', gui, mocker.MagicMock(), homedir)
+    co.logout = mocker.MagicMock()
+    co.gui = mocker.MagicMock()
+    co.gui.update_error_status = mocker.MagicMock()
+
+    co.on_sync_failure(exception())
+
+    co.gui.update_error_status.assert_called_once_with(
+        'The SecureDrop server cannot be reached. Trying to reconnect...', duration=0)
+
+
 def test_Controller_on_sync_success(homedir, config, mocker):
     """
     If there's a result to syncing, then update local storage.
@@ -1482,7 +1499,7 @@ def test_APICallRunner_api_call_timeout(mocker, exception):
 
 def test_Controller_on_queue_paused(homedir, config, mocker, session_maker):
     '''
-    Check that a paused queue is communicated to the user via the error status bar with retry option
+    Check that a paused queue is communicated to the user via the error status bar
     '''
     mock_gui = mocker.MagicMock()
     co = Controller('http://localhost', mock_gui, session_maker, homedir)
@@ -1491,7 +1508,7 @@ def test_Controller_on_queue_paused(homedir, config, mocker, session_maker):
     co.show_last_sync_timer = mocker.MagicMock()
     co.on_queue_paused()
     mock_gui.update_error_status.assert_called_once_with(
-        'The SecureDrop server cannot be reached.', duration=0, retry=True)
+        'The SecureDrop server cannot be reached. Trying to reconnect...', duration=0)
     co.show_last_sync_timer.start.assert_called_once_with(TIME_BETWEEN_SHOWING_LAST_SYNC_MS)