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

Standardize connection errors to "Trying to reconnect" language without Retry #811

Closed
eloquence opened this issue Feb 19, 2020 · 5 comments · Fixed by #823 or #952
Closed

Standardize connection errors to "Trying to reconnect" language without Retry #811

eloquence opened this issue Feb 19, 2020 · 5 comments · Fixed by #823 or #952
Assignees
Labels
enhancement New feature or request network

Comments

@eloquence
Copy link
Member

eloquence commented Feb 19, 2020

Description

We've agreed to the following changes to network error handling:

  • All network-related errors (connection errors or timeouts) should display the same error message. If the error message is already being displayed, no change to the message is necessary.

  • The error message should not display a "Retry" link, as the client will automatically and continuously attempt to restore connectivity.

  • The message should be changed to make that clearer. "The SecureDrop server cannot be reached. Trying to reconnect." sounds reasonable, but we can use this issue to kick around exact language and UI display.

  • The message should not be ephemeral; it should continue to be the only message displayed until connectivity is restored.

Motivation

  • There is message display contention between multiple places in the client that can trigger messages to the same effect for the same reason. In other words, an "operation failed" message can compete with a "network down" message (not the actual wording in the client).
  • Offering a "Retry" button that may disappear at any time -- including while the user is attempting to click it -- is confusing.
  • The exact behavior of "Retry" is not always clear.

Out of scope for this issue:

  • Any changes to the sync widget or the top bar of the client (e.g., offline mode styling)
  • Any changes to what actions are available to the user in the client (e.g., preventing the user from sending additional replies)
@eloquence eloquence added enhancement New feature or request network labels Feb 19, 2020
@eloquence eloquence added this to the 0.2.0beta milestone Feb 19, 2020
@eloquence
Copy link
Member Author

I'm not labeling this a release blocker just yet as the behavior is confusing but not completely broken, especially once #604 (which I've marked as a release blocker) is fixed. However, I would recommend that it should at least be a very high priority once other client blockers are resolved.

#798 is related to this; once the message is standardized, the MetadataSync can be another way to trigger it if it's not already being displayed.

@sssoleileraaa
Copy link
Contributor

Resolving this issue will also resolve #676

@eloquence
Copy link
Member Author

@creviera I still see "Failed to update star" as a special case error message e.g. if I take down the server while the client is running. Should that also trigger the "Trying to reconnect" error or is there a reason why it shouldn't?

@eloquence
Copy link
Member Author

eloquence commented Mar 9, 2020

I'm reopening this issue since we still have two cases where we want to display "Trying to reconnect" if there is a connection error:

  • If a starring operation failed due to a connection error and is being retried
  • If a deletion operation failed due to a connection error and is being retried.

@eloquence eloquence reopened this Mar 9, 2020
@sssoleileraaa
Copy link
Contributor

I'm reopening this issue since we still have two cases where we want to display "Trying to reconnect" if there is a connection error:

  • If a starring operation failed due to a connection error and is being retried

This will be resolved in #952 and will close this issue.

  • If a deletion operation failed due to a connection error and is being retried.

This was resolved in #879

@rmol rmol closed this as completed in #952 Mar 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request network
Projects
None yet
2 participants