Skip to content

Handle unrecoverable long-poll failures #186

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

Closed
gnprice opened this issue Jun 14, 2023 · 2 comments
Closed

Handle unrecoverable long-poll failures #186

gnprice opened this issue Jun 14, 2023 · 2 comments
Labels
a-api Implementing specific parts of the Zulip server API a-sync Event queue; retry; local echo; races

Comments

@gnprice
Copy link
Member

gnprice commented Jun 14, 2023

As described in #184, when a long-poll for events fails, the current prototype just stops polling. That issue is about transient failures, and #185 is about a non-transient but recoverable error. That leaves failures that we can't expect to fix, neither by just retrying nor by some other request: e.g., the user's auth credentials were rejected.

At a minimum, in this case we should give the user some indication that the data they're seeing is no longer being updated: e.g. set the isStale flag mentioned in #185.

If specifically what happened is that authentication failed, then we should probably ask the user to log in again to that account.

If we do those two things, and also make sure the user always has the option to navigate away so they can add or choose another account to use, then I think that's pretty satisfactory. This situation shouldn't be common, so it's OK if it isn't super smooth as long as there's some reasonable way for the user to recover.

@gnprice gnprice added the a-api Implementing specific parts of the Zulip server API label Jun 14, 2023
@gnprice gnprice added this to the Launch milestone Jun 14, 2023
@gnprice gnprice added the a-sync Event queue; retry; local echo; races label Oct 22, 2024
@gnprice
Copy link
Member Author

gnprice commented Nov 8, 2024

Now that the following two issues are complete:

the current behavior is:

At a minimum, in this case we should give the user some indication that the data they're seeing is no longer being updated

This is done (as #465).

If specifically what happened is that authentication failed, then we should probably ask the user to log in again to that account.

The user will get a "snack bar"-style popup saying generically "Error connecting to Zulip. Retrying…". That has a "Details" button; if they hit that, they get a modal saying something like:

Error

Error connecting to Zulip at https://chat.zulip.org. Will retry:

ZulipApiException: 401 UNAUTHORIZED getEvents: Invalid API key

(The exact message at the end, from "Invalid" on, might vary depending on the nature of the auth failure.)

So with that, it's not as clear or convenient as it could be, but the information is there. And the user can indeed always navigate away so they can add or choose another account. Moreover after we have #463 (coming soon), the user can forget the account that had the problem, in order to add it anew.


In short I think this issue is done according to its title: we do handle these failures. I'll file a fresh issue for the remaining task of noticing auth failures specifically and flagging those for the user.

@gnprice
Copy link
Member Author

gnprice commented Nov 8, 2024

I'll file a fresh issue for the remaining task of noticing auth failures specifically and flagging those for the user.

Done:

While writing it up I realized that registerQueue is actually the more important case than getEvents (i.e. long-poll), for reasons explained there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a-api Implementing specific parts of the Zulip server API a-sync Event queue; retry; local echo; races
Projects
Status: Done
Development

No branches or pull requests

1 participant