-
Notifications
You must be signed in to change notification settings - Fork 262
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
Handle invalid API key on register-queue #737
Comments
The status quo of this is extremely annoying when it comes up — the app just sits at the loading page forever, and all you can do is switch to a different account. For example see this long-running thread, where we just today figured out that the cause is probably that the API key (or email / API key pair, anyway) was invalid. That would be substantially mitigated by #890, telling the user what the problem is. I'm leaving this with a later milestone only because we have that one in the current milestone M5 Launch. |
This is near term fix for a user-reported issue: https://chat.zulip.org/#narrow/channel/48-mobile/topic/0.2E0.2E19.20Flutter.20.3A.20Cant.20connect.20to.20self.20hosted.20instance/near/2004042 It is not intended to be the full fix. With a better UX, we would bring the user back to the choose-account page without them manually doing so. That's covered by zulip#737 but out-of-scope for this commit. Signed-off-by: Zixuan James Li <zixuan@zulip.com>
This is near term fix for a user-reported issue: https://chat.zulip.org/#narrow/channel/48-mobile/topic/0.2E0.2E19.20Flutter.20.3A.20Cant.20connect.20to.20self.20hosted.20instance/near/2004042 It is not intended to be the full fix. With a better UX, we would bring the user back to the choose-account page without them manually doing so. That's covered by zulip#737 but out-of-scope for this commit. Signed-off-by: Zixuan James Li <zixuan@zulip.com>
This is near term fix for a user-reported issue: https://chat.zulip.org/#narrow/channel/48-mobile/topic/0.2E0.2E19.20Flutter.20.3A.20Cant.20connect.20to.20self.20hosted.20instance/near/2004042 It is not intended to be the full fix. With a better UX, we would bring the user back to the choose-account page without them manually doing so. That's covered by zulip#737 but out-of-scope for this commit. Signed-off-by: Zixuan James Li <zixuan@zulip.com>
This happens automatically if you call |
This is near term fix for a user-reported issue: https://chat.zulip.org/#narrow/channel/48-mobile/topic/0.2E0.2E19.20Flutter.20.3A.20Cant.20connect.20to.20self.20hosted.20instance/near/2004042 It is not intended to be the full fix. With a better UX, we would bring the user back to the choose-account page without them manually doing so. That's covered by zulip#737 but out-of-scope for this commit. Signed-off-by: Zixuan James Li <zixuan@zulip.com>
This is near term fix for a user-reported issue: https://chat.zulip.org/#narrow/channel/48-mobile/topic/0.2E0.2E19.20Flutter.20.3A.20Cant.20connect.20to.20self.20hosted.20instance/near/2004042 It is not intended to be the full fix. With a better UX, we would bring the user back to the choose-account page without them manually doing so. That's covered by zulip#737 but out-of-scope for this commit. Signed-off-by: Zixuan James Li <zixuan@zulip.com>
This is near term fix for a user-reported issue: https://chat.zulip.org/#narrow/channel/48-mobile/topic/0.2E0.2E19.20Flutter.20.3A.20Cant.20connect.20to.20self.20hosted.20instance/near/2004042 It is not intended to be the full fix. With a better UX, we would bring the user back to the choose-account page without them manually doing so. That's covered by zulip#737 but out-of-scope for this commit. Signed-off-by: Zixuan James Li <zixuan@zulip.com>
Fixes: zulip#737 Signed-off-by: Zixuan James Li <zixuan@zulip.com>
Fixes: zulip#737 Signed-off-by: Zixuan James Li <zixuan@zulip.com>
Fixes: zulip#737 Signed-off-by: Zixuan James Li <zixuan@zulip.com>
This fixes the user reported issue by logging out the account if the API key is found invalid: https://chat.zulip.org/#narrow/channel/48-mobile/topic/0.2E0.2E19.20Flutter.20.3A.20Cant.20connect.20to.20self.20hosted.20instance/near/2004042 Fixes: zulip#737 Signed-off-by: Zixuan James Li <zixuan@zulip.com>
This fixes the user reported issue by logging out the account if the API key is found invalid: https://chat.zulip.org/#narrow/channel/48-mobile/topic/0.2E0.2E19.20Flutter.20.3A.20Cant.20connect.20to.20self.20hosted.20instance/near/2004042 Fixes: zulip#737 Signed-off-by: Zixuan James Li <zixuan@zulip.com>
Fixes: zulip#737 Signed-off-by: Zixuan James Li <zixuan@zulip.com>
Fixes: zulip#737 Signed-off-by: Zixuan James Li <zixuan@zulip.com>
Fixes: zulip#737 Signed-off-by: Zixuan James Li <zixuan@zulip.com>
Fixes: zulip#737 Signed-off-by: Zixuan James Li <zixuan@zulip.com>
The method loadPerAccount has two call sites, i.e. places where we send register-queue requests: 1. _reloadPerAccount through [UpdateMachine._handlePollError] 2. perAccount through [PerAccountStoreWidget] (the common case) We utilize the existing [AccountNotFoundException] because invalidating invalid auth keys effectively logs out the account, that it can no longer be found in the store. [PerAccountStoreWidget] already expects this error by ignoring it and waiting for the route to be removed: ```dart try { // If this succeeds, globalStore will notify listeners, and // [didChangeDependencies] will run again, this time in the // `store != null` case above. await globalStore.perAccount(widget.accountId); } on AccountNotFoundException { // The account was logged out while its store was loading. // This widget will be showing [placeholder] perpetually, // but that's OK as long as other code will be removing it from the UI // (usually by using [routeToRemoveOnLogout]). } ``` (included because unchanged code is not in the diff) To handle 1, we apply the same expectation that the account gets logged out when the exception happens. For `_handlePollError`, while the store was not replaced at all, the end result of having the old store disposed is still expected. This partly addresses zulip#890 by handling authentication errors for register-queue. Fixes: zulip#737 Signed-off-by: Zixuan James Li <zixuan@zulip.com>
The method loadPerAccount has two call sites, i.e. places where we send register-queue requests: 1. _reloadPerAccount through [UpdateMachine._handlePollError] 2. perAccount through [PerAccountStoreWidget] (the common case) We utilize the existing [AccountNotFoundException] because invalidating invalid auth keys effectively logs out the account, that it can no longer be found in the store. [PerAccountStoreWidget] already expects this error by ignoring it and waiting for the route to be removed: ```dart try { // If this succeeds, globalStore will notify listeners, and // [didChangeDependencies] will run again, this time in the // `store != null` case above. await globalStore.perAccount(widget.accountId); } on AccountNotFoundException { // The account was logged out while its store was loading. // This widget will be showing [placeholder] perpetually, // but that's OK as long as other code will be removing it from the UI // (usually by using [routeToRemoveOnLogout]). } ``` (included because unchanged code is not in the diff) To handle 1, we apply the same expectation that the account gets logged out when the exception happens. For `_handlePollError`, while the store was not replaced at all, the end result of having the old store disposed is still expected. This partly addresses zulip#890 by handling authentication errors for register-queue. Fixes: zulip#737 Signed-off-by: Zixuan James Li <zixuan@zulip.com>
The method loadPerAccount has two call sites, i.e. places where we send register-queue requests: 1. _reloadPerAccount through [UpdateMachine._handlePollError] 2. perAccount through [PerAccountStoreWidget] (the common case) We utilize the existing [AccountNotFoundException] because invalidating invalid auth keys effectively logs out the account, that it can no longer be found in the store. [PerAccountStoreWidget] already expects this error by ignoring it and waiting for the route to be removed: ```dart try { // If this succeeds, globalStore will notify listeners, and // [didChangeDependencies] will run again, this time in the // `store != null` case above. await globalStore.perAccount(widget.accountId); } on AccountNotFoundException { // The account was logged out while its store was loading. // This widget will be showing [placeholder] perpetually, // but that's OK as long as other code will be removing it from the UI // (usually by using [routeToRemoveOnLogout]). } ``` (included because unchanged code is not in the diff) To handle 1, we apply the same expectation that the account gets logged out when the exception happens. For `_handlePollError`, while the store was not replaced at all, the end result of having the old store disposed is still expected. This partly addresses zulip#890 by handling authentication errors for register-queue. Fixes: zulip#737 Signed-off-by: Zixuan James Li <zixuan@zulip.com>
The method loadPerAccount has two call sites, i.e. places where we send register-queue requests: 1. _reloadPerAccount through [UpdateMachine._handlePollError] 2. perAccount through [PerAccountStoreWidget] (the common case) We utilize the existing [AccountNotFoundException] because invalidating invalid auth keys effectively logs out the account, that it can no longer be found in the store. [PerAccountStoreWidget] already expects this error by ignoring it and waiting for the route to be removed: ```dart try { // If this succeeds, globalStore will notify listeners, and // [didChangeDependencies] will run again, this time in the // `store != null` case above. await globalStore.perAccount(widget.accountId); } on AccountNotFoundException { // The account was logged out while its store was loading. // This widget will be showing [placeholder] perpetually, // but that's OK as long as other code will be removing it from the UI // (usually by using [routeToRemoveOnLogout]). } ``` (included because unchanged code is not in the diff) To handle 1, we apply the same expectation that the account gets logged out when the exception happens. For `_handlePollError`, while the store was not replaced at all, the end result of having the old store disposed is still expected. This partly addresses zulip#890 by handling authentication errors for register-queue. Fixes: zulip#737 Signed-off-by: Zixuan James Li <zixuan@zulip.com>
The method loadPerAccount has two call sites, i.e. places where we send register-queue requests: 1. _reloadPerAccount through [UpdateMachine._handlePollError] 2. perAccount through [PerAccountStoreWidget] (the common case) We utilize the existing [AccountNotFoundException] because invalidating invalid auth keys effectively logs out the account, that it can no longer be found in the store. [PerAccountStoreWidget] already expects this error by ignoring it and assuming that the route will be removed: ```dart try { // If this succeeds, globalStore will notify listeners, and // [didChangeDependencies] will run again, this time in the // `store != null` case above. await globalStore.perAccount(widget.accountId); } on AccountNotFoundException { // The account was logged out while its store was loading. // This widget will be showing [placeholder] perpetually, // but that's OK as long as other code will be removing it from the UI // (usually by using [routeToRemoveOnLogout]). } ``` (included because unchanged code is not in the diff) To handle 1, we apply the same expectation that the account gets logged out when the exception happens. For `_handlePollError`, while the event queue was not replaced at all, the end result of having the old store disposed is still expected. This partly addresses zulip#890 by handling authentication errors for register-queue. Fixes: zulip#737 Signed-off-by: Zixuan James Li <zixuan@zulip.com>
The method loadPerAccount has two call sites, i.e. places where we send register-queue requests: 1. _reloadPerAccount through [UpdateMachine._handlePollError] 2. perAccount through [PerAccountStoreWidget] (the common case) We utilize the existing [AccountNotFoundException] because invalidating invalid auth keys effectively logs out the account, that it can no longer be found in the store. [PerAccountStoreWidget] already expects this error by ignoring it and assuming that the route will be removed: ```dart try { // If this succeeds, globalStore will notify listeners, and // [didChangeDependencies] will run again, this time in the // `store != null` case above. await globalStore.perAccount(widget.accountId); } on AccountNotFoundException { // The account was logged out while its store was loading. // This widget will be showing [placeholder] perpetually, // but that's OK as long as other code will be removing it from the UI // (usually by using [routeToRemoveOnLogout]). } ``` (included because unchanged code is not in the diff) To handle 1, we apply the same expectation that the account gets logged out when the exception happens. For `_handlePollError`, while the event queue was not replaced at all, the end result of having the old store disposed is still expected. This partly addresses zulip#890 by handling authentication errors for register-queue. Fixes: zulip#737 Signed-off-by: Zixuan James Li <zixuan@zulip.com>
The method loadPerAccount has two call sites, i.e. places where we send register-queue requests: 1. _reloadPerAccount through [UpdateMachine._handlePollError] 2. perAccount through [PerAccountStoreWidget] (the common case) We utilize the existing [AccountNotFoundException] because invalidating invalid auth keys effectively logs out the account, that it can no longer be found in the store. [PerAccountStoreWidget] already expects this error by ignoring it and assuming that the route will be removed: ```dart try { // If this succeeds, globalStore will notify listeners, and // [didChangeDependencies] will run again, this time in the // `store != null` case above. await globalStore.perAccount(widget.accountId); } on AccountNotFoundException { // The account was logged out while its store was loading. // This widget will be showing [placeholder] perpetually, // but that's OK as long as other code will be removing it from the UI // (usually by using [routeToRemoveOnLogout]). } ``` (included because unchanged code is not in the diff) To handle 1, we apply the same expectation that the account gets logged out when the exception happens. For `_handlePollError`, while the event queue was not replaced at all, the end result of having the old store disposed is still expected. This partly addresses zulip#890 by handling authentication errors for register-queue. Fixes: zulip#737 Signed-off-by: Zixuan James Li <zixuan@zulip.com>
The method loadPerAccount has two call sites, i.e. places where we send register-queue requests: 1. _reloadPerAccount through [UpdateMachine._handlePollError] 2. perAccount through [PerAccountStoreWidget] (the common case) We utilize the existing [AccountNotFoundException] because invalidating invalid auth keys effectively logs out the account, that it can no longer be found in the store. [PerAccountStoreWidget] already expects this error by ignoring it and assuming that the route will be removed: ```dart try { // If this succeeds, globalStore will notify listeners, and // [didChangeDependencies] will run again, this time in the // `store != null` case above. await globalStore.perAccount(widget.accountId); } on AccountNotFoundException { // The account was logged out while its store was loading. // This widget will be showing [placeholder] perpetually, // but that's OK as long as other code will be removing it from the UI // (usually by using [routeToRemoveOnLogout]). } ``` (included because unchanged code is not in the diff) To handle 1, we apply the same expectation that the account gets logged out when the exception happens. For `_handlePollError`, while the event queue was not replaced at all, the end result of having the old store disposed is still expected. This partly addresses zulip#890 by handling authentication errors for register-queue. Fixes: zulip#737 Signed-off-by: Zixuan James Li <zixuan@zulip.com>
The method loadPerAccount has two call sites, i.e. places where we send register-queue requests: 1. _reloadPerAccount through [UpdateMachine._handlePollError] 2. perAccount through [PerAccountStoreWidget] (the common case) We utilize the existing [AccountNotFoundException] because invalidating invalid auth keys effectively logs out the account, that it can no longer be found in the store. [PerAccountStoreWidget] already expects this error by ignoring it and assuming that the route will be removed: ```dart try { // If this succeeds, globalStore will notify listeners, and // [didChangeDependencies] will run again, this time in the // `store != null` case above. await globalStore.perAccount(widget.accountId); } on AccountNotFoundException { // The account was logged out while its store was loading. // This widget will be showing [placeholder] perpetually, // but that's OK as long as other code will be removing it from the UI // (usually by using [routeToRemoveOnLogout]). } ``` (included because unchanged code is not in the diff) To handle 1, we apply the same expectation that the account gets logged out when the exception happens. For `_handlePollError`, while the event queue was not replaced at all, the end result of having the old store disposed is still expected. This partly addresses zulip#890 by handling authentication errors for register-queue. Fixes: zulip#737 Signed-off-by: Zixuan James Li <zixuan@zulip.com>
The method loadPerAccount has two call sites, i.e. places where we send register-queue requests: 1. _reloadPerAccount through [UpdateMachine._handlePollError] 2. perAccount through [PerAccountStoreWidget] (the common case) We utilize the existing [AccountNotFoundException] because invalidating invalid auth keys effectively logs out the account, that it can no longer be found in the store. [PerAccountStoreWidget] already expects this error by ignoring it and assuming that the route will be removed: ```dart try { // If this succeeds, globalStore will notify listeners, and // [didChangeDependencies] will run again, this time in the // `store != null` case above. await globalStore.perAccount(widget.accountId); } on AccountNotFoundException { // The account was logged out while its store was loading. // This widget will be showing [placeholder] perpetually, // but that's OK as long as other code will be removing it from the UI // (usually by using [routeToRemoveOnLogout]). } ``` (included because unchanged code is not in the diff) To handle 1, we apply the same expectation that the account gets logged out when the exception happens. For `_handlePollError`, while the event queue was not replaced at all, the end result of having the old store disposed is still expected. This partly addresses zulip#890 by handling authentication errors for register-queue. Fixes: zulip#737 Signed-off-by: Zixuan James Li <zixuan@zulip.com>
The method loadPerAccount has two call sites, i.e. places where we send register-queue requests: 1. _reloadPerAccount through [UpdateMachine._handlePollError] 2. perAccount through [PerAccountStoreWidget] (the common case) We utilize the existing [AccountNotFoundException] because invalidating invalid auth keys effectively logs out the account, that it can no longer be found in the store. [PerAccountStoreWidget] already expects this error by ignoring it and assuming that the route will be removed: ```dart try { // If this succeeds, globalStore will notify listeners, and // [didChangeDependencies] will run again, this time in the // `store != null` case above. await globalStore.perAccount(widget.accountId); } on AccountNotFoundException { // The account was logged out while its store was loading. // This widget will be showing [placeholder] perpetually, // but that's OK as long as other code will be removing it from the UI // (usually by using [routeToRemoveOnLogout]). } ``` (included because unchanged code is not in the diff) To handle 1, we apply the same expectation that the account gets logged out when the exception happens. For `_handlePollError`, while the event queue was not replaced at all, the end result of having the old store disposed is still expected. This partly addresses zulip#890 by handling authentication errors for register-queue. Fixes: zulip#737 Signed-off-by: Zixuan James Li <zixuan@zulip.com>
The method loadPerAccount has two call sites, i.e. places where we send register-queue requests: 1. _reloadPerAccount through [UpdateMachine._handlePollError] 2. perAccount through [PerAccountStoreWidget] (the common case) We utilize the existing [AccountNotFoundException] because invalidating invalid auth keys effectively logs out the account, that it can no longer be found in the store. [PerAccountStoreWidget] already expects this error by ignoring it and assuming that the route will be removed: ```dart try { // If this succeeds, globalStore will notify listeners, and // [didChangeDependencies] will run again, this time in the // `store != null` case above. await globalStore.perAccount(widget.accountId); } on AccountNotFoundException { // The account was logged out while its store was loading. // This widget will be showing [placeholder] perpetually, // but that's OK as long as other code will be removing it from the UI // (usually by using [routeToRemoveOnLogout]). } ``` (included because unchanged code is not in the diff) To handle 1, we apply the same expectation that the account gets logged out when the exception happens. For `_handlePollError`, while the event queue was not replaced at all, the end result of having the old store disposed is still expected. This partly addresses zulip#890 by handling authentication errors for register-queue. Fixes: zulip#737 Signed-off-by: Zixuan James Li <zixuan@zulip.com>
The method loadPerAccount has two call sites, i.e. places where we send register-queue requests: 1. _reloadPerAccount through [UpdateMachine._handlePollError] (e.g.: expired event queue) 2. perAccount through [PerAccountStoreWidget] (e.g.: loading for the first time) Both sites already expect [AccountNotFoundException] by assuming that the `loadPerAccount` fail is irrecoverable and is handled elsewhere. This partly addresses zulip#890 by handling authentication errors for register-queue. Fixes: zulip#737 Signed-off-by: Zixuan James Li <zixuan@zulip.com>
The method loadPerAccount has two call sites, i.e. places where we send register-queue requests: 1. _reloadPerAccount through [UpdateMachine._handlePollError] (e.g.: expired event queue) 2. perAccount through [PerAccountStoreWidget] (e.g.: loading for the first time) Both sites already expect [AccountNotFoundException] by assuming that the `loadPerAccount` fail is irrecoverable and is handled elsewhere. This partly addresses zulip#890 by handling authentication errors for register-queue. Fixes: zulip#737 Signed-off-by: Zixuan James Li <zixuan@zulip.com>
The method loadPerAccount has two call sites, i.e. places where we send register-queue requests: 1. _reloadPerAccount through [UpdateMachine._handlePollError] (e.g.: expired event queue) 2. perAccount through [PerAccountStoreWidget] (e.g.: loading for the first time) Both sites already expect [AccountNotFoundException] by assuming that the `loadPerAccount` fail is irrecoverable and is handled elsewhere. This partly addresses zulip#890 by handling authentication errors for register-queue. Fixes: zulip#737 Signed-off-by: Zixuan James Li <zixuan@zulip.com>
The method loadPerAccount has two call sites, i.e. places where we send register-queue requests: 1. _reloadPerAccount through [UpdateMachine._handlePollError] (e.g.: expired event queue) 2. perAccount through [PerAccountStoreWidget] (e.g.: loading for the first time) Both sites already expect [AccountNotFoundException] by assuming that the `loadPerAccount` fail is irrecoverable and is handled elsewhere. This partly addresses zulip#890 by handling authentication errors for register-queue. Fixes: zulip#737 Signed-off-by: Zixuan James Li <zixuan@zulip.com>
When the user has logged in, but then their API key changes, the server requests we make will start failing because they're using the old API key.
Currently the main symptom of this is that you open the app, try to use that account, and the spinner keeps going forever as the app just keeps retrying the registerQueue request. The logs look like:
Instead, we should detect that this error is non-transient and won't be resolved by retrying. When that happens:
Don't retry.
Show an error dialog, after popping from the navigation stack any screens that were on the affected account. (This and the previous step are the same as at Disallow connecting to unsupported ancient servers #267 (comment) .)
Forget about the affected account, akin to Support logging out / forgetting about an account #463, so that the user can log in afresh.
(This differs from what we do for unsupported servers Disallow connecting to unsupported ancient servers #267; for an unsupported server the only thing to do is to retry after the server has been upgraded, and logging in again won't help, whereas for this situation the only thing to do is to log in again and retrying without that won't help.)
As a followup, we might introduce some sort of streamlined flow that saves the user from entering the server URL again. But that's out of scope for this PR.
Thanks to @PIG208 for reporting this issue; see chat thread.
The text was updated successfully, but these errors were encountered: