Use independent timeouts for polling queries. (#4800)

The last time I touched this code in #4243, I thought it was worthwhile to
preserve the behavior of sometimes batching polling query fetches in the
same tick of the event loop.

However, as @voxtex points out in #4786, almost any batching strategy will
lead to unpredictable fetch timing, skipped fetches, poor accounting for
fetch duration (which could exceed the polling interval), and so on. On
top of that, even the premise that "batching is good" comes into question
if it doesn't happen consistently.

An implementation which uses independent timers seems much simpler and
more predictable.

packages/apollo-client/src/core/QueryManager.ts

@@ -1341,15 +1341,10 @@ export class QueryManager<TStore> {
   // Map from client ID to { interval, options }.
   private pollingInfoByQueryId = new Map<string, {
     interval: number;
-    lastPollTimeMs: number;
+    timeout: NodeJS.Timeout;
     options: WatchQueryOptions;
   }>();
 
-  private nextPoll: {
-    time: number;
-    timeout: NodeJS.Timeout;
-  } | null = null;
-
   public startPollingQuery(
     options: WatchQueryOptions,
     queryId: string,
@@ -1364,95 +1359,50 @@ export class QueryManager<TStore> {
 
     // Do not poll in SSR mode
     if (!this.ssrMode) {
-      this.pollingInfoByQueryId.set(queryId, {
-        interval: pollInterval!,
-        // Avoid polling until at least pollInterval milliseconds from now.
-        // The -10 is a fudge factor to help with tests that rely on simulated
-        // timeouts via jest.runTimersToTime.
-        lastPollTimeMs: Date.now() - 10,
-        options: {
-          ...options,
-          fetchPolicy: 'network-only',
-        },
-      });
+      let info = this.pollingInfoByQueryId.get(queryId)!;
+      if (!info) {
+        this.pollingInfoByQueryId.set(queryId, (info = {} as any));
+      }
+
+      info.interval = pollInterval!;
+      info.options = {
+        ...options,
+        fetchPolicy: 'network-only',
+      };
+
+      const maybeFetch = () => {
+        const info = this.pollingInfoByQueryId.get(queryId);
+        if (info) {
+          if (this.checkInFlight(queryId)) {
+            poll();
+          } else {
+            this.fetchQuery(queryId, info.options, FetchType.poll).then(
+              poll,
+              poll,
+            );
+          }
+        }
+      };
+
+      const poll = () => {
+        const info = this.pollingInfoByQueryId.get(queryId);
+        if (info) {
+          clearTimeout(info.timeout);
+          info.timeout = setTimeout(maybeFetch, info.interval);
+        }
+      };
 
       if (listener) {
         this.addQueryListener(queryId, listener);
       }
 
-      this.schedulePoll(pollInterval!);
+      poll();
     }
 
     return queryId;
   }
 
   public stopPollingQuery(queryId: string) {
-    // Since the master polling interval dynamically adjusts to the contents of
-    // this.pollingInfoByQueryId, stopping a query from polling is as easy as
-    // removing it from the map.
     this.pollingInfoByQueryId.delete(queryId);
   }
-
-  // Calling this method ensures a poll will happen within the specified time
-  // limit, canceling any pending polls that would not happen in time.
-  private schedulePoll(timeLimitMs: number) {
-    const now = Date.now();
-
-    if (this.nextPoll) {
-      if (timeLimitMs < this.nextPoll.time - now) {
-        // The next poll will happen too far in the future, so cancel it, and
-        // fall through to scheduling a new timeout.
-        clearTimeout(this.nextPoll.timeout);
-      } else {
-        // The next poll will happen within timeLimitMs, so all is well.
-        return;
-      }
-    }
-
-    this.nextPoll = {
-      // Estimated time when the timeout will fire.
-      time: now + timeLimitMs,
-
-      timeout: setTimeout(() => {
-        this.nextPoll = null;
-        let nextTimeLimitMs = Infinity;
-
-        this.pollingInfoByQueryId.forEach((info, queryId) => {
-          // Pick next timeout according to current minimum interval.
-          if (info.interval < nextTimeLimitMs) {
-            nextTimeLimitMs = info.interval;
-          }
-
-          if (!this.checkInFlight(queryId)) {
-            // If this query was last polled more than interval milliseconds
-            // ago, poll it now. Note that there may be a small delay between
-            // the desired polling time and the actual polling time (equal to
-            // at most the minimum polling interval across all queries), but
-            // that's the tradeoff to batching polling intervals.
-            if (Date.now() - info.lastPollTimeMs >= info.interval) {
-              const updateLastPollTime = () => {
-                info.lastPollTimeMs = Date.now();
-              };
-              this.fetchQuery(queryId, info.options, FetchType.poll).then(
-                // Set info.lastPollTimeMs after the fetch completes, whether
-                // or not it succeeded. Promise.prototype.finally would be nice
-                // here, but we don't have a polyfill for that at the moment,
-                // and this code has historically silenced errors, which is not
-                // the behavior of .finally(updateLastPollTime).
-                updateLastPollTime,
-                updateLastPollTime
-              );
-            }
-          }
-        });
-
-        // If there were no entries in this.pollingInfoByQueryId, then
-        // nextTimeLimitMs will still be Infinity, so this.schedulePoll will
-        // not be called, thus ending the master polling interval.
-        if (isFinite(nextTimeLimitMs)) {
-          this.schedulePoll(nextTimeLimitMs);
-        }
-      }, timeLimitMs),
-    };
-  }
 }