store: Report polling errors with details.

We extend the checkRetry helper to support simulating multiple
consecutive errors.

Signed-off-by: Zixuan James Li <[email protected]>

Author: PIG208

assets/l10n/app_en.arb

@@ -160,6 +160,18 @@
       "message": {"type": "String", "example": "Invalid format"}
     }
   },
+  "errorLoadingServerDataShort": "Error connecting to Zulip. Retrying…",
+  "@errorLoadingServerDataShort": {
+    "description": "Short error message for a generic unknown error connecting to the server."
+  },
+  "errorLoadingServerDataDetails": "Error connecting to Zulip at {serverUrl}. Will retry:\n\n{error}",
+  "@errorLoadingServerDataDetails": {
+    "description": "Dialog error message for a generic unknown error connecting to server with details.",
+    "placeholders": {
+      "serverUrl": {"type": "String", "example": "http://example.com/"},
+      "error": {"type": "String", "example": "Invalid format"}
+    }
+  },
   "errorSharingFailed": "Sharing failed",
   "@errorSharingFailed": {
     "description": "Error message when sharing a message failed."

lib/model/store.dart

@@ -20,6 +20,7 @@ import '../log.dart';
 import '../notifications/receive.dart';
 import 'autocomplete.dart';
 import 'database.dart';
+import 'localizations.dart';
 import 'message.dart';
 import 'message_list.dart';
 import 'recent_dm_conversations.dart';
@@ -770,6 +771,8 @@ class UpdateMachine {
 
   void poll() async {
     BackoffMachine? backoffMachine;
+    int accumulatedTransientFailureCount = 0;
+    const transientFailureCountNotifyThreshold = 5;
 
     while (true) {
       if (_debugLoopSignal != null) {
@@ -786,6 +789,8 @@ class UpdateMachine {
           queueId: queueId, lastEventId: lastEventId);
       } catch (e) {
         store.isLoading = true;
+        final localizations = GlobalLocalizations.zulipLocalizations;
+        final serverUrl = store.connection.realmUrl.origin;
         switch (e) {
           case ZulipApiException(code: 'BAD_EVENT_QUEUE_ID'):
             assert(debugLog('Lost event queue for $store.  Replacing…'));
@@ -797,15 +802,25 @@ class UpdateMachine {
           case Server5xxException() || NetworkException():
             assert(debugLog('Transient error polling event queue for $store: $e\n'
                 'Backing off, then will retry…'));
-            // TODO tell user if transient polling errors persist
+            accumulatedTransientFailureCount++;
+            if (accumulatedTransientFailureCount > transientFailureCountNotifyThreshold) {
+              reportErrorToUserBriefly(
+                localizations.errorLoadingServerDataShort,
+                details: localizations.errorLoadingServerDataDetails(
+                  serverUrl, e.toString()));
+            }
             await (backoffMachine ??= BackoffMachine()).wait();
             assert(debugLog('… Backoff wait complete, retrying poll.'));
             continue;
 
           default:
             assert(debugLog('Error polling event queue for $store: $e\n'
                 'Backing off and retrying even though may be hopeless…'));
-            // TODO tell user on non-transient error in polling
+            // TODO(#186): Handle unrecoverable failures
+            reportErrorToUserBriefly(
+              localizations.errorLoadingServerDataShort,
+              details: localizations.errorLoadingServerDataDetails(
+                serverUrl, e.toString()));
             await (backoffMachine ??= BackoffMachine()).wait();
             assert(debugLog('… Backoff wait complete, retrying poll.'));
             continue;
@@ -829,6 +844,9 @@ class UpdateMachine {
       // and failures, the successes themselves should space out the requests.
       backoffMachine = null;
       store.isLoading = false;
+      // Dismiss existing errors, if any.
+      reportErrorToUserBriefly(null);
+      accumulatedTransientFailureCount = 0;
 
       final events = result.events;
       for (final event in events) {

test/model/store_test.dart

@@ -8,6 +8,7 @@ import 'package:zulip/api/model/events.dart';
 import 'package:zulip/api/model/model.dart';
 import 'package:zulip/api/route/events.dart';
 import 'package:zulip/api/route/messages.dart';
+import 'package:zulip/log.dart';
 import 'package:zulip/model/store.dart';
 import 'package:zulip/notifications/receive.dart';
 
@@ -391,6 +392,22 @@ void main() {
       check(store.userSettings!.twentyFourHourTime).isTrue();
     }));
 
+    String? lastReportedError;
+    String? takeLastReportedError() {
+      final result = lastReportedError;
+      lastReportedError = null;
+      return result;
+    }
+
+    void logAndReportErrorToUserBriefly(String? message, {String? details}) {
+      if (message == null) return;
+      lastReportedError = '$message\n$details';
+    }
+    // This overrides the function globally, but we don't need to worry about
+    // leaking the state because that is only relevant to widget tests.
+    // See `lib/log.dart ` for details.
+    reportErrorToUserBriefly = logAndReportErrorToUserBriefly;
+
     test('handles expired queue', () => awaitFakeAsync((async) async {
       await prepareStore();
       updateMachine.debugPauseLoop();
@@ -428,19 +445,41 @@ void main() {
     }));
 
     group('retries on errors', () {
-      void checkRetry(void Function() prepareError) {
+      void checkRetry(void Function() prepareError, {
+        int expectedFailureCountNotifyThreshold = 0,
+      }) {
+        final expectedErrorMessage =
+          'Error connecting to Zulip. Retrying…\n'
+          'Error connecting to Zulip at ${eg.realmUrl.origin}. Will retry';
+
         awaitFakeAsync((async) async {
           await prepareStore(lastEventId: 1);
           updateMachine.debugPauseLoop();
           updateMachine.poll();
           check(async.pendingTimers).length.equals(0);
 
-          // Make the request, inducing an error in it.
-          prepareError();
-          updateMachine.debugAdvanceLoop();
-          async.elapse(Duration.zero);
-          checkLastRequest(lastEventId: 1);
-          check(store).isLoading.isTrue();
+          // Need to add 1 to the upperbound for that one additional request
+          // to trigger error reporting.
+          for (int i = 0; i < expectedFailureCountNotifyThreshold + 1; i++) {
+            // Make the request, inducing an error in it.
+            prepareError();
+            if (i > 0) {
+              // End polling backoff from the previous iteration.
+              async.flushTimers();
+            }
+            updateMachine.debugAdvanceLoop();
+            check(lastReportedError).isNull();
+            async.elapse(Duration.zero);
+            if (i < expectedFailureCountNotifyThreshold) {
+              // The error message should not appear until the `updateMachine`
+              // has retried the given number of times.
+              check(takeLastReportedError()).isNull();
+            } else {
+              check(takeLastReportedError()).isNotNull().contains(expectedErrorMessage);
+            }
+            checkLastRequest(lastEventId: 1);
+            check(store).isLoading.isTrue();
+          }
 
           // Polling doesn't resume immediately; there's a timer.
           check(async.pendingTimers).length.equals(1);
@@ -461,11 +500,13 @@ void main() {
       }
 
       test('Server5xxException', () {
-        checkRetry(() => connection.prepare(httpStatus: 500, body: 'splat'));
+        checkRetry(() => connection.prepare(httpStatus: 500, body: 'splat'),
+          expectedFailureCountNotifyThreshold: 5);
       });
 
       test('NetworkException', () {
-        checkRetry(() => connection.prepare(exception: Exception("failed")));
+        checkRetry(() => connection.prepare(exception: Exception("failed")),
+          expectedFailureCountNotifyThreshold: 5);
       });
 
       test('ZulipApiException', () {