revision wip

Signed-off-by: Zixuan James Li <[email protected]>
PIG208 · Feb 7, 2025 · 707ccb3 · 707ccb3
1 parent f8864d2
commit 707ccb3
Show file tree

Hide file tree

Showing 4 changed files with 56 additions and 32 deletions.
diff --git a/lib/model/store.dart b/lib/model/store.dart
@@ -156,7 +156,7 @@ abstract class GlobalStore extends ChangeNotifier {
       store = await doLoadPerAccount(accountId);
     } catch (e) {
       switch (e) {
-        case ZulipApiException(code: 'INVALID_API_KEY'):
+        case HttpException(httpStatus: 401):
           // The API key is invalid and the store can never be loaded
           // unless the user retries manually.
           final zulipLocalizations = GlobalLocalizations.zulipLocalizations;
@@ -939,14 +939,14 @@ class UpdateMachine {
         // Print stack trace in its own log entry; log entries are truncated
         // at 1 kiB (at least on Android), and stack can be longer than that.
         assert(debugLog('Stack:\n$s'));
-        assert(debugLog('Backing off, then will retry…'));
         // TODO(#890): tell user if initial-fetch errors persist, or look non-transient
         switch (e) {
-          case ZulipApiException(code: 'INVALID_API_KEY'):
+          case HttpException(httpStatus: 401):
             // We cannot recover from this error through retrying.
             // Leave it to [GlobalStore.loadPerAccount].
             rethrow;
         }
+        assert(debugLog('Backing off, then will retry…'));
         await (backoffMachine ??= BackoffMachine()).wait();
         assert(debugLog('… Backoff wait complete, retrying initial fetch.'));
       }
@@ -1078,13 +1078,7 @@ class UpdateMachine {
       }
     } catch (e) {
       if (_disposed) return;
-      try {
-        await _handlePollError(e);
-      } on AccountNotFoundException {
-        // Cannot recover by replacing the store because the account
-        // was logged out.
-        return;
-      }
+      await _handlePollError(e);
       assert(_disposed);
       return;
     }
@@ -1217,6 +1211,10 @@ class UpdateMachine {
         // The old event queue is gone, so we need a new one.  This is normal.
         isUnexpected = false;
 
+      case HttpException(httpStatus: 401):
+        assert(debugLog('Got invalid auth keys for $store. Replacing with anticipated failure…'));
+        isUnexpected = false;
+
       case _EventHandlingException(:final cause, :final event):
         assert(debugLog('BUG: Error handling an event: $cause\n' // TODO(log)
           '  event: $event\n'
@@ -1252,8 +1250,14 @@ class UpdateMachine {
       if (_disposed) return;
     }
 
-    await store._globalStore._reloadPerAccount(store.accountId);
-    assert(_disposed);
+    try {
+      await store._globalStore._reloadPerAccount(store.accountId);
+    } on AccountNotFoundException {
+      assert(debugLog('… Event queue not replaced; account logged out.'));
+      return;
+    } finally {
+      assert(_disposed);
+    }
     assert(debugLog('… Event queue replaced.'));
   }
 

diff --git a/test/model/store_test.dart b/test/model/store_test.dart
@@ -13,6 +13,7 @@ import 'package:zulip/api/model/model.dart';
 import 'package:zulip/api/route/events.dart';
 import 'package:zulip/api/route/messages.dart';
 import 'package:zulip/api/route/realm.dart';
+import 'package:zulip/model/actions.dart';
 import 'package:zulip/model/message_list.dart';
 import 'package:zulip/model/narrow.dart';
 import 'package:zulip/log.dart';
@@ -123,6 +124,39 @@ void main() {
     check(completers(1)).length.equals(1);
   });
 
+  test('GlobalStore.perAccount loading fails with HTTP status code 401', () => awaitFakeAsync((async) async {
+    addTearDown(testBinding.reset);
+
+    await testBinding.globalStore.insertAccount(eg.selfAccount.toCompanion(false));
+    testBinding.globalStore.loadPerAccountException = ZulipApiException(
+      routeName: '/register', code: 'UNAUTHORIZED', httpStatus: 401,
+      data: {}, message: '');
+    await check(testBinding.globalStore.perAccount(eg.selfAccount.id))
+      .throws<AccountNotFoundException>();
+
+    check(testBinding.globalStore.takeDoRemoveAccountCalls())
+      .single.equals(eg.selfAccount.id);
+  }));
+
+  test('GlobalStore.perAccount logs out account; then loading fails with HTTP status code 401', () => awaitFakeAsync((async) async {
+    addTearDown(testBinding.reset);
+
+    await testBinding.globalStore.insertAccount(eg.selfAccount.toCompanion(false));
+
+    testBinding.globalStore.loadPerAccountDuration = Duration(seconds: 2);
+    testBinding.globalStore.loadPerAccountException = ZulipApiException(
+      routeName: '/register', code: 'UNAUTHORIZED', httpStatus: 401,
+      data: {}, message: '');
+    final future = testBinding.globalStore.perAccount(eg.selfAccount.id);
+    check(testBinding.globalStore.takeDoRemoveAccountCalls()).isEmpty();
+
+    await logOutAccount(testBinding.globalStore, eg.selfAccount.id);
+    check(testBinding.globalStore.takeDoRemoveAccountCalls()).single;
+
+    await check(future).throws<AccountNotFoundException>();
+    check(testBinding.globalStore.takeDoRemoveAccountCalls()).isEmpty();
+  }));
+
   // TODO test insertAccount
 
   group('GlobalStore.updateAccount', () {
@@ -501,20 +535,6 @@ void main() {
         users.map((expected) => (it) => it.fullName.equals(expected.fullName)));
     }));
 
-    test('GlobalStore.perAccount on INVALID_API_KEY', () => awaitFakeAsync((async) async {
-      addTearDown(testBinding.reset);
-
-      await testBinding.globalStore.insertAccount(eg.selfAccount.toCompanion(false));
-      testBinding.globalStore.loadPerAccountException = ZulipApiException(
-        routeName: '/register', code: 'INVALID_API_KEY', httpStatus: 400,
-        data: {}, message: '');
-      await check(testBinding.globalStore.perAccount(eg.selfAccount.id))
-        .throws<AccountNotFoundException>();
-
-      check(testBinding.globalStore.takeDoRemoveAccountCalls())
-        .single.equals(eg.selfAccount.id);
-    }));
-
     // TODO test UpdateMachine.load starts polling loop
     // TODO test UpdateMachine.load calls registerNotificationToken
   });
@@ -858,7 +878,7 @@ void main() {
       check(store.debugMessageListViews).isEmpty();
     }));
 
-    test('log out if fail to reload on unexpected errors', () => awaitFakeAsync((async) async {
+    test('unexpected poll error, but reload fails with HTTP status code 401; log out', () => awaitFakeAsync((async) async {
       await preparePoll();
 
       prepareUnexpectedLoopError();
@@ -867,7 +887,7 @@ void main() {
       check(store).isLoading.isTrue();
 
       globalStore.loadPerAccountException = ZulipApiException(
-        routeName: '/register', code: 'INVALID_API_KEY', httpStatus: 400,
+        routeName: '/register', code: 'UNAUTHORIZED', httpStatus: 401,
         data: {}, message: '');
       // The reload doesn't happen immediately; there's a timer.
       check(async.pendingTimers).length.equals(1);

diff --git a/test/widgets/app_test.dart b/test/widgets/app_test.dart
@@ -83,7 +83,7 @@ void main() {
       assert(loadPerAccountDuration > kTryAnotherAccountWaitPeriod);
       testBinding.globalStore.loadPerAccountDuration = loadPerAccountDuration;
       testBinding.globalStore.loadPerAccountException = ZulipApiException(
-        routeName: '/register', code: 'INVALID_API_KEY', httpStatus: 400,
+        routeName: '/register', code: 'UNAUTHORIZED', httpStatus: 401,
         data: {}, message: '');
       await testBinding.globalStore.insertAccount(eg.selfAccount.toCompanion(false));
       await prepare(tester);
@@ -115,7 +115,7 @@ void main() {
     testWidgets('push route when popping last route on stack', (tester) async {
       testBinding.globalStore.loadPerAccountDuration = Duration.zero;
       testBinding.globalStore.loadPerAccountException = ZulipApiException(
-        routeName: '/register', code: 'INVALID_API_KEY', httpStatus: 400,
+        routeName: '/register', code: 'UNAUTHORIZED', httpStatus: 401,
         data: {}, message: '');
       await testBinding.globalStore.insertAccount(eg.selfAccount.toCompanion(false));
       await prepare(tester);

diff --git a/test/widgets/store_test.dart b/test/widgets/store_test.dart
@@ -132,7 +132,7 @@ void main() {
     assert(loadPerAccountDuration > kTryAnotherAccountWaitPeriod);
     testBinding.globalStore.loadPerAccountDuration = loadPerAccountDuration;
     testBinding.globalStore.loadPerAccountException = ZulipApiException(
-      routeName: '/register', code: 'INVALID_API_KEY', httpStatus: 400,
+      routeName: '/register', code: 'UNAUTHORIZED', httpStatus: 401,
       data: {}, message: '');
     await testBinding.globalStore.insertAccount(eg.selfAccount.toCompanion(false));
     await tester.pumpWidget(ZulipApp(navigatorObservers: [testNavObserver]));
@@ -174,7 +174,7 @@ void main() {
 
     testBinding.globalStore.loadPerAccountDuration = Duration.zero;
     testBinding.globalStore.loadPerAccountException = ZulipApiException(
-      routeName: '/register', code: 'INVALID_API_KEY', httpStatus: 400,
+      routeName: '/register', code: 'UNAUTHORIZED', httpStatus: 401,
       data: {}, message: '');
     await testBinding.globalStore.insertAccount(eg.selfAccount.toCompanion(false));
     await tester.pumpWidget(ZulipApp(navigatorObservers: [testNavObserver]));