store: Pass dont_block in first poll attempt after a failure

Fixes zulip#979.

Discussion:
  zulip#979 (comment)

Author: chrisbobbe

lib/model/store.dart

@@ -1427,7 +1427,13 @@ class UpdateMachine {
         final GetEventsResult result;
         try {
           result = await getEvents(store.connection,
-            queueId: store.queueId, lastEventId: lastEventId);
+            queueId: store.queueId,
+            lastEventId: lastEventId,
+            // If the UI shows we're busy getting event-polling to work again,
+            // ask the server to tell us immediately that it's working again,
+            // rather than waiting for an event, which could take up to a minute
+            // in the case of a heartbeat event. See #979.
+            dontBlock: store.isRecoveringEventStream ? true : null);
           if (_disposed) return;
         } catch (e, stackTrace) {
           if (_disposed) return;
@@ -1494,11 +1500,11 @@ class UpdateMachine {
     // if we stayed at the max backoff interval; partway toward what would
     // happen if we weren't backing off at all.
     //
-    // But at least for [getEvents] requests, as here, it should be OK,
-    // because this is a long-poll.  That means a typical successful request
-    // takes a long time to come back; in fact longer than our max backoff
-    // duration (which is 10 seconds).  So if we're getting a mix of successes
-    // and failures, the successes themselves should space out the requests.
+    // Successful retries won't actually space out the requests, because retries
+    // are done with the `dont_block` param, asking the server to respond
+    // immediately instead of waiting through the long-poll period.
+    // (See comments on that code for why this behavior is helpful.)
+    // If server logs show pressure from too many requests, we can investigate.
     _pollBackoffMachine = null;
 
     store.isRecoveringEventStream = false;

test/model/store_test.dart

@@ -782,13 +782,14 @@ void main() {
       updateMachine.poll();
     }
 
-    void checkLastRequest({required int lastEventId}) {
+    void checkLastRequest({required int lastEventId, bool expectDontBlock = false}) {
       check(connection.takeRequests()).single.isA<http.Request>()
         ..method.equals('GET')
         ..url.path.equals('/api/v1/events')
         ..url.queryParameters.deepEquals({
           'queue_id': store.queueId,
           'last_event_id': lastEventId.toString(),
+          if (expectDontBlock) 'dont_block': 'true',
         });
     }
 
@@ -878,7 +879,7 @@ void main() {
         prepareError();
         updateMachine.debugAdvanceLoop();
         async.elapse(Duration.zero);
-        checkLastRequest(lastEventId: 1);
+        checkLastRequest(lastEventId: 1, expectDontBlock: false);
         check(store).isRecoveringEventStream.isTrue();
 
         // Polling doesn't resume immediately; there's a timer.
@@ -893,7 +894,7 @@ void main() {
           HeartbeatEvent(id: 2),
         ], queueId: null).toJson());
         async.flushTimers();
-        checkLastRequest(lastEventId: 1);
+        checkLastRequest(lastEventId: 1, expectDontBlock: true);
         check(updateMachine.lastEventId).equals(2);
         check(store).isRecoveringEventStream.isFalse();
       });
@@ -1032,10 +1033,12 @@ void main() {
         await preparePoll(lastEventId: 1);
       }
 
-      void pollAndFail(FakeAsync async, {bool shouldCheckRequest = true}) {
+      void pollAndFail(FakeAsync async, {bool shouldCheckRequest = true, bool expectDontBlock = false}) {
         updateMachine.debugAdvanceLoop();
         async.elapse(Duration.zero);
-        if (shouldCheckRequest) checkLastRequest(lastEventId: 1);
+        if (shouldCheckRequest) {
+          checkLastRequest(lastEventId: 1, expectDontBlock: expectDontBlock);
+        }
         check(store).isRecoveringEventStream.isTrue();
       }
 
@@ -1054,21 +1057,26 @@ void main() {
         return awaitFakeAsync((async) async {
           await prepare();
 
+          bool expectDontBlock = false;
           for (int i = 0; i < UpdateMachine.transientFailureCountNotifyThreshold; i++) {
             prepareError();
-            pollAndFail(async);
+            pollAndFail(async, expectDontBlock: expectDontBlock);
+            expectDontBlock = true;
             check(takeLastReportedError()).isNull();
             async.flushTimers();
             if (!identical(store, globalStore.perAccountSync(store.accountId))) {
               // Store was reloaded.
               updateFromGlobalStore();
               updateMachine.debugPauseLoop();
               updateMachine.poll();
+              // Loading indicator is cleared on successful /register;
+              // we don't need dont_block for the new queue's first poll.
+              expectDontBlock = false;
             }
           }
 
           prepareError();
-          pollAndFail(async);
+          pollAndFail(async, expectDontBlock: expectDontBlock);
           return check(takeLastReportedError()).isNotNull();
         });
       }
@@ -1077,21 +1085,26 @@ void main() {
         return awaitFakeAsync((async) async {
           await prepare();
 
+          bool expectDontBlock = false;
           for (int i = 0; i < UpdateMachine.transientFailureCountNotifyThreshold; i++) {
             prepareError();
-            pollAndFail(async);
+            pollAndFail(async, expectDontBlock: expectDontBlock);
+            expectDontBlock = true;
             check(takeLastReportedError()).isNull();
             async.flushTimers();
             if (!identical(store, globalStore.perAccountSync(store.accountId))) {
               // Store was reloaded.
               updateFromGlobalStore();
               updateMachine.debugPauseLoop();
               updateMachine.poll();
+              // Loading indicator is cleared on successful /register;
+              // we don't need dont_block for the new queue's first poll.
+              expectDontBlock = false;
             }
           }
 
           prepareError();
-          pollAndFail(async);
+          pollAndFail(async, expectDontBlock: expectDontBlock);
           // Still no error reported, even after the same number of iterations
           // where other errors get reported (as [checkLateReported] checks).
           check(takeLastReportedError()).isNull();