Time out initial fetch, and go to account-picker screen

src/__tests__/lib/lolex.js

@@ -86,6 +86,18 @@ export class Lolex {
     this._clock.runToLast();
   }
 
+  async runOnlyPendingTimersAsync(): Promise<void> {
+    this._clock.runToLastAsync();
+  }
+
+  runAllTimers(): void {
+    this._clock.runAll();
+  }
+
+  async runAllTimersAsync(): Promise<void> {
+    this._clock.runAllAsync();
+  }
+
   advanceTimersByTime(msToRun: number): void {
     this._clock.tick(msToRun);
   }

src/actionConstants.js

@@ -20,6 +20,7 @@ export const DO_NARROW: 'DO_NARROW' = 'DO_NARROW';
 export const INIT_SAFE_AREA_INSETS: 'INIT_SAFE_AREA_INSETS' = 'INIT_SAFE_AREA_INSETS';
 export const INITIAL_FETCH_START: 'INITIAL_FETCH_START' = 'INITIAL_FETCH_START';
 export const INITIAL_FETCH_COMPLETE: 'INITIAL_FETCH_COMPLETE' = 'INITIAL_FETCH_COMPLETE';
+export const INITIAL_FETCH_ABORT: 'INITIAL_FETCH_ABORT' = 'INITIAL_FETCH_ABORT';
 export const INIT_TOPICS: 'INIT_TOPICS' = 'INIT_TOPICS';
 
 export const EVENT: 'EVENT' = 'EVENT';

src/actionTypes.js

@@ -19,6 +19,7 @@ import {
   MESSAGE_FETCH_COMPLETE,
   INITIAL_FETCH_START,
   INITIAL_FETCH_COMPLETE,
+  INITIAL_FETCH_ABORT,
   SETTINGS_CHANGE,
   DRAFT_UPDATE,
   DO_NARROW,
@@ -212,6 +213,10 @@ type InitialFetchCompleteAction = {|
   type: typeof INITIAL_FETCH_COMPLETE,
 |};
 
+type InitialFetchAbortAction = {|
+  type: typeof INITIAL_FETCH_ABORT,
+|};
+
 type ServerEvent = {|
   id: number,
 |};
@@ -568,7 +573,11 @@ type AccountAction =
   | LoginSuccessAction
   | LogoutAction;
 
-type LoadingAction = DeadQueueAction | InitialFetchStartAction | InitialFetchCompleteAction;
+type LoadingAction =
+  | DeadQueueAction
+  | InitialFetchStartAction
+  | InitialFetchCompleteAction
+  | InitialFetchAbortAction;
 
 type MessageAction = MessageFetchStartAction | MessageFetchCompleteAction;
 

src/message/__tests__/fetchActions-test.js

@@ -1,8 +1,11 @@
 import mockStore from 'redux-mock-store'; // eslint-disable-line
 
-import { fetchMessages, fetchOlder, fetchNewer } from '../fetchActions';
+import { Lolex } from '../../__tests__/lib/lolex';
+import { fetchMessages, fetchOlder, fetchNewer, tryFetch } from '../fetchActions';
 import { streamNarrow, HOME_NARROW, HOME_NARROW_STR } from '../../utils/narrow';
 import { navStateWithNarrow } from '../../utils/testHelpers';
+import { ApiError } from '../../api/apiErrors';
+import { TimeoutError } from '../../utils/async';
 
 const narrow = streamNarrow('some stream');
 const streamNarrowStr = JSON.stringify(narrow);
@@ -14,6 +17,79 @@ describe('fetchActions', () => {
     fetch.reset();
   });
 
+  describe('tryFetch', () => {
+    const lolex = new Lolex();
+    afterAll(() => {
+      lolex.dispose();
+    });
+
+    afterEach(() => {
+      // clear any unset timers
+      lolex.clearAllTimers();
+    });
+
+    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 tryFetchPromise = tryFetch(async () => {
+        await new Promise(r => setTimeout(r, 10));
+        return thrower();
+      });
+      await lolex.runAllTimersAsync();
+      const result = await tryFetchPromise;
+
+      expect(result).toEqual('hello');
+    });
+
+    test('Rethrows a 4xx error without retrying', async () => {
+      const apiError = new ApiError(400, {
+        code: 'BAD_REQUEST',
+        msg: 'Bad Request',
+        result: 'error',
+      });
+
+      const func = jest.fn().mockImplementation(async () => {
+        throw apiError;
+      });
+      const tryFetchPromise = tryFetch(func);
+
+      await lolex.runAllTimersAsync();
+      expect(func).toHaveBeenCalledTimes(1);
+      return expect(tryFetchPromise).rejects.toThrow(apiError);
+    });
+
+    test('times out after many short-duration 5xx errors', async () => {
+      const tryFetchPromise = tryFetch(async () => {
+        await new Promise(r => setTimeout(r, 50));
+        throw new ApiError(500, {
+          code: 'SOME_ERROR_CODE',
+          msg: 'Internal Server Error',
+          result: 'error',
+        });
+      });
+
+      await lolex.runAllTimersAsync();
+      return expect(tryFetchPromise).rejects.toThrow(TimeoutError);
+    });
+
+    test('times out after hanging on one request', async () => {
+      const tryFetchPromise = tryFetch(async () => {
+        await new Promise((resolve, reject) => {});
+      });
+
+      await lolex.runAllTimersAsync();
+      return expect(tryFetchPromise).rejects.toThrow(TimeoutError);
+    });
+  });
+
   describe('fetchMessages', () => {
     test('message fetch success action is dispatched after successful fetch', async () => {
       const store = mockStore({

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,
@@ -23,12 +24,14 @@ import config from '../config';
 import {
   INITIAL_FETCH_START,
   INITIAL_FETCH_COMPLETE,
+  INITIAL_FETCH_ABORT,
   MESSAGE_FETCH_START,
   MESSAGE_FETCH_COMPLETE,
 } 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, promiseTimeout, TimeoutError } from '../utils/async';
+import * as logging from '../utils/logging';
 import { initNotifications } from '../notification/notificationActions';
 import { addToOutbox, sendOutbox } from '../outbox/outboxActions';
 import { realmInit } from '../realm/realmActions';
@@ -126,6 +129,10 @@ const initialFetchComplete = (): Action => ({
   type: INITIAL_FETCH_COMPLETE,
 });
 
+const initialFetchAbort = (): Action => ({
+  type: INITIAL_FETCH_ABORT,
+});
+
 const isFetchNeededAtAnchor = (state: GlobalState, narrow: Narrow, anchor: number): boolean => {
   // Ideally this would detect whether, even if we don't have *all* the
   // messages in the narrow, we have enough of them around the anchor
@@ -178,14 +185,12 @@ export const fetchMessagesInNarrow = (
  */
 const fetchPrivateMessages = () => async (dispatch: Dispatch, getState: GetState) => {
   const auth = getAuth(getState());
-  const { messages, found_newest, found_oldest } = await tryUntilSuccessful(() =>
-    api.getMessages(auth, {
-      narrow: ALL_PRIVATE_NARROW,
-      anchor: LAST_MESSAGE_ANCHOR,
-      numBefore: 100,
-      numAfter: 0,
-    }),
-  );
+  const { messages, found_newest, found_oldest } = await api.getMessages(auth, {
+    narrow: ALL_PRIVATE_NARROW,
+    anchor: LAST_MESSAGE_ANCHOR,
+    numBefore: 100,
+    numAfter: 0,
+  });
   dispatch(
     messageFetchComplete({
       messages,
@@ -222,6 +227,54 @@ const fetchTopMostNarrow = () => 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.
+ *
+ * After a certain duration, times out with a TimeoutError.
+ */
+export async function tryFetch<T>(func: () => Promise<T>): Promise<T> {
+  const MAX_TIME_MS: number = 60000;
+  const backoffMachine = new BackoffMachine();
+
+  // TODO: Use AbortController instead of this stateful flag; #4170
+  let timerHasExpired = false;
+
+  return promiseTimeout(
+    (async () => {
+      // eslint-disable-next-line no-constant-condition
+      while (true) {
+        if (timerHasExpired) {
+          // No one is listening for this Promise's outcome, so stop
+          // doing more work.
+          throw new Error();
+        }
+        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();
+    })(),
+    MAX_TIME_MS,
+    () => {
+      timerHasExpired = true;
+      throw new TimeoutError();
+    },
+  );
+}
+
 /**
  * Fetch lots of state from the server, and start an event queue.
  *
@@ -245,7 +298,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,
@@ -257,12 +310,18 @@ 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
-    // happen when `auth` is no longer valid.  No use retrying; just log out.
-    dispatch(logout());
+    if (isClientError(e)) {
+      dispatch(logout());
+    } else if (e instanceof TimeoutError) {
+      dispatch(initialFetchAbort());
+    } else {
+      logging.warn(e, {
+        message: 'Unexpected error during initial fetch and serverSettings fetch.',
+      });
+    }
     return;
   }
 

src/nav/__tests__/navReducer-test.js

@@ -1,6 +1,11 @@
 import deepFreeze from 'deep-freeze';
 
-import { LOGIN_SUCCESS, INITIAL_FETCH_COMPLETE, REHYDRATE } from '../../actionConstants';
+import {
+  LOGIN_SUCCESS,
+  INITIAL_FETCH_COMPLETE,
+  INITIAL_FETCH_ABORT,
+  REHYDRATE,
+} from '../../actionConstants';
 import navReducer, { getStateForRoute } from '../navReducer';
 import { NULL_OBJECT } from '../../nullObjects';
 
@@ -42,6 +47,26 @@ describe('navReducer', () => {
     });
   });
 
+  describe('INITIAL_FETCH_ABORT', () => {
+    test('navigate to account screen', () => {
+      const prevState = getStateForRoute('main');
+
+      const action = deepFreeze({
+        type: INITIAL_FETCH_ABORT,
+      });
+
+      const expectedState = {
+        index: 0,
+        routes: [{ routeName: 'account' }],
+      };
+
+      const newState = navReducer(prevState, action);
+
+      expect(newState.index).toEqual(expectedState.index);
+      expect(newState.routes[0].routeName).toEqual(expectedState.routes[0].routeName);
+    });
+  });
+
   describe('REHYDRATE', () => {
     test('when no previous navigation is given do not throw but return some result', () => {
       const initialState = NULL_OBJECT;
@@ -60,7 +85,7 @@ describe('navReducer', () => {
       expect(nav.routes).toHaveLength(1);
     });
 
-    test('if logged in, go to main screen', () => {
+    test('if logged in and users is empty, go to loading', () => {
       const initialState = NULL_OBJECT;
 
       const action = deepFreeze({
@@ -74,6 +99,24 @@ describe('navReducer', () => {
 
       const nav = navReducer(initialState, action);
 
+      expect(nav.routes).toHaveLength(1);
+      expect(nav.routes[0].routeName).toEqual('loading');
+    });
+
+    test('if logged in and users is not empty, go to main', () => {
+      const initialState = NULL_OBJECT;
+
+      const action = deepFreeze({
+        type: REHYDRATE,
+        payload: {
+          accounts: [{ apiKey: '123' }],
+          users: [{ user_id: 123 }],
+          realm: {},
+        },
+      });
+
+      const nav = navReducer(initialState, action);
+
       expect(nav.routes).toHaveLength(1);
       expect(nav.routes[0].routeName).toEqual('main');
     });

src/nav/navReducer.js

@@ -6,6 +6,7 @@ import AppNavigator from './AppNavigator';
 import {
   REHYDRATE,
   INITIAL_FETCH_COMPLETE,
+  INITIAL_FETCH_ABORT,
   ACCOUNT_SWITCH,
   LOGIN_SUCCESS,
   LOGOUT,
@@ -60,7 +61,7 @@ const rehydrate = (state, action) => {
   // will have set `needsInitialFetch`, too, so we really will be loading.
   //
   // (Valid server data must have a user: the self user, at a minimum.)
-  if (rehydratedState.users === undefined) {
+  if (rehydratedState.users === undefined || rehydratedState.users.length === 0) {
     return getStateForRoute('loading');
   }
 
@@ -83,6 +84,7 @@ export default (state: NavigationState = initialState, action: Action): Navigati
     case INITIAL_FETCH_COMPLETE:
       return state.routes[0].routeName === 'main' ? state : getStateForRoute('main');
 
+    case INITIAL_FETCH_ABORT:
     case LOGOUT:
       return getStateForRoute('account');