Skip to content

Remove the HTTP PusherPing heartbeat #63

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
adhorodyski wants to merge 1 commit into from
Closed

Remove the HTTP PusherPing heartbeat #63

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions src/libs/API/parameters/PusherPingParams.ts
View file
Open in desktop

This file was deleted.

1 change: 0 additions & 1 deletion src/libs/API/parameters/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,6 @@ export type {default as OpenPolicyReceiptPartnersPageParams} from './OpenPolicyR
export type {default as OpenPolicyHRPageParams} from './OpenPolicyHRPageParams';
export type {default as PaymentCardParams} from './PaymentCardParams';
export type {default as AddPersonalPlaidCardParams} from './AddPersonalPlaidCardParams';
export type {default as PusherPingParams} from './PusherPingParams';
export type {default as ReconnectAppParams} from './ReconnectAppParams';
export type {default as ReferTeachersUniteVolunteerParams} from './ReferTeachersUniteVolunteerParams';
export type {default as ReportVirtualExpensifyCardFraudParams} from './ReportVirtualExpensifyCardFraudParams';
Expand Down
2 changes: 0 additions & 2 deletions src/libs/API/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1649,7 +1649,6 @@ const SIDE_EFFECT_REQUEST_COMMANDS = {
IMPORT_PER_DIEM_RATES: 'ImportPerDiemRates',
IMPORT_TAGS_SPREADSHEET: 'ImportTagsSpreadsheet',
OPEN_OLD_DOT_LINK: 'OpenOldDotLink',
PUSHER_PING: 'PusherPing',
RECONNECT_APP: 'ReconnectApp',
REVEAL_EXPENSIFY_TRAVEL_CARD_DETAILS: 'RevealExpensifyTravelCardDetails',
TWO_FACTOR_AUTH_VALIDATE: 'TwoFactorAuth_Validate',
Expand Down Expand Up @@ -1698,7 +1697,6 @@ type SideEffectRequestCommandParameters = {
[SIDE_EFFECT_REQUEST_COMMANDS.IMPORT_PER_DIEM_RATES]: Parameters.ImportPerDiemRatesParams;
[SIDE_EFFECT_REQUEST_COMMANDS.IMPORT_TAGS_SPREADSHEET]: Parameters.ImportTagsSpreadsheetParams;
[SIDE_EFFECT_REQUEST_COMMANDS.OPEN_OLD_DOT_LINK]: Parameters.OpenOldDotLinkParams;
[SIDE_EFFECT_REQUEST_COMMANDS.PUSHER_PING]: Parameters.PusherPingParams;
[SIDE_EFFECT_REQUEST_COMMANDS.REVEAL_EXPENSIFY_TRAVEL_CARD_DETAILS]: Parameters.RevealExpensifyCardDetailsParams;
[SIDE_EFFECT_REQUEST_COMMANDS.GET_MISSING_ONYX_MESSAGES]: Parameters.GetMissingOnyxMessagesParams;
[SIDE_EFFECT_REQUEST_COMMANDS.RECONNECT_APP]: Parameters.ReconnectAppParams;
Expand Down
2 changes: 0 additions & 2 deletions src/libs/Pusher/EventType.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@ export default {
CONCIERGE_DRAFT_FAILED: 'conciergeDraftFailed',
CONCIERGE_DRAFT_CLEARED: 'conciergeDraftCleared',

// An event that the server sends back to the client in response to a "ping" API command
PONG: 'pong',
MULTIPLE_EVENT_TYPE: {
ONYX_API_UPDATE: 'onyxApiUpdate',
RECONNECT_APP: 'reconnectApp',
Expand Down
7 changes: 0 additions & 7 deletions src/libs/Pusher/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,6 @@ type UserIsLeavingRoomEvent = Record<string, boolean> & {
userLogin?: string;
};

type PingPongEvent = Record<string, string | number> & {
pingID: string;
pingTimestamp: number;
};

type ConciergeReasoningEvent = {
reasoning: string;
agentZeroRequestID: string;
Expand Down Expand Up @@ -74,7 +69,6 @@ type ConciergeDraftEventsEvent = {
type PusherEventMap = {
[TYPE.USER_IS_TYPING]: UserIsTypingEvent;
[TYPE.USER_IS_LEAVING_ROOM]: UserIsLeavingRoomEvent;
[TYPE.PONG]: PingPongEvent;
[TYPE.CONCIERGE_REASONING]: ConciergeReasoningEvent;
[TYPE.CONCIERGE_DRAFT_EVENTS]: ConciergeDraftEventsEvent;
[TYPE.CONCIERGE_DRAFT_STARTED]: ConciergeDraftEvent;
Expand Down Expand Up @@ -136,7 +130,6 @@ export type {
Args,
UserIsTypingEvent,
UserIsLeavingRoomEvent,
PingPongEvent,
ConciergeDraftEvent,
ConciergeDraftEventsEvent,
EventData,
Expand Down
8 changes: 3 additions & 5 deletions src/libs/PusherUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@ import type {AnyOnyxUpdatesFromServer, OnyxServerUpdate} from '@src/types/onyx/O

import type {OnyxKey} from 'react-native-onyx';

import type {PingPongEvent} from './Pusher/types';

import {reconnect} from './actions/Reconnect';
import Log from './Log';
import Pusher from './Pusher';
Expand Down Expand Up @@ -37,14 +35,14 @@ function triggerMultiEventHandler<TKey extends OnyxKey>(eventType: string, data:
/**
* Abstraction around subscribing to private user channel events. Handles all logs and errors automatically.
*/
function subscribeToPrivateUserChannelEvent(eventName: string, accountID: string, onEvent: (pushJSON: AnyOnyxUpdatesFromServer | PingPongEvent) => void) {
function subscribeToPrivateUserChannelEvent(eventName: string, accountID: string, onEvent: (pushJSON: AnyOnyxUpdatesFromServer) => void) {
const pusherChannelName = getUserChannelName(accountID);

function logPusherEvent(pushJSON: AnyOnyxUpdatesFromServer | PingPongEvent) {
function logPusherEvent(pushJSON: AnyOnyxUpdatesFromServer) {
Log.info(`[Report] Handled ${eventName} event sent by Pusher`, false, pushJSON);
}

function onEventPush(pushJSON: AnyOnyxUpdatesFromServer | PingPongEvent) {
function onEventPush(pushJSON: AnyOnyxUpdatesFromServer) {
logPusherEvent(pushJSON);
onEvent(pushJSON);
}
Expand Down
87 changes: 2 additions & 85 deletions src/libs/actions/User.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import type {
CloseAccountParams,
DeleteContactMethodParams,
GetStatementPDFParams,
PusherPingParams,
RequestContactMethodValidateCodeParams,
ResendValidateCodeParams,
RevokeDeviceParams,
Expand All @@ -31,11 +30,8 @@ import Log from '@libs/Log';
import createDynamicRoute from '@libs/Navigation/helpers/dynamicRoutesUtils/createDynamicRoute';
import Navigation from '@libs/Navigation/Navigation';
import * as SequentialQueue from '@libs/Network/SequentialQueue';
import {getIsOffline} from '@libs/NetworkState';
import * as NumberUtils from '@libs/NumberUtils';
import * as PersonalDetailsUtils from '@libs/PersonalDetailsUtils';
import Pusher from '@libs/Pusher';
import type {PingPongEvent} from '@libs/Pusher/types';
import PusherUtils from '@libs/PusherUtils';
import * as ReportActionsUtils from '@libs/ReportActionsUtils';
import * as ReportUtils from '@libs/ReportUtils';
Expand All @@ -51,7 +47,7 @@ import type {ExpenseRuleForm, FlagForReviewRuleForm, MerchantRuleForm, MerchantT
import type {AppReview, BlockedFromConcierge, CustomStatusDraft, ExpenseRule, NewLogin, ReportAttributesDerivedValue} from '@src/types/onyx';
import type Login from '@src/types/onyx/Login';
import type {Errors} from '@src/types/onyx/OnyxCommon';
import type {AnyOnyxServerUpdate, OnyxServerUpdate, OnyxUpdateEvent} from '@src/types/onyx/OnyxUpdatesFromServer';
import type {AnyOnyxServerUpdate, OnyxServerUpdate} from '@src/types/onyx/OnyxUpdatesFromServer';
import type {CurrentUserPersonalDetails, Status} from '@src/types/onyx/PersonalDetails';
import type ReportAction from '@src/types/onyx/ReportAction';
import type {AnyOnyxUpdate} from '@src/types/onyx/Request';
Expand Down Expand Up @@ -798,81 +794,6 @@ function playSoundForMessageType<TKey extends OnyxKey>(pushJSON: Array<OnyxServe
});
}

let lastPingSentTimestamp = Date.now();
let lastPongReceivedTimestamp = Date.now();
function subscribeToPusherPong(currentUserAccountID: number) {
// If there is no user accountID yet (because the app isn't fully setup yet), the channel can't be subscribed to so return early
if (!currentUserAccountID) {
return;
}

PusherUtils.subscribeToPrivateUserChannelEvent(Pusher.TYPE.PONG, currentUserAccountID.toString(), (pushJSON) => {
Log.info(`[Pusher PINGPONG] Received a PONG event from the server`, false, pushJSON);
lastPongReceivedTimestamp = Date.now();

const pongEvent = pushJSON as PingPongEvent;
const latency = Date.now() - Number(pongEvent.pingTimestamp);
Log.info(`[Pusher PINGPONG] The event took ${latency} ms`);
});
}

const PING_INTERVAL_LENGTH_IN_SECONDS = 30;

const MISSING_PONG_THRESHOLD_IN_SECONDS = 2 * PING_INTERVAL_LENGTH_IN_SECONDS;

function pingPusher() {
if (getIsOffline()) {
Log.info('[Pusher PINGPONG] Skipping PING because the client is offline');
return;
}
const pingID = NumberUtils.rand64();
const pingTimestamp = Date.now();

// In local development, there can end up being multiple intervals running because when JS code is replaced with hot module replacement, the old interval is not cleared
// and keeps running. This little bit of logic will attempt to keep multiple pings from happening.
if (pingTimestamp - lastPingSentTimestamp < PING_INTERVAL_LENGTH_IN_SECONDS * 1000) {
return;
}
lastPingSentTimestamp = pingTimestamp;

const parameters: PusherPingParams = {pingID, pingTimestamp};

// The heartbeat is a probe, not a user's write, so it must not be persisted to disk, retried, or hold the
// head of the queue while a real write waits behind it. The Logging middleware already logs any failure.
// eslint-disable-next-line rulesdir/no-api-side-effects-method
API.makeRequestWithSideEffects(SIDE_EFFECT_REQUEST_COMMANDS.PUSHER_PING, parameters).catch(() => {});
Log.info(`[Pusher PINGPONG] Sending a PING to the server: ${pingID} timestamp: ${pingTimestamp}`);

const timeSinceLastPongReceived = pingTimestamp - lastPongReceivedTimestamp;
if (timeSinceLastPongReceived > MISSING_PONG_THRESHOLD_IN_SECONDS * 1000) {
Log.info(`[Pusher PINGPONG] The server has not sent a PONG in ${timeSinceLastPongReceived} ms, leaving recovery to the Pusher SDK`);
}
}

let pingPusherIntervalID: ReturnType<typeof setInterval>;
function initializePusherPingPong(currentUserAccountID: number) {
// Only run the ping pong from the leader client
if (!ActiveClientManager.isClientTheLeader()) {
Log.info("[Pusher PINGPONG] Not starting PING PONG because this instance isn't the leader client");
return;
}

Log.info(`[Pusher PINGPONG] Starting Pusher PING PONG and pinging every ${PING_INTERVAL_LENGTH_IN_SECONDS} seconds`);

lastPongReceivedTimestamp = Date.now();

// Subscribe to the pong event from Pusher. Unfortunately, there is no way of knowing when the client is actually subscribed
// so there could be a little delay before the client is actually listening to this event.
subscribeToPusherPong(currentUserAccountID);

// If things are initializing again (which is fine because it will reinitialize each time Pusher authenticates), clear the old intervals
if (pingPusherIntervalID) {
clearInterval(pingPusherIntervalID);
}

pingPusherIntervalID = setInterval(pingPusher, PING_INTERVAL_LENGTH_IN_SECONDS * 1000);
}

/**
* Handles the newest events from Pusher where a single mega multipleEvents contains
* an array of singular events all in one event
Expand Down Expand Up @@ -904,9 +825,7 @@ function subscribeToUserEvents(
const updates = {
type: CONST.ONYX_UPDATE_TYPES.PUSHER,
lastUpdateID: Number(pushEventData.lastUpdateID ?? CONST.DEFAULT_NUMBER_ID),
// specific key type is not known from the Pusher event data
// eslint-disable-next-line @typescript-eslint/no-explicit-any
updates: (pushEventData.updates as Array<OnyxUpdateEvent<any>>) ?? [],
updates: pushEventData.updates ?? [],
previousUpdateID: Number(pushJSON.previousUpdateID ?? CONST.DEFAULT_NUMBER_ID),
};
Log.info('[subscribeToUserEvents] Applying Onyx updates');
Expand Down Expand Up @@ -954,8 +873,6 @@ function subscribeToUserEvents(
reconnectApp();
return Promise.resolve();
});

initializePusherPingPong(currentUserAccountID);
}

/**
Expand Down
67 changes: 0 additions & 67 deletions tests/unit/PusherPingPongTest.ts
View file
Open in desktop

This file was deleted.

14 changes: 7 additions & 7 deletions tests/unit/PusherSubscribeWebTest.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ describe('Pusher.subscribe on web', () => {
it('should authorize again when a caller subscribes after a subscription error', async () => {
// Given a channel that failed authorization, because pusher-js keeps such a channel and the
// old guard read it as usable and never asked for authorization again
const failed = Pusher.subscribe(CHANNEL, 'pong', () => {});
const failed = Pusher.subscribe(CHANNEL, 'reportComment', () => {});
await jest.runAllTimersAsync();

const channel = mockChannels.get(CHANNEL);
Expand All @@ -221,7 +221,7 @@ describe('Pusher.subscribe on web', () => {
await expect(failed).rejects.toBe('Forbidden');

// When a caller subscribes to the same channel after that failure
const retried = Pusher.subscribe(CHANNEL, 'pong', () => {});
const retried = Pusher.subscribe(CHANNEL, 'reportComment', () => {});
await jest.runAllTimersAsync();
channel?.completeHandshake();
await retried;
Expand All @@ -235,7 +235,7 @@ describe('Pusher.subscribe on web', () => {
// channel on its own at the next reconnect and the caller must not stay silent
const onEvent = jest.fn();

const failed = Pusher.subscribe(CHANNEL, 'pong', onEvent);
const failed = Pusher.subscribe(CHANNEL, 'reportComment', onEvent);
await jest.runAllTimersAsync();

const channel = mockChannels.get(CHANNEL);
Expand All @@ -245,7 +245,7 @@ describe('Pusher.subscribe on web', () => {
// When pusher-js authorizes the channel again and the channel then carries one event
channel?.startSubscription();
channel?.completeHandshake();
channel?.receiveEvent('pong', {});
channel?.receiveEvent('reportComment', {});

// Then the caller reads the event, because its success handler stays bound after a failure
expect(onEvent).toHaveBeenCalledTimes(1);
Expand All @@ -254,7 +254,7 @@ describe('Pusher.subscribe on web', () => {
it('should reject every caller waiting on a failed handshake, not only the first', async () => {
// Given two callers that wait on the same first handshake, because one shared error handler
// rejected only the caller that opened the channel
const first = Pusher.subscribe(CHANNEL, 'pong', () => {});
const first = Pusher.subscribe(CHANNEL, 'reportComment', () => {});
const second = Pusher.subscribe(CHANNEL, 'multipleEvents', () => {});
await jest.runAllTimersAsync();

Expand Down Expand Up @@ -386,13 +386,13 @@ describe('Pusher.subscribe on web', () => {
});

it('should trigger one reconnect per drop, however many events subscribe to the private user channel', async () => {
// Given a private user channel with both PONG and MULTIPLE_EVENTS subscribed, because each
// Given a private user channel with both REPORT_COMMENT and MULTIPLE_EVENTS subscribed, because each
// call carried its own onResubscribe and one drop sent two ReconnectApp requests
const accountID = '1';
const userChannel = `${CONST.PUSHER.PRIVATE_USER_CHANNEL_PREFIX}${accountID}${CONFIG.PUSHER.SUFFIX}`;

PusherUtils.onPrivateUserChannelResubscribe(accountID);
PusherUtils.subscribeToPrivateUserChannelEvent(Pusher.TYPE.PONG, accountID, () => {});
PusherUtils.subscribeToPrivateUserChannelEvent(Pusher.TYPE.REPORT_COMMENT, accountID, () => {});
PusherUtils.subscribeToPrivateUserChannelEvent(Pusher.TYPE.MULTIPLE_EVENTS, accountID, () => {});
await jest.runAllTimersAsync();

Expand Down
Loading