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

Conversation

sssoleileraaa
Copy link
Contributor

@sssoleileraaa sssoleileraaa commented Aug 12, 2019

Description

The queues now pause when there is either an auth error, api connection failure, or request timeout error. The queues resume when the user clicks "Retry" or upon a sync/refresh after the issues are resolved.

Resolves #443
Towards #391

Test Plan

Make sure acceptance criteria is satisfied: #391 (comment), and follow these steps:

RequestTimeoutError with Retry link

  1. Send reply after pausing the staging app server vm, wait and observe 5 Keyring access qubes notication popups and reply bar turn red in the gui and the SecureDrop error message and Retry link.
  2. Unpause the staging app server vm
  3. Click Retry link and wait until the next sync happens or manually click the refresh icon and see the reply bar turn blue

ApiInaccessibleError

  1. Send reply after pausing the staging app server vm
  2. Log out before the 5th timeout retry so that the final error will be an ApiInaccessibleError and see the reply bar turn red in the gui and the SecureDrop error message without the Retry link.
  3. Unpause the staging app server vm
  4. Log back in
  5. The queue is automatically resumedd and you should see the reply bar turn blue

Checklist

If these changes modify code paths involving cryptography, the opening of files in VMs or network (via the RPC service) traffic, Qubes testing in the staging environment is required. For fine tuning of the graphical user interface, testing in any environment in Qubes is required. Please check as applicable:

  • I have tested these changes in the appropriate Qubes environment
  • I do not have an appropriate Qubes OS workstation set up (the reviewer will need to test these changes)
  • These changes should not need testing in Qubes

@eloquence eloquence changed the title Issue 443 pause queue Pause queue on auth errors, connection failures and timeouts Aug 12, 2019
@eloquence eloquence added this to Ready for review in SecureDrop Team Board Aug 12, 2019
@redshiftzero redshiftzero moved this from Ready for review to Under Review in SecureDrop Team Board Aug 22, 2019
Copy link
Contributor

@redshiftzero redshiftzero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

excellent work! 😍 based on visual review of the diff lgtm (i.e. i haven't run through the test plan yet in qubes), just one minor comment inline, otherwise all my comments from #514 (comment) have been addressed

(btw also needs rebase on top of latest)

securedrop_client/queue.py Outdated Show resolved Hide resolved
@redshiftzero redshiftzero moved this from Under Review to In Development in SecureDrop Team Board Aug 27, 2019
@sssoleileraaa
Copy link
Contributor Author

sssoleileraaa commented Aug 28, 2019

Notes for the reviewer of this PR:

  • We still need to move the metadata sync to the queue, which is being tracked here: Create a MetadataSyncJob #461

    When a metadata sync sees a request timeout, the user will see "The connection to the SecureDrop server timed out. Please try again." instead of the "The SecureDrop server cannot be reached." with retry link. Once Create a MetadataSyncJob #461 is complete, a metadata sync timeout will also pause the queue. Right now a user can click the refresh link when the "retry" error message is showing, which does not remove the retry error message and the refresh state will remain active.

  • Auto-resume for the queue is being tracked here: health checker - auto resume #491

    Right now a user has to click "retry" in order to resume the queue. Also the error message will not go away until retry is clicked. Once the client can detect that the issue has been resolved, the error message will automatically go away.

  • The behavior where a pending reply -> <Reply not yet available> -> contents of the reply is being tracked here: Treat all sync-related actions as part of sync #468

    Once metadata sync is moved to the queue, we will not update the GUI until the message has been fully decrypted.

@sssoleileraaa sssoleileraaa moved this from In Development to Ready for review in SecureDrop Team Board Aug 28, 2019
Copy link
Contributor

@redshiftzero redshiftzero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all my comments were addressed, thanks! :shipit:

@redshiftzero redshiftzero merged commit 2737155 into master Aug 31, 2019
SecureDrop Team Board automation moved this from Ready for review to Done Aug 31, 2019
@redshiftzero redshiftzero deleted the issue-443-pause-queue branch August 31, 2019 00:24
@sssoleileraaa sssoleileraaa mentioned this pull request Oct 31, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

Pausing the queue
2 participants