[No QA] stop the pong watchdog from reconnecting Pusher - #98884
Conversation
The watchdog compared wall-clock time against a 60 second threshold while the real PING cadence in production is also about 60 seconds, because browsers throttle setInterval in background tabs. The threshold equalled the cadence of the signal it watched, so it fired on timer jitter: 516 users produced 31,518 "presumed dead" fires per 4 hours while receiving about 118 PONGs each. The probe cannot answer the question either way. The PING goes out over HTTP and the PONG comes back over the websocket, so a missing PONG does not say which of the two failed, and the request has no retry on a failed fetch. Every Pusher SDK already detects a dead socket with a websocket ping frame answered on the connection under test, and reconnects itself: pusher-js 8.3.0 (120s/30s), PusherSwift 10.1.6 (server activity_timeout/30s), pusher-java-client 2.4.4 (120s/30s). The only cost of deferring to them is reaction time. The log line stays, restated, so the old string falling to zero is the proof and the new string sizes whatever the SDKs do not cover. Pusher.reconnect() itself stays: PusherConnectionManager and Session both still call it. The staleness check now rides the existing 30 second PING interval instead of a second timer with its own delayed start, which also removes the delay that read 60 milliseconds where it meant 60 seconds. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. Swish! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
|
@codex review |
|
@MelvinBot please review this PR |
|
@ZhenjaHorbach Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
PR Review — looks good ✅The change is sound and does exactly what the issue describes: the watchdog stops discarding live sockets. The core premise checks out. Recovery is delegated to pusher-js, and our Pusher init only sets Verification details
Minor / non-blocking observations:
Web smoke test (no UI change, so just a regression check): signed in, reached Home, opened a chat — report list and messages render, and |
Reviewer Checklist
Screenshots/VideosAndroid: HybridApp2026-08-19.17.13.01.movAndroid: mWeb Chrome2026-08-19.17.13.01.moviOS: HybridApp2026-08-19.17.08.21.moviOS: mWeb Safari2026-08-19.17.06.37.movMacOS: Chrome / Safari2026-08-19.17.03.49.mov |
|
Changes look good! |
mountiny
left a comment
There was a problem hiding this comment.
Thanks, seems like this will bring nice drop
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚧 mountiny has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
Explanation of Change
The watchdog declares a live socket dead. The 60 second threshold equals the PING cadence the app reaches in production.
This change deletes the reconnect. Recovery moves to the Pusher SDK, which detects a dead socket with a websocket ping frame on the connection under test. A false alarm no longer discards a working socket or pulls a full
ReconnectAppsync behind it.Proof
Baseline window
2026-08-12T10:30:00Zto14:30:00Z. The window matches the weekday and the hours of the original measurement. After values are the prediction this PR is measured against.presumed deadfires per 4hpresumed alivelog lines per 4hReconnectAppper user per 4hGetMissingOnyxMessages did not advanceFixed Issues
$ #98886
PROPOSAL:
Tests
npx jest tests/unit/PusherPingPongTest.ts. Two tests pass. No PONG for 300 seconds produces the log line and noPusher.reconnect()call. The PING still goes off the durable write queue.pusher-js.[Pusher PINGPONG] The server has not sent a PONG in ... leaving recovery to the Pusher SDKin place of the oldpresumed dead and Pusher is being reconnected.Offline tests
[Pusher PINGPONG] Skipping PING because the client is offline. Confirm no PONG staleness line appears.QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
This PR changes
src/libs/actions/User.tsand a unit test file. It has no UI change. There is nothing to capture in the sections below.Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari