Skip to content

Commit

Permalink
startEventPolling [nfc]: Be clearer that this is a per-account action
Browse files Browse the repository at this point in the history
In a multi-account, post-zulip#5005 world, we're likely to want a live
event queue for all the logged-in accounts. So we'll want this
function to start polling for "this account", not for the global
"active account". The action is already typed as a `ThunkAction`,
not a `GlobalThunkAction`; that's a good start.

The `auth` object we're working with after the code change is the
same one as before, we just get it more directly. (`tryGetAuth` uses
`getAccount`, which, pending zulip#5005, assumes "the account" is the
active account; see the implementation comment in `getAccount`.)
And, with zulip#5005, it will naturally change to be the Auth object for
"this account", the one the caller cares about.

Also expand a few comments and clarify that they only apply in the
pre-zulip#5005 world, and add TODOs to remove them when we complete zulip#5005
so we don't forget.
  • Loading branch information
chrisbobbe committed Dec 8, 2021
1 parent 99775a9 commit a324a93
Showing 1 changed file with 16 additions and 11 deletions.
27 changes: 16 additions & 11 deletions src/events/eventActions.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
/* @flow strict-local */
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 } from '../selectors';
import { BackoffMachine } from '../utils/async';
import { ApiError } from '../api/apiErrors';
import * as logging from '../utils/logging';
Expand Down Expand Up @@ -53,26 +52,32 @@ 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
// the one in the previous loop iteration, if we did a backoff wait.
// Pre-#5005, `auth` secretly represents the active account. It might
// be different from the one in the previous loop iteration, if
// we did a backoff wait.
// TODO(#5009): Is that really quite OK?
// TODO(#5005): After #5005, it should be OK; remove these comments.

let events = undefined;
try {
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.
// Pre-#5005, this will catch that the user has switched accounts,
// as long as the old and new accounts' queue IDs don't collide.
// It'll catch the switch on REGISTER_COMPLETE, which is a while
// after the switch happens in the UI. But the essential thing
// (again, pre-#5005) is that it's caught before we're set up with
// a new loop for the new account, so that the old and new account
// aren't writing to the same piece of state at the same time.
// TODO(#5005): Remove the "Pre-#5005" comment above.
// TODO(#5022): TODO(#5009): Catch a logout.
break;
}
} catch (e) {
Expand Down

0 comments on commit a324a93

Please sign in to comment.