Skip to content

Commit dab106a

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 185c8c5 commit dab106a

File tree

4 files changed

+80
-4
lines changed

4 files changed

+80
-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: 60 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

@@ -969,6 +970,65 @@ void main() {
969970
});
970971
});
971972

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

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)