Skip to content

Commit 2859759

Browse files
PIG208PIG208
committed
store: Handle invalid API key on register-queue
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 #890 by handling authentication errors for register-queue. Fixes: #737 Signed-off-by: Zixuan James Li <[email protected]>
1 parent d4d4d49 commit 2859759

13 files changed

+214
-8
lines changed

‎assets/l10n/app_en.arb‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -515,6 +515,13 @@
515515
"@topicValidationErrorMandatoryButEmpty": {
516516
"description": "Topic validation error when topic is required but was empty."
517517
},
518+
"errorInvalidApiKeyMessage": "Your account at {url} could not be authenticated. Please try logging in again or use another account.",
519+
"@errorInvalidApiKeyMessage": {
520+
"description": "Error message in the dialog for invalid API key.",
521+
"placeholders": {
522+
"url": {"type": "String", "example": "http://chat.example.com/"}
523+
}
524+
},
518525
"errorInvalidResponse": "The server sent an invalid response",
519526
"@errorInvalidResponse": {
520527
"description": "Error message when an API call returned an invalid response."

‎lib/generated/l10n/zulip_localizations.dart‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -795,6 +795,12 @@ abstract class ZulipLocalizations {
795795
/// **'Topics are required in this organization.'**
796796
String get topicValidationErrorMandatoryButEmpty;
797797

798+
/// Error message in the dialog for invalid API key.
799+
///
800+
/// In en, this message translates to:
801+
/// **'Your account at {url} could not be authenticated. Please try logging in again or use another account.'**
802+
String errorInvalidApiKeyMessage(String url);
803+
798804
/// Error message when an API call returned an invalid response.
799805
///
800806
/// In en, this message translates to:

‎lib/generated/l10n/zulip_localizations_ar.dart‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -399,6 +399,11 @@ class ZulipLocalizationsAr extends ZulipLocalizations {
399399
@override
400400
String get topicValidationErrorMandatoryButEmpty => 'Topics are required in this organization.';
401401

402+
@override
403+
String errorInvalidApiKeyMessage(String url) {
404+
return 'Your account at $url could not be authenticated. Please try logging in again or use another account.';
405+
}
406+
402407
@override
403408
String get errorInvalidResponse => 'The server sent an invalid response';
404409

‎lib/generated/l10n/zulip_localizations_en.dart‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -399,6 +399,11 @@ class ZulipLocalizationsEn extends ZulipLocalizations {
399399
@override
400400
String get topicValidationErrorMandatoryButEmpty => 'Topics are required in this organization.';
401401

402+
@override
403+
String errorInvalidApiKeyMessage(String url) {
404+
return 'Your account at $url could not be authenticated. Please try logging in again or use another account.';
405+
}
406+
402407
@override
403408
String get errorInvalidResponse => 'The server sent an invalid response';
404409

‎lib/generated/l10n/zulip_localizations_ja.dart‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -399,6 +399,11 @@ class ZulipLocalizationsJa extends ZulipLocalizations {
399399
@override
400400
String get topicValidationErrorMandatoryButEmpty => 'Topics are required in this organization.';
401401

402+
@override
403+
String errorInvalidApiKeyMessage(String url) {
404+
return 'Your account at $url could not be authenticated. Please try logging in again or use another account.';
405+
}
406+
402407
@override
403408
String get errorInvalidResponse => 'The server sent an invalid response';
404409

‎lib/generated/l10n/zulip_localizations_nb.dart‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -399,6 +399,11 @@ class ZulipLocalizationsNb extends ZulipLocalizations {
399399
@override
400400
String get topicValidationErrorMandatoryButEmpty => 'Topics are required in this organization.';
401401

402+
@override
403+
String errorInvalidApiKeyMessage(String url) {
404+
return 'Your account at $url could not be authenticated. Please try logging in again or use another account.';
405+
}
406+
402407
@override
403408
String get errorInvalidResponse => 'The server sent an invalid response';
404409

‎lib/generated/l10n/zulip_localizations_pl.dart‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -399,6 +399,11 @@ class ZulipLocalizationsPl extends ZulipLocalizations {
399399
@override
400400
String get topicValidationErrorMandatoryButEmpty => 'Wątki są wymagane przez tę organizację.';
401401

402+
@override
403+
String errorInvalidApiKeyMessage(String url) {
404+
return 'Your account at $url could not be authenticated. Please try logging in again or use another account.';
405+
}
406+
402407
@override
403408
String get errorInvalidResponse => 'Nieprawidłowa odpowiedź serwera';
404409

‎lib/generated/l10n/zulip_localizations_ru.dart‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -399,6 +399,11 @@ class ZulipLocalizationsRu extends ZulipLocalizations {
399399
@override
400400
String get topicValidationErrorMandatoryButEmpty => 'Темы обязательны в этой организации.';
401401

402+
@override
403+
String errorInvalidApiKeyMessage(String url) {
404+
return 'Your account at $url could not be authenticated. Please try logging in again or use another account.';
405+
}
406+
402407
@override
403408
String get errorInvalidResponse => 'Получен недопустимый ответ сервера';
404409

‎lib/generated/l10n/zulip_localizations_sk.dart‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -399,6 +399,11 @@ class ZulipLocalizationsSk extends ZulipLocalizations {
399399
@override
400400
String get topicValidationErrorMandatoryButEmpty => 'Topics are required in this organization.';
401401

402+
@override
403+
String errorInvalidApiKeyMessage(String url) {
404+
return 'Your account at $url could not be authenticated. Please try logging in again or use another account.';
405+
}
406+
402407
@override
403408
String get errorInvalidResponse => 'Server poslal nesprávnu odpoveď';
404409

‎lib/model/store.dart‎

Lines changed: 47 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import '../api/backoff.dart';
1919
import '../api/route/realm.dart';
2020
import '../log.dart';
2121
import '../notifications/receive.dart';
22+
import 'actions.dart';
2223
import 'autocomplete.dart';
2324
import 'database.dart';
2425
import 'emoji.dart';
@@ -149,7 +150,29 @@ abstract class GlobalStore extends ChangeNotifier {
149150
/// and/or [perAccountSync].
150151
Future<PerAccountStore> loadPerAccount(int accountId) async {
151152
assert(_accounts.containsKey(accountId));
152-
final store = await doLoadPerAccount(accountId);
153+
final realmUrl = getAccount(accountId)!.realmUrl;
154+
final PerAccountStore store;
155+
try {
156+
store = await doLoadPerAccount(accountId);
157+
} catch (e) {
158+
switch (e) {
159+
case HttpException(httpStatus: 401):
160+
// The API key is invalid and the store can never be loaded
161+
// unless the user retries manually.
162+
final zulipLocalizations = GlobalLocalizations.zulipLocalizations;
163+
reportErrorToUserModally(
164+
zulipLocalizations.errorCouldNotConnectTitle,
165+
details: zulipLocalizations.errorInvalidApiKeyMessage(
166+
realmUrl.toString()));
167+
// The account could be logged out from the choose-account page.
168+
if (_accounts.containsKey(accountId)) {
169+
await logOutAccount(this, accountId);
170+
}
171+
throw AccountNotFoundException();
172+
default:
173+
rethrow;
174+
}
175+
}
153176
if (!_accounts.containsKey(accountId)) {
154177
// [removeAccount] was called during [doLoadPerAccount].
155178
store.dispose();
@@ -912,12 +935,19 @@ class UpdateMachine {
912935
try {
913936
return await registerQueue(connection);
914937
} catch (e, s) {
915-
assert(debugLog('Error fetching initial snapshot: $e'));
916-
// Print stack trace in its own log entry; log entries are truncated
917-
// at 1 kiB (at least on Android), and stack can be longer than that.
918-
assert(debugLog('Stack:\n$s'));
919-
assert(debugLog('Backing off, then will retry…'));
920938
// TODO(#890): tell user if initial-fetch errors persist, or look non-transient
939+
switch (e) {
940+
case HttpException(httpStatus: 401):
941+
// We cannot recover from this error through retrying.
942+
// Leave it to [GlobalStore.loadPerAccount].
943+
rethrow;
944+
default:
945+
assert(debugLog('Error fetching initial snapshot: $e'));
946+
// Print stack trace in its own log entry; log entries are truncated
947+
// at 1 kiB (at least on Android), and stack can be longer than that.
948+
assert(debugLog('Stack:\n$s'));
949+
}
950+
assert(debugLog('Backing off, then will retry…'));
921951
await (backoffMachine ??= BackoffMachine()).wait();
922952
assert(debugLog('… Backoff wait complete, retrying initial fetch.'));
923953
}
@@ -1218,8 +1248,17 @@ class UpdateMachine {
12181248
if (_disposed) return;
12191249
}
12201250

1221-
await store._globalStore._reloadPerAccount(store.accountId);
1222-
assert(_disposed);
1251+
try {
1252+
await store._globalStore._reloadPerAccount(store.accountId);
1253+
} on AccountNotFoundException {
1254+
// The account was logged out while we retry to replace the store,
1255+
// but that's OK as long as other code will be removing it from the UI
1256+
// (usually by using [routeToRemoveOnLogout]).
1257+
assert(debugLog('… Event queue not replaced; account logged out.'));
1258+
return;
1259+
} finally {
1260+
assert(_disposed);
1261+
}
12231262
assert(debugLog('… Event queue replaced.'));
12241263
}
12251264

0 commit comments

Comments
 (0)