diff --git a/src/message/__tests__/fetchActions-test.js b/src/message/__tests__/fetchActions-test.js index 08fb9ab0f0d..30b3034243c 100644 --- a/src/message/__tests__/fetchActions-test.js +++ b/src/message/__tests__/fetchActions-test.js @@ -22,7 +22,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]); @@ -140,6 +140,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', () => { diff --git a/src/message/fetchActions.js b/src/message/fetchActions.js index acc15b339ba..efddce93243 100644 --- a/src/message/fetchActions.js +++ b/src/message/fetchActions.js @@ -26,7 +26,8 @@ import { } from '../actionConstants'; import { FIRST_UNREAD_ANCHOR, LAST_MESSAGE_ANCHOR } from '../anchor'; import { ALL_PRIVATE_NARROW, apiNarrowOfNarrow } from '../utils/narrow'; -import { BackoffMachine } 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'; @@ -192,8 +193,6 @@ const initialFetchAbortPlain = (): Action => ({ type: INITIAL_FETCH_ABORT, }); -// This will be used in an upcoming commit. -/* eslint-disable-next-line no-unused-vars */ export const initialFetchAbort = () => async (dispatch: Dispatch, getState: GetState) => { NavigationService.dispatch(resetToAccountPicker()); dispatch(initialFetchAbortPlain()); @@ -288,26 +287,46 @@ const fetchPrivateMessages = () => async (dispatch: Dispatch, getState: GetState * 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(func: () => Promise): Promise { + const MAX_TIME_MS: number = 60000; 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 could be returned, - // which is inconsistent with the stated Promise return type. - // eslint-disable-next-line no-unreachable - throw new Error(); + // 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 could be returned, + // which is inconsistent with the stated Promise return type. + // eslint-disable-next-line no-unreachable + throw new Error(); + })(), + MAX_TIME_MS, + () => { + timerHasExpired = true; + throw new TimeoutError(); + }, + ); } /** @@ -356,9 +375,15 @@ export const doInitialFetch = () => async (dispatch: Dispatch, getState: GetStat 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; }