feat: match WhatsApp Web keepalive, dead socket, and reconnect behavior - #341
Conversation
Cross-referenced against WAWebCommsConfig, WAComms, and WAWebCommsHandleStanza from captured WhatsApp Web JS bundle.
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds per-client send/receive timestamps, replaces linear reconnect delay with a Fibonacci backoff (with jitter and cap), reworks keepalive to IQ-based RTT measurement with activity gating and dead-socket detection (20s), and adds an RTT-aware server time offset update method. Changes
Sequence DiagramsequenceDiagram
participant Client
participant Keepalive
participant Server
participant SessionMgr as UnifiedSessionManager
loop keepalive loop
Keepalive->>Keepalive: ms_since / is_dead_socket(last_sent,last_received)
alt dead socket
Keepalive->>Client: request reconnect (fibonacci_backoff)
else recent activity
Keepalive->>Keepalive: reset errors, skip ping
else no recent activity
Keepalive->>Server: send IQ keepalive (record start_time)
Server-->>Keepalive: IQ response (contains node.t)
Keepalive->>Keepalive: compute rtt_ms
Keepalive->>SessionMgr: update_server_time_offset_with_rtt(node,start_time_ms,rtt_ms)
SessionMgr->>SessionMgr: compute and store server_time_offset_ms
end
end
note over Client,Keepalive: Data events update last_data_received_ms & last_data_sent_ms
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Bencher Report- Reset last_data_sent_ms and last_data_received_ms in cleanup_connection_state to prevent stale timestamps from triggering immediate dead-socket reconnect on next connection. - Adjust reconnect() delay from fibonacci(2)=2s to fibonacci(4)=5s to maintain the offline window e2e tests depend on (old linear backoff was 4s).
The negative-assertion timeouts (3s) in offline receipt and presence coalescing tests assumed the old linear backoff delay (~4s). With fibonacci backoff, reconnect() produces ~2s delay (fibonacci(2)), so B reconnects before the 3s check. Reduce timeouts to 1s. Also reset dead-socket timestamps in cleanup_connection_state to prevent stale values from triggering immediate reconnect.
…ndow Replace magic number with a named constant (RECONNECT_BACKOFF_STEP=4) so the reconnect() delay is self-documenting and tests can reason about it. fibonacci_backoff(4) ≈ 5s, which is longer than the mock server's CHATSTATE_TTL_SECS=3 so TTL-expiry tests pass. Update all e2e test comments to reference the constant instead of hardcoded timing assumptions.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/e2e/tests/chatstate_ttl.rs (1)
17-21:⚠️ Potential issue | 🟡 MinorInconsistent timing reference in doc comment.
Line 17 correctly states
~5s offline window (see RECONNECT_BACKOFF_STEP), but line 21 still references~4s reconnect. Withfibonacci_backoff(4)yielding ~5s, line 21 should be updated for consistency.📝 Suggested fix
-/// Requires mock server with CHATSTATE_TTL_SECS=3 (so TTL expires before the ~4s reconnect). +/// Requires mock server with CHATSTATE_TTL_SECS=3 (so TTL expires before the ~5s reconnect).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/e2e/tests/chatstate_ttl.rs` around lines 17 - 21, Update the doc comment to use a consistent timing reference: replace the "~4s reconnect" wording with "~5s reconnect" (or reference RECONNECT_BACKOFF_STEP/fibonacci_backoff(4)) so it matches the earlier "~5s offline window (see RECONNECT_BACKOFF_STEP)" and the actual fibonacci_backoff(4) result used in the test.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@tests/e2e/tests/chatstate_ttl.rs`:
- Around line 17-21: Update the doc comment to use a consistent timing
reference: replace the "~4s reconnect" wording with "~5s reconnect" (or
reference RECONNECT_BACKOFF_STEP/fibonacci_backoff(4)) so it matches the earlier
"~5s offline window (see RECONNECT_BACKOFF_STEP)" and the actual
fibonacci_backoff(4) result used in the test.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 205463c0-bb12-439f-8445-995d528ab919
📒 Files selected for processing (3)
src/client.rstests/e2e/tests/chatstate_ttl.rstests/e2e/tests/offline_receipts.rs
1 participant
Summary
Aligns keepalive, dead-socket detection, and reconnection logic with real WhatsApp Web behavior, cross-referenced against captured JS modules (
WAWebCommsConfig,WAComms,WAWebCommsHandleStanza).Changes
Keepalive ping (
src/keepalive.rs)healthCheckInterval = 15, formula15*(1+random())= 15-30sactivePing || pendingIqs.sizeguardmaybeScheduleHealthCheckevent-driven reschedulingtfield — matches WA WebMath.round((startTime + rtt/2) / 1000 - serverTime)viaonClockSkewUpdateDead socket timer (
src/keepalive.rs+src/client.rs)last_data_sent_mson everysend_node— matches WA WebcallStanza → deadSocketTimer.onOrBefore(deadSocketTime)last_data_received_mson everyDataReceived— matches WA WebparseAndHandleStanza → deadSocketTimer.cancel()deadSocketTime = 20_000 → softCloseSocket()reconnect_immediately()— matches WA Web socket loop reconnectionFibonacci reconnect backoff (
src/client.rs){ type: "fibonacci", first: 1000, second: 1000, jitter: 0.1, max: 9e5 }Clock skew (
src/unified_session.rs)update_server_time_offset_with_rttfor RTT-adjusted midpoint calculationWA Web modules referenced
WAWebCommsConfig(I7eq_pFgCZw.js) —healthCheckInterval,deadSocketTime,socketReconnectBackoffAlgoWAComms(TSXjrkVar4k.js) —sendPing,maybeScheduleHealthCheck,deadSocketTimer,parseAndHandleStanza,callStanzaWAWebCommsHandleStanza(3vCxym4tMOo.js) — server ping response formatTest plan
ms_sincehelper testsis_dead_sockettests (all edge cases: never sent, received after send, sent recently, stale no reply, old reply, recent reply)fibonacci_backofftests (sequence, max cap, first attempt)cargo fmt --allcleancargo clippy --all --testscleanSummary by CodeRabbit
New Features
Bug Fixes
Performance
Tests & Docs