diff --git a/src/actionConstants.js b/src/actionConstants.js index 785c59eb743..f2d82a5970b 100644 --- a/src/actionConstants.js +++ b/src/actionConstants.js @@ -15,6 +15,7 @@ export const REALM_INIT: 'REALM_INIT' = 'REALM_INIT'; 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'; diff --git a/src/actionTypes.js b/src/actionTypes.js index 1f03382c60d..8cbce7cc064 100644 --- a/src/actionTypes.js +++ b/src/actionTypes.js @@ -18,6 +18,7 @@ import { MESSAGE_FETCH_COMPLETE, INITIAL_FETCH_START, INITIAL_FETCH_COMPLETE, + INITIAL_FETCH_ABORT, SETTINGS_CHANGE, DRAFT_UPDATE, PRESENCE_RESPONSE, @@ -194,7 +195,7 @@ type MessageFetchStartAction = {| /** * Any unexpected failure in a message fetch. * - * Includes internal server errors and any errors we throw when + * Includes request timeout errors and any errors we throw when * validating and reshaping the server data at the edge. * * In an ideal crunchy-shell world [1], none of these will be thrown @@ -235,6 +236,19 @@ type InitialFetchCompleteAction = {| type: typeof INITIAL_FETCH_COMPLETE, |}; +export type InitialFetchAbortReason = 'server' | 'network' | 'timeout' | 'unexpected'; + +/** + * Notify Redux that we've given up on the initial fetch. + * + * Not for unrecoverable errors, like ApiErrors, which indicate that we + * tried and failed, not that we gave up trying. + */ +type InitialFetchAbortAction = {| + type: typeof INITIAL_FETCH_ABORT, + reason: InitialFetchAbortReason, +|}; + type ServerEvent = {| id: number, |}; @@ -602,7 +616,11 @@ type InitTopicsAction = {| type AccountAction = AccountSwitchAction | AccountRemoveAction | LoginSuccessAction | LogoutAction; -type LoadingAction = DeadQueueAction | InitialFetchStartAction | InitialFetchCompleteAction; +type LoadingAction = + | DeadQueueAction + | InitialFetchStartAction + | InitialFetchCompleteAction + | InitialFetchAbortAction; type MessageAction = MessageFetchStartAction | MessageFetchErrorAction | MessageFetchCompleteAction; diff --git a/src/chat/FetchError.js b/src/chat/FetchError.js index c8268c095e7..5c554bff084 100644 --- a/src/chat/FetchError.js +++ b/src/chat/FetchError.js @@ -5,6 +5,7 @@ import { StyleSheet, View } from 'react-native'; import type { Narrow } from '../types'; import { Label } from '../common'; +import { TimeoutError } from '../utils/async'; const styles = StyleSheet.create({ container: { @@ -28,7 +29,13 @@ export default class FetchError extends PureComponent { render() { return ( - ); } diff --git a/src/config.js b/src/config.js index e0b6d28ad95..d4c34613e0f 100644 --- a/src/config.js +++ b/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, diff --git a/src/message/__tests__/fetchActions-test.js b/src/message/__tests__/fetchActions-test.js index 1cc8492d78d..8c9c047d61c 100644 --- a/src/message/__tests__/fetchActions-test.js +++ b/src/message/__tests__/fetchActions-test.js @@ -19,9 +19,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, Server5xxError } from '../../api/apiErrors'; +import { Server5xxError, NetworkError } from '../../api/apiErrors'; import { fakeSleep } from '../../__tests__/lib/fakeTimers'; -import { BackoffMachine } from '../../utils/async'; +import { TimeoutError, BackoffMachine } from '../../utils/async'; import * as logging from '../../utils/logging'; const mockStore = configureStore([thunk]); @@ -104,65 +104,129 @@ describe('fetchActions', () => { jest.clearAllTimers(); }); - test('resolves any promise, if there is no exception', async () => { - const tryFetchFunc = jest.fn(async () => { - await fakeSleep(10); - return 'hello'; - }); + describe.each([false, true])( + 'whether or not asked to retry; was asked this time? %s', + withRetry => { + test('resolves any promise, if there is no exception', async () => { + const tryFetchFunc = jest.fn(async () => { + await fakeSleep(10); + return 'hello'; + }); - await expect(tryFetch(tryFetchFunc)).resolves.toBe('hello'); + await expect(tryFetch(tryFetchFunc, withRetry)).resolves.toBe('hello'); - expect(tryFetchFunc).toHaveBeenCalledTimes(1); - await expect(tryFetchFunc.mock.results[0].value).resolves.toBe('hello'); + expect(tryFetchFunc).toHaveBeenCalledTimes(1); + await expect(tryFetchFunc.mock.results[0].value).resolves.toBe('hello'); - jest.runAllTimers(); - }); + jest.runAllTimers(); + }); - // 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 Server5xxError(500); - - // fail on first call, succeed second time - let callCount = 0; - const thrower = jest.fn(() => { - callCount++; - if (callCount === 1) { - throw serverError; - } - return 'hello'; - }); + test('Rethrows an unexpected error without retrying', async () => { + const unexpectedError = new Error('You have displaced the mirth.'); + + const func = jest.fn(async () => { + throw unexpectedError; + }); + + await expect(tryFetch(func, withRetry)).rejects.toThrow(unexpectedError); + expect(func).toHaveBeenCalledTimes(1); + + jest.runAllTimers(); + }); + + test('times out after hanging on one request', async () => { + const tryFetchPromise = tryFetch(async () => { + await new Promise((resolve, reject) => {}); + }, withRetry); + + await fakeSleep(60000); + return expect(tryFetchPromise).rejects.toThrow(TimeoutError); + }); + }, + ); + + describe('if asked to retry', () => { + test('retries a call if there is a recoverable error', async () => { + const serverError = new Server5xxError(500); + + // fail on first call, succeed second time + let callCount = 0; + const thrower = jest.fn(() => { + callCount++; + if (callCount === 1) { + throw serverError; + } + return 'hello'; + }); + + const tryFetchFunc = jest.fn(async () => { + await fakeSleep(10); + return thrower(); + }); + + await expect(tryFetch(tryFetchFunc, true)).resolves.toBe('hello'); + + expect(tryFetchFunc).toHaveBeenCalledTimes(2); + await expect(tryFetchFunc.mock.results[0].value).rejects.toThrow(serverError); + await expect(tryFetchFunc.mock.results[1].value).resolves.toBe('hello'); - const tryFetchFunc = jest.fn(async () => { - await fakeSleep(10); - return thrower(); + jest.runAllTimers(); }); - await expect(tryFetch(tryFetchFunc)).resolves.toBe('hello'); + test('retries a call if there is a network error', async () => { + const networkError = new NetworkError(); + + // fail on first call, succeed second time + let callCount = 0; + const thrower = jest.fn(() => { + callCount++; + if (callCount === 1) { + throw networkError; + } + return 'hello'; + }); - expect(tryFetchFunc).toHaveBeenCalledTimes(2); - await expect(tryFetchFunc.mock.results[0].value).rejects.toThrow(serverError); - await expect(tryFetchFunc.mock.results[1].value).resolves.toBe('hello'); + const tryFetchFunc = jest.fn(async () => { + await fakeSleep(10); + return thrower(); + }); - jest.runAllTimers(); - }); + await expect(tryFetch(tryFetchFunc)).resolves.toBe('hello'); - test('Rethrows a 4xx error without retrying', async () => { - const apiError = new ApiError(400, { - code: 'BAD_REQUEST', - msg: 'Bad Request', - result: 'error', + expect(tryFetchFunc).toHaveBeenCalledTimes(2); + await expect(tryFetchFunc.mock.results[0].value).rejects.toThrow(networkError); + await expect(tryFetchFunc.mock.results[1].value).resolves.toBe('hello'); + + jest.runAllTimers(); }); - const func = jest.fn(async () => { - throw apiError; + test('times out after many short-duration 5xx errors', async () => { + const func = jest.fn(async () => { + await fakeSleep(50); + throw new Server5xxError(500); + }); + + await expect(tryFetch(func, true)).rejects.toThrow(TimeoutError); + + expect(func.mock.calls.length).toBeGreaterThan(50); }); + }); - await expect(tryFetch(func)).rejects.toThrow(apiError); - expect(func).toHaveBeenCalledTimes(1); + describe('if not asked to retry', () => { + test('does not retry a call if there is a server error', async () => { + const serverError = new Server5xxError(500); - jest.runAllTimers(); + const tryFetchFunc = jest.fn(async () => { + await fakeSleep(10); + throw serverError; + }); + + await expect(tryFetch(tryFetchFunc, false)).rejects.toThrow(serverError); + + expect(tryFetchFunc).toHaveBeenCalledTimes(1); + + jest.runAllTimers(); + }); }); }); @@ -245,6 +309,12 @@ describe('fetchActions', () => { }); describe('failure', () => { + beforeAll(() => { + // suppress `logging.warn` output + // $FlowFixMe[prop-missing]: Jest mock + logging.warn.mockReturnValue(); + }); + test('rejects when user is not logged in, dispatches MESSAGE_FETCH_ERROR', async () => { const stateWithoutAccount = { ...baseState, diff --git a/src/message/fetchActions.js b/src/message/fetchActions.js index 1d8b47639e5..e7c048c575d 100644 --- a/src/message/fetchActions.js +++ b/src/message/fetchActions.js @@ -1,9 +1,14 @@ /* @flow strict-local */ +import * as logging from '../utils/logging'; +import * as NavigationService from '../nav/NavigationService'; import type { Narrow, Dispatch, GetState, GlobalState, Message, Action, UserId } from '../types'; +import { ensureUnreachable } from '../types'; +import type { InitialFetchAbortReason } from '../actionTypes'; import type { ApiResponseServerSettings } from '../api/settings/getServerSettings'; import type { InitialData } from '../api/initialDataTypes'; import * as api from '../api'; -import { ApiError } from '../api/apiErrors'; +import { ApiError, Server5xxError, NetworkError } from '../api/apiErrors'; +import { resetToAccountPicker } from '../actions'; import { getAuth, getSession, @@ -11,25 +16,29 @@ import { getLastMessageId, getCaughtUpForNarrow, getFetchingForNarrow, + getIsAdmin, + getActiveAccount, } from '../selectors'; import config from '../config'; import { INITIAL_FETCH_START, INITIAL_FETCH_COMPLETE, + INITIAL_FETCH_ABORT, MESSAGE_FETCH_START, MESSAGE_FETCH_ERROR, MESSAGE_FETCH_COMPLETE, } 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 { showErrorAlert } from '../utils/info'; +import { ALL_PRIVATE_NARROW, apiNarrowOfNarrow, caseNarrow } from '../utils/narrow'; +import { BackoffMachine, promiseTimeout, TimeoutError } from '../utils/async'; import { initNotifications } from '../notification/notificationActions'; import { addToOutbox, sendOutbox } from '../outbox/outboxActions'; import { realmInit } from '../realm/realmActions'; import { startEventPolling } from '../events/eventActions'; import { logout } from '../account/accountActions'; import { ZulipVersion } from '../utils/zulipVersion'; -import { getAllUsersById, getOwnUserId } from '../users/userSelectors'; +import { getAllUsersById, getHaveServerData, getOwnUserId } from '../users/userSelectors'; import { MIN_RECENTPMS_SERVER_VERSION } from '../pm-conversations/pmConversationsModel'; const messageFetchStart = (narrow: Narrow, numBefore: number, numAfter: number): Action => ({ @@ -96,11 +105,17 @@ export const fetchMessages = (fetchArgs: {| |}) => async (dispatch: Dispatch, getState: GetState): Promise => { dispatch(messageFetchStart(fetchArgs.narrow, fetchArgs.numBefore, fetchArgs.numAfter)); try { - const { messages, found_newest, found_oldest } = await api.getMessages(getAuth(getState()), { - ...fetchArgs, - narrow: apiNarrowOfNarrow(fetchArgs.narrow, getAllUsersById(getState())), - useFirstUnread: fetchArgs.anchor === FIRST_UNREAD_ANCHOR, // TODO: don't use this; see #4203 - }); + const { messages, found_newest, found_oldest } = + // TODO: If `MESSAGE_FETCH_ERROR` isn't the right way to respond + // to a timeout, maybe make a new action. + // eslint-disable-next-line no-use-before-define + await tryFetch(() => + api.getMessages(getAuth(getState()), { + ...fetchArgs, + narrow: apiNarrowOfNarrow(fetchArgs.narrow, getAllUsersById(getState())), + useFirstUnread: fetchArgs.anchor === FIRST_UNREAD_ANCHOR, // TODO: don't use this; see #4203 + }), + ); dispatch( messageFetchComplete({ ...fetchArgs, @@ -118,6 +133,25 @@ export const fetchMessages = (fetchArgs: {| error: e, }), ); + logging.warn(e, { + message: 'Message-fetch error', + + // Describe the narrow without sending sensitive data to Sentry. + narrow: caseNarrow(fetchArgs.narrow, { + stream: name => 'stream', + topic: (streamName, topic) => 'topic', + pm: ids => (ids.length > 1 ? 'pm (group)' : 'pm (1:1)'), + home: () => 'all', + starred: () => 'starred', + mentioned: () => 'mentioned', + allPrivate: () => 'all-pm', + search: query => 'search', + }), + + anchor: fetchArgs.anchor, + numBefore: fetchArgs.numBefore, + numAfter: fetchArgs.numAfter, + }); throw e; } }; @@ -168,6 +202,45 @@ const initialFetchComplete = (): Action => ({ type: INITIAL_FETCH_COMPLETE, }); +const initialFetchAbortPlain = (reason: InitialFetchAbortReason): Action => ({ + type: INITIAL_FETCH_ABORT, + reason, +}); + +export const initialFetchAbort = (reason: InitialFetchAbortReason) => async ( + dispatch: Dispatch, + getState: GetState, +) => { + showErrorAlert( + // TODO: Set up these user-facing strings for translation once + // `initialFetchAbort`'s callers all have access to a `GetText` + // function. As of adding the strings, the initial fetch is dispatched + // from `AppDataFetcher` which isn't a descendant of + // `TranslationProvider`. + 'Connection failed', + (() => { + const realmStr = getActiveAccount(getState()).realm.toString(); + switch (reason) { + case 'server': + return getIsAdmin(getState()) + ? `Could not connect to ${realmStr} because the server encountered an error. Please check the server logs.` + : `Could not connect to ${realmStr} because the server encountered an error. Please ask an admin to check the server logs.`; + case 'network': + return `The network request to ${realmStr} failed.`; + case 'timeout': + return `Gave up trying to connect to ${realmStr} after waiting too long.`; + case 'unexpected': + return `Unexpected error while trying to connect to ${realmStr}.`; + default: + ensureUnreachable(reason); + return ''; + } + })(), + ); + NavigationService.dispatch(resetToAccountPicker()); + dispatch(initialFetchAbortPlain(reason)); +}; + /** Private; exported only for tests. */ export const isFetchNeededAtAnchor = ( state: GlobalState, @@ -252,33 +325,57 @@ const fetchPrivateMessages = () => async (dispatch: Dispatch, getState: GetState }; /** - * Calls an async function and if unsuccessful retries the call. + * Makes a request with a timeout. If asked, retries on + * server/network operational errors until success. + * + * Waits between retries with a backoff. * - * 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. + * Other, non-retryable errors (client errors and all unexpected errors) + * will always 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. The time spent waiting in backoff is included + * in that. */ -export async function tryFetch(func: () => Promise): Promise { +export async function tryFetch( + func: () => Promise, + shouldRetry?: boolean = true, +): Promise { const backoffMachine = new BackoffMachine(); - // eslint-disable-next-line no-constant-condition - while (true) { - try { - return await func(); - } catch (e) { - // TODO: This should be narrowed; we should fail early if we encounter - // unrecognized / unexpected errors. - if (e instanceof ApiError) { - 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 (!(shouldRetry && (e instanceof Server5xxError || e instanceof NetworkError))) { + 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 could be returned, - // which is inconsistent with the stated Promise return type. - // eslint-disable-next-line no-unreachable - throw new Error(); } /** @@ -309,37 +406,63 @@ export const doInitialFetch = () => async (dispatch: Dispatch, getState: GetStat let initData: InitialData; let serverSettings: ApiResponseServerSettings; + const haveServerData = getHaveServerData(getState()); + try { [initData, serverSettings] = await Promise.all([ - tryFetch(() => - // Currently, no input we're giving `registerForEvents` is - // conditional on the server version / feature level. If we - // need to do that, make sure that data is up-to-date -- we've - // been using this `registerForEvents` call to update the - // feature level in Redux, which means the value in Redux will - // be from the *last* time it was run. That could be a long - // time ago, like from the previous app startup. - api.registerForEvents(auth, { - // Event types not supported by the server are ignored; see - // https://zulip.com/api/register-queue#parameter-fetch_event_types. - fetch_event_types: config.serverDataOnStartup, + tryFetch( + () => + // Currently, no input we're giving `registerForEvents` is + // conditional on the server version / feature level. If we + // need to do that, make sure that data is up-to-date -- we've + // been using this `registerForEvents` call to update the + // feature level in Redux, which means the value in Redux will + // be from the *last* time it was run. That could be a long + // time ago, like from the previous app startup. + api.registerForEvents(auth, { + // Event types not supported by the server are ignored; see + // https://zulip.com/api/register-queue#parameter-fetch_event_types. + fetch_event_types: config.serverDataOnStartup, - apply_markdown: true, - include_subscribers: false, - client_gravatar: true, - client_capabilities: { - notification_settings_null: true, - bulk_message_deletion: true, - user_avatar_url_field_optional: true, - }, - }), + apply_markdown: true, + include_subscribers: false, + client_gravatar: true, + client_capabilities: { + notification_settings_null: true, + bulk_message_deletion: true, + user_avatar_url_field_optional: true, + }, + }), + // We might have (potentially stale) server data already. If + // we do, we'll be showing some UI that lets the user see that + // data. If we don't, we'll be showing a full-screen loading + // indicator that prevents the user from doing anything useful + // -- if that's the case, don't bother retrying on 5xx errors, + // to save the user's time and patience. They can retry + // manually if they want. + haveServerData, ), - tryFetch(() => api.getServerSettings(auth.realm)), + tryFetch(() => api.getServerSettings(auth.realm), haveServerData), ]); } 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 (e instanceof ApiError) { + // This should only happen when `auth` is no longer valid. No + // use retrying; just log out. + dispatch(logout()); + } else if (e instanceof Server5xxError) { + dispatch(initialFetchAbort('server')); + } else if (e instanceof NetworkError) { + dispatch(initialFetchAbort('network')); + } else if (e instanceof TimeoutError) { + // We always want to abort if we've kept the user waiting an + // unreasonably long time. + dispatch(initialFetchAbort('timeout')); + } else { + dispatch(initialFetchAbort('unexpected')); + logging.warn(e, { + message: 'Unexpected error during initial fetch and serverSettings fetch.', + }); + } return; } diff --git a/src/session/__tests__/sessionReducer-test.js b/src/session/__tests__/sessionReducer-test.js index de7dec1753a..12ef8db89ec 100644 --- a/src/session/__tests__/sessionReducer-test.js +++ b/src/session/__tests__/sessionReducer-test.js @@ -6,6 +6,7 @@ import { LOGOUT, APP_ONLINE, INITIAL_FETCH_COMPLETE, + INITIAL_FETCH_ABORT, APP_ORIENTATION, GOT_PUSH_TOKEN, TOGGLE_OUTBOX_SENDING, @@ -74,6 +75,15 @@ describe('sessionReducer', () => { expect(newState).toEqual({ ...baseState, isOnline: true }); }); + test('INITIAL_FETCH_ABORT', () => { + const state = deepFreeze({ ...baseState, needsInitialFetch: true, loading: true }); + const newState = sessionReducer( + state, + deepFreeze({ type: INITIAL_FETCH_ABORT, reason: 'server' }), + ); + expect(newState).toEqual({ ...baseState, needsInitialFetch: false, loading: false }); + }); + test('INITIAL_FETCH_COMPLETE', () => { const state = deepFreeze({ ...baseState, needsInitialFetch: true, loading: true }); const newState = sessionReducer(state, deepFreeze({ type: INITIAL_FETCH_COMPLETE })); diff --git a/src/session/sessionReducer.js b/src/session/sessionReducer.js index 5f17b4373f1..55cf4657e00 100644 --- a/src/session/sessionReducer.js +++ b/src/session/sessionReducer.js @@ -8,6 +8,7 @@ import { ACCOUNT_SWITCH, REALM_INIT, INITIAL_FETCH_COMPLETE, + INITIAL_FETCH_ABORT, INITIAL_FETCH_START, APP_ORIENTATION, TOGGLE_OUTBOX_SENDING, @@ -166,6 +167,7 @@ export default (state: SessionState = initialState, action: Action): SessionStat loading: true, }; + case INITIAL_FETCH_ABORT: case INITIAL_FETCH_COMPLETE: return { ...state, diff --git a/src/utils/__tests__/promiseTimeout-test.js b/src/utils/__tests__/promiseTimeout-test.js new file mode 100644 index 00000000000..f3996f00ac9 --- /dev/null +++ b/src/utils/__tests__/promiseTimeout-test.js @@ -0,0 +1,47 @@ +/* @flow strict-local */ +import { promiseTimeout, TimeoutError } from '../async'; +import { fakeSleep } from '../../__tests__/lib/fakeTimers'; + +const ONE_MINUTE_MS: number = 1000 * 60; + +jest.useFakeTimers('modern'); +// Will throw if not actually using the "modern" implementation +jest.getRealSystemTime(); + +describe('promiseTimeout', () => { + afterEach(() => { + // clear any unset timers + expect(jest.getTimerCount()).toBe(0); + jest.clearAllTimers(); + }); + + test('If `promise` resolves with `x` before time is up, resolves with `x`', async () => { + const x = Math.random(); + const quickPromise = fakeSleep(1).then(() => x); + + await expect(promiseTimeout(quickPromise, ONE_MINUTE_MS)).resolves.toBe(x); + + jest.runAllTimers(); + }); + + test('If `promise` rejects before time is up, rejects with that reason', async () => { + const x = Math.random(); + const quickPromise = (async () => { + await fakeSleep(1); + throw new Error(x.toString()); + })(); + + await expect(promiseTimeout(quickPromise, ONE_MINUTE_MS)).rejects.toThrow(x.toString()); + + jest.runAllTimers(); + }); + + test('If time is up, throws a TimeoutError', async () => { + const endlessPromise = new Promise((resolve, reject) => {}); + + const endlessPromiseWithTimeout = promiseTimeout(endlessPromise, ONE_MINUTE_MS); + jest.runAllTimers(); + + await expect(endlessPromiseWithTimeout).rejects.toThrow(TimeoutError); + }); +}); diff --git a/src/utils/async.js b/src/utils/async.js index 79470d3c110..2bdd6bebe54 100644 --- a/src/utils/async.js +++ b/src/utils/async.js @@ -1,5 +1,24 @@ /* @flow strict-local */ +export class TimeoutError extends Error { + name = 'TimeoutError'; +} + +/** + * Time-out a Promise after `timeLimitMs` has passed. + * + * Returns a new Promise that rejects with a TimeoutError if the + * passed `promise` doesn't settle (resolve/reject) within the time + * limit specified by `timeLimitMs`; otherwise, it settles the way + * `promise` does. + */ +export async function promiseTimeout(promise: Promise, timeLimitMs: number): Promise { + return Promise.race([ + promise, + new Promise((_, reject) => setTimeout(() => reject(new TimeoutError()), timeLimitMs)), + ]); +} + /** Like setTimeout(..., 0), but returns a Promise of the result. */ export function delay(callback: () => T): Promise { return new Promise(resolve => resolve()).then(callback); diff --git a/static/translations/messages_en.json b/static/translations/messages_en.json index b4867b2eeee..59e31998ada 100644 --- a/static/translations/messages_en.json +++ b/static/translations/messages_en.json @@ -37,6 +37,8 @@ "Password": "Password", "Why not start the conversation?": "Why not start the conversation?", "That conversation doesn't seem to exist.": "That conversation doesn't seem to exist.", + "Request timed out.": "Request timed out.", + "Oops! Something went wrong.": "Oops! Something went wrong.", "Chat": "Chat", "Sign in with {method}": "Sign in with {method}", "Cannot connect to server": "Cannot connect to server",