Merge pull request #531 from freedomofpress/issue-443-pause-queue

Pause queue on auth errors, connection failures and timeouts

securedrop_client/api_jobs/base.py

@@ -9,7 +9,7 @@
 
 DEFAULT_NUM_ATTEMPTS = 5
 
-ApiJobType = TypeVar('ApiJobType', bound='ApiJob')
+QueueJobType = TypeVar('QueueJobType', bound='QueueJob')
 
 
 class ApiInaccessibleError(Exception):
@@ -21,7 +21,31 @@ def __init__(self, message: Optional[str] = None) -> None:
         super().__init__(message)
 
 
-class ApiJob(QObject):
+class QueueJob(QObject):
+    def __init__(self) -> None:
+        super().__init__()
+        self.order_number = None  # type: Optional[int]
+
+    def __lt__(self, other: QueueJobType) -> bool:
+        '''
+        Python's PriorityQueue requires that QueueJobs are sortable as it
+        retrieves the next job using sorted(list(entries))[0].
+
+        For QueueJobs that have equal priority, we need to use the order_number key
+        to break ties to ensure that objects are retrieved in FIFO order.
+        '''
+        if self.order_number is None or other.order_number is None:
+            raise ValueError('cannot compare jobs without order_number!')
+
+        return self.order_number < other.order_number
+
+
+class PauseQueueJob(QueueJob):
+    def __init__(self) -> None:
+        super().__init__()
+
+
+class ApiJob(QueueJob):
 
     '''
     Signal that is emitted after an job finishes successfully.
@@ -34,22 +58,8 @@ class ApiJob(QObject):
     failure_signal = pyqtSignal(Exception)
 
     def __init__(self, remaining_attempts: int = DEFAULT_NUM_ATTEMPTS) -> None:
-        super().__init__(None)  # `None` because the QOjbect has no parent
+        super().__init__()
         self.remaining_attempts = remaining_attempts
-        self.order_number = None  # type: Optional[int]
-
-    def __lt__(self, other: ApiJobType) -> bool:
-        '''
-        Python's PriorityQueue requires that ApiJobs are sortable as it
-        retrieves the next job using sorted(list(entries))[0].
-
-        For ApiJobs that have equal priority, we need to use the order_number key
-        to break ties to ensure that objects are retrieved in FIFO order.
-        '''
-        if self.order_number is None or other.order_number is None:
-            raise ValueError('cannot compare jobs without order_number!')
-
-        return self.order_number < other.order_number
 
     def _do_call_api(self, api_client: API, session: Session) -> None:
         if not api_client:
@@ -59,7 +69,7 @@ def _do_call_api(self, api_client: API, session: Session) -> None:
             try:
                 self.remaining_attempts -= 1
                 result = self.call_api(api_client, session)
-            except (ApiInaccessibleError, AuthError) as e:
+            except (AuthError, ApiInaccessibleError) as e:
                 raise ApiInaccessibleError() from e
             except RequestTimeoutError as e:
                 if self.remaining_attempts == 0:

securedrop_client/gui/main.py

@@ -178,12 +178,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):
+    def update_error_status(self, message: str, duration=10000, retry=False) -> 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)
+        self.top_pane.update_error_status(message, duration, retry)
 
     def clear_error_status(self):
         """

securedrop_client/gui/widgets.py

@@ -99,6 +99,7 @@ def __init__(self):
 
     def setup(self, controller):
         self.refresh.setup(controller)
+        self.error_status_bar.setup(controller)
 
     def enable_refresh(self):
         self.refresh.enable()
@@ -109,8 +110,8 @@ def disable_refresh(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):
-        self.error_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 clear_error_status(self):
         self.error_status_bar.clear_message()
@@ -307,6 +308,15 @@ 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):
@@ -338,15 +348,22 @@ 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()
@@ -356,6 +373,7 @@ 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()
@@ -365,12 +383,28 @@ def _show(self):
     def _on_status_timeout(self):
         self._hide()
 
-    def update_message(self, message: str, duration: int):
+    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:
         """
-        Display a status message to the user for a given duration.
+        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)
-        self.status_timer.start(duration)
+
+        if duration != 0:
+            self.status_timer.start(duration)
+
         self._show()
 
     def clear_message(self):
@@ -1830,13 +1864,6 @@ def __init__(self, source_db_object: Source, controller: Controller):
 
         self.update_conversation(self.source.collection)
 
-        # Refresh the session to update any replies that failed from a network timeout
-        self.controller.reply_succeeded.connect(self.refresh_conversation)
-
-    def refresh_conversation(self):
-        self.controller.session.refresh(self.source)
-        self.update_conversation(self.source.collection)
-
     def clear_conversation(self):
         while self.conversation_layout.count():
             child = self.conversation_layout.takeAt(0)

securedrop_client/logic.py

@@ -171,6 +171,7 @@ def __init__(self, hostname: str, gui, session_maker: sessionmaker,
 
         # Queue that handles running API job
         self.api_job_queue = ApiJobQueue(self.api, self.session_maker)
+        self.api_job_queue.paused.connect(self.on_queue_paused)
 
         # Contains active threads calling the API.
         self.api_threads = {}  # type: Dict[str, Dict]
@@ -246,7 +247,6 @@ def call_api(self,
             lambda: self.completed_api_call(new_thread_id, success_callback))
         new_api_runner.call_failed.connect(
             lambda: self.completed_api_call(new_thread_id, failure_callback))
-        new_api_runner.call_timed_out.connect(self.on_api_timeout)
 
         # when the thread starts, we want to run `call_api` on `api_runner`
         new_api_thread.started.connect(new_api_runner.call_api)
@@ -260,9 +260,17 @@ def call_api(self,
         # Start the thread and related activity.
         new_api_thread.start()
 
-    def on_api_timeout(self) -> None:
-        self.gui.update_error_status(_('The connection to the SecureDrop server timed out. '
-                                       'Please try again.'))
+    def on_queue_paused(self) -> None:
+        if self.api is None:
+            self.gui.update_error_status(_('The SecureDrop server cannot be reached.'))
+        else:
+            self.gui.update_error_status(
+                _('The SecureDrop server cannot be reached.'),
+                duration=0,
+                retry=True)
+
+    def resume_queues(self) -> None:
+        self.api_job_queue.resume_queues()
 
     def completed_api_call(self, thread_id, user_callback):
         """
@@ -303,11 +311,8 @@ def on_authenticate_success(self, result):
                       self.on_get_current_user_failure)
         self.api_job_queue.login(self.api)
 
-        # Clear the sidebar error status bar if a message was shown
-        # to the user indicating they should log in.
-        self.gui.clear_error_status()
-
         self.is_authenticated = True
+        self.resume_queues()
 
     def on_authenticate_failure(self, result: Exception) -> None:
         # Failed to authenticate. Reset state with failure message.
@@ -415,7 +420,10 @@ def on_sync_failure(self, result: Exception) -> None:
         """
         Called when syncronisation of data via the API fails.
         """
-        logger.debug('Sync failed: "{}".'.format(result))
+        self.gui.update_error_status(
+            _('The SecureDrop server cannot be reached.'),
+            duration=0,
+            retry=True)
 
     def update_sync(self):
         """
@@ -438,7 +446,6 @@ def on_update_star_success(self, result) -> None:
         After we star a source, we should sync the API such that the local database is updated.
         """
         self.sync_api()  # Syncing the API also updates the source list UI
-        self.gui.clear_error_status()
 
     def on_update_star_failure(self, result: UpdateStarJobException) -> None:
         """
@@ -456,13 +463,8 @@ def update_star(self, source_db_object):
         if not self.api:  # Then we should tell the user they need to login.
             self.on_action_requiring_login()
             return
-        else:  # Clear the error status bar
-            self.gui.clear_error_status()
 
-        job = UpdateStarJob(
-                source_db_object.uuid,
-                source_db_object.is_starred
-        )
+        job = UpdateStarJob(source_db_object.uuid, source_db_object.is_starred)
         job.success_signal.connect(self.on_update_star_success, type=Qt.QueuedConnection)
         job.failure_signal.connect(self.on_update_star_failure, type=Qt.QueuedConnection)
 
@@ -625,7 +627,6 @@ def on_delete_source_success(self, result) -> None:
         Handler for when a source deletion succeeds.
         """
         self.sync_api()
-        self.gui.clear_error_status()
 
     def on_delete_source_failure(self, result: Exception) -> None:
         logging.info("failed to delete source at server")
@@ -665,6 +666,7 @@ def send_reply(self, source_uuid: str, reply_uuid: str, message: str) -> None:
     def on_reply_success(self, reply_uuid: str) -> None:
         logger.debug('{} sent successfully'.format(reply_uuid))
         self.reply_succeeded.emit(reply_uuid)
+        self.sync_api()
 
     def on_reply_failure(
         self,