Skip to content

Commit b5e0fda

Browse files
committed
store: Expect AccountNotFoundException when reloading store
This can happen when `removeAccount` was called during `doLoadPerAccount`, possibly due to the user logging out from choose-account page while the store is getting reloaded. However, it is currently not reachable live because of a bug in `UpdateMachine.load`. See #1354. Modulo the bug, `UpdateMachine` can safely ignore this error acknowledging that the reload has failed, given that the user can continue using the app by navigating from choose-account page. Signed-off-by: Zixuan James Li <[email protected]>
1 parent 33e2613 commit b5e0fda

File tree

4 files changed

+81
-4
lines changed

4 files changed

+81
-4
lines changed

lib/model/store.dart

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1220,8 +1220,14 @@ class UpdateMachine {
12201220
if (_disposed) return;
12211221
}
12221222

1223-
await store._globalStore._reloadPerAccount(store.accountId);
1224-
assert(_disposed);
1223+
try {
1224+
await store._globalStore._reloadPerAccount(store.accountId);
1225+
} on AccountNotFoundException {
1226+
assert(debugLog('… Event queue not replaced; account was logged out.'));
1227+
return;
1228+
} finally {
1229+
assert(_disposed);
1230+
}
12251231
assert(debugLog('… Event queue replaced.'));
12261232
}
12271233

test/example_data.dart

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -879,6 +879,7 @@ ChannelUpdateEvent channelUpdateEvent(
879879
TestGlobalStore globalStore({List<Account> accounts = const []}) {
880880
return TestGlobalStore(accounts: accounts);
881881
}
882+
const _globalStore = globalStore;
882883

883884
InitialSnapshot initialSnapshot({
884885
String? queueId,
@@ -949,10 +950,14 @@ InitialSnapshot initialSnapshot({
949950
}
950951
const _initialSnapshot = initialSnapshot;
951952

952-
PerAccountStore store({Account? account, InitialSnapshot? initialSnapshot}) {
953+
PerAccountStore store({
954+
GlobalStore? globalStore,
955+
Account? account,
956+
InitialSnapshot? initialSnapshot,
957+
}) {
953958
final effectiveAccount = account ?? selfAccount;
954959
return PerAccountStore.fromInitialSnapshot(
955-
globalStore: globalStore(accounts: [effectiveAccount]),
960+
globalStore: globalStore ?? _globalStore(accounts: [effectiveAccount]),
956961
accountId: effectiveAccount.id,
957962
initialSnapshot: initialSnapshot ?? _initialSnapshot(),
958963
);

test/model/store_test.dart

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import 'package:zulip/api/route/events.dart';
1313
import 'package:zulip/api/route/messages.dart';
1414
import 'package:zulip/api/route/realm.dart';
1515
import 'package:zulip/log.dart';
16+
import 'package:zulip/model/actions.dart';
1617
import 'package:zulip/model/store.dart';
1718
import 'package:zulip/notifications/receive.dart';
1819

@@ -966,6 +967,66 @@ void main() {
966967
));
967968
});
968969
});
970+
971+
group('reload failure', () {
972+
late LoadingTestGlobalStore globalStore;
973+
974+
List<Completer<PerAccountStore>> completers() =>
975+
globalStore.completers[eg.selfAccount.id]!;
976+
977+
Future<void> prepareReload(FakeAsync async) async {
978+
globalStore = LoadingTestGlobalStore(accounts: [eg.selfAccount]);
979+
final future = globalStore.perAccount(eg.selfAccount.id);
980+
completers()
981+
..single.complete(
982+
eg.store(globalStore: globalStore, account: eg.selfAccount))
983+
..clear();
984+
final store = await future;
985+
final updateMachine = globalStore.updateMachines[eg.selfAccount.id] =
986+
UpdateMachine.fromInitialSnapshot(
987+
store: store, initialSnapshot: eg.initialSnapshot());
988+
updateMachine.debugPauseLoop();
989+
updateMachine.poll();
990+
991+
(store.connection as FakeApiConnection).prepare(
992+
apiException: eg.apiExceptionBadEventQueueId());
993+
updateMachine.debugAdvanceLoop();
994+
async.elapse(Duration.zero);
995+
check(store).isLoading.isTrue();
996+
}
997+
998+
void checkReloadFailure({
999+
required Future<void> Function() completeLoading,
1000+
}) {
1001+
awaitFakeAsync((async) async {
1002+
await prepareReload(async);
1003+
1004+
check(completers()).single.isCompleted.isFalse();
1005+
await completeLoading();
1006+
check(completers()).single.isCompleted.isTrue();
1007+
check(globalStore.takeDoRemoveAccountCalls()).single.equals(eg.selfAccount.id);
1008+
1009+
async.elapse(TestGlobalStore.removeAccountDuration);
1010+
check(globalStore.perAccountSync(eg.selfAccount.id)).isNull();
1011+
1012+
async.flushTimers();
1013+
// Reload never succeeds and there is no unhandled errors.
1014+
check(globalStore.perAccountSync(eg.selfAccount.id)).isNull();
1015+
});
1016+
}
1017+
1018+
Future<void> logOutAndCompleteWithNewStore() async {
1019+
// [PerAccountStore.fromInitialSnapshot] requires the account
1020+
// to be in the global store when called; do so before logging out.
1021+
final newStore = eg.store(globalStore: globalStore, account: eg.selfAccount);
1022+
await logOutAccount(globalStore, eg.selfAccount.id);
1023+
completers().single.complete(newStore);
1024+
}
1025+
1026+
test('user logged out before new store is loaded', () => awaitFakeAsync((async) async {
1027+
checkReloadFailure(completeLoading: logOutAndCompleteWithNewStore);
1028+
}));
1029+
});
9691030
});
9701031

9711032
group('UpdateMachine.registerNotificationToken', () {

test/stdlib_checks.dart

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
/// part of the Dart standard library.
66
library;
77

8+
import 'dart:async';
89
import 'dart:convert';
910

1011
import 'package:checks/checks.dart';
@@ -74,6 +75,10 @@ Object? deepToJson(Object? object) {
7475
return (result, true);
7576
}
7677

78+
extension CompleterChecks<T> on Subject<Completer<T>> {
79+
Subject<bool> get isCompleted => has((x) => x.isCompleted, 'isCompleted');
80+
}
81+
7782
extension JsonChecks on Subject<Object?> {
7883
/// Expects that the value is deeply equal to [expected],
7984
/// after calling [deepToJson] on both.

0 commit comments

Comments
 (0)