Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pause queue on auth errors, connection failures and timeouts #531

Merged
merged 19 commits into from
Aug 31, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 28 additions & 18 deletions securedrop_client/api_jobs/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

DEFAULT_NUM_ATTEMPTS = 5

ApiJobType = TypeVar('ApiJobType', bound='ApiJob')
QueueJobType = TypeVar('QueueJobType', bound='QueueJob')


class ApiInaccessibleError(Exception):
Expand All @@ -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.
Expand All @@ -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:
Expand All @@ -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:
Expand Down
4 changes: 2 additions & 2 deletions securedrop_client/gui/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
"""
Expand Down
51 changes: 39 additions & 12 deletions securedrop_client/gui/widgets.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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()
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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()
Expand All @@ -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()
Expand All @@ -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):
Expand Down Expand Up @@ -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)
Expand Down
36 changes: 19 additions & 17 deletions securedrop_client/logic.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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)
Expand All @@ -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):
"""
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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):
"""
Expand All @@ -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:
"""
Expand All @@ -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)

Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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,
Expand Down
Loading