fix: re-subscribe rooms stream after forced socket reopen - #7362
Merged
Conversation
diegolmello
temporarily deployed
to
approve_e2e_testing
May 29, 2026 20:52 — with 
GitHub Actions
Inactive
GitHub Actions
Inactive
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📜 Recent review details⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
WalkthroughThe PR calls ChangesStream Close Unsubscribe Rooms Guard Reset
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
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 |
diegolmello
had a problem deploying
to
upload_android
May 29, 2026 21:36 — with
GitHub Actions
Error
GitHub Actions
Error
|
Android Build Available Rocket.Chat 4.73.0.108999 Internal App Sharing: https://play.google.com/apps/test/RQQ8k09hlnQ/ahAO29uNSzEZLV48QqMLm01fO63uOynryyNiHxSqus-xRmI9xiF90aqh3JWOzuKZ_cCOI-x38Z8tkEGnT-xDQ7OYOc |
|
iOS Build Available Rocket.Chat 4.73.0.109000 |
A long background marks the DDP socket stale, so foregrounding triggers checkAndReopen -> forceReopen, which drops all SDK subscriptions and reopens the socket without going through connect(). The module-level roomsSubscription guard therefore stayed set, so subscribeRooms() skipped re-subscribing stream-notify-user and the rooms list silently stopped reflecting subscriptions/favorites/reads until a manual reconnect. Reset the guard from the socket 'close' listener (via unsubscribeRooms), the same teardown connect() already performs, so the resume-login that follows the reopen re-subscribes stream-notify-user. Other streams (permissions, presence, settings, roles) subscribe unconditionally and were never affected.
diegolmello
force-pushed
the
rooms-freeze-android
branch
from
June 1, 2026 20:15
19944a4 to
6dd8818
Compare
diegolmello
had a problem deploying
to
approve_e2e_testing
June 1, 2026 20:15 — with
GitHub Actions
Failure
GitHub Actions
Failure
diegolmello
had a problem deploying
to
android_build
June 1, 2026 20:19 — with
GitHub Actions
Failure
GitHub Actions
Failure
This was referenced Jun 9, 2026
steffenkleinle
pushed a commit
to netzbegruenung/chatbegruenung-app
that referenced
this pull request
Jun 16, 2026
Brings the 4.73.1 release into the single-server line. Conflict resolution (mirrors the 4.73.0 single-server merge, NATIVE-1205): - Version files (package.json, build.gradle, Info.plist x2, pbxproj) -> 4.73.1 - android/gradle.properties -> single-server values (APPLICATION_ID=chat.rocket.reactnative, VERSIONCODE=1, keystore block) - Firebase configs kept deleted (google-services.json, GoogleService-Info.plist, debug strings.xml) - connect.ts / connect.test.ts -> 4.73.1 (single-server merely lacked the unsubscribeRooms() close-listener fix RocketChat#7362) - pnpm-lock.yaml / Podfile.lock -> 4.73.1 (only delta was picker 2.11.1 -> 2.11.4) - Single-server product customizations intact (app.json hardcoded server, disabled NewServerView, disabled ServersList switcher)
10 tasks
This was referenced Jul 23, 2026
diegolmello
added a commit
that referenced
this pull request
Jul 31, 2026
)" This reverts commit cd6f2a8.
diegolmello
added a commit
that referenced
this pull request
Jul 31, 2026
…fixes (#7521) * Revert "fix: re-subscribe room streams after re-authentication on reconnect (#7475)" This reverts commit 6e98b8b. * Revert "fix: re-subscribe room streams on DDP reconnect (#7426)" This reverts commit da389be. * Revert "fix: re-subscribe rooms stream after forced socket reopen (#7362)" This reverts commit cd6f2a8. * Revert "fix: keep live socket on deeplink login (prevent orphan WebSocket clobber) (#7380)" This reverts commit 80989f5. * Revert "fix(voip,connect): probe Meteor Connect on foreground and lock-screen accept (#7298)" This reverts commit 2af4bda. * fix: rewire pendingHangups drain without awaitDdpLoggedIn * fix: remove 7380 orphan-socket gate tests from deepLinking * test: cover handleClickCallPush new-server path and pendingHangups login-wait drain * refactor: extract isLoginReady helper in connect.ts * fix: restore orphan-socket guard in SDK patch * test: restore updateMessage concurrency and subscribe coverage * refactor: address review nits in connect and MediaSessionInstance * fix(voip): await processSignal before native-accept replay processSignal returns a promise and mutates the state that tryAnswerIfNativeAcceptedNotification reads, so both call sites must sequence on it. Also corrects the isLoginReady comment: close does clear meteor.connected, but neither it nor ddp.loggedIn survives a silent background death.
diegolmello
added a commit
that referenced
this pull request
Jul 31, 2026
* Revert "fix: re-subscribe room streams after re-authentication on reconnect (#7475)" This reverts commit 6e98b8b. * Revert "fix: re-subscribe room streams on DDP reconnect (#7426)" This reverts commit da389be. * Revert "fix: re-subscribe rooms stream after forced socket reopen (#7362)" This reverts commit cd6f2a8. * Revert "fix: keep live socket on deeplink login (prevent orphan WebSocket clobber) (#7380)" This reverts commit 80989f5. * Revert "fix(voip,connect): probe Meteor Connect on foreground and lock-screen accept (#7298)" This reverts commit 2af4bda. * fix: rewire pendingHangups drain without awaitDdpLoggedIn * fix: remove 7380 orphan-socket gate tests from deepLinking * test: cover handleClickCallPush new-server path and pendingHangups login-wait drain * refactor: extract isLoginReady helper in connect.ts * fix: restore orphan-socket guard in SDK patch * test: restore updateMessage concurrency and subscribe coverage * refactor: address review nits in connect and MediaSessionInstance * feat(sdk): add reopenNow, liveness probe, and disconnected emit to DDP socket The SDK send() waits on a 'disconnected' event that nothing ever emitted, so zombie sockets caused in-flight sends to hang forever. Add reopenNow() to force a single shared reconnect and emit 'disconnected' to reject those sends, plus a bounded probe() for gray-zone liveness checks. Restore ddpSocket.test.ts with coverage for probe, reopenNow, subscription preservation, concurrent reconnect deduplication, and the send() listener-leak fix. Add @rocket.chat/sdk and tiny-events to Jest's transform-ignore exceptions so the SDK's TypeScript source is transformed. * fix(voip): gate native call accept on socket readiness After a long suspension the DDP socket can be a zombie: readyState=1 and connected=true while sends hang and no pong arrives. The native accept path previously replayed/answered immediately, so the WebRTC setup timed out at the remote-sdp stage. Add a single guarded accept helper that every accept path funnels through: - classify the socket by lastPing age and force reopenNow() when stale; - wait for login readiness and for the media-signal/media-calls subscriptions to be acked on the current socket; - replay REST state signals and answer only if the call is not already bound; - on timeout/failure terminate the native call, reset the native accepted id, and queue a best-effort hangup. Expose the socket via sdk.current.ddp and add DDPDriver passthroughs plus a waitForNotifyUserMediaSubs readiness helper in the SDK patch. Also guard checkVoipPermission so it does not reset the media session while a call is active or being accepted. * fix(sdk): serialize forced reopen against concurrent open, harden probe * refactor(voip): share socket health classification Extract the age/ping classification into classifySocketHealth in waitForLoginReady.ts and make the foreground-saga helper getSocketStaleness delegate to it. The accept gate imports from the lightweight helper file to avoid pulling connect.ts (and its heavy deps) into the VoIP unit tests. * fix(connect): reconnect immediately when foregrounding a stale socket On foreground after long suspension the DDP socket can be zombie while redux still reads connected=true. Classify socket freshness via lastPing and pingInterval: reopen immediately when stale, probe in the gray zone, and keep the existing checkAndReopen path for healthy/fresh sockets. Adds an in-flight probe guard so rapid AppState flaps do not stack probes. * fix(voip): stop aborted accept gates from terminating the call Aborted gates now return early without running the failure ladder (terminate/endCall). activeGates cleanup only deletes its own controller so newer gates survive. Live-signal 'accepted' notifications from the stream listener funnel through acceptNativeCallWithReadiness instead of calling answerCall directly. SDK waitForNotifyUserMediaSubs now polls up to the timeout for media-signal/media-calls subscriptions to appear after a forced reopen. Also type the DDP shape in acceptNativeCall and remove "as any" from the AbortSignal fallback. * test(voip): align call-lifecycle integration tests with gated accept * fix(voip): await processSignal before native-accept replay processSignal returns a promise and mutates the state that tryAnswerIfNativeAcceptedNotification reads, so both call sites must sequence on it. Also corrects the isLoginReady comment: close does clear meteor.connected, but neither it nor ddp.loggedIn survives a silent background death. * test(voip): tidy timer cleanup and duplicate mock in accept tests * fix(voip): terminate native call when readiness sequence throws * fix(sdk): require both media subs and reuse sub id when resubscribing * test(voip): integration coverage for accept gate ladder and socket probe * chore(voip): temporary reconnect-latency trace instrumentation, dev-only Marks app-foreground classification, reopen start, socket connecting/connected, login success, rooms sync done, and VoIP gate stages. Persists to a file so marks survive device lock (Metro disconnected), dumps to Metro console after foreground. Drop before merge. * fix(voip): classify closed socket as reopen regardless of ping age Ping-age-only classification labeled a known-dead socket healthy when the last pong was recent, bypassing the reopen ladder on foreground and in the native-call accept gate. Short-circuit on the driver connected getter, which already checks readyState. * fix(sdk): lower hardcoded ddp ping interval to 10s DDPDriver overrode the ping interval to 20s, doubling every freshness threshold derived from it. Also apply the sdk patch via pnpm patchedDependencies instead of the patch-package postinstall. * fix(sdk): apply sdk patch via pnpm only pnpm patchedDependencies already applies the patch at install, so the patch-package postinstall failed trying to apply it again and broke CI installs. Move the patch out of patches/ so patch-package never sees it. * fix(sdk): revert to patch-package, keep 10s ping interval The pnpm patchedDependencies migration double-applied the sdk patch with the patch-package postinstall and broke CI installs. patch-package 8 also cannot author patches under pnpm, so a single mechanism wins: restore patch-package as the sole applier and hand-add the timeout hunk to its patch. Verified against pristine with patch-package's own apply engine. * fix(voip): guard AppState access in reconnect trace The react-native mock leaves AppState undefined, crashing every suite that imports the trace module. * test(voip): match integration ping interval to 10s sdk patch * fix(voip): always confirm socket health with a round trip `classifySocketHealth` returned 'healthy' for any ping younger than one interval, and the accept gate skipped its verification round trip on that verdict. A young `lastPing` proves nothing: the SDK's `onOpen` sets it before awaiting the connect reply and `onMessage` refreshes it on any frame, so a frozen socket can carry a fresh timestamp. Collapse the classification to 'probe' | 'reopen' and have the accept gate verify every non-reopen verdict, so a frozen or mid-handshake socket gets reopened instead of answered into silence. The foreground ladder follows: a quiet-but-fresh socket now probes rather than falling through to `checkAndReopen`, and 'fresh' is reachable only when the SDK lacks the probe/reopen hooks. Also drop the `connected` half of the `appHasComeBackToForeground` guard. A real socket close sets `meteor.connected = false`, so that guard returned early exactly when `reopenNow()` was needed. * remove reconnectMark * refactor(connection): extract socket health module with unit suite * test(connection): add socket health integration suite * refactor(connection): move foreground socket recovery onto socket health module * refactor(voip): gate call accept on socket health module * test(connection): type the socket health integration scaffolding * refactor(connection): consolidate onAbort into shared helper Three copies of onAbort (waitForLoginReady, acceptNativeCall, socketHealth) collapse into app/lib/methods/helpers/onAbort.ts. The helper is addEventListener-only: React Native's AbortController polyfill (abort-controller/event-target-shim) always provides addEventListener, so the legacy onabort fallback was dead code and carried a last-writer-wins overwrite hazard. Superset behavior wins where the copies diverged: null signal is a no-op and an already-aborted signal fires the callback immediately, which closes recoverSocket's blind spot where a pre-aborted signal never resolved 'abandoned'. * refactor(connection): fold executeRecovery into shareRecovery * Revert "fix: re-subscribe room streams after re-authentication on reconnect (#7475)" This reverts commit 6e98b8b. * Revert "fix: re-subscribe room streams on DDP reconnect (#7426)" This reverts commit da389be. * Revert "fix: re-subscribe rooms stream after forced socket reopen (#7362)" This reverts commit cd6f2a8. * Revert "fix: keep live socket on deeplink login (prevent orphan WebSocket clobber) (#7380)" This reverts commit 80989f5. * Revert "fix(voip,connect): probe Meteor Connect on foreground and lock-screen accept (#7298)" This reverts commit 2af4bda. * fix: rewire pendingHangups drain without awaitDdpLoggedIn * fix: remove 7380 orphan-socket gate tests from deepLinking * test: cover handleClickCallPush new-server path and pendingHangups login-wait drain * refactor: extract isLoginReady helper in connect.ts * fix: restore orphan-socket guard in SDK patch * test: restore updateMessage concurrency and subscribe coverage * refactor: address review nits in connect and MediaSessionInstance * fix(voip): await processSignal before native-accept replay processSignal returns a promise and mutates the state that tryAnswerIfNativeAcceptedNotification reads, so both call sites must sequence on it. Also corrects the isLoginReady comment: close does clear meteor.connected, but neither it nor ddp.loggedIn survives a silent background death.
diegolmello
added a commit
that referenced
this pull request
Jul 31, 2026
* Revert "fix: re-subscribe room streams after re-authentication on reconnect (#7475)" This reverts commit 6e98b8b. * Revert "fix: re-subscribe room streams on DDP reconnect (#7426)" This reverts commit da389be. * Revert "fix: re-subscribe rooms stream after forced socket reopen (#7362)" This reverts commit cd6f2a8. * Revert "fix: keep live socket on deeplink login (prevent orphan WebSocket clobber) (#7380)" This reverts commit 80989f5. * Revert "fix(voip,connect): probe Meteor Connect on foreground and lock-screen accept (#7298)" This reverts commit 2af4bda. * fix: rewire pendingHangups drain without awaitDdpLoggedIn * fix: remove 7380 orphan-socket gate tests from deepLinking * test: cover handleClickCallPush new-server path and pendingHangups login-wait drain * refactor: extract isLoginReady helper in connect.ts * fix(sync): derive the message sync cursor from the server clock `subscription.lastOpen` did double duty as both the `chat.syncMessages` fetch cursor and the unread-separator anchor, and every writer stamped it from the DEVICE clock. The server compares that cursor against message `_updatedAt`, which is a SERVER clock value. Any forward device skew — or simply closing a room, which stamped `lastOpen = new Date()` regardless of what had been fetched — pushed the cursor past messages that existed on the server but had never reached the device. Those messages were then permanently invisible: the server would never report anything "newer" than the cursor again. `lastOpen` now means one thing only: the max `_updatedAt` of the server response actually received for that room. - add `writeSyncWatermark`, which reads the max `_updatedAt` off the RAW payload. It must run before `normalizeMessage`, which invents a device-clock `_updatedAt` for rows lacking one. It is deliberately not monotonic, so an already-poisoned future cursor can heal. - write the watermark only where the payload proves coverage: from `updated` once the `chat.syncMessages` UPDATED cursor drains, and from the first batch of an initial tail load. The jump paths never write it — a ts-ordered forward walk never sees the `_updatedAt` of pre-anchor messages. - delete every device-clock writer: `updateLastOpen`, the `unsubscribe()` call that closed a room by advancing the cursor, and the `readMessages` `updateLastOpen` flag. Closing a room offline no longer loses messages. - treat a cursor in the future as absent so a skewed room falls back to a full tail load instead of syncing from nothing. - `loadMissedMessages` no longer accepts a `lastOpen` argument, removing the last path by which a caller could inject a device clock. - move the unread separator to its own RoomView state field `lastSeen`, fed from `room.ls`, so it no longer reads a fetch watermark. The DB column keeps its `lastOpen` name; no migration. * fix(state): re-sync the open room on foreground over REST Backgrounding can outlive the DDP socket without emitting `connected` or `close`, so `RoomSubscription.handleConnection` never fires and messages sent during the window never land. `chat.syncMessages` rides REST, so calling `loadMissedMessages` from the foreground saga delivers the intent of the reverted socket-probing change with no connection-layer risk: no probing, no forced reopen, nothing racing the SDK reconnect timer. `checkAndReopen()` is deleted — it no-ops when connected, which is exactly the state the foreground gate requires, so it could never heal anything. Also resets `RoomSubscription.isAlive` in `subscribe`, which was only ever set in the constructor; a reused instance would immediately unsubscribe itself. * fix(sync): self-heal a sync cursor that sits ahead of the server Deriving the cursor from the server clock stops new poisoning, but it cannot undo it. Rows already in users' databases carry a device-clock `lastOpen` written by the deleted `updateLastOpen` — closing a room while offline stamped `new Date()` regardless of what had actually been fetched. Such a cursor sits ahead of the server's newest `_updatedAt` for the room, so every `chat.syncMessages` drains an empty UPDATED page, there is no `_updatedAt` to take as a watermark, and the gap is permanent. The future-skew clamp does not catch it: the value was device-now when written and is now in the past. There is deliberately no migration and no backfill, so those cursors heal themselves on the next open instead. When the UPDATED cursor drains with an empty payload, compare the subscription's server-supplied `lastMessage._id` against the local messages table. If the server says a newest message exists and it was never delivered here, the cursor is provably lying: run a full tail load, which re-anchors the watermark from a real server response. The check costs one `getMessageById` on a sync that had nothing to do anyway, never runs mid-pagination or on a non-empty payload, skips a room with no `lastMessage`, and recovers via `loadMessagesForRoom` so it cannot recurse. Also document the timestamp trust boundary in CONTEXT.md: a `_updatedAt` from a server response is server truth and the only legitimate cursor source, while the same field on a WatermelonDB row is device-tainted. * fix(state): request the rooms delta on foreground `roomsRequest` rode only `LOGIN.SUCCESS`. If the socket died silently while backgrounded, every `stream-notify-user` update was lost and this delta fetch is the only thing that heals the rooms list. * fix(RoomView): re-run init when the subscription row arrives and bound the retry A notification tap for a room the user was just added to routes through canOpenRoom, which returns a bare { rid } and creates no subscription row. RoomView therefore falls into findAndObserveRoom, which throws and installs observeSubscriptions. When the row later arrives from the rooms sync, observeSubscriptions only swapped it into state, so init() never re-ran: the room got no ls-based unread separator and sent no read receipt. Re-run init() on the transition from "no row" to "row present", guarded by a one-shot flag plus an in-flight flag so a concurrent or repeated emission cannot re-enter it. The init() failure path also re-armed a 300ms timer with no cap, hammering a room that had no row yet. It is now capped at 5 attempts with exponential backoff, the previous handle is cleared before re-arming, and the counter resets once init() succeeds. * test(RoomView): share one awaiting act flush helper The nine inline `act(async () => {})` calls tripped require-await and no-await-in-loop, failing the eslint CI gate. A single helper that awaits a microtask expresses the same flush honestly instead of suppressing the rules. * fix(RoomView): lazy state reads in init to heal notification-tap race * refactor(sync): align cursor terminology and drop dead code * fix: restore orphan-socket guard in SDK patch * test: restore updateMessage concurrency and subscribe coverage * fix(sync): derive the cursor from every fetched batch An edited older message can postdate batch 1's newest _updatedAt, so a first-batch-only snapshot could leave the cursor below it and miss later edits. Snapshot the raw _updatedAt of all batches before updateMessages mutates rows, and collapse updateLastOpen's max loop to Math.max with the invalid-date filter kept. * refactor: address review nits in connect and MediaSessionInstance * fix(RoomView): dispatch init load on cursor presence, not subscription row A room with lastOpen resumes via the missed-messages loader; a room without one pulls history directly, ending the double fetch when a push-notification open starts before the subscription row arrives. Also renames the read-receipt local newLastOpen to readReceiptTime so it no longer masquerades as a cursor. * fix(sync): throttle self-heal tail load per room * fix(sync): throttle foreground rooms-delta request Rapid foreground/background cycles were dispatching roomsRequest() on every FOREGROUND, hammering the rooms-delta endpoint. Add a 60-second module-level throttle so cycles inside the window collapse to a single delta fetch, while a foreground after the window still heals the rooms list over REST. The 60-second window balances spam protection against missed-rooms risk: the call exists to recover from a silently dead socket, so a window much longer would re-open real missed-rooms windows, while the spam scenario is seconds to a minute. * test(sagas): pin Date.now past the rooms-delta throttle window Two foreground tests in state.test.ts ran within the same real-time 60s window, so the throttle in state.js suppressed the second dispatch and CI went red. Mock Date.now to jump 2 minutes per read so each test's foreground is always eligible. * feat(sdk): add reopenNow, liveness probe, and disconnected emit to DDP socket The SDK send() waits on a 'disconnected' event that nothing ever emitted, so zombie sockets caused in-flight sends to hang forever. Add reopenNow() to force a single shared reconnect and emit 'disconnected' to reject those sends, plus a bounded probe() for gray-zone liveness checks. Restore ddpSocket.test.ts with coverage for probe, reopenNow, subscription preservation, concurrent reconnect deduplication, and the send() listener-leak fix. Add @rocket.chat/sdk and tiny-events to Jest's transform-ignore exceptions so the SDK's TypeScript source is transformed. * fix(voip): gate native call accept on socket readiness After a long suspension the DDP socket can be a zombie: readyState=1 and connected=true while sends hang and no pong arrives. The native accept path previously replayed/answered immediately, so the WebRTC setup timed out at the remote-sdp stage. Add a single guarded accept helper that every accept path funnels through: - classify the socket by lastPing age and force reopenNow() when stale; - wait for login readiness and for the media-signal/media-calls subscriptions to be acked on the current socket; - replay REST state signals and answer only if the call is not already bound; - on timeout/failure terminate the native call, reset the native accepted id, and queue a best-effort hangup. Expose the socket via sdk.current.ddp and add DDPDriver passthroughs plus a waitForNotifyUserMediaSubs readiness helper in the SDK patch. Also guard checkVoipPermission so it does not reset the media session while a call is active or being accepted. * fix(sdk): serialize forced reopen against concurrent open, harden probe * refactor(voip): share socket health classification Extract the age/ping classification into classifySocketHealth in waitForLoginReady.ts and make the foreground-saga helper getSocketStaleness delegate to it. The accept gate imports from the lightweight helper file to avoid pulling connect.ts (and its heavy deps) into the VoIP unit tests. * fix(connect): reconnect immediately when foregrounding a stale socket On foreground after long suspension the DDP socket can be zombie while redux still reads connected=true. Classify socket freshness via lastPing and pingInterval: reopen immediately when stale, probe in the gray zone, and keep the existing checkAndReopen path for healthy/fresh sockets. Adds an in-flight probe guard so rapid AppState flaps do not stack probes. * fix(voip): stop aborted accept gates from terminating the call Aborted gates now return early without running the failure ladder (terminate/endCall). activeGates cleanup only deletes its own controller so newer gates survive. Live-signal 'accepted' notifications from the stream listener funnel through acceptNativeCallWithReadiness instead of calling answerCall directly. SDK waitForNotifyUserMediaSubs now polls up to the timeout for media-signal/media-calls subscriptions to appear after a forced reopen. Also type the DDP shape in acceptNativeCall and remove "as any" from the AbortSignal fallback. * test(voip): align call-lifecycle integration tests with gated accept * fix(sync): drop the lying-cursor heal `normalizeCursor` read `subscription.lastMessage._id` and, when that id was absent from the local `messages` table, declared the persisted cursor a lie and ran a full tail load behind a 5-minute per-room cooldown. The predicate cannot detect what it claims to. `app/lib/methods/subscriptions/rooms.ts:190-215` persists `subscription.lastMessage` as a message row whenever the room is not the one currently open, so finding that `_id` locally proves nothing about contiguity between the cursor and it. The rooms-delta back-fills the row while the messages between the cursor and it are still missing, which means the heal goes quiet in exactly the back-filled poisoned-cursor case this branch exists to close. The only case it does fire on is a tombstone `lastMessage` — a deleted newest message that never resolves locally — which is also the sole reason the cooldown existed. The reported repros are already covered without it: `load()` discards a cursor that sits ahead of server time, and a room with no usable cursor falls back to a full recent-history load. Both are kept and still tested. * fix(voip): await processSignal before native-accept replay processSignal returns a promise and mutates the state that tryAnswerIfNativeAcceptedNotification reads, so both call sites must sequence on it. Also corrects the isLoginReady comment: close does clear meteor.connected, but neither it nor ddp.loggedIn survives a silent background death. * revert(sync): drop the foreground re-sync chain Coming back to the foreground resumes the connection through `checkAndReopen` again, as it did before this branch: no per-room `chat.syncMessages` re-sync, no extra rooms-delta dispatch, and no throttle around it. This PR is scoped to one change only — deriving the message sync cursor from the server's `_updatedAt` instead of the device clock. The foreground re-sync is an independent chain: it targets a different failure (a silently dead socket that drops stream updates), it reaches into the connection layer, and it collides with the `checkAndReopen` work owned elsewhere. Restoring `connect.ts` byte-identical to base keeps that ownership clean in either merge order. The saga tests that only covered the removed behavior go with it, including the source guard asserting the saga no longer references `checkAndReopen`. * revert(RoomView): drop the init control-flow chain Opening a room initializes once again: no re-entrancy guard, no bounded retry with exponential backoff, no `init()` re-run driven by the subscription-row observer, and no lazy re-reads of room and joined state after the network await. That chain targets the notification-tap race — a room opened before its subscription row exists — which is a separate defect from the one this PR fixes. This PR is scoped to deriving the sync cursor from the server's `_updatedAt`, and the `lastOpen`/`lastSeen` column split that makes the cursor a distinct column from the read receipt. That split stays: `lastOpen` is the cursor, `lastSeen` anchors the unread separator, and a room is routed by whether it already holds a cursor. The RoomView cases covering retry, re-entrancy and row adoption go with the behavior; the cursor-predicate cases remain. * fix(sync): snapshot the cursor from the first batch only The initial tail load can recurse over several pages of history when hidden system messages consume the visible page. Take the Last Open snapshot from the first batch only, instead of accumulating the raw `_updatedAt` of every page. Accumulating over every batch maxes over a superset of the first batch, so it can only push the cursor higher — and higher is the unsafe direction: cursor < change._updatedAt -> server returns the change again -> wasteful, safe cursor > change._updatedAt -> server stays silent -> the missing-message bug The earlier every-batch change argued the opposite and was wrong about which way the risk points. The asymmetry is structural: the tail load paginates by creation time while the cursor lives on modification time, so no maximum taken over a tail load can honestly mean "seen everything below this". The conservative snapshot is the one that re-fetches. `updateLastOpen` keeps its rewritten body — a `Math.max` over the filtered numeric times, invalid dates dropped, empty payload a no-op — and the write gate is untouched: older pages and gap fills still never move the cursor. * docs: describe only the cursor mechanisms that still exist The Lying Cursor paragraph named a heal that this branch no longer carries, and the vocabulary around it belonged to the carved foreground-resync and RoomView-init chains rather than to the cursor change this PR is scoped to. Replace it with the direction of safety stated the right way round: cursor < change._updatedAt -> server returns the change again -> wasteful, safe cursor > change._updatedAt -> server stays silent -> the missing-message bug The Timestamp Trust Boundary section and the Last Open / Last Seen / Server Timestamp / Device Timestamp table stay: they document the vocabulary the cursor change is written in. * test(voip): tidy timer cleanup and duplicate mock in accept tests * fix(voip): terminate native call when readiness sequence throws * fix(sdk): require both media subs and reuse sub id when resubscribing * fix(read): let the server own the ls timestamp readMessages wrote `ls` from the device clock while POSTing subscriptions.read, so a skewed clock drew the unread separator early or late until the next subscriptions.get corrected it. The server already stamps `ls` and the subscription stream delivers it via createOrUpdateSubscription, so drop the optimistic write and the now unused `ls` parameter. * fix(sync): drop loadMissedMessages' cursor guards getMessages is the sole gate between loadMessagesForRoom and loadMissedMessages, so the no-cursor fallback and the future-cursor guard duplicated its job. Cursor writes are server-derived now, so a skewed device clock can no longer poison lastOpen. * fix(sync): take the Last Open from every batch and page Both loaders now accumulate the raw server `_updatedAt` of every batch or page they walk and write the cursor once, at the end, instead of trusting a single batch to carry the newest stamp. `loadMessagesForRoom` snapshots inside `fetchBatch` for every batch, not only the first: the tail load can recurse over several pages of history when hidden system messages consume the visible page, and the fetch order within that load is not part of the contract. Taking the max across all of them is future proof — if the first batch ever became the oldest one, a first-batch-only snapshot would silently lower the cursor. `loadMissedMessages` threads the accumulator through its pagination. Its pages descend from the newest, so writing from the last page alone landed the cursor on the oldest page walked. The write still fires only once the UPDATED cursor has drained, and only on a call that actually fetched an UPDATED page — a DELETED-only continuation must not write again. An abort mid-pagination writes no cursor: the run is re-fetched. `updateLastOpen` is untouched and stays deliberately non-monotonic, so an older server value can still heal a poisoned cursor. The write gate in `loadMessagesForRoom` is untouched too: older pages (`latest`) and gap fills (`loaderItem`) never reach the newest history regardless of fetch order, so letting them write could only lower the cursor. * test(voip): integration coverage for accept gate ladder and socket probe * fix(sync): return early from loadMissedMessages without a subscription * refactor(sync): name the Server Timestamp carrier and its snapshot * chore(sync): drop review-flagged comments * fix(sync): handle CodeRabbit round-3 findings - catch rejections from the un-awaited pagination continuation - skip null _updatedAt payload rows so a null-only payload can't write an epoch-0 cursor - annotate the resume-sync fixture helper and assert the persisted cursor is sent to chat.syncMessages - re-point the null-lastOpen resume test: the history fallback was removed, RoomView's getMessages owns the initial load * chore: format code and fix lint issues * chore(voip): temporary reconnect-latency trace instrumentation, dev-only Marks app-foreground classification, reopen start, socket connecting/connected, login success, rooms sync done, and VoIP gate stages. Persists to a file so marks survive device lock (Metro disconnected), dumps to Metro console after foreground. Drop before merge. * fix(voip): classify closed socket as reopen regardless of ping age Ping-age-only classification labeled a known-dead socket healthy when the last pong was recent, bypassing the reopen ladder on foreground and in the native-call accept gate. Short-circuit on the driver connected getter, which already checks readyState. * fix(sdk): lower hardcoded ddp ping interval to 10s DDPDriver overrode the ping interval to 20s, doubling every freshness threshold derived from it. Also apply the sdk patch via pnpm patchedDependencies instead of the patch-package postinstall. * fix(sdk): apply sdk patch via pnpm only pnpm patchedDependencies already applies the patch at install, so the patch-package postinstall failed trying to apply it again and broke CI installs. Move the patch out of patches/ so patch-package never sees it. * fix(sdk): revert to patch-package, keep 10s ping interval The pnpm patchedDependencies migration double-applied the sdk patch with the patch-package postinstall and broke CI installs. patch-package 8 also cannot author patches under pnpm, so a single mechanism wins: restore patch-package as the sole applier and hand-add the timeout hunk to its patch. Verified against pristine with patch-package's own apply engine. * fix(voip): guard AppState access in reconnect trace The react-native mock leaves AppState undefined, crashing every suite that imports the trace module. * test(voip): match integration ping interval to 10s sdk patch * fix(voip): always confirm socket health with a round trip `classifySocketHealth` returned 'healthy' for any ping younger than one interval, and the accept gate skipped its verification round trip on that verdict. A young `lastPing` proves nothing: the SDK's `onOpen` sets it before awaiting the connect reply and `onMessage` refreshes it on any frame, so a frozen socket can carry a fresh timestamp. Collapse the classification to 'probe' | 'reopen' and have the accept gate verify every non-reopen verdict, so a frozen or mid-handshake socket gets reopened instead of answered into silence. The foreground ladder follows: a quiet-but-fresh socket now probes rather than falling through to `checkAndReopen`, and 'fresh' is reachable only when the SDK lacks the probe/reopen hooks. Also drop the `connected` half of the `appHasComeBackToForeground` guard. A real socket close sets `meteor.connected = false`, so that guard returned early exactly when `reopenNow()` was needed. * remove reconnectMark * refactor(connection): extract socket health module with unit suite * test(connection): add socket health integration suite * refactor(connection): move foreground socket recovery onto socket health module * refactor(voip): gate call accept on socket health module * test(connection): type the socket health integration scaffolding * refactor(connection): consolidate onAbort into shared helper Three copies of onAbort (waitForLoginReady, acceptNativeCall, socketHealth) collapse into app/lib/methods/helpers/onAbort.ts. The helper is addEventListener-only: React Native's AbortController polyfill (abort-controller/event-target-shim) always provides addEventListener, so the legacy onabort fallback was dead code and carried a last-writer-wins overwrite hazard. Superset behavior wins where the copies diverged: null signal is a no-op and an already-aborted signal fires the callback immediately, which closes recoverSocket's blind spot where a pre-aborted signal never resolved 'abandoned'. * refactor(connection): fold executeRecovery into shareRecovery * Revert "fix: re-subscribe room streams after re-authentication on reconnect (#7475)" This reverts commit 6e98b8b. * Revert "fix: re-subscribe room streams on DDP reconnect (#7426)" This reverts commit da389be. * Revert "fix: re-subscribe rooms stream after forced socket reopen (#7362)" This reverts commit cd6f2a8. * Revert "fix: keep live socket on deeplink login (prevent orphan WebSocket clobber) (#7380)" This reverts commit 80989f5. * Revert "fix(voip,connect): probe Meteor Connect on foreground and lock-screen accept (#7298)" This reverts commit 2af4bda. * fix: rewire pendingHangups drain without awaitDdpLoggedIn * fix: remove 7380 orphan-socket gate tests from deepLinking * test: cover handleClickCallPush new-server path and pendingHangups login-wait drain * refactor: extract isLoginReady helper in connect.ts * fix: restore orphan-socket guard in SDK patch * test: restore updateMessage concurrency and subscribe coverage * refactor: address review nits in connect and MediaSessionInstance * fix(voip): await processSignal before native-accept replay processSignal returns a promise and mutates the state that tryAnswerIfNativeAcceptedNotification reads, so both call sites must sequence on it. Also corrects the isLoginReady comment: close does clear meteor.connected, but neither it nor ddp.loggedIn survives a silent background death. --------- Co-authored-by: diegolmello <diegolmello@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed changes
After the app sits in the background for a while the DDP socket goes stale, so foregrounding triggers
checkAndReopen→forceReopen.forceReopendrops every SDK subscription and reopens the socket without going throughconnect(), so the module-levelroomsSubscriptionguard insubscribeRoomsstays set. The nextsubscribeRooms()therefore short-circuits and never re-subscribesstream-notify-user, and the rooms list silently stops reflecting subscription/favorite/read changes until a manual reconnect or app restart.This resets that guard from the socket
'close'listener (viaunsubscribeRooms()) — the same teardownconnect()already performs at startup — so the resume-login that follows the reopen re-subscribesstream-notify-user.stream-notify-useris the only stream behind that guard; the other streams (permissions, presence, settings, roles) callsdk.subscribe(...)unconditionally and were never affected.Issue(s)
https://rocketchat.atlassian.net/browse/SUP-1047
How to test or reproduce
subscriptions-changedevent — e.g. favorite/unfavorite a room or mark one read.Verified on an Android 16 emulator: after foregrounding,
subscribeRooms()re-subscribesstream-notify-userand a favorite toggle then fires the handler and the WatermelonDB write. A regression test inconnect.test.tsasserts the'close'listener callsunsubscribeRooms()(and fails if the call is removed).Types of changes
Checklist
Further comments
The fix lives in the
'close'listener rather than insubscribeRoomsitself because'close'is already the place app state is reset for reconnect (it dispatchesdisconnectAction()to flip Reduxmeteor.connected→ false). The SDK'sforceReopendocuments this contract: it emits'close'soconnect.tsflips that state, otherwise the next'connected'short-circuits and the loginRequest → subscribeNotifyUser chain never re-runs. TheroomsSubscriptionguard is just a second piece of "am I subscribed?" state that has to be reset in that same spot.Summary by CodeRabbit
Bug Fixes
Tests