Prep for timing out the initial fetch.

jest.config.js

@@ -24,34 +24,58 @@ const transformModulesWhitelist = [
 // (This value is correctly a string, not a RegExp.)
 const transformIgnorePattern = `node_modules/(?!${transformModulesWhitelist.join('|')})`;
 
-const common = {
-  // Finding and transforming source code.
-
-  testPathIgnorePatterns: ['/node_modules/', '/src/__tests__/lib/', '-testlib.js$'],
-
-  // When some source file foo.js says `import 'bar'`, Jest looks in the
-  // directories above foo.js for a directory like `node_modules` to find
-  // `bar` in.  If foo.js is behind a `yarn link` symlink and outside our
-  // tree, that won't work; so have it look at our node_modules too.
-  moduleDirectories: ['node_modules', '<rootDir>/node_modules'],
-
-  transform: {
-    '^.+\\.js$': '<rootDir>/node_modules/react-native/jest/preprocessor.js',
-  },
-  transformIgnorePatterns: [transformIgnorePattern],
-
-  // The runtime test environment.
-  globals: {
-    __TEST__: true,
-  },
-  setupFiles: ['./jest/globalFetch.js', './node_modules/react-native-gesture-handler/jestSetup.js'],
-  setupFilesAfterEnv: ['./jest/jestSetup.js', 'jest-extended'],
+const projectForPlatform = platform => {
+  if (!['ios', 'android'].includes(platform)) {
+    throw new Error(`Unsupported platform '${platform}'.`);
+  }
+  return {
+    displayName: platform,
+
+    // Ideally, these would simply be `jest-expo/ios` and
+    // `jest-expo/android`; see
+    // https://github.com/expo/expo/blob/master/packages/jest-expo/README.md#platforms.
+    // These custom presets are a workaround for a bug:
+    //
+    // `jest-expo`'s presets are based on `react-native`'s preset,
+    // which does something messy: it overwrites the global `Promise`.
+    // That's facebook/react-native#29303. Jest doesn't work well with
+    // that; that's facebook/jest#10221.
+    //
+    // So, until one of those is fixed, we use these custom presets to
+    // sandwich the code that replaces `global.Promise` with a fix:
+    //
+    // 1) save `global.Promise` to something else on `global`
+    // 2) let the `react-native` preset do its thing (like mocking RN
+    //    libraries)
+    // 3) assign `global.Promise` back to what we saved in step 1
+    preset: platform === 'ios' ? './jest/presetIos' : './jest/presetAndroid',
+
+    // Finding and transforming source code.
+    testPathIgnorePatterns: ['/node_modules/', '/src/__tests__/lib/', '-testlib.js$'],
+
+    // When some source file foo.js says `import 'bar'`, Jest looks in the
+    // directories above foo.js for a directory like `node_modules` to find
+    // `bar` in.  If foo.js is behind a `yarn link` symlink and outside our
+    // tree, that won't work; so have it look at our node_modules too.
+    moduleDirectories: ['node_modules', '<rootDir>/node_modules'],
+
+    transform: {
+      '^.+\\.js$': '<rootDir>/node_modules/react-native/jest/preprocessor.js',
+    },
+    transformIgnorePatterns: [transformIgnorePattern],
+
+    // The runtime test environment.
+    globals: {
+      __TEST__: true,
+    },
+    setupFiles: [
+      './jest/globalFetch.js',
+      './node_modules/react-native-gesture-handler/jestSetup.js',
+    ],
+    setupFilesAfterEnv: ['./jest/jestSetup.js', 'jest-extended'],
+  };
 };
 
 module.exports = {
-  // See https://github.com/expo/expo/blob/master/packages/jest-expo/README.md#platforms.
-  projects: [
-    { ...common, displayName: 'ios', preset: 'jest-expo/ios' },
-    { ...common, displayName: 'android', preset: 'jest-expo/android' },
-  ],
+  projects: [projectForPlatform('ios'), projectForPlatform('android')],
 };

jest/presetAndroid.js

@@ -0,0 +1,14 @@
+/**
+ * The preset we're making tweaks to.
+ */
+const basePreset = require('../node_modules/jest-expo/android/jest-preset.js');
+
+// See comment in our Jest config about how this is used.
+module.exports = {
+  ...basePreset,
+  setupFiles: [
+    require.resolve('./savePromise.js'),
+    ...basePreset.setupFiles,
+    require.resolve('./restorePromise.js'),
+  ],
+};

jest/presetIos.js

@@ -0,0 +1,14 @@
+/**
+ * The preset we're making tweaks to.
+ */
+const basePreset = require('../node_modules/jest-expo/ios/jest-preset.js');
+
+// See comment in our Jest config about how this is used.
+module.exports = {
+  ...basePreset,
+  setupFiles: [
+    require.resolve('./savePromise.js'),
+    ...basePreset.setupFiles,
+    require.resolve('./restorePromise.js'),
+  ],
+};

jest/restorePromise.js

@@ -0,0 +1,3 @@
+// For facebook/jest#10221. After savePromise.js and Jest's setup
+// files have run, restore the natural value of `global.Promise`.
+global.Promise = global.originalPromise;

jest/savePromise.js

@@ -0,0 +1,4 @@
+// For facebook/jest#10221. Before Jest's setup files have run, take
+// note of what the natural value of `global.Promise` is, so we can
+// restore it in restorePromise.js.
+global.originalPromise = Promise;

src/message/__tests__/fetchActions-test.js

@@ -18,6 +18,9 @@ import type { ServerMessage } from '../../api/messages/getMessages';
 import { streamNarrow, HOME_NARROW, HOME_NARROW_STR, keyFromNarrow } from '../../utils/narrow';
 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';
 
 const mockStore = configureStore([thunk]);
 
@@ -26,15 +29,20 @@ const streamNarrowStr = keyFromNarrow(narrow);
 
 global.FormData = class FormData {};
 
+const BORING_RESPONSE = JSON.stringify({
+  messages: [],
+  result: 'success',
+});
+
 describe('fetchActions', () => {
   afterEach(() => {
     fetch.reset();
   });
 
   describe('isFetchNeededAtAnchor', () => {
     test("false if we're caught up, even if there are no messages", () => {
-      const state = eg.reduxState({
-        session: { ...eg.baseReduxState.session, isHydrated: true },
+      const state = eg.reduxStatePlus({
+        session: { ...eg.plusReduxState.session, isHydrated: true },
         caughtUp: {
           [streamNarrowStr]: {
             newer: true,
@@ -55,8 +63,8 @@ describe('fetchActions', () => {
       const message1 = eg.streamMessage({ id: 1 });
       const message2 = eg.streamMessage({ id: 2 });
 
-      const state = eg.reduxState({
-        session: { ...eg.baseReduxState.session, isHydrated: true },
+      const state = eg.reduxStatePlus({
+        session: { ...eg.plusReduxState.session, isHydrated: true },
         caughtUp: {
           [streamNarrowStr]: {
             newer: false,
@@ -76,34 +84,87 @@ describe('fetchActions', () => {
   });
 
   describe('tryFetch', () => {
-    test('resolves any value when there is no exception', async () => {
-      const result = await tryFetch(async () => 'hello');
+    beforeAll(() => {
+      jest.useFakeTimers('modern');
+
+      // So we don't have to think about the (random, with jitter)
+      // duration of these waits in these tests. `BackoffMachine` has
+      // its own unit tests already, so we don't have to test that it
+      // waits for the right amount of time.
+      // $FlowFixMe[cannot-write]
+      BackoffMachine.prototype.wait = async function wait() {
+        return fakeSleep(100);
+      };
+    });
 
-      expect(result).toEqual('hello');
+    afterEach(() => {
+      expect(jest.getTimerCount()).toBe(0);
+      jest.clearAllTimers();
     });
 
     test('resolves any promise, if there is no exception', async () => {
-      const result = await tryFetch(
-        () => new Promise(resolve => setTimeout(() => resolve('hello'), 100)),
-      );
+      const tryFetchFunc = jest.fn(async () => {
+        await fakeSleep(10);
+        return 'hello';
+      });
+
+      await expect(tryFetch(tryFetchFunc)).resolves.toBe('hello');
 
-      expect(result).toEqual('hello');
+      expect(tryFetchFunc).toHaveBeenCalledTimes(1);
+      await expect(tryFetchFunc.mock.results[0].value).resolves.toBe('hello');
+
+      jest.runAllTimers();
     });
 
-    test('retries a call, if there is an exception', async () => {
+    // TODO: test more errors, like regular `new Error()`s. Unexpected
+    // errors should actually cause the retry loop to break; we'll fix
+    // that soon.
+    test('retries a call if there is a non-client error', async () => {
+      const serverError = new ApiError(500, {
+        code: 'SOME_ERROR_CODE',
+        msg: 'Internal Server Error',
+        result: 'error',
+      });
+
       // fail on first call, succeed second time
       let callCount = 0;
-      const thrower = () => {
+      const thrower = jest.fn(() => {
         callCount++;
         if (callCount === 1) {
-          throw new Error('First run exception');
+          throw serverError;
         }
         return 'hello';
-      };
+      });
+
+      const tryFetchFunc = jest.fn(async () => {
+        await fakeSleep(10);
+        return thrower();
+      });
+
+      await expect(tryFetch(tryFetchFunc)).resolves.toBe('hello');
 
-      const result = await tryFetch(async () => thrower());
+      expect(tryFetchFunc).toHaveBeenCalledTimes(2);
+      await expect(tryFetchFunc.mock.results[0].value).rejects.toThrow(serverError);
+      await expect(tryFetchFunc.mock.results[1].value).resolves.toBe('hello');
 
-      expect(result).toEqual('hello');
+      jest.runAllTimers();
+    });
+
+    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(async () => {
+        throw apiError;
+      });
+
+      await expect(tryFetch(func)).rejects.toThrow(apiError);
+      expect(func).toHaveBeenCalledTimes(1);
+
+      jest.runAllTimers();
     });
   });
 
@@ -127,9 +188,7 @@ describe('fetchActions', () => {
       avatar_url: null, // Null in server data will be transformed to a GravatarURL
     };
 
-    const baseState = eg.reduxState({
-      accounts: [eg.makeAccount()],
-      realm: { ...eg.baseReduxState.realm, user_id: eg.selfUser.user_id },
+    const baseState = eg.reduxStatePlus({
       narrows: Immutable.Map({
         [streamNarrowStr]: [message1.id],
       }),
@@ -287,16 +346,9 @@ describe('fetchActions', () => {
       });
     });
 
-    const BORING_RESPONSE = JSON.stringify({
-      messages: [],
-      result: 'success',
-    });
-
     test('when messages to be fetched both before and after anchor, numBefore and numAfter are greater than zero', async () => {
       const store = mockStore<GlobalState, Action>(
-        eg.reduxState({
-          accounts: [eg.selfAccount],
-          realm: { ...eg.baseReduxState.realm, user_id: eg.selfUser.user_id },
+        eg.reduxStatePlus({
           narrows: Immutable.Map({
             [streamNarrowStr]: [1],
           }),
@@ -320,9 +372,7 @@ describe('fetchActions', () => {
 
     test('when no messages to be fetched before the anchor, numBefore is not greater than zero', async () => {
       const store = mockStore<GlobalState, Action>(
-        eg.reduxState({
-          accounts: [eg.selfAccount],
-          realm: { ...eg.baseReduxState.realm, user_id: eg.selfUser.user_id },
+        eg.reduxStatePlus({
           narrows: Immutable.Map({
             [streamNarrowStr]: [1],
           }),
@@ -345,9 +395,7 @@ describe('fetchActions', () => {
 
     test('when no messages to be fetched after the anchor, numAfter is not greater than zero', async () => {
       const store = mockStore<GlobalState, Action>(
-        eg.reduxState({
-          accounts: [eg.selfAccount],
-          realm: { ...eg.baseReduxState.realm, user_id: eg.selfUser.user_id },
+        eg.reduxStatePlus({
           narrows: Immutable.Map({
             [streamNarrowStr]: [1],
           }),
@@ -373,9 +421,8 @@ describe('fetchActions', () => {
     const message1 = eg.streamMessage({ id: 1 });
     const message2 = eg.streamMessage({ id: 2 });
 
-    const baseState = eg.reduxState({
-      accounts: [eg.selfAccount],
-      narrows: eg.baseReduxState.narrows.merge(
+    const baseState = eg.reduxStatePlus({
+      narrows: eg.plusReduxState.narrows.merge(
         Immutable.Map({
           [streamNarrowStr]: [message2.id],
           [HOME_NARROW_STR]: [message1.id, message2.id],
@@ -384,6 +431,10 @@ describe('fetchActions', () => {
       messages: eg.makeMessagesState([message1, message2]),
     });
 
+    beforeEach(() => {
+      fetch.mockResponseSuccess(BORING_RESPONSE);
+    });
+
     test('message fetch start action is dispatched with numBefore greater than zero', async () => {
       const store = mockStore<GlobalState, Action>({
         ...baseState,
@@ -462,9 +513,8 @@ describe('fetchActions', () => {
     const message1 = eg.streamMessage({ id: 1 });
     const message2 = eg.streamMessage({ id: 2 });
 
-    const baseState = eg.reduxState({
-      accounts: [eg.selfAccount],
-      narrows: eg.baseReduxState.narrows.merge(
+    const baseState = eg.reduxStatePlus({
+      narrows: eg.plusReduxState.narrows.merge(
         Immutable.Map({
           [streamNarrowStr]: [message2.id],
           [HOME_NARROW_STR]: [message1.id, message2.id],
@@ -473,6 +523,10 @@ describe('fetchActions', () => {
       messages: eg.makeMessagesState([message1, message2]),
     });
 
+    beforeEach(() => {
+      fetch.mockResponseSuccess(BORING_RESPONSE);
+    });
+
     test('message fetch start action is dispatched with numAfter greater than zero', async () => {
       const store = mockStore<GlobalState, Action>({
         ...baseState,

src/message/fetchActions.js

@@ -265,6 +265,9 @@ export async function tryFetch<T>(func: () => Promise<T>): Promise<T> {
     try {
       return await func();
     } catch (e) {
+      // TODO: This should be narrowed to `!isServerError(e)`; we
+      // should fail early if we encounter unrecognized / unexpected
+      // errors.
       if (isClientError(e)) {
         throw e;
       }