From f89d16e97b1e3ce68758864e91cf14cef743b9ed Mon Sep 17 00:00:00 2001 From: Adam Horodyski Date: Mon, 3 Aug 2026 17:09:21 +0200 Subject: [PATCH 1/3] Send the Pusher heartbeat off the durable write queue --- src/libs/API/types.ts | 4 ++-- src/libs/actions/User.ts | 6 +++++- tests/unit/PusherPingPongTest.ts | 10 ++++++++++ 3 files changed, 17 insertions(+), 3 deletions(-) diff --git a/src/libs/API/types.ts b/src/libs/API/types.ts index ec6640cc58c9..58990ddec7a4 100644 --- a/src/libs/API/types.ts +++ b/src/libs/API/types.ts @@ -140,7 +140,6 @@ const WRITE_COMMANDS = { ADD_EMOJI_REACTION: 'AddEmojiReaction', REMOVE_EMOJI_REACTION: 'RemoveEmojiReaction', LEAVE_ROOM: 'LeaveRoom', - PUSHER_PING: 'PusherPing', LEAVE_GROUP_CHAT: 'LeaveGroupChat', INVITE_TO_ROOM: 'InviteToRoom', INVITE_TO_GROUP_CHAT: 'InviteToGroupChat', @@ -789,7 +788,6 @@ type WriteCommandParameters = { [WRITE_COMMANDS.INVITE_TO_GROUP_CHAT]: Parameters.InviteToGroupChatParams; [WRITE_COMMANDS.UPDATE_GROUP_CHAT_AVATAR]: Parameters.UpdateGroupChatAvatarParams; [WRITE_COMMANDS.UPDATE_ROOM_AVATAR]: Parameters.UpdateRoomAvatarParams; - [WRITE_COMMANDS.PUSHER_PING]: Parameters.PusherPingParams; [WRITE_COMMANDS.LEAVE_GROUP_CHAT]: Parameters.LeaveGroupChatParams; [WRITE_COMMANDS.REMOVE_FROM_GROUP_CHAT]: Parameters.RemoveFromGroupChatParams; [WRITE_COMMANDS.UPDATE_GROUP_CHAT_MEMBER_ROLES]: Parameters.UpdateGroupChatMemberRolesParams; @@ -1569,6 +1567,7 @@ 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', @@ -1616,6 +1615,7 @@ 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; diff --git a/src/libs/actions/User.ts b/src/libs/actions/User.ts index 98503fb5d3ba..cda775647ca3 100644 --- a/src/libs/actions/User.ts +++ b/src/libs/actions/User.ts @@ -846,7 +846,11 @@ function pingPusher() { lastPingSentTimestamp = pingTimestamp; const parameters: PusherPingParams = {pingID, pingTimestamp}; - API.writeWithNoDuplicatesConflictAction(WRITE_COMMANDS.PUSHER_PING, parameters); + + // 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}`); } diff --git a/tests/unit/PusherPingPongTest.ts b/tests/unit/PusherPingPongTest.ts index d0ec6146baeb..914171b2b835 100644 --- a/tests/unit/PusherPingPongTest.ts +++ b/tests/unit/PusherPingPongTest.ts @@ -1,4 +1,6 @@ import {subscribeToUserEvents} from '@libs/actions/User'; +import * as API from '@libs/API'; +import {SIDE_EFFECT_REQUEST_COMMANDS} from '@libs/API/types'; import type * as NetworkStateModule from '@libs/NetworkState'; import Pusher from '@libs/Pusher'; import PusherUtils from '@libs/PusherUtils'; @@ -33,6 +35,9 @@ describe('Pusher PINGPONG watchdog', () => { Onyx.init({keys: ONYXKEYS}); reconnectSpy = jest.spyOn(Pusher, 'reconnect').mockImplementation(() => {}); + // The automock returns undefined, and pingPusher chains a .catch on the returned promise + jest.mocked(API.makeRequestWithSideEffects).mockResolvedValue(undefined); + subscribeToUserEvents(123, 'test@example.com', () => undefined); const callback = jest.mocked(PusherUtils.subscribeToPrivateUserChannelEvent).mock.calls.find(([eventName]) => eventName === Pusher.TYPE.PONG)?.[2]; @@ -78,4 +83,9 @@ describe('Pusher PINGPONG watchdog', () => { await jest.advanceTimersByTimeAsync(CHECK_INTERVAL_MS); expect(reconnectSpy).toHaveBeenCalledTimes(1); }); + + it('sends the PING off the durable write queue', () => { + expect(API.makeRequestWithSideEffects).toHaveBeenCalledWith(SIDE_EFFECT_REQUEST_COMMANDS.PUSHER_PING, expect.objectContaining({pingID: expect.any(String)})); + expect(API.writeWithNoDuplicatesConflictAction).not.toHaveBeenCalled(); + }); }); From 7649b85e56d0af3450c2f85dca455e7a4b4f1a63 Mon Sep 17 00:00:00 2001 From: Adam Horodyski Date: Mon, 3 Aug 2026 19:17:54 +0200 Subject: [PATCH 2/3] Hoist the API mock handle to satisfy no-multiple-api-calls The no-multiple-api-calls rule counts `API` tokens per function body, so the third assertion pushed the describe block over the limit. One module-scope mockAPI handle keeps the count at zero inside functions. Drop the pingID shape assertion too: pingPusher builds a typed PusherPingParams, so tsc already guarantees it is a string. Co-Authored-By: Claude Opus 5 (1M context) --- tests/unit/PusherPingPongTest.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tests/unit/PusherPingPongTest.ts b/tests/unit/PusherPingPongTest.ts index 914171b2b835..57d855f56b0c 100644 --- a/tests/unit/PusherPingPongTest.ts +++ b/tests/unit/PusherPingPongTest.ts @@ -10,6 +10,8 @@ import ONYXKEYS from '@src/ONYXKEYS'; import Onyx from 'react-native-onyx'; jest.mock('@libs/API'); +const mockAPI = jest.mocked(API); + jest.mock('@libs/PusherUtils'); jest.mock('@libs/ActiveClientManager', () => ({ isClientTheLeader: jest.fn(() => true), @@ -36,7 +38,7 @@ describe('Pusher PINGPONG watchdog', () => { reconnectSpy = jest.spyOn(Pusher, 'reconnect').mockImplementation(() => {}); // The automock returns undefined, and pingPusher chains a .catch on the returned promise - jest.mocked(API.makeRequestWithSideEffects).mockResolvedValue(undefined); + mockAPI.makeRequestWithSideEffects.mockResolvedValue(undefined); subscribeToUserEvents(123, 'test@example.com', () => undefined); @@ -85,7 +87,7 @@ describe('Pusher PINGPONG watchdog', () => { }); it('sends the PING off the durable write queue', () => { - expect(API.makeRequestWithSideEffects).toHaveBeenCalledWith(SIDE_EFFECT_REQUEST_COMMANDS.PUSHER_PING, expect.objectContaining({pingID: expect.any(String)})); - expect(API.writeWithNoDuplicatesConflictAction).not.toHaveBeenCalled(); + expect(mockAPI.makeRequestWithSideEffects).toHaveBeenCalledWith(SIDE_EFFECT_REQUEST_COMMANDS.PUSHER_PING, expect.anything()); + expect(mockAPI.writeWithNoDuplicatesConflictAction).not.toHaveBeenCalled(); }); }); From 92c35ab2e287111731be051d34763eed3928f872 Mon Sep 17 00:00:00 2001 From: Adam Horodyski Date: Tue, 4 Aug 2026 12:44:19 +0200 Subject: [PATCH 3/3] Make the PING assertion drive its own timer tick The test advanced no timers, so it only passed because the two watchdog tests ahead of it had already run the 30s ping interval and the mock was never cleared. Run alone it saw zero calls. Clearing the mock and advancing one ping interval makes it independent of test order. Co-Authored-By: Claude Opus 5 (1M context) --- tests/unit/PusherPingPongTest.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/unit/PusherPingPongTest.ts b/tests/unit/PusherPingPongTest.ts index 57d855f56b0c..7b9714af0704 100644 --- a/tests/unit/PusherPingPongTest.ts +++ b/tests/unit/PusherPingPongTest.ts @@ -26,6 +26,8 @@ jest.mock('@libs/NetworkState', () => ({ // The watchdog checks every 60s; each reconnect skips the following check, so while PONGs stay missing it fires on every second check tick (~2 minutes) const CHECK_INTERVAL_MS = 60_000; +const PING_INTERVAL_MS = 30_000; + describe('Pusher PINGPONG watchdog', () => { let reconnectSpy: jest.SpyInstance; let pongCallback: Parameters[2]; @@ -86,7 +88,10 @@ describe('Pusher PINGPONG watchdog', () => { expect(reconnectSpy).toHaveBeenCalledTimes(1); }); - it('sends the PING off the durable write queue', () => { + it('sends the PING off the durable write queue', async () => { + mockAPI.makeRequestWithSideEffects.mockClear(); + await jest.advanceTimersByTimeAsync(PING_INTERVAL_MS); + expect(mockAPI.makeRequestWithSideEffects).toHaveBeenCalledWith(SIDE_EFFECT_REQUEST_COMMANDS.PUSHER_PING, expect.anything()); expect(mockAPI.writeWithNoDuplicatesConflictAction).not.toHaveBeenCalled(); });