msglist: Throttle fetchOlder retries

The main purpose of having a separate class is to wait for the
BackoffMachine without blocking fetchOlder so that we can reach
the finally block and return.

This approach is different from how a BackoffMachine is typically used,
because the message list doesn't send and retry requests in a loop; its
caller retries rapidly on scroll changes, and we want to ignore the
excessive requests.

The test drops irrelevant requests with `connection.takeRequests`
without checking, as we are only interested in verifying that no request
was sent.

Fixes: #945

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

Author: PIG208

lib/model/message_list.dart

@@ -1,6 +1,7 @@
 import 'package:collection/collection.dart';
 import 'package:flutter/foundation.dart';
 
+import '../api/backoff.dart';
 import '../api/model/events.dart';
 import '../api/model/model.dart';
 import '../api/route/messages.dart';
@@ -469,7 +470,7 @@ class MessageListView with ChangeNotifier, _MessageSequence {
   Future<void> fetchInitial() async {
     // TODO(#80): fetch from anchor firstUnread, instead of newest
     // TODO(#82): fetch from a given message ID as anchor
-    assert(!fetched && !haveOldest && !fetchingOlder);
+    assert(!fetched && !haveOldest && !fetchingOlder && !_fetchOlderBackoff.isWaiting);
     assert(messages.isEmpty && contents.isEmpty);
     // TODO schedule all this in another isolate
     final generation = this.generation;
@@ -493,24 +494,33 @@ class MessageListView with ChangeNotifier, _MessageSequence {
     notifyListeners();
   }
 
+  final _fetchOlderBackoff = _RetryBackoff();
+
   /// Fetch the next batch of older messages, if applicable.
   Future<void> fetchOlder() async {
     if (haveOldest) return;
     if (fetchingOlder) return;
+    if (_fetchOlderBackoff.isWaiting) return;
     assert(fetched);
     assert(messages.isNotEmpty);
     _fetchingOlder = true;
     _updateEndMarkers();
     notifyListeners();
     final generation = this.generation;
     try {
-      final result = await getMessages(store.connection,
-        narrow: narrow.apiEncode(),
-        anchor: NumericAnchor(messages[0].id),
-        includeAnchor: false,
-        numBefore: kMessageListFetchBatchSize,
-        numAfter: 0,
-      );
+      final GetMessagesResult result;
+      try {
+        result = await getMessages(store.connection,
+          narrow: narrow.apiEncode(),
+          anchor: NumericAnchor(messages[0].id),
+          includeAnchor: false,
+          numBefore: kMessageListFetchBatchSize,
+          numAfter: 0,
+        );
+      } catch (e) {
+        _fetchOlderBackoff.start();
+        rethrow;
+      }
       if (this.generation > generation) return;
 
       if (result.messages.isNotEmpty
@@ -528,6 +538,7 @@ class MessageListView with ChangeNotifier, _MessageSequence {
 
       _insertAllMessages(0, fetchedMessages);
       _haveOldest = result.foundOldest;
+      _fetchOlderBackoff.reset();
     } finally {
       if (this.generation == generation) {
         _fetchingOlder = false;
@@ -708,3 +719,21 @@ class MessageListView with ChangeNotifier, _MessageSequence {
     notifyListeners();
   }
 }
+
+class _RetryBackoff {
+  bool get isWaiting => _isWaiting;
+  bool _isWaiting = false;
+
+  BackoffMachine? _backoffMachine;
+
+  void start() async {
+    assert(!_isWaiting);
+    _isWaiting = true;
+    await (_backoffMachine ??= BackoffMachine()).wait();
+    _isWaiting = false;
+  }
+
+  void reset() {
+    _backoffMachine = null;
+  }
+}

test/model/message_list_test.dart

@@ -3,6 +3,7 @@ import 'dart:convert';
 import 'package:checks/checks.dart';
 import 'package:http/http.dart' as http;
 import 'package:test/scaffolding.dart';
+import 'package:zulip/api/exception.dart';
 import 'package:zulip/api/model/events.dart';
 import 'package:zulip/api/model/model.dart';
 import 'package:zulip/api/model/narrow.dart';
@@ -236,6 +237,33 @@ void main() {
       ..messages.length.equals(30);
   });
 
+  test('fetchOlder nop during backoff', () => awaitFakeAsync((async) async {
+    final olderMessages = List.generate(5, (i) => eg.streamMessage());
+    final initialMessages = List.generate(5, (i) => eg.streamMessage());
+    await prepare(narrow: const CombinedFeedNarrow());
+    await prepareMessages(foundOldest: false, messages: initialMessages);
+    check(model).messages.length.equals(5);
+
+    connection.prepare(httpStatus: 400, json: {
+      'result': 'error', 'code': 'BAD_REQUEST', 'msg': 'Bad request'});
+    check(async.pendingTimers).isEmpty();
+    await check(model.fetchOlder()).throws<ZulipApiException>();
+    // The backoff timer has started.
+    check(async.pendingTimers).single;
+
+    connection.takeRequests();
+    await model.fetchOlder();
+    check(connection.lastRequest).isNull();
+    check(async.pendingTimers).single;
+    check(model).messages.length.equals(5);
+
+    async.flushTimers();
+    connection.prepare(json: olderResult(
+      anchor: 1000, foundOldest: false, messages: olderMessages).toJson());
+    await model.fetchOlder();
+    check(model).messages.length.equals(10);
+  }));
+
   test('fetchOlder handles servers not understanding includeAnchor', () async {
     const narrow = CombinedFeedNarrow();
     await prepare(narrow: narrow);