Skip to content

Commit 501aaa6

Browse files
committed
store: Handle invalid API key on register-queue
Fixes: zulip#737 Signed-off-by: Zixuan James Li <[email protected]>
1 parent 90e4159 commit 501aaa6

14 files changed

+338
-7
lines changed

assets/l10n/app_en.arb

+7
Original file line numberDiff line numberDiff line change
@@ -458,6 +458,13 @@
458458
"@topicValidationErrorMandatoryButEmpty": {
459459
"description": "Topic validation error when topic is required but was empty."
460460
},
461+
"errorInvalidApiKeyMessage": "Your account at {url} could not be authenticated. Please try logging in again or use another account.",
462+
"@errorInvalidApiKeyMessage": {
463+
"description": "Error message in the dialog for invalid API key.",
464+
"placeholders": {
465+
"url": {"type": "String", "example": "http://chat.example.com/"}
466+
}
467+
},
461468
"errorInvalidResponse": "The server sent an invalid response",
462469
"@errorInvalidResponse": {
463470
"description": "Error message when an API call returned an invalid response."

lib/generated/l10n/zulip_localizations.dart

+6
Original file line numberDiff line numberDiff line change
@@ -721,6 +721,12 @@ abstract class ZulipLocalizations {
721721
/// **'Topics are required in this organization.'**
722722
String get topicValidationErrorMandatoryButEmpty;
723723

724+
/// Error message in the dialog for invalid API key.
725+
///
726+
/// In en, this message translates to:
727+
/// **'Your account at {url} could not be authenticated. Please try logging in again or use another account.'**
728+
String errorInvalidApiKeyMessage(String url);
729+
724730
/// Error message when an API call returned an invalid response.
725731
///
726732
/// In en, this message translates to:

lib/generated/l10n/zulip_localizations_ar.dart

+5
Original file line numberDiff line numberDiff line change
@@ -357,6 +357,11 @@ class ZulipLocalizationsAr extends ZulipLocalizations {
357357
@override
358358
String get topicValidationErrorMandatoryButEmpty => 'Topics are required in this organization.';
359359

360+
@override
361+
String errorInvalidApiKeyMessage(String url) {
362+
return 'Your account at $url could not be authenticated. Please try logging in again or use another account.';
363+
}
364+
360365
@override
361366
String get errorInvalidResponse => 'The server sent an invalid response';
362367

lib/generated/l10n/zulip_localizations_en.dart

+5
Original file line numberDiff line numberDiff line change
@@ -357,6 +357,11 @@ class ZulipLocalizationsEn extends ZulipLocalizations {
357357
@override
358358
String get topicValidationErrorMandatoryButEmpty => 'Topics are required in this organization.';
359359

360+
@override
361+
String errorInvalidApiKeyMessage(String url) {
362+
return 'Your account at $url could not be authenticated. Please try logging in again or use another account.';
363+
}
364+
360365
@override
361366
String get errorInvalidResponse => 'The server sent an invalid response';
362367

lib/generated/l10n/zulip_localizations_fr.dart

+5
Original file line numberDiff line numberDiff line change
@@ -357,6 +357,11 @@ class ZulipLocalizationsFr extends ZulipLocalizations {
357357
@override
358358
String get topicValidationErrorMandatoryButEmpty => 'Topics are required in this organization.';
359359

360+
@override
361+
String errorInvalidApiKeyMessage(String url) {
362+
return 'Your account at $url could not be authenticated. Please try logging in again or use another account.';
363+
}
364+
360365
@override
361366
String get errorInvalidResponse => 'The server sent an invalid response';
362367

lib/generated/l10n/zulip_localizations_ja.dart

+5
Original file line numberDiff line numberDiff line change
@@ -357,6 +357,11 @@ class ZulipLocalizationsJa extends ZulipLocalizations {
357357
@override
358358
String get topicValidationErrorMandatoryButEmpty => 'Topics are required in this organization.';
359359

360+
@override
361+
String errorInvalidApiKeyMessage(String url) {
362+
return 'Your account at $url could not be authenticated. Please try logging in again or use another account.';
363+
}
364+
360365
@override
361366
String get errorInvalidResponse => 'The server sent an invalid response';
362367

lib/generated/l10n/zulip_localizations_pl.dart

+5
Original file line numberDiff line numberDiff line change
@@ -357,6 +357,11 @@ class ZulipLocalizationsPl extends ZulipLocalizations {
357357
@override
358358
String get topicValidationErrorMandatoryButEmpty => 'Wątki są wymagane przez tę organizację.';
359359

360+
@override
361+
String errorInvalidApiKeyMessage(String url) {
362+
return 'Your account at $url could not be authenticated. Please try logging in again or use another account.';
363+
}
364+
360365
@override
361366
String get errorInvalidResponse => 'Nieprawidłowa odpowiedź serwera';
362367

lib/generated/l10n/zulip_localizations_ru.dart

+5
Original file line numberDiff line numberDiff line change
@@ -357,6 +357,11 @@ class ZulipLocalizationsRu extends ZulipLocalizations {
357357
@override
358358
String get topicValidationErrorMandatoryButEmpty => 'Topics are required in this organization.';
359359

360+
@override
361+
String errorInvalidApiKeyMessage(String url) {
362+
return 'Your account at $url could not be authenticated. Please try logging in again or use another account.';
363+
}
364+
360365
@override
361366
String get errorInvalidResponse => 'The server sent an invalid response';
362367

lib/model/store.dart

+37-6
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 '../widgets/actions.dart';
2223
import 'autocomplete.dart';
2324
import 'database.dart';
2425
import 'emoji.dart';
@@ -149,13 +150,31 @@ 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+
PerAccountStore? store;
154+
try {
155+
store = await doLoadPerAccount(accountId);
156+
} catch (e) {
157+
switch (e) {
158+
case ZulipApiException(code: 'INVALID_API_KEY'):
159+
// The API key is invalid and the store can never be loaded
160+
// unless the user retries manually.
161+
final zulipLocalizations = GlobalLocalizations.zulipLocalizations;
162+
final account = getAccount(accountId)!;
163+
reportErrorToUserModally(
164+
zulipLocalizations.errorCouldNotConnectTitle,
165+
details: zulipLocalizations.errorInvalidApiKeyMessage(
166+
account.realmUrl.toString()));
167+
await logOutAccount(this, accountId);
168+
default:
169+
rethrow;
170+
}
171+
}
153172
if (!_accounts.containsKey(accountId)) {
154-
// [removeAccount] was called during [doLoadPerAccount].
155-
store.dispose();
173+
// [removeAccount] was called during or after [doLoadPerAccount].
174+
store?.dispose();
156175
throw AccountNotFoundException();
157176
}
158-
return store;
177+
return store!;
159178
}
160179

161180
/// Load per-account data for the given account, unconditionally.
@@ -912,7 +931,13 @@ class UpdateMachine {
912931
// at 1 kiB (at least on Android), and stack can be longer than that.
913932
assert(debugLog('Stack:\n$s'));
914933
assert(debugLog('Backing off, then will retry…'));
915-
// TODO tell user if initial-fetch errors persist, or look non-transient
934+
// TODO(#890): tell user if initial-fetch errors persist, or look non-transient
935+
switch (e) {
936+
case ZulipApiException(code: 'INVALID_API_KEY'):
937+
// We cannot recover from this error through retrying.
938+
// Leave it to [GlobalStore.loadPerAccount].
939+
rethrow;
940+
}
916941
await (backoffMachine ??= BackoffMachine()).wait();
917942
assert(debugLog('… Backoff wait complete, retrying initial fetch.'));
918943
}
@@ -1044,7 +1069,13 @@ class UpdateMachine {
10441069
}
10451070
} catch (e) {
10461071
if (_disposed) return;
1047-
await _handlePollError(e);
1072+
try {
1073+
await _handlePollError(e);
1074+
} on AccountNotFoundException {
1075+
// Cannot recover by replacing the store because the account
1076+
// was logged out.
1077+
return;
1078+
}
10481079
assert(_disposed);
10491080
return;
10501081
}

lib/widgets/app.dart

+36-1
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,11 @@ class _ZulipAppState extends State<ZulipApp> with WidgetsBindingObserver {
189189
final themeData = zulipThemeData(context);
190190
return GlobalStoreWidget(
191191
child: Builder(builder: (context) {
192+
final effectiveNavigatorObservers = [
193+
_EmptyStackNavigatorObserver(),
194+
if (widget.navigatorObservers != null)
195+
...widget.navigatorObservers!,
196+
];
192197
final globalStore = GlobalStoreWidget.of(context);
193198
// TODO(#524) choose initial account as last one used
194199
final initialAccountId = globalStore.accounts.firstOrNull?.id;
@@ -199,7 +204,7 @@ class _ZulipAppState extends State<ZulipApp> with WidgetsBindingObserver {
199204
theme: themeData,
200205

201206
navigatorKey: ZulipApp.navigatorKey,
202-
navigatorObservers: widget.navigatorObservers ?? const [],
207+
navigatorObservers: effectiveNavigatorObservers,
203208
builder: (BuildContext context, Widget? child) {
204209
if (!ZulipApp.ready.value) {
205210
SchedulerBinding.instance.addPostFrameCallback(
@@ -230,6 +235,36 @@ class _ZulipAppState extends State<ZulipApp> with WidgetsBindingObserver {
230235
}
231236
}
232237

238+
class _EmptyStackNavigatorObserver extends NavigatorObserver {
239+
void _pushIfEmpty() async {
240+
final navigator = await ZulipApp.navigator;
241+
bool isEmptyStack = true;
242+
// TODO: find a better way to inspect the navigator stack
243+
navigator.popUntil((route) {
244+
isEmptyStack = false;
245+
return true; // never actually pops
246+
});
247+
if (isEmptyStack) {
248+
unawaited(navigator.push(
249+
MaterialWidgetRoute(page: const ChooseAccountPage())));
250+
}
251+
}
252+
253+
@override
254+
void didRemove(Route<void> route, Route<void>? previousRoute) async {
255+
if (previousRoute == null) {
256+
_pushIfEmpty();
257+
}
258+
}
259+
260+
@override
261+
void didPop(Route<void> route, Route<void>? previousRoute) async {
262+
if (previousRoute == null) {
263+
_pushIfEmpty();
264+
}
265+
}
266+
}
267+
233268
class ChooseAccountPage extends StatelessWidget {
234269
const ChooseAccountPage({super.key});
235270

test/model/store_test.dart

+34
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import 'package:flutter/foundation.dart';
77
import 'package:http/http.dart' as http;
88
import 'package:test/scaffolding.dart';
99
import 'package:zulip/api/core.dart';
10+
import 'package:zulip/api/exception.dart';
1011
import 'package:zulip/api/model/events.dart';
1112
import 'package:zulip/api/model/model.dart';
1213
import 'package:zulip/api/route/events.dart';
@@ -500,6 +501,20 @@ void main() {
500501
users.map((expected) => (it) => it.fullName.equals(expected.fullName)));
501502
}));
502503

504+
test('GlobalStore.perAccount on INVALID_API_KEY', () => awaitFakeAsync((async) async {
505+
addTearDown(testBinding.reset);
506+
507+
await testBinding.globalStore.insertAccount(eg.selfAccount.toCompanion(false));
508+
testBinding.globalStore.loadPerAccountException = ZulipApiException(
509+
routeName: '/register', code: 'INVALID_API_KEY', httpStatus: 400,
510+
data: {}, message: '');
511+
await check(testBinding.globalStore.perAccount(eg.selfAccount.id))
512+
.throws<AccountNotFoundException>();
513+
514+
check(testBinding.globalStore.takeDoRemoveAccountCalls())
515+
.single.equals(eg.selfAccount.id);
516+
}));
517+
503518
// TODO test UpdateMachine.load starts polling loop
504519
// TODO test UpdateMachine.load calls registerNotificationToken
505520
});
@@ -843,6 +858,25 @@ void main() {
843858
check(store.debugMessageListViews).isEmpty();
844859
}));
845860

861+
test('log out if fail to reload on unexpected errors', () => awaitFakeAsync((async) async {
862+
await preparePoll();
863+
864+
prepareUnexpectedLoopError();
865+
updateMachine.debugAdvanceLoop();
866+
async.elapse(Duration.zero);
867+
check(store).isLoading.isTrue();
868+
869+
globalStore.loadPerAccountException = ZulipApiException(
870+
routeName: '/register', code: 'INVALID_API_KEY', httpStatus: 400,
871+
data: {}, message: '');
872+
// The reload doesn't happen immediately; there's a timer.
873+
check(async.pendingTimers).length.equals(1);
874+
async.flushTimers();
875+
876+
check(globalStore.takeDoRemoveAccountCalls().single)
877+
.equals(eg.selfAccount.id);
878+
}));
879+
846880
group('report error', () {
847881
String? lastReportedError;
848882
String? takeLastReportedError() {

test/model/test_store.dart

+4
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,7 @@ class TestGlobalStore extends GlobalStore {
126126

127127
static const Duration removeAccountDuration = Duration(milliseconds: 1);
128128
Duration? loadPerAccountDuration;
129+
Object? loadPerAccountException;
129130

130131
/// Consume the log of calls made to [doRemoveAccount].
131132
List<int> takeDoRemoveAccountCalls() {
@@ -147,6 +148,9 @@ class TestGlobalStore extends GlobalStore {
147148
if (loadPerAccountDuration != null) {
148149
await Future<void>.delayed(loadPerAccountDuration!);
149150
}
151+
if (loadPerAccountException != null) {
152+
throw loadPerAccountException!;
153+
}
150154
final initialSnapshot = _initialSnapshots[accountId]!;
151155
final store = PerAccountStore.fromInitialSnapshot(
152156
globalStore: this,

0 commit comments

Comments
 (0)