diff --git a/src/chat/ChatScreen.js b/src/chat/ChatScreen.js index a7a922f902c..75648fd67d2 100644 --- a/src/chat/ChatScreen.js +++ b/src/chat/ChatScreen.js @@ -86,11 +86,12 @@ const useMessagesWithFetch = args => { } }, [dispatch, narrow]); - // When the event queue changes, schedule a fetch. (Currently, we never - // set this to null from a non-null value, so this really does mean the - // event queue has changed; it can't mean that we had a queue ID and - // dropped it.) - React.useEffect(scheduleFetch, [eventQueueId]); + // When we have a new event queue, schedule a fetch. + React.useEffect(() => { + if (eventQueueId !== null) { + scheduleFetch(); + } + }, [eventQueueId]); // If we stop having any data at all about the messages in this narrow -- // we don't know any, and nor do we know if there are some -- then diff --git a/src/events/eventActions.js b/src/events/eventActions.js index 8cd2663161c..b777527ac58 100644 --- a/src/events/eventActions.js +++ b/src/events/eventActions.js @@ -1,14 +1,16 @@ /* @flow strict-local */ import { addBreadcrumb } from '@sentry/react-native'; +// $FlowFixMe[untyped-import] +import isEqual from 'lodash.isequal'; import type { GeneralEvent, ThunkAction } from '../types'; -import { assumeSecretlyGlobalState } from '../reduxTypes'; import * as api from '../api'; import { logout } from '../account/accountActions'; import { deadQueue } from '../session/sessionActions'; import eventToAction from './eventToAction'; import doEventActionSideEffects from './doEventActionSideEffects'; -import { tryGetActiveAccountState, tryGetAuth } from '../selectors'; +import { tryGetAuth, getIdentity } from '../selectors'; +import { identityOfAuth } from '../account/accountMisc'; import { BackoffMachine } from '../utils/async'; import { ApiError } from '../api/apiErrors'; import * as logging from '../utils/logging'; @@ -61,11 +63,9 @@ export const startEventPolling = ( // eslint-disable-next-line no-constant-condition while (true) { - const globalState = assumeSecretlyGlobalState(getState()); - const state = tryGetActiveAccountState(globalState); - const auth = state ? tryGetAuth(state) : undefined; + const auth = tryGetAuth(getState()); if (!auth) { - // There is no logged-in active account. + // This account is not logged in. break; } // `auth` represents the active account. It might be different from @@ -77,10 +77,28 @@ export const startEventPolling = ( const response = await api.pollForEvents(auth, queueId, lastEventId); events = response.events; - if (queueId !== getState().session.eventQueueId) { - // The user switched accounts or logged out. - // TODO(#5022): TODO(#5009): This doesn't seem like an adequate - // check for that; it looks like it only detects a new REGISTER_COMPLETE. + if (getState().session.eventQueueId === null) { + // We don't want to keep polling, e.g., because we've logged out; + // see `PerAccountSessionState.eventQueueId` for other cases. + break; + } else if (!isEqual(getIdentity(getState()), identityOfAuth(auth))) { + // During the last poll, the active account changed. Stop polling + // for the previous one. + // TODO(#5005): Remove this conditional as unreachable, once `auth` + // represents the current account (instead of secretly + // representing the global "active account" which can change.) + break; + } else if (queueId !== getState().session.eventQueueId) { + // While the most recent poll was happening, another queue was + // established for this account, and we've started polling on that + // one. Stop polling on this one. + // + // In theory this could happen if you logged out of this account and + // logged back in again. + // + // TODO(#5009): Instead of this conditional, abort the + // `api.pollForEvents` immediately when `eventQueueId` becomes + // `null`, then break at that time? break; } } catch (e) { diff --git a/src/session/sessionReducer.js b/src/session/sessionReducer.js index e462e1cab64..2b6508025c2 100644 --- a/src/session/sessionReducer.js +++ b/src/session/sessionReducer.js @@ -24,6 +24,15 @@ import { getHasAuth } from '../account/accountsSelectors'; * See {@link SessionState} for discussion of what "non-persistent" means. */ export type PerAccountSessionState = $ReadOnly<{ + /** + * The event queue ID that we're currently polling on, if any. + * + * Null when we're not polling on any event queue: + * - Between startup and registering a queue + * - After the server tells us our old queue was invalid, and before we've + * registered a new one + * - While this account is logged out + */ eventQueueId: string | null, /** @@ -162,12 +171,20 @@ export default (state: SessionState = initialState, action: Action): SessionStat ...state, needsInitialFetch: true, loading: false, + + // The server told us that the old queue ID is invalid. Forget it, + // so we don't try to use it. + eventQueueId: null, }; case LOGIN_SUCCESS: return { ...state, needsInitialFetch: true, + + // We're about to request a new event queue; no use hanging on to + // any old one we might have. + eventQueueId: null, }; case LOGOUT: @@ -175,6 +192,9 @@ export default (state: SessionState = initialState, action: Action): SessionStat ...state, needsInitialFetch: false, loading: false, + + // Stop polling this event queue. + eventQueueId: null, }; case ACCOUNT_SWITCH: @@ -182,6 +202,12 @@ export default (state: SessionState = initialState, action: Action): SessionStat ...state, needsInitialFetch: true, loading: false, + + // Stop polling this event queue. (We'll request a new one soon, + // for the new account.) + // TODO(#5005): Keep polling on accounts other than the active + // account. + eventQueueId: null, }; case REHYDRATE: