fetchActions: Add a timeout to tryFetch.

So far, `tryFetch`'s only callers are in the initial fetch; so, add
handling for the `TimeoutError` there.

The choice of value for `requestLongTimeoutMs` comes from a
suggestion in zulip#4165's description:

> As a minimal version, if the initial fetch takes a completely
> unreasonable amount of time (maybe one minute), we should give up
> and take you to the account-picker screen so you can try a
> different account and server.

Fixes: zulip#4165

src/config.js

@@ -2,6 +2,7 @@
 const isDevelopment = process.env.NODE_ENV === 'development';
 
 type Config = {|
+  requestLongTimeoutMs: number,
   messagesPerRequest: number,
   messageListThreshold: number,
   enableReduxLogging: boolean,
@@ -14,6 +15,11 @@ type Config = {|
 |};
 
 const config: Config = {
+  // A completely unreasonable amount of time for a request, or
+  // several retries of a request, to take. If this elapses, we're
+  // better off giving up.
+  requestLongTimeoutMs: 60 * 1000,
+
   messagesPerRequest: 100,
   messageListThreshold: 4000,
   enableReduxLogging: isDevelopment && !!global.btoa,

src/message/__tests__/fetchActions-test.js

@@ -20,7 +20,7 @@ import { GravatarURL } from '../../utils/avatar';
 import * as eg from '../../__tests__/lib/exampleData';
 import { ApiError } from '../../api/apiErrors';
 import { fakeSleep } from '../../__tests__/lib/fakeTimers';
-import { BackoffMachine } from '../../utils/async';
+import { TimeoutError, BackoffMachine } from '../../utils/async';
 
 const mockStore = configureStore([thunk]);
 
@@ -176,6 +176,30 @@ describe('fetchActions', () => {
 
       jest.runAllTimers();
     });
+
+    test('times out after many short-duration 5xx errors', async () => {
+      const func = jest.fn(async () => {
+        await fakeSleep(50);
+        throw new ApiError(500, {
+          code: 'SOME_ERROR_CODE',
+          msg: 'Internal Server Error',
+          result: 'error',
+        });
+      });
+
+      await expect(tryFetch(func)).rejects.toThrow(TimeoutError);
+
+      expect(func.mock.calls.length).toBeGreaterThan(50);
+    });
+
+    test('times out after hanging on one request', async () => {
+      const tryFetchPromise = tryFetch(async () => {
+        await new Promise((resolve, reject) => {});
+      });
+
+      await fakeSleep(60000);
+      return expect(tryFetchPromise).rejects.toThrow(TimeoutError);
+    });
   });
 
   describe('fetchMessages', () => {

src/message/fetchActions.js

@@ -30,7 +30,7 @@ import {
 import { FIRST_UNREAD_ANCHOR, LAST_MESSAGE_ANCHOR } from '../anchor';
 import { showErrorAlert } from '../utils/info';
 import { ALL_PRIVATE_NARROW, apiNarrowOfNarrow } from '../utils/narrow';
-import { BackoffMachine } from '../utils/async';
+import { BackoffMachine, promiseTimeout, TimeoutError } from '../utils/async';
 import { initNotifications } from '../notification/notificationActions';
 import { addToOutbox, sendOutbox } from '../outbox/outboxActions';
 import { realmInit } from '../realm/realmActions';
@@ -181,8 +181,6 @@ const initialFetchAbortPlain = (reason: 'server' | 'network' | 'timeout'): Actio
   reason,
 });
 
-// This will be used in an upcoming commit.
-/* eslint-disable-next-line no-unused-vars */
 export const initialFetchAbort = (reason: 'server' | 'network' | 'timeout') => async (
   dispatch: Dispatch,
   getState: GetState,
@@ -304,31 +302,53 @@ const fetchPrivateMessages = () => async (dispatch: Dispatch, getState: GetState
 };
 
 /**
- * Makes a request that retries until success or a non-retryable error.
+ * Makes a request that retries until success or a non-retryable error, with
+ *   timeout.
  *
  * Waits between retries with a backoff.
  *
  * A non-retryable error will propagate to the caller to be handled.
+ *
+ * The timeout's length is `config.requestLongTimeoutMs` and it is absolute:
+ * it triggers after that time has elapsed no matter whether the time was
+ * spent waiting to hear back from one request, or retrying a request
+ * unsuccessfully many times (and the time spent in backoff is included in
+ * that).
  */
 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 (!isRetryable(e)) {
-        throw e;
-      }
-      await backoffMachine.wait();
+
+  // TODO: Use AbortController instead of this stateful flag; #4170
+  let timerHasExpired = false;
+
+  try {
+    return await promiseTimeout(
+      (async () => {
+        // eslint-disable-next-line no-constant-condition
+        while (true) {
+          if (timerHasExpired) {
+            // No one is listening for this Promise to settle, so stop
+            // doing more work.
+            throw new Error();
+          }
+          try {
+            return await func();
+          } catch (e) {
+            if (!isRetryable(e)) {
+              throw e;
+            }
+            await backoffMachine.wait();
+          }
+        }
+      })(),
+      config.requestLongTimeoutMs,
+    );
+  } catch (e) {
+    if (e instanceof TimeoutError) {
+      timerHasExpired = true;
     }
+    throw e;
   }
-
-  // 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();
 }
 
 /**
@@ -391,6 +411,8 @@ export const doInitialFetch = () => async (dispatch: Dispatch, getState: GetStat
       // This should only happen when `auth` is no longer valid. No
       // use retrying; just log out.
       dispatch(logout());
+    } else if (e instanceof TimeoutError) {
+      dispatch(initialFetchAbort('timeout'));
     } else {
       logging.warn(e, {
         message: 'Unexpected error during initial fetch and serverSettings fetch.',