fix: recover messages lost while an open room reconnects - #7547
fix: recover messages lost while an open room reconnects#7547diegolmello wants to merge 5 commits into
Conversation
Emits a per-room event when the server acks the room's stream-room-messages subscription, on first connect and on every reconnect. No consumer yet.
The fetch ran on the SDK `connected` event — the raw socket open, before the DDP handshake and before login — while the room's stream-room-messages subscription is only re-sent after the resume login. Messages the server accepted in between reached neither the fetch nor the stream, and nothing fetched again, so an open room silently lost them until a full app restart. The fetch now runs on the room stream ready signal, so its snapshot overlaps the live stream. It only runs when the socket dropped or re-handshaked since the last fetch, so opening a room still leaves the initial load to RoomView, and it keeps that flag set until a fetch succeeds so a failed one is retried on the next ack. How the `lastOpen` cursor is computed is untouched.
A catch-up fetch started around a socket close can resolve long after a new connection cycle already fetched. Its pages are older, and the sync cursor has no monotonic clamp, so a late result could overwrite rows the live stream had already updated and lower `lastOpen`. Each fetch now captures the connection cycle it belongs to and is dropped — before its pages are written, before it paginates further, and before the cursor is touched — once that cycle has ended or the room was left. Dropping it does not clear the reconnect flag, so the current cycle still fetches on its own ack. How the cursor is computed is untouched.
GitHub Actions
Inactive
WalkthroughRoom subscriptions now emit readiness after server acknowledgement, trigger missed-message loading after reconnect, and reject stale fetch results. Cursorless rooms use history loading, and synchronization pagination is capped. ChangesRoom stream synchronization
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Socket
participant RoomSubscription
participant DDP
participant loadMissedMessages
participant Emitter
Socket->>RoomSubscription: signal reconnection
RoomSubscription->>DDP: subscribe to room message stream
DDP-->>RoomSubscription: acknowledge stream subscription
RoomSubscription->>Emitter: emit roomStreamReady
RoomSubscription->>loadMissedMessages: fetch missed messages with cycle check
loadMissedMessages-->>RoomSubscription: update messages and cursor
Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (2)
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 |
GitHub Actions
Inactive
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
app/lib/methods/subscriptions/room.ts (2)
154-195: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd explicit return type annotations to the new methods.
handleDisconnection,handleReconnection,handleStreamReadySignal, andfetchMissedMessageshave no return type annotations.removeListenerat line 143 already declaresPromise<void>. Match that style so the async handlers state their contract.As per coding guidelines: "Use TypeScript for type safety; add explicit type annotations to function parameters and return types".
♻️ Proposed annotations
- handleDisconnection = () => { + handleDisconnection = (): void => { this.hasReconnected = true; this.connectionCycle += 1; reduxStore.dispatch(clearUserTyping()); }; /** A new DDP handshake also means a reconnect, including reopens that emit no `close`. */ - handleReconnection = () => { + handleReconnection = (): void => { this.hasReconnected = true; this.connectionCycle += 1; }; @@ - handleStreamReadySignal = async () => { + handleStreamReadySignal = async (): Promise<void> => { @@ - fetchMissedMessages = async (isStale: () => boolean) => { + fetchMissedMessages = async (isStale: () => boolean): Promise<void> => {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/lib/methods/subscriptions/room.ts` around lines 154 - 195, Add explicit return type annotations to handleDisconnection and handleReconnection using void, and to the async methods handleStreamReadySignal and fetchMissedMessages using Promise<void>. Keep their existing parameters and behavior unchanged, matching the Promise<void> style used by removeListener.Source: Coding guidelines
176-186: 🩺 Stability & Availability | 🔵 Trivial | ⚖️ Poor tradeoff
hasReconnectedclears before pagination finishes.
fetchMissedMessagesresolves after the first page.loadMissedMessagescontinues later pages through a detached recursive call, as shown inapp/lib/methods/loadMissedMessages.tslines 114-122.handleStreamReadySignaltherefore clearshasReconnectedwhile later pages are still in flight. If a later page fails, the next stream ack does not retry it, and thelastOpencursor is not advanced until a subsequent reconnect.The staleness checks still block stale writes, so no incorrect data lands. Consider returning the pagination promise from
loadMissedMessagesso the retry flag reflects the whole walk.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/lib/methods/subscriptions/room.ts` around lines 176 - 186, Update loadMissedMessages so its recursive pagination path returns or awaits the promise for subsequent pages, allowing fetchMissedMessages to resolve only after the full walk completes. Preserve the existing staleness checks and ensure handleStreamReadySignal clears hasReconnected only after all pages succeed, while failures keep the retry flag set.app/lib/methods/subscriptions/room.reconnectFetch.test.ts (1)
104-148: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUnsubscribe created
RoomSubscriptioninstances after each test. ARoomSubscriptionkeepshandleStreamReadySignalregistered on the module-level emitter untilunsubscriberuns. Both suites create instances and leave most of them alive, so instances accumulate across tests in the same file and still react to every readiness emit.app/lib/methods/subscriptions/room.staleFetch.test.tslines 125-129 already documents this hazard and solves it with anafterEachteardown.
app/lib/methods/subscriptions/room.reconnectFetch.test.ts#L104-L148: track every instance created byopenRoomand by the tests at lines 192 and 248, then unsubscribe them in anafterEach. Leftover instances whosehasReconnectedis still true issue their ownsdk.getcalls on a laterackRoomStream(), which makes the exact-count assertions at lines 223, 233, and 245 depend on test order.app/lib/methods/subscriptions/room.streamReady.test.ts#L47-L68: track the instances created in each test and unsubscribe them in the existingafterEach, next to theemitter.offcall.♻️ Proposed teardown for room.reconnectFetch.test.ts
let listeners: Record<string, (message: any) => void>; + /** Subscriptions opened by a test, torn down afterwards: the ready signal is module-level. */ + let opened: RoomSubscription[]; @@ beforeEach(() => { jest.clearAllMocks(); batched.length = 0; listeners = {}; + opened = []; @@ + afterEach(async () => { + await Promise.all(opened.map(subscription => subscription.unsubscribe())); + }); + const openRoom = async () => { const subscription = new RoomSubscription(RID); + opened.push(subscription); await subscription.subscribe();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/lib/methods/subscriptions/room.reconnectFetch.test.ts` around lines 104 - 148, Track every RoomSubscription created by openRoom and the direct test constructions in room.reconnectFetch.test.ts, then unsubscribe all tracked instances in an afterEach teardown. In room.streamReady.test.ts, track each test-created RoomSubscription and unsubscribe them in the existing afterEach beside the emitter.off cleanup, ensuring no subscription remains registered across tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/lib/services/sdk.ts`:
- Around line 184-187: Update getSubscriptionById in the SDK service to declare
an explicit optional return type matching the DDP subscription shape, so
consumers such as room subscription methods receive typed name and params
properties while preserving the existing lookup behavior.
---
Nitpick comments:
In `@app/lib/methods/subscriptions/room.reconnectFetch.test.ts`:
- Around line 104-148: Track every RoomSubscription created by openRoom and the
direct test constructions in room.reconnectFetch.test.ts, then unsubscribe all
tracked instances in an afterEach teardown. In room.streamReady.test.ts, track
each test-created RoomSubscription and unsubscribe them in the existing
afterEach beside the emitter.off cleanup, ensuring no subscription remains
registered across tests.
In `@app/lib/methods/subscriptions/room.ts`:
- Around line 154-195: Add explicit return type annotations to
handleDisconnection and handleReconnection using void, and to the async methods
handleStreamReadySignal and fetchMissedMessages using Promise<void>. Keep their
existing parameters and behavior unchanged, matching the Promise<void> style
used by removeListener.
- Around line 176-186: Update loadMissedMessages so its recursive pagination
path returns or awaits the promise for subsequent pages, allowing
fetchMissedMessages to resolve only after the full walk completes. Preserve the
existing staleness checks and ensure handleStreamReadySignal clears
hasReconnected only after all pages succeed, while failures keep the retry flag
set.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 461fd0fc-68e5-46e2-a5e9-807229ae446c
📒 Files selected for processing (10)
CONTEXT.mdapp/lib/methods/helpers/emitter.tsapp/lib/methods/loadMissedMessages.tsapp/lib/methods/subscriptions/room.reconnectFetch.test.tsapp/lib/methods/subscriptions/room.resumeSync.test.tsapp/lib/methods/subscriptions/room.staleFetch.test.tsapp/lib/methods/subscriptions/room.streamReady.test.tsapp/lib/methods/subscriptions/room.tsapp/lib/methods/subscriptions/roomCloseCursor.test.tsapp/lib/services/sdk.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: ESLint and Test / run-eslint-and-test
- GitHub Check: format
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,jsx,tsx}: Use descriptive names for functions, variables, and classes that clearly convey their purpose
Write comments that explain the 'why' behind code decisions, not the 'what'
Keep functions small and focused on a single responsibility
Use const by default, let when reassignment is needed, and avoid var
Prefer async/await over .then() chains for handling asynchronous operations
Use explicit error handling with try/catch blocks for async operations
Avoid deeply nested code; refactor complex logic into helper functions
Files:
app/lib/methods/subscriptions/roomCloseCursor.test.tsapp/lib/services/sdk.tsapp/lib/methods/subscriptions/room.reconnectFetch.test.tsapp/lib/methods/helpers/emitter.tsapp/lib/methods/subscriptions/room.resumeSync.test.tsapp/lib/methods/subscriptions/room.streamReady.test.tsapp/lib/methods/loadMissedMessages.tsapp/lib/methods/subscriptions/room.staleFetch.test.tsapp/lib/methods/subscriptions/room.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use TypeScript for type safety; add explicit type annotations to function parameters and return types
Prefer interfaces over type aliases for defining object shapes in TypeScript
Use enums for sets of related constants rather than magic strings or numbers
Files:
app/lib/methods/subscriptions/roomCloseCursor.test.tsapp/lib/services/sdk.tsapp/lib/methods/subscriptions/room.reconnectFetch.test.tsapp/lib/methods/helpers/emitter.tsapp/lib/methods/subscriptions/room.resumeSync.test.tsapp/lib/methods/subscriptions/room.streamReady.test.tsapp/lib/methods/loadMissedMessages.tsapp/lib/methods/subscriptions/room.staleFetch.test.tsapp/lib/methods/subscriptions/room.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{js,jsx,ts,tsx}: Format JavaScript and TypeScript code with Oxfmt using the repository configuration: tabs, single quotes, 130-character width, no trailing commas, omitted arrow-function parentheses where possible, and same-line brackets.
Follow Oxlint rules configured in.oxlintrc.json, including the import, React, Jest, TypeScript, and React Native plugins.
Files:
app/lib/methods/subscriptions/roomCloseCursor.test.tsapp/lib/services/sdk.tsapp/lib/methods/subscriptions/room.reconnectFetch.test.tsapp/lib/methods/helpers/emitter.tsapp/lib/methods/subscriptions/room.resumeSync.test.tsapp/lib/methods/subscriptions/room.streamReady.test.tsapp/lib/methods/loadMissedMessages.tsapp/lib/methods/subscriptions/room.staleFetch.test.tsapp/lib/methods/subscriptions/room.ts
🧠 Learnings (2)
📚 Learning: 2026-04-30T17:07:51.020Z
Learnt from: diegolmello
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7274
File: app/lib/services/voip/MediaCallEvents.ts:0-0
Timestamp: 2026-04-30T17:07:51.020Z
Learning: In this Rocket.Chat React Native codebase, the ESLint rule `no-void: error` is enforced. When you see a promise returned from an async call that is not awaited (a “floating promise”), do not silence it with the `void somePromise()` pattern. Instead, handle the promise explicitly by attaching `.catch(...)` (or otherwise awaiting/handling the error) so unhandled-rejection risks are addressed in a way that satisfies the existing ESLint configuration.
Applied to files:
app/lib/methods/subscriptions/roomCloseCursor.test.tsapp/lib/services/sdk.tsapp/lib/methods/subscriptions/room.reconnectFetch.test.tsapp/lib/methods/helpers/emitter.tsapp/lib/methods/subscriptions/room.resumeSync.test.tsapp/lib/methods/subscriptions/room.streamReady.test.tsapp/lib/methods/loadMissedMessages.tsapp/lib/methods/subscriptions/room.staleFetch.test.tsapp/lib/methods/subscriptions/room.ts
📚 Learning: 2026-06-25T18:37:25.526Z
Learnt from: diegolmello
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7434
File: app/views/ScreenLockConfigView.test.tsx:16-22
Timestamp: 2026-06-25T18:37:25.526Z
Learning: In Rocket.Chat ReactNative tests that mock selectors for `useAppSelector`, don’t require the mocked selector input to be typed as `IApplicationState` when the fixture only includes a partial Redux state slice (e.g., only `server` and `settings`). Requiring the full `IApplicationState` type in that scenario forces unsafe `as IApplicationState` casts and undermines type-safety. For these narrowly scoped selector-mock fixtures, use a less strict type (e.g., `any`) to keep the mock focused on the slice under test.
Applied to files:
app/lib/methods/subscriptions/roomCloseCursor.test.tsapp/lib/methods/subscriptions/room.reconnectFetch.test.tsapp/lib/methods/subscriptions/room.resumeSync.test.tsapp/lib/methods/subscriptions/room.streamReady.test.tsapp/lib/methods/subscriptions/room.staleFetch.test.ts
🔇 Additional comments (13)
app/lib/methods/helpers/emitter.ts (2)
18-28: LGTM!
9-17: 📐 Maintainability & Code QualityManual verification needed for
TKeyEmitterEventconsumers.The available check could not complete, so inspect code that maps over
TKeyEmitterEvent, buildsRecord<TKeyEmitterEvent, ...>, or switches exhaustively on it after the template-literal index signature was added.app/lib/methods/subscriptions/room.ts (2)
77-95: LGTM!
112-141: LGTM!app/lib/methods/subscriptions/room.streamReady.test.ts (1)
70-148: LGTM!CONTEXT.md (1)
200-206: LGTM!app/lib/methods/loadMissedMessages.ts (2)
92-94: LGTM!Also applies to: 108-112
114-122: LGTM!app/lib/methods/subscriptions/room.reconnectFetch.test.ts (1)
150-257: LGTM!app/lib/methods/subscriptions/room.staleFetch.test.ts (2)
100-137: LGTM!
139-230: LGTM!app/lib/methods/subscriptions/room.resumeSync.test.ts (1)
68-68: LGTM!Also applies to: 85-85
app/lib/methods/subscriptions/roomCloseCursor.test.ts (1)
118-118: LGTM!
| /** Look up a live DDP subscription by the id the server acks in a `ready` message. */ | ||
| getSubscriptionById(id: string) { | ||
| return this.current?.ddp?.subscriptions?.[id]; | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm the DDP subscription registry shape in the installed SDK.
fd -i -t f 'ddp*' node_modules/@rocket.chat/sdk 2>/dev/null | head -20
rg -nP -C4 'subscriptions\s*[:=]|this\.subscriptions' node_modules/@rocket.chat/sdk 2>/dev/null | head -60
rg -nP '"`@rocket`\.chat/sdk"' package.jsonRepository: RocketChat/Rocket.Chat.ReactNative
Length of output: 237
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "package manager and sdk references"
git ls-files | rg '(^|/)(package(-lock)?\.json|pnpm-lock\.yaml|yarn\.lock)$' || true
echo
echo "sdk.ts outline and relevant lines"
fd -a -t f '^sdk\.ts$' app | sed 's#^\./##' || true
file=$(fd -t f '^sdk\.ts$' app | head -1 || true)
if [ -n "${file:-}" ]; then
wc -l "$file"
ast-grep outline "$file" --match getSubscriptionById --view expanded || true
sed -n '1,240p' "$file"
fi
echo
echo "room.ts relevant usages"
file=$(fd -t f '^room\.ts$' app | head -1 || true)
if [ -n "${file:-}" ]; then
wc -l "$file"
sed -n '100,170p' "$file"
fi
echo
echo "dependency lock/sdk source references"
if [ -f package.json ]; then
rg -n '"`@rocket`\.chat/sdk"|RocketChat/Rocket.Chat.js.SDK#mobile' package.json package-lock.json pnpm-lock.yaml yarn.lock 2>/dev/null || true
fi
echo
echo "read-only local package lookup"
node - <<'JS'
const fs = require('fs');
const path = 'node_modules/@rocket.chat/sdk';
console.log(fs.existsSync(path));
if (!fs.existsSync(path)) process.exit(0);
for (const f of fs.readdirSync(path).slice(0, 80)) {
const p = `${path}/${f}`;
console.log(`${fs.lstatSync(p).isDirectory() ? 'DIR ' : 'FILE '} ${f}`);
}
JSRepository: RocketChat/Rocket.Chat.ReactNative
Length of output: 8075
🌐 Web query:
RocketChat.Rocket.Chat.js.SDK mobile getSubscriptionById ddp subscriptions ready
💡 Result:
In the context of the Rocket.Chat JavaScript SDKs, managing subscription readiness and retrieving subscriptions by ID follows different patterns depending on whether you are using the legacy Rocket.Chat.js.SDK or the newer @rocket.chat/ddp-client. Legacy Rocket.Chat.js.SDK This SDK uses an underlying Asteroid DDP connection to manage interactions [1][2]. - Subscriptions: The driver.subscribe(topic, roomId) method initiates a Meteor subscription and returns a promise that resolves with the subscription instance, which includes an ID property [1][2]. - Subscription Management: The driver manages an internal list of subscriptions [3]. Because these are DDP subscriptions, readiness is typically handled by the underlying Asteroid library, which receives a ready message from the server [4][5]. There is no explicit getSubscriptionById method in the high-level API; you generally work with the subscription instances returned by the subscribe methods or access the internal subscriptions array if necessary [3][2]. New @rocket.chat/ddp-client SDK This modern SDK provides more direct control over DDP streams [6]. - Stream subscriptions: You use the sdk.stream(topic, roomId, callback) method to subscribe to publications [6]. This returns a stream object that allows you to stop the subscription [6]. - Readiness: In DDP, the server sends a ready message once the initial data for a subscription has been sent. In testing environments for this SDK, subscription readiness can be verified by awaiting a ready method on the subscription object [4]. Key Technical Notes - DDP Ready State: When sending a DDP subscription request (msg: sub), the server confirms completion by sending a ready message for that specific subscription ID [5]. - Retrieving Subscriptions: If you need to retrieve actual room subscription data (e.g., room details, names, or IDs), this is typically done by calling the subscriptions/get method via the API client or the driver's method calling capabilities, rather than DDP stream subscription IDs [7][5]. The subscriptions/get method returns an array of subscription objects containing specific room and user information [7].
Citations:
- 1: https://github.com/RocketChat/Rocket.Chat.js.SDK/blob/master/README.md
- 2: https://github.com/RocketChat/Rocket.Chat.js.SDK
- 3: https://github.com/RocketChat/Rocket.Chat.js.SDK/blob/master/dist/lib/driver.d.ts
- 4: https://gitlab.ow2.org/RocketChat/Rocket.Chat/-/commit/5bb857fe564b8f6813cea9fb2f744fe2a402b754?file_path=packages%2Fddp-client%2F__tests__%2FDDPSDK.spec.ts
- 5: https://developer.rocket.chat/apidocs/subscriptions-method-calls
- 6: https://www.npmjs.com/package/@rocket.chat/ddp-client
- 7: https://developer.rocket.chat/apidocs/get-subscriptions-realtime
🌐 Web query:
github.com RocketChat Rocket.Chat.js.SDK mobile ddp.ts subscriptions
💡 Result:
In the context of Rocket.Chat development, DDP (Distributed Data Protocol) is used for real-time WebSocket communication, including subscriptions [1][2]. While older integrations frequently used the legacy Rocket.Chat.js.SDK (which relies on Asteroid for DDP connections and provides driver.subscribe methods) [3][4], current development and modern mobile applications are migrating to @rocket.chat/ddp-client [5][6]. Key points regarding DDP subscriptions in this ecosystem include: 1. Evolution of Libraries: The legacy Rocket.Chat.js.SDK managed subscriptions via an Asteroid DDP connection, with specific methods like driver.subscribeToMessages() and driver.subscribe(topic, roomId) [3][4]. Newer implementations, such as those in the Rocket.Chat React Native mobile app, have migrated to @rocket.chat/ddp-client to improve reliability, lifecycle management, and standard integration [5][6]. 2. DDP Subscription Mechanics: Subscriptions in Rocket.Chat are initiated via a DDP sub message, which includes a unique subscription ID, the stream name, and relevant parameters (e.g., room ID) [1][7]. The server responds with ready when the subscription is established [8][1]. Developers must ensure they are authenticated before sending these requests [1][9]. 3. Modern Implementation (@rocket.chat/ddp-client): This modern client simplifies streaming. For example, you can use sdk.stream('room-messages', roomId, callback) to listen to real-time message updates [6][10]. It is important to call stream.stop() when a subscription is no longer needed to optimize resources [6][10]. 4. Warning on Deprecation: Rocket.Chat's official documentation notes that legacy DDP method calls are considered deprecated [2][9]. While the Realtime API (including subscriptions) remains functional, developers are increasingly encouraged to utilize the REST API for critical functionality to ensure long-term stability [2][9]. For developers working with internal codebase files like ddp.ts (often found in the server or client core), these files typically define the schema and message handling logic for DDP communication, such as the sub, unsub, ready, and nosub messages that structure the protocol [8].
Citations:
- 1: https://developer.rocket.chat/apidocs/subscriptions-method-calls
- 2: https://developer.rocket.chat/apidocs/realtimeapi
- 3: https://github.com/RocketChat/Rocket.Chat.js.SDK
- 4: https://github.com/RocketChat/Rocket.Chat.js.SDK/blob/master/dist/lib/driver.d.ts
- 5: chore: ddp client #6800
- 6: https://registry.npmjs.org/@rocket.chat/ddp-client
- 7: https://github.com/RocketChat/developer-docs/blob/master/reference/api/realtime-api/README.md
- 8: https://github.com/RocketChat/Rocket.Chat/blob/d33009ab/apps/meteor/client/lib/streamer/ddp.ts
- 9: https://rocketchat-developer.us.document360.io/docs/realtime-api
- 10: https://www.npmjs.com/package/@rocket.chat/ddp-client
🌐 Web query:
RocketChat Rocket.Chat.js.SDK ddp.ts subscriptions: Record subscriptions this.subscriptions
💡 Result:
In the Rocket.Chat.js.SDK, the subscriptions variable (often referenced as this.subscriptions in related contexts like the core streamer or internal mock classes) is used to track and manage active DDP (Distributed Data Protocol) subscriptions [1][2][3]. Key technical details regarding subscriptions and their management in the SDK: 1. Subscription Tracking: The subscriptions variable generally holds an array or map of active subscription objects [1][2]. Within the Rocket.Chat.js.SDK driver, export declare let subscriptions: ISubscription[] is used to maintain a list of active Meteor subscriptions [1]. This variable is initialized after prepMeteorSubscriptions is called [1]. 2. Role in DDP: These subscription objects are critical for ensuring that the client remains connected to specific data streams (e.g., room message streams) [4][5]. The driver.subscribe(topic, roomId) method adds successful subscriptions to this tracking array, allowing the SDK to manage, poll, or clean them up as needed [1][6]. 3. Core Streamer Management: In the broader Rocket.Chat architecture (e.g., apps/meteor/client/lib/streamer/streamer.ts), this.subscriptions functions as a SubscriptionMap [2]. It maps event names to their respective subscription instances, providing methods to subscribe, unsubscribe, or stop individual streams while ensuring that redundant subscriptions are not created for the same event [2][7]. 4. Usage in SDK: Developers using the SDK typically interact with these subscriptions via high-level methods like driver.subscribeToMessages(), which simplifies the DDP sub-message process by defaulting to the stream-room-messages topic and the my_messages room ID [6][8]. Manual management of the subscriptions array is generally handled internally by the driver, though it is exported for diagnostic or polling purposes by custom adapters [1]. For developers implementing extensions or custom bots, it is recommended to use the provided driver methods (such as subscribe, unsubscribe, or reactToMessages) rather than manually manipulating the subscriptions array, to ensure state consistency [1][6].
Citations:
- 1: https://github.com/RocketChat/Rocket.Chat.js.SDK/blob/master/dist/lib/driver.d.ts
- 2: https://github.com/RocketChat/Rocket.Chat/blob/d33009ab/apps/meteor/client/lib/streamer/streamer.ts
- 3: https://github.com/RocketChat/Rocket.Chat/blob/e75965c0/packages/mock-providers/src/MockedAppRootBuilder.tsx
- 4: https://github.com/RocketChat/Rocket.Chat.js.SDK
- 5: https://github.com/rocketchat/Rocket.Chat.js.SDK
- 6: https://github.com/RocketChat/Rocket.Chat.js.SDK/blob/master/README.md
- 7: https://stackoverflow.com/questions/75151229/how-to-handle-stream-down-error-in-chat-application
- 8: https://github.com/rocketchat/rocket.chat.js.sdk
🏁 Script executed:
#!/bin/bash
set -euo pipefail
git ls-files | rg 'app/actions/room\.ts|app/(lib/services/sdk\.ts|actions/.*room|.*Room)' | head -80
if [ -f app/actions/room.ts ]; then
echo "room.ts size:"
wc -l app/actions/room.ts
echo "getSubscriptionById usages:"
rg -n "getSubscriptionById|subscription\?\.name|subscription\?\.params|MESSAGES_STREAM|ready" app/actions/room.ts app -g '*.ts' -g '*.tsx' || true
fi
echo "pnpm lock entry for mobile branch:"
sed -n '80,105p' pnpm-lock.yamlRepository: RocketChat/Rocket.Chat.ReactNative
Length of output: 26194
🌐 Web query:
github.com RocketChat Rocket.Chat.js.SDK b6d2b3f25b0ff8283dd71faf6dae720c47fecd8f ddp.ts subscriptions
💡 Result:
The specific commit b6d2b3f25b0ff8283dd71faf6dae720c47fecd8f refers to a change within the RocketChat/Rocket.Chat.js.SDK repository [1]. This SDK is a utility designed to allow bots and applications to interact with Rocket.Chat instances using DDP (via the Asteroid library) and REST APIs [1][2]. In the context of ddp.ts (often associated with the driver module in this SDK), the subscriptions variable and related functions are used to manage real-time event tracking from the Rocket.Chat server [3][2]. Key details regarding subscriptions in this SDK include: 1. Subscription Management: The subscriptions variable is an exported array used to track active Meteor/DDP subscriptions [3]. It is typically populated when prepMeteorSubscriptions is invoked [3]. 2. Core Functions: The SDK provides specific methods to interact with these subscriptions, including: - subscribe(topic, roomId): The base method for creating a subscription to a specific topic (e.g., room messages) [3][2]. - unsubscribe(subscription): Removes a specific subscription [3][2]. - subscribeToMessages(): A high-level helper that defaults to the stream-room-messages topic for my_messages [2]. - reactToMessages(callback) and respondToMessages(callback): Higher-level wrappers that allow developers to attach callbacks to handle incoming message stream data [3][2]. 3. Technical Context: The SDK relies on the Asteroid library to handle the underlying DDP connection [2][4]. Recent usage of this SDK has encountered compatibility challenges with newer Rocket.Chat versions (e.g., 7.1.0) due to changes in message validation and data structures [5]. If you are investigating this specific commit, it likely contains a modification to how these DDP subscriptions are initialized, tracked, or cleaned up within the driver logic [3]. You can view the specific diff by appending /commit/b6d2b3f25b0ff8283dd71faf6dae720c47fecd8f to the main repository URL on GitHub.
Citations:
- 1: https://github.com/RocketChat/Rocket.Chat.js.SDK
- 2: https://github.com/rocketchat/rocket.chat.js.sdk
- 3: https://github.com/RocketChat/Rocket.Chat.js.SDK/blob/master/dist/lib/driver.d.ts
- 4: https://github.com/RocketChat/Rocket.Chat.js.SDK/blob/master/package.json
- 5: SDK does not work with Rocket.Chat 7.1.0 Rocket.Chat.js.SDK#169
Keep getSubscriptionById return type explicit.
getSubscriptionById has no explicit return type, so optional chaining still leaves properties like .name and .params?.[0] untyped in app/lib/methods/subscriptions/room.ts. Add an explicit optional return type for the DDP subscription shape.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/lib/services/sdk.ts` around lines 184 - 187, Update getSubscriptionById
in the SDK service to declare an explicit optional return type matching the DDP
subscription shape, so consumers such as room subscription methods receive typed
name and params properties while preserving the existing lookup behavior.
Source: Coding guidelines
GitHub Actions
Error
|
Android Build Available Rocket.Chat 4.76.0.109498 Internal App Sharing: https://play.google.com/apps/test/RQQ8k09hlnQ/ahAO29uNT_Dcj1atTC59RLODvB8kxu7WH9_r2aynbpjVKryQ6zhIM3hkoMzWMytkkOje4_1yWxP3mE_tOe7JLnFgD8 |
|
iOS Build Available Rocket.Chat 4.76.0.109499 |
On reconnect, a room with no `lastOpen` built no sync request and returned clean, so messages that arrived while the connection was down were silently and permanently absent with nothing left that could fetch them. The catch-up now delegates such a room to `loadMessagesForRoom`, which is batch-capped and emits a loader row for what it could not reach, and seeds `lastOpen` from real server timestamps so the next reconnect syncs normally. The staleness guard is threaded into that path, so a superseded connection cycle still writes nothing. Also in the same area: - the sync walk is capped at 10 pages, so a room can no longer walk unbounded history across repeated connection cycles; - servers below 7.1.0 no longer receive a request with an undefined timestamp, since the no-cursor case short-circuits before the legacy branch; - a sync response carrying no `cursor` no longer throws, which used to leave the room refetching on every stream acknowledgement; - the direct-message subscription stub no longer invents `ts`, `ls` or `roomUpdatedAt` from the device clock — `ls` anchors the unread separator, so that poisoning was already user-visible. The subscription type widens to admit their absence.
GitHub Actions
Waiting
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)
app/lib/methods/loadMessagesForRoom.ts (1)
130-138: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winCheck staleness before the internal loader write.
isStaleis checked only afterloadresolves. A full batch with too few visible messages callsupdateMessagesinsideloadbefore that check. An already stale cycle can therefore persist messages and mutate loader UI state.
app/lib/methods/loadMessagesForRoom.ts#L130-L138: passisStaleintoloadand stop before itsupdateMessagescall and subsequent batch requests.app/lib/methods/loadMessagesForRoom.test.ts#L173-L186: use a full batch that triggers the internal loader path, then assert that stale execution does not callupdateMessages.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/lib/methods/loadMessagesForRoom.ts` around lines 130 - 138, Update app/lib/methods/loadMessagesForRoom.ts lines 130-138 to pass isStale into load and have the internal loader flow check it before updateMessages and any subsequent batch requests. Update app/lib/methods/loadMessagesForRoom.test.ts lines 173-186 to use a full batch that exercises the internal loader path and assert stale execution does not call updateMessages.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@app/lib/methods/loadMessagesForRoom.ts`:
- Around line 130-138: Update app/lib/methods/loadMessagesForRoom.ts lines
130-138 to pass isStale into load and have the internal loader flow check it
before updateMessages and any subsequent batch requests. Update
app/lib/methods/loadMessagesForRoom.test.ts lines 173-186 to use a full batch
that exercises the internal loader path and assert stale execution does not call
updateMessages.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 24c09762-ea94-4e93-8026-4f09a935577d
📒 Files selected for processing (10)
app/definitions/ISubscription.tsapp/lib/methods/createDirectMessageSubscriptionStub.test.tsapp/lib/methods/createDirectMessageSubscriptionStub.tsapp/lib/methods/loadMessagesForRoom.test.tsapp/lib/methods/loadMessagesForRoom.tsapp/lib/methods/loadMissedMessages.test.tsapp/lib/methods/loadMissedMessages.tsapp/lib/methods/subscriptions/room.reconnectFetch.test.tsapp/lib/methods/subscriptions/room.resumeSync.test.tsapp/views/RoomView/index.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- app/lib/methods/subscriptions/room.resumeSync.test.ts
- app/lib/methods/loadMissedMessages.ts
- app/lib/methods/subscriptions/room.reconnectFetch.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
- GitHub Check: ESLint and Test / run-eslint-and-test
- GitHub Check: E2E Shard Preflight
- GitHub Check: format
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,jsx,tsx}: Use descriptive names for functions, variables, and classes that clearly convey their purpose
Write comments that explain the 'why' behind code decisions, not the 'what'
Keep functions small and focused on a single responsibility
Use const by default, let when reassignment is needed, and avoid var
Prefer async/await over .then() chains for handling asynchronous operations
Use explicit error handling with try/catch blocks for async operations
Avoid deeply nested code; refactor complex logic into helper functions
Files:
app/lib/methods/loadMessagesForRoom.test.tsapp/lib/methods/createDirectMessageSubscriptionStub.test.tsapp/views/RoomView/index.tsxapp/lib/methods/createDirectMessageSubscriptionStub.tsapp/definitions/ISubscription.tsapp/lib/methods/loadMessagesForRoom.tsapp/lib/methods/loadMissedMessages.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use TypeScript for type safety; add explicit type annotations to function parameters and return types
Prefer interfaces over type aliases for defining object shapes in TypeScript
Use enums for sets of related constants rather than magic strings or numbers
Files:
app/lib/methods/loadMessagesForRoom.test.tsapp/lib/methods/createDirectMessageSubscriptionStub.test.tsapp/views/RoomView/index.tsxapp/lib/methods/createDirectMessageSubscriptionStub.tsapp/definitions/ISubscription.tsapp/lib/methods/loadMessagesForRoom.tsapp/lib/methods/loadMissedMessages.test.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{js,jsx,ts,tsx}: Format JavaScript and TypeScript code with Oxfmt using the repository configuration: tabs, single quotes, 130-character width, no trailing commas, omitted arrow-function parentheses where possible, and same-line brackets.
Follow Oxlint rules configured in.oxlintrc.json, including the import, React, Jest, TypeScript, and React Native plugins.
Files:
app/lib/methods/loadMessagesForRoom.test.tsapp/lib/methods/createDirectMessageSubscriptionStub.test.tsapp/views/RoomView/index.tsxapp/lib/methods/createDirectMessageSubscriptionStub.tsapp/definitions/ISubscription.tsapp/lib/methods/loadMessagesForRoom.tsapp/lib/methods/loadMissedMessages.test.ts
🧠 Learnings (4)
📚 Learning: 2026-04-30T17:07:51.020Z
Learnt from: diegolmello
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7274
File: app/lib/services/voip/MediaCallEvents.ts:0-0
Timestamp: 2026-04-30T17:07:51.020Z
Learning: In this Rocket.Chat React Native codebase, the ESLint rule `no-void: error` is enforced. When you see a promise returned from an async call that is not awaited (a “floating promise”), do not silence it with the `void somePromise()` pattern. Instead, handle the promise explicitly by attaching `.catch(...)` (or otherwise awaiting/handling the error) so unhandled-rejection risks are addressed in a way that satisfies the existing ESLint configuration.
Applied to files:
app/lib/methods/loadMessagesForRoom.test.tsapp/lib/methods/createDirectMessageSubscriptionStub.test.tsapp/views/RoomView/index.tsxapp/lib/methods/createDirectMessageSubscriptionStub.tsapp/definitions/ISubscription.tsapp/lib/methods/loadMessagesForRoom.tsapp/lib/methods/loadMissedMessages.test.ts
📚 Learning: 2026-06-25T18:37:25.526Z
Learnt from: diegolmello
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7434
File: app/views/ScreenLockConfigView.test.tsx:16-22
Timestamp: 2026-06-25T18:37:25.526Z
Learning: In Rocket.Chat ReactNative tests that mock selectors for `useAppSelector`, don’t require the mocked selector input to be typed as `IApplicationState` when the fixture only includes a partial Redux state slice (e.g., only `server` and `settings`). Requiring the full `IApplicationState` type in that scenario forces unsafe `as IApplicationState` casts and undermines type-safety. For these narrowly scoped selector-mock fixtures, use a less strict type (e.g., `any`) to keep the mock focused on the slice under test.
Applied to files:
app/lib/methods/loadMessagesForRoom.test.tsapp/lib/methods/createDirectMessageSubscriptionStub.test.tsapp/lib/methods/loadMissedMessages.test.ts
📚 Learning: 2026-06-24T22:58:43.390Z
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7157
File: app/views/MessagesView/index.tsx:392-392
Timestamp: 2026-06-24T22:58:43.390Z
Learning: When wrapping a React Native component (e.g., via `withSafeAreaInsets`) ensure `hoistNonReactStatics` is only required if the wrapped component actually defines static properties/methods that consumers rely on. If the component has no statics (as in `app/views/MessagesView/index.tsx`), you can omit `hoistNonReactStatics` for this case.
Applied to files:
app/views/RoomView/index.tsx
📚 Learning: 2026-06-25T18:37:44.793Z
Learnt from: diegolmello
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7434
File: app/views/ScreenLockConfigView.tsx:101-141
Timestamp: 2026-06-25T18:37:44.793Z
Learning: In the Rocket.Chat React Native codebase, do not treat passing an `async` function directly to an event prop in React/React Native UI components (e.g., `onPress={async () => ...}` in TSX) as a “floating promises” CI-blocking lint issue—this repo does not enable the ESLint `no-floating-promises` rule (while `no-void` is enforced). Only raise robustness follow-ups when there are genuinely unhandled promise paths (e.g., fire-and-forget calls like `save()` that return a Promise that is neither awaited nor handled), and prefer making sure failure paths are explicitly handled/reported rather than blocking on lint-style floating-promise concerns.
Applied to files:
app/views/RoomView/index.tsx
🔇 Additional comments (5)
app/definitions/ISubscription.ts (1)
47-48: LGTM!Also applies to: 64-64
app/lib/methods/createDirectMessageSubscriptionStub.ts (1)
77-79: LGTM!app/lib/methods/createDirectMessageSubscriptionStub.test.ts (1)
101-104: LGTM!app/views/RoomView/index.tsx (1)
689-689: LGTM!app/lib/methods/loadMissedMessages.test.ts (1)
6-6: LGTM!Also applies to: 28-28, 39-39, 71-144
GitHub Actions
Waiting
1 participant
Proposed changes
An open room silently loses messages that arrive while the app reconnects.
On reconnect,
RoomSubscription.handleConnectionran the catch-up fetch (loadMissedMessages) the moment the raw WebSocket opened — the SDK emitsconnectedfrom the socket'sopenhandler, before the DDP handshake and before login. But the room'sstream-room-messagessubscription is only re-sent after the resume login succeeds. Messages the server accepts between the fetch snapshot and the subscription'sreadyack reach neither the fetch nor the stream, and nothing fetches again. Leaving and re-entering the room does not recover them; only an app restart does.This re-times the fetch so it runs after the room stream is acked, making the fetch snapshot and the live stream overlap so no interval is covered by nothing.
Three commits:
stream-room-messagesis acked, so the fetch has something real to wait on rather than inferring readiness.Deliberately narrow: it re-times the one existing fetch on the one existing recovery path. It adds no re-subscribe, and it does not touch how the
lastOpensync cursor is computed — the single-writer, server-_updatedAt-only contract from6f9a093b0is unchanged. That matters because6f9a093b0reverted five earlier re-subscribe fixes that each added code on a side path.Issue(s)
https://rocketchat.atlassian.net/browse/NATIVE-1476
How to test or reproduce
Needs a busy room and a reconnect, since the loss band is only ~1.1-1.7s wide.
/api/v1/chat.postMessage).Before this change, messages whose server
tsfell between the subscription send and itsreadywere missing — measured as 2 of 100 — and room re-entry did not bring them back.Verified on an Android emulator (API 36) against mobile.qa over 6 reconnect cycles, zero messages missing in every one:
Messages were checked against the app's own storage, not the UI: the WatermelonDB SQLite file was pulled out of the app sandbox (
/data/data/chat.rocket.android/mobile.qa.rocket.chat.db.db, including the-wal— read it without the WAL and you get stale state) andSELECT msg FROM messagescompared against the set of messages the server confirmed it created. The flood records only responses withsuccess: true, so rate-limiter rejections (HTTP 429, which do occur under sustained sending) never enter the comparison. Syntheticload-more-*boundary rows are excluded. The room was never left during any cycle.Each cycle also carries direct evidence it took the real reconnect path rather than the silent socket reopen:
reduxStore.dispatchwas instrumented at runtime to record connection actions, and every counted cycle showsMETEOR_CONNECT_DISCONNECT→METEOR_CONNECT_REQUEST→METEOR_CONNECT_SUCCESS→LOGIN_REQUEST, one per cycle. Without that check a reconnect that skipped login would show 100% loss and be misread as this bug.The "landed during offline window" column matters more than the totals — a cycle where no message arrived while the device was offline exercises nothing, so each cycle was confirmed to have messages inside the airplane-mode window.
Note when reproducing: some reconnects take the silent socket-reopen path (NATIVE-1471) where no login runs at all. That path loses everything and is a different bug — confirm the reconnect actually re-logged in before counting a round.
Screenshots
Not applicable — no visible UI change.
Types of changes
Checklist
Further comments
Three new test suites cover the change:
room.streamReady.test.ts(the ready signal),room.reconnectFetch.test.ts(fetch ordering against the stream ack), androom.staleFetch.test.ts(in-flight fetches dropped across cycles). 28 tests across 6 suites pass locally.The alternative was to keep the fetch where it was and re-subscribe the stream earlier. Rejected: it widens the reconnect path rather than closing the gap, and re-subscribe changes on side paths are exactly what
6f9a093b0had to revert.Scope left out on purpose: the silent socket reopen that skips the resume login (NATIVE-1471), the missing Subscription row on a notification-opened room, and credential-lifecycle defects on the same path. This fix is only reachable when the resume login runs, so NATIVE-1471 is what makes it apply broadly — worth landing that alongside.
Summary by CodeRabbit
Bug Fixes
Tests