Show detailed register-queue feedback #890
Labels
a-sync
Event queue; retry; local echo; races
beta feedback
Things beta users have specifically asked for
Milestone
Comments
|
A new issue today is a reminder of the malformed-server-data case: |
gnprice
added a commit
to gnprice/zulip-flutter
that referenced
this issue
Nov 25, 2024
For a malformed-response error this is essential because it pinpoints where in the data schema the mismatch occurred between our expectations and the server's behavior. There's more to do here (zulip#890 for getting this to the user in cases like a malformed response, so they can screenshot it when reporting an issue; zulip#1083 for including stack traces when relevant in showing other errors to the user) but this is a start.
chrisbobbe
pushed a commit
that referenced
this issue
Nov 25, 2024
For a malformed-response error this is essential because it pinpoints where in the data schema the mismatch occurred between our expectations and the server's behavior. There's more to do here (#890 for getting this to the user in cases like a malformed response, so they can screenshot it when reporting an issue; #1083 for including stack traces when relevant in showing other errors to the user) but this is a start.
|
One case where register-queue naturally fails is if the user's API key, or email, changed so that the credentials the app had saved are no longer valid. We realized today that that's probably what was happening in a long-running thread where things mysteriously weren't working for a user — in fact, the same one that originally prompted this issue. So we should be sure to handle at least that case in the nearer term. The full-service version of handling it would be #737; but if at a minimum we give the user a message clearly saying the problem is their login isn't working, that would give them the information they need to go delete the account from the list and log in again. |
PIG208
added a commit
to PIG208/zulip-flutter
that referenced
this issue
Feb 8, 2025
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 <[email protected]>
PIG208
added a commit
to PIG208/zulip-flutter
that referenced
this issue
Feb 8, 2025
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 <[email protected]>
PIG208
added a commit
to PIG208/zulip-flutter
that referenced
this issue
Feb 8, 2025
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 <[email protected]>
PIG208
added a commit
to PIG208/zulip-flutter
that referenced
this issue
Feb 8, 2025
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 <[email protected]>
PIG208
added a commit
to PIG208/zulip-flutter
that referenced
this issue
Feb 11, 2025
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 <[email protected]>
PIG208
added a commit
to PIG208/zulip-flutter
that referenced
this issue
Feb 11, 2025
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 <[email protected]>
PIG208
added a commit
to PIG208/zulip-flutter
that referenced
this issue
Feb 11, 2025
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 <[email protected]>
PIG208
added a commit
to PIG208/zulip-flutter
that referenced
this issue
Feb 12, 2025
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 <[email protected]>
PIG208
added a commit
to PIG208/zulip-flutter
that referenced
this issue
Feb 12, 2025
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 <[email protected]>
PIG208
added a commit
to PIG208/zulip-flutter
that referenced
this issue
Feb 12, 2025
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 <[email protected]>
PIG208
added a commit
to PIG208/zulip-flutter
that referenced
this issue
Feb 12, 2025
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 <[email protected]>
PIG208
added a commit
to PIG208/zulip-flutter
that referenced
this issue
Feb 12, 2025
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 <[email protected]>
PIG208
added a commit
to PIG208/zulip-flutter
that referenced
this issue
Feb 14, 2025
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 <[email protected]>
PIG208
added a commit
to PIG208/zulip-flutter
that referenced
this issue
Feb 14, 2025
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 <[email protected]>
PIG208
added a commit
to PIG208/zulip-flutter
that referenced
this issue
Feb 14, 2025
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 <[email protected]>
PIG208
added a commit
to PIG208/zulip-flutter
that referenced
this issue
Feb 15, 2025
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 <[email protected]>
PIG208
added a commit
to PIG208/zulip-flutter
that referenced
this issue
Feb 18, 2025
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 <[email protected]>
PIG208
added a commit
to PIG208/zulip-flutter
that referenced
this issue
Feb 18, 2025
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 <[email protected]>
PIG208
added a commit
to PIG208/zulip-flutter
that referenced
this issue
Feb 18, 2025
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 <[email protected]>
PIG208
added a commit
to PIG208/zulip-flutter
that referenced
this issue
Feb 18, 2025
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 <[email protected]>
PIG208
added a commit
to PIG208/zulip-flutter
that referenced
this issue
Feb 19, 2025
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 <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
This is like #555 but for the step where we call
registerQueue, rather thangetEvents.On the implementation, some details in chat thread:
https://chat.zulip.org/#narrow/stream/48-mobile/topic/0.2E0.2E19.20Flutter.20.3A.20Cant.20connect.20to.20self.20hosted.20instance/near/1923246
This will let us distinguish between:
and will give the user details they can send us in a screenshot. Particularly in the case of malformed server data, we'll want the stack trace so we can debug.
Related issues:
The text was updated successfully, but these errors were encountered: