Skip to content

Commit 7cd25f2

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] (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 #890 by handling authentication errors for register-queue. Fixes: #737 Signed-off-by: Zixuan James Li <[email protected]>
1 parent 9d69074 commit 7cd25f2

13 files changed

+207
-7
lines changed

‎assets/l10n/app_en.arb‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -523,6 +523,13 @@
523523
"@topicValidationErrorMandatoryButEmpty": {
524524
"description": "Topic validation error when topic is required but was empty."
525525
},
526+
"errorInvalidApiKeyMessage": "Your account at {url} could not be authenticated. Please try logging in again or use another account.",
527+
"@errorInvalidApiKeyMessage": {
528+
"description": "Error message in the dialog for invalid API key.",
529+
"placeholders": {
530+
"url": {"type": "String", "example": "http://chat.example.com/"}
531+
}
532+
},
526533
"errorInvalidResponse": "The server sent an invalid response",
527534
"@errorInvalidResponse": {
528535
"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
@@ -801,6 +801,12 @@ abstract class ZulipLocalizations {
801801
/// **'Topics are required in this organization.'**
802802
String get topicValidationErrorMandatoryButEmpty;
803803

804+
/// Error message in the dialog for invalid API key.
805+
///
806+
/// In en, this message translates to:
807+
/// **'Your account at {url} could not be authenticated. Please try logging in again or use another account.'**
808+
String errorInvalidApiKeyMessage(String url);
809+
804810
/// Error message when an API call returned an invalid response.
805811
///
806812
/// 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
@@ -404,6 +404,11 @@ class ZulipLocalizationsAr extends ZulipLocalizations {
404404
@override
405405
String get topicValidationErrorMandatoryButEmpty => 'Topics are required in this organization.';
406406

407+
@override
408+
String errorInvalidApiKeyMessage(String url) {
409+
return 'Your account at $url could not be authenticated. Please try logging in again or use another account.';
410+
}
411+
407412
@override
408413
String get errorInvalidResponse => 'The server sent an invalid response';
409414

‎lib/generated/l10n/zulip_localizations_en.dart‎

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

407+
@override
408+
String errorInvalidApiKeyMessage(String url) {
409+
return 'Your account at $url could not be authenticated. Please try logging in again or use another account.';
410+
}
411+
407412
@override
408413
String get errorInvalidResponse => 'The server sent an invalid response';
409414

‎lib/generated/l10n/zulip_localizations_ja.dart‎

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

407+
@override
408+
String errorInvalidApiKeyMessage(String url) {
409+
return 'Your account at $url could not be authenticated. Please try logging in again or use another account.';
410+
}
411+
407412
@override
408413
String get errorInvalidResponse => 'The server sent an invalid response';
409414

‎lib/generated/l10n/zulip_localizations_nb.dart‎

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

407+
@override
408+
String errorInvalidApiKeyMessage(String url) {
409+
return 'Your account at $url could not be authenticated. Please try logging in again or use another account.';
410+
}
411+
407412
@override
408413
String get errorInvalidResponse => 'The server sent an invalid response';
409414

‎lib/generated/l10n/zulip_localizations_pl.dart‎

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

407+
@override
408+
String errorInvalidApiKeyMessage(String url) {
409+
return 'Your account at $url could not be authenticated. Please try logging in again or use another account.';
410+
}
411+
407412
@override
408413
String get errorInvalidResponse => 'Nieprawidłowa odpowiedź serwera';
409414

‎lib/generated/l10n/zulip_localizations_ru.dart‎

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

407+
@override
408+
String errorInvalidApiKeyMessage(String url) {
409+
return 'Your account at $url could not be authenticated. Please try logging in again or use another account.';
410+
}
411+
407412
@override
408413
String get errorInvalidResponse => 'Получен недопустимый ответ сервера';
409414

‎lib/generated/l10n/zulip_localizations_sk.dart‎

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

407+
@override
408+
String errorInvalidApiKeyMessage(String url) {
409+
return 'Your account at $url could not be authenticated. Please try logging in again or use another account.';
410+
}
411+
407412
@override
408413
String get errorInvalidResponse => 'Server poslal nesprávnu odpoveď';
409414

‎lib/model/store.dart‎

Lines changed: 41 additions & 6 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,34 @@ 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 PerAccountStore store;
154+
try {
155+
store = await doLoadPerAccount(accountId);
156+
} catch (e) {
157+
switch (e) {
158+
case HttpException(httpStatus: 401):
159+
// The API key is invalid and the store can never be loaded
160+
// unless the user retries manually.
161+
final account = getAccount(accountId);
162+
if (account == null) {
163+
// The account was logged out during `await doLoadPerAccount`.
164+
// Here, that seems possible only by the user's own action;
165+
// the logout can't have been done programmatically.
166+
// Even if it were, it would have come with its own UI feedback.
167+
// Anyway, skip showing feedback, to not be confusing or repetitive.
168+
throw AccountNotFoundException();
169+
}
170+
final zulipLocalizations = GlobalLocalizations.zulipLocalizations;
171+
reportErrorToUserModally(
172+
zulipLocalizations.errorCouldNotConnectTitle,
173+
message: zulipLocalizations.errorInvalidApiKeyMessage(
174+
account.realmUrl.toString()));
175+
await logOutAccount(this, accountId);
176+
throw AccountNotFoundException();
177+
default:
178+
rethrow;
179+
}
180+
}
153181
if (!_accounts.containsKey(accountId)) {
154182
// TODO(#1354): handle this earlier
155183
// [removeAccount] was called during [doLoadPerAccount].
@@ -914,12 +942,19 @@ class UpdateMachine {
914942
try {
915943
return await registerQueue(connection);
916944
} catch (e, s) {
917-
assert(debugLog('Error fetching initial snapshot: $e'));
918-
// Print stack trace in its own log entry; log entries are truncated
919-
// at 1 kiB (at least on Android), and stack can be longer than that.
920-
assert(debugLog('Stack:\n$s'));
921-
assert(debugLog('Backing off, then will retry…'));
922945
// TODO(#890): tell user if initial-fetch errors persist, or look non-transient
946+
switch (e) {
947+
case HttpException(httpStatus: 401):
948+
// We cannot recover from this error through retrying.
949+
// Leave it to [GlobalStore.loadPerAccount].
950+
rethrow;
951+
default:
952+
assert(debugLog('Error fetching initial snapshot: $e'));
953+
// Print stack trace in its own log entry; log entries are truncated
954+
// at 1 kiB (at least on Android), and stack can be longer than that.
955+
assert(debugLog('Stack:\n$s'));
956+
}
957+
assert(debugLog('Backing off, then will retry…'));
923958
await (backoffMachine ??= BackoffMachine()).wait();
924959
assert(debugLog('… Backoff wait complete, retrying initial fetch.'));
925960
}

0 commit comments

Comments
 (0)