wip store [nfc]: move backoff/reload inside error helpers

Author: gnprice

lib/model/store.dart

@@ -929,7 +929,6 @@ class UpdateMachine {
     try {
       assert(!_disposed);
 
-      BackoffMachine? backoffMachine;
       while (true) {
         if (_debugLoopSignal != null) {
           await _debugLoopSignal!.future;
@@ -945,33 +944,13 @@ class UpdateMachine {
           result = await getEvents(store.connection,
             queueId: queueId, lastEventId: lastEventId);
           if (_disposed) return;
-        } catch (e) {
+        } catch (e, stackTrace) {
           if (_disposed) return;
-          final shouldRetry = _triagePollRequestError(e);
-          if (!shouldRetry) rethrow;
-          await (backoffMachine ??= BackoffMachine()).wait();
+          await _handlePollRequestError(e, stackTrace); // may rethrow
           if (_disposed) return;
-          assert(debugLog('… Backoff wait complete, retrying poll.'));
           continue;
         }
 
-        // After one successful request, we reset backoff to its initial state.
-        // That way if the user is off the network and comes back on, the app
-        // doesn't wind up in a state where it's slow to recover the next time
-        // one request fails.
-        //
-        // This does mean that if the server is having trouble and handling some
-        // but not all of its requests, we'll end up doing a lot more retries than
-        // 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.
-        backoffMachine = null;
-
         _clearPollErrors();
 
         final events = result.events;
@@ -991,22 +970,14 @@ class UpdateMachine {
       }
     } catch (e) {
       if (_disposed) return;
-
-      // An error occurred, other than the transient request errors we retry on.
-      // This means either a lost/expired event queue on the server (which is
-      // normal after the app is offline for a period like 10 minutes),
-      // or an unexpected exception representing a bug in our code or the server.
-      // Either way, the show must go on.  So reload server data from scratch.
-
-      _reportPollError(e);
-
-      // This disposes the store, which disposes this update machine.
-      await store._globalStore._reloadPerAccount(store.accountId);
-      assert(debugLog('… Event queue replaced.'));
+      await _handlePollError(e);
+      assert(_disposed);
       return;
     }
   }
 
+  BackoffMachine? _pollBackoffMachine;
+
   /// This controls when we start to report transient errors to the user when
   /// polling.
   ///
@@ -1017,17 +988,42 @@ class UpdateMachine {
   int _accumulatedTransientFailureCount = 0;
 
   void _clearPollErrors() {
+    // After one successful request, we reset backoff to its initial state.
+    // That way if the user is off the network and comes back on, the app
+    // doesn't wind up in a state where it's slow to recover the next time
+    // one request fails.
+    //
+    // This does mean that if the server is having trouble and handling some
+    // but not all of its requests, we'll end up doing a lot more retries than
+    // 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.
+    _pollBackoffMachine = null;
+
     store.isLoading = false;
     _accumulatedTransientFailureCount = 0;
     reportErrorToUserBriefly(null);
   }
 
-  /// Sort out an error from the network request in [poll].
+  /// Sort out an error from the network request in [poll]:
+  /// either wait for a backoff duration (and possibly report the error),
+  /// or rethrow.
   ///
-  /// If the request should be retried, this method returns true,
+  /// If the request should be retried, this method uses [_pollBackoffMachine]
+  /// to wait an appropriate backoff duration for that retry,
   /// after reporting the error if appropriate to the user and/or developer.
-  /// Otherwise, this method returns false with no side effects.
-  bool _triagePollRequestError(Object error) {
+  ///
+  /// Otherwise this method rethrows the error, with no other side effects.
+  ///
+  /// See also:
+  ///  * [_handlePollError], which handles errors from the rest of [poll]
+  ///    and errors this method rethrows.
+  Future<void> _handlePollRequestError(Object error, StackTrace stackTrace) async {
     store.isLoading = true;
 
     bool shouldReportToUser;
@@ -1046,19 +1042,32 @@ class UpdateMachine {
         shouldReportToUser = true;
 
       default:
-        return false;
+        Error.throwWithStackTrace(error, stackTrace);
     }
 
     assert(debugLog('Transient error polling event queue for $store: $error\n'
         'Backing off, then will retry…'));
     if (shouldReportToUser) {
       _maybeReportToUserTransientError(error);
     }
-    return true;
+    await (_pollBackoffMachine ??= BackoffMachine()).wait();
+    if (_disposed) return;
+    assert(debugLog('… Backoff wait complete, retrying poll.'));
   }
 
-  /// Report an error in [poll], if appropriate, to the user and/or developer.
-  void _reportPollError(Object error) {
+  /// Deal with an error in [poll]: reload server data to replace the store,
+  /// after reporting the error as appropriate to the user and/or developer.
+  ///
+  /// See also:
+  ///  * [_handlePollRequestError], which handles certain errors
+  ///    and causes them not to reach this method.
+  Future<void> _handlePollError(Object error) async {
+    // An error occurred, other than the transient request errors we retry on.
+    // This means either a lost/expired event queue on the server (which is
+    // normal after the app is offline for a period like 10 minutes),
+    // or an unexpected exception representing a bug in our code or the server.
+    // Either way, the show must go on.  So reload server data from scratch.
+
     store.isLoading = true;
 
     switch (error) {
@@ -1091,6 +1100,10 @@ class UpdateMachine {
         // The bug here could be in the server if it's an ApiRequestException,
         // but our remedy is the same either way.
     }
+
+    await store._globalStore._reloadPerAccount(store.accountId);
+    assert(_disposed);
+    assert(debugLog('… Event queue replaced.'));
   }
 
   /// This only reports transient errors after reaching