async [nfc]: Move tryUntilSuccessful to fetchActions and rename.

As Greg says [1], it's always been closely tied to
`doInitialFetch`'s specific needs, with its handling of 4xx errors.
We're about to make it more so, by adding timeout logic.

[1]: zulip#4165 (comment)

src/message/__tests__/fetchActions-test.js

@@ -4,7 +4,13 @@ import thunk from 'redux-thunk';
 
 import type { GlobalState } from '../../reduxTypes';
 import type { Action } from '../../actionTypes';
-import { isFetchNeededAtAnchor, fetchMessages, fetchOlder, fetchNewer } from '../fetchActions';
+import {
+  isFetchNeededAtAnchor,
+  fetchMessages,
+  fetchOlder,
+  fetchNewer,
+  tryFetch,
+} from '../fetchActions';
 import { FIRST_UNREAD_ANCHOR } from '../../anchor';
 import { streamNarrow, HOME_NARROW, HOME_NARROW_STR } from '../../utils/narrow';
 import { navStateWithNarrow } from '../../utils/testHelpers';
@@ -71,6 +77,38 @@ describe('fetchActions', () => {
     });
   });
 
+  describe('tryFetch', () => {
+    test('resolves any value when there is no exception', async () => {
+      const result = await tryFetch(async () => 'hello');
+
+      expect(result).toEqual('hello');
+    });
+
+    test('resolves any promise, if there is no exception', async () => {
+      const result = await tryFetch(
+        () => new Promise(resolve => setTimeout(() => resolve('hello'), 100)),
+      );
+
+      expect(result).toEqual('hello');
+    });
+
+    test('retries a call, if there is an exception', async () => {
+      // fail on first call, succeed second time
+      let callCount = 0;
+      const thrower = () => {
+        callCount++;
+        if (callCount === 1) {
+          throw new Error('First run exception');
+        }
+        return 'hello';
+      };
+
+      const result = await tryFetch(async () => thrower());
+
+      expect(result).toEqual('hello');
+    });
+  });
+
   describe('fetchMessages', () => {
     const message1 = eg.streamMessage({ id: 1 });
     const message2 = eg.streamMessage({ id: 2 });

src/message/fetchActions.js

@@ -10,6 +10,7 @@ import type {
 } from '../types';
 import type { InitialData } from '../api/initialDataTypes';
 import * as api from '../api';
+import { isClientError } from '../api/apiErrors';
 import {
   getAuth,
   getSession,
@@ -28,7 +29,7 @@ import {
 } from '../actionConstants';
 import { FIRST_UNREAD_ANCHOR, LAST_MESSAGE_ANCHOR } from '../anchor';
 import { ALL_PRIVATE_NARROW } from '../utils/narrow';
-import { tryUntilSuccessful } from '../utils/async';
+import { BackoffMachine } from '../utils/async';
 import { initNotifications } from '../notification/notificationActions';
 import { addToOutbox, sendOutbox } from '../outbox/outboxActions';
 import { realmInit } from '../realm/realmActions';
@@ -240,6 +241,34 @@ const fetchPrivateMessages = () => async (dispatch: Dispatch, getState: GetState
   );
 };
 
+/**
+ * Calls an async function and if unsuccessful retries the call.
+ *
+ * If the function is an API call and the response has HTTP status code 4xx
+ * the error is considered unrecoverable and the exception is rethrown, to be
+ * handled further up in the call stack.
+ */
+export async function tryFetch<T>(func: () => Promise<T>): Promise<T> {
+  const backoffMachine = new BackoffMachine();
+  // eslint-disable-next-line no-constant-condition
+  while (true) {
+    try {
+      return await func();
+    } catch (e) {
+      if (isClientError(e)) {
+        throw e;
+      }
+      await backoffMachine.wait();
+    }
+  }
+
+  // Without this, Flow 0.92.1 does not know this code is unreachable,
+  // and it incorrectly thinks Promise<undefined> could be returned,
+  // which is inconsistent with the stated Promise<T> return type.
+  // eslint-disable-next-line no-unreachable
+  throw new Error();
+}
+
 /**
  * Fetch lots of state from the server, and start an event queue.
  *
@@ -270,7 +299,7 @@ export const doInitialFetch = () => async (dispatch: Dispatch, getState: GetStat
 
   try {
     [initData, serverSettings] = await Promise.all([
-      tryUntilSuccessful(() =>
+      tryFetch(() =>
         api.registerForEvents(auth, {
           fetch_event_types: config.serverDataOnStartup,
           apply_markdown: true,
@@ -282,7 +311,7 @@ export const doInitialFetch = () => async (dispatch: Dispatch, getState: GetStat
           },
         }),
       ),
-      tryUntilSuccessful(() => api.getServerSettings(auth.realm)),
+      tryFetch(() => api.getServerSettings(auth.realm)),
     ]);
   } catch (e) {
     // This should only happen on a 4xx HTTP status, which should only

src/utils/__tests__/async-test.js

@@ -1,5 +1,5 @@
 /* @flow strict-local */
-import { sleep, tryUntilSuccessful } from '../async';
+import { sleep } from '../async';
 import { Lolex } from '../../__tests__/lib/lolex';
 
 const sleepMeasure = async (expectedMs: number) => {
@@ -60,35 +60,3 @@ describe('sleep (real)', () => {
     //     unpredictable loads.
   });
 });
-
-describe('tryUntilSuccessful', () => {
-  test('resolves any value when there is no exception', async () => {
-    const result = await tryUntilSuccessful(async () => 'hello');
-
-    expect(result).toEqual('hello');
-  });
-
-  test('resolves any promise, if there is no exception', async () => {
-    const result = await tryUntilSuccessful(
-      () => new Promise(resolve => setTimeout(() => resolve('hello'), 100)),
-    );
-
-    expect(result).toEqual('hello');
-  });
-
-  test('retries a call, if there is an exception', async () => {
-    // fail on first call, succeed second time
-    let callCount = 0;
-    const thrower = () => {
-      callCount++;
-      if (callCount === 1) {
-        throw new Error('First run exception');
-      }
-      return 'hello';
-    };
-
-    const result = await tryUntilSuccessful(async () => thrower());
-
-    expect(result).toEqual('hello');
-  });
-});

src/utils/async.js

@@ -1,5 +1,4 @@
 /* @flow strict-local */
-import { isClientError } from '../api/apiErrors';
 
 /** Like setTimeout(..., 0), but returns a Promise of the result. */
 export function delay<T>(callback: () => T): Promise<T> {
@@ -78,31 +77,3 @@ export class BackoffMachine {
     this._waitsCompleted++;
   }
 }
-
-/**
- * Calls an async function and if unsuccessful retries the call.
- *
- * If the function is an API call and the response has HTTP status code 4xx
- * the error is considered unrecoverable and the exception is rethrown, to be
- * handled further up in the call stack.
- */
-export async function tryUntilSuccessful<T>(func: () => Promise<T>): Promise<T> {
-  const backoffMachine = new BackoffMachine();
-  // eslint-disable-next-line no-constant-condition
-  while (true) {
-    try {
-      return await func();
-    } catch (e) {
-      if (isClientError(e)) {
-        throw e;
-      }
-      await backoffMachine.wait();
-    }
-  }
-
-  // Without this, Flow 0.92.1 does not know this code is unreachable,
-  // and it incorrectly thinks Promise<undefined> could be returned,
-  // which is inconsistent with the stated Promise<T> return type.
-  // eslint-disable-next-line no-unreachable
-  throw new Error();
-}