diff --git a/app/lib/methods/subscriptions/room.test.ts b/app/lib/methods/subscriptions/room.test.ts index 830d0d0bf1d..bf0b0e9576d 100644 --- a/app/lib/methods/subscriptions/room.test.ts +++ b/app/lib/methods/subscriptions/room.test.ts @@ -1,6 +1,4 @@ import RoomSubscription from './room'; -import { loadMissedMessages } from '../loadMissedMessages'; -import { clearUserTyping } from '../../../actions/usersTyping'; import { getMessageById } from '../../database/services/Message'; import { getThreadById } from '../../database/services/Thread'; import log from '../helpers/log'; @@ -17,14 +15,10 @@ jest.mock('../../services/sdk', () => ({ } })); -const mockStoreGetState = jest.fn<{ meteor: { connected: boolean } }, []>(() => ({ - meteor: { connected: false } -})); -const mockStoreDispatch = jest.fn(); jest.mock('../../store/auxStore', () => ({ store: { - getState: () => mockStoreGetState(), - dispatch: (action: unknown) => mockStoreDispatch(action) + getState: jest.fn(() => ({})), + dispatch: jest.fn() } })); @@ -78,11 +72,7 @@ jest.mock('../../../actions/room', () => ({ jest.mock('../../encryption', () => ({ Encryption: { - decryptMessage: jest.fn((msg: unknown) => Promise.resolve(msg)), - decryptPendingSubscriptions: jest.fn(), - decryptPendingMessages: jest.fn(), - getRoomInstance: jest.fn(), - stopRoom: jest.fn() + decryptMessage: jest.fn((msg: unknown) => Promise.resolve(msg)) } })); @@ -147,143 +137,6 @@ describe('RoomSubscription', () => { }); }); - describe('handleLogin', () => { - it('calls subscribeRoom, dispatches clearUserTyping, loads missed messages, and reads', async () => { - await sub.handleLogin(); - - expect(mockSubscribeRoom).toHaveBeenCalledWith(rid); - expect(mockStoreDispatch).toHaveBeenCalledWith(clearUserTyping()); - expect(loadMissedMessages).toHaveBeenCalledWith({ rid }); - }); - - it('handles subscribeRoom rejection gracefully', async () => { - mockSubscribeRoom.mockRejectedValueOnce(new Error('boom')); - - await expect(sub.handleLogin()).resolves.toBeUndefined(); - }); - }); - - describe('handleClose', () => { - it('does not call subscribeRoom or loadMissedMessages, but dispatches clearUserTyping', async () => { - await sub.handleClose(); - - expect(mockSubscribeRoom).not.toHaveBeenCalled(); - expect(loadMissedMessages).not.toHaveBeenCalled(); - expect(mockStoreDispatch).toHaveBeenCalledWith(clearUserTyping()); - }); - }); - - describe('DDP subscription recovery after forceReopen', () => { - it('handleLogin re-subscribes the room to restore lost DDP subscriptions', async () => { - await sub.subscribe(); - mockSubscribeRoom.mockClear(); - - await sub.handleLogin(); - - expect(mockSubscribeRoom).toHaveBeenCalledTimes(1); - expect(mockSubscribeRoom).toHaveBeenCalledWith(rid); - }); - - it('handleClose does NOT re-subscribe (only reconnects restore subscriptions, not disconnects)', async () => { - await sub.subscribe(); - mockSubscribeRoom.mockClear(); - - await sub.handleClose(); - - expect(mockSubscribeRoom).not.toHaveBeenCalled(); - }); - - it('tears down stale subscriptions on reconnect and tracks fresh ones for later cleanup', async () => { - const staleSub = { unsubscribe: jest.fn(() => Promise.resolve()) }; - const freshSub = { unsubscribe: jest.fn(() => Promise.resolve()) }; - mockSubscribeRoom.mockResolvedValueOnce([staleSub]).mockResolvedValueOnce([freshSub]); - - await sub.subscribe(); - await sub.handleLogin(); - await sub.unsubscribe(); - - expect(staleSub.unsubscribe).toHaveBeenCalledTimes(1); - expect(freshSub.unsubscribe).toHaveBeenCalledTimes(1); - }); - - it('does not accumulate subscriptions across repeated handleLogin calls (simulates sequential reopen)', async () => { - const first = { unsubscribe: jest.fn(() => Promise.resolve()) }; - const second = { unsubscribe: jest.fn(() => Promise.resolve()) }; - mockSubscribeRoom.mockResolvedValueOnce([first]).mockResolvedValueOnce([second]); - - await sub.subscribe(); - expect(mockSubscribeRoom).toHaveBeenCalledTimes(1); - - // First reopen → tears down [first], creates [second] - await sub.handleLogin(); - expect(mockSubscribeRoom).toHaveBeenCalledTimes(2); - expect(first.unsubscribe).toHaveBeenCalledTimes(1); - expect(second.unsubscribe).not.toHaveBeenCalled(); - - // Second reopen → tears down [second], creates [] - await sub.handleLogin(); - expect(mockSubscribeRoom).toHaveBeenCalledTimes(3); - expect(second.unsubscribe).toHaveBeenCalledTimes(1); - - // Final cleanup → empty batch, no more unsubscribes - await sub.unsubscribe(); - expect(first.unsubscribe).toHaveBeenCalledTimes(1); - expect(second.unsubscribe).toHaveBeenCalledTimes(1); - }); - - it('does not call onStreamData inside handleLogin (listeners persist across reopen)', async () => { - await sub.subscribe(); - mockOnStreamData.mockClear(); - - await sub.handleLogin(); - - expect(mockOnStreamData).not.toHaveBeenCalled(); - }); - - it('re-subscribes on the authenticated "login" event, not the pre-auth "connected" event', async () => { - await sub.subscribe(); - - expect(mockOnStreamData).toHaveBeenCalledWith('login', sub.handleLogin); - expect(mockOnStreamData).not.toHaveBeenCalledWith('connected', expect.anything()); - }); - - it('survives a poisoned subscription array (undefined entry from a rejected sub) and still re-subscribes', async () => { - // A pre-auth subscribe rejected by the server (nosub) resolves to undefined inside Promise.all. - const freshSub = { unsubscribe: jest.fn(() => Promise.resolve()) }; - mockSubscribeRoom.mockResolvedValueOnce([undefined as any]).mockResolvedValueOnce([freshSub]); - - await sub.subscribe(); - mockSubscribeRoom.mockClear(); - - await expect(sub.handleLogin()).resolves.toBeUndefined(); - expect(mockSubscribeRoom).toHaveBeenCalledTimes(1); - expect(mockSubscribeRoom).toHaveBeenCalledWith(rid); - }); - }); - - describe('isAlive guard', () => { - it('handleLogin does nothing once the subscription is no longer alive (race with unsubscribe)', async () => { - await sub.subscribe(); - await sub.unsubscribe(); - jest.clearAllMocks(); - - await sub.handleLogin(); - - expect(mockSubscribeRoom).not.toHaveBeenCalled(); - expect(loadMissedMessages).not.toHaveBeenCalled(); - expect(mockStoreDispatch).not.toHaveBeenCalled(); - }); - - it('handleLogin re-subscribes while the subscription is still alive', async () => { - await sub.subscribe(); - mockSubscribeRoom.mockClear(); - - await sub.handleLogin(); - - expect(mockSubscribeRoom).toHaveBeenCalledWith(rid); - }); - }); - describe('updateMessage concurrency', () => { const makeRecord = (debugName: string) => ({ _preparedState: null as string | null, @@ -315,13 +168,7 @@ describe('RoomSubscription', () => { const message = { _id, rid, tlm: { $date: 1 } } as any; - // updateMessage's promise never resolves on the happy path, so fire both and flush the queues. - sub.updateMessage({ ...message }); - sub.updateMessage({ ...message }); - await Array.from({ length: 10 }).reduce>( - chain => chain.then(() => new Promise(resolve => setImmediate(resolve))), - Promise.resolve() - ); + await Promise.all([sub.updateMessage({ ...message }), sub.updateMessage({ ...message })]); const loggedPendingChanges = (log as jest.Mock).mock.calls.some(([err]) => /pending changes/.test(err?.message)); expect(loggedPendingChanges).toBe(false); diff --git a/app/lib/methods/subscriptions/room.ts b/app/lib/methods/subscriptions/room.ts index 1e88d165453..1fdf573e7d7 100644 --- a/app/lib/methods/subscriptions/room.ts +++ b/app/lib/methods/subscriptions/room.ts @@ -34,7 +34,7 @@ export default class RoomSubscription { private rid: string; private isAlive: boolean; private promises?: Promise; - private loginListener?: Promise; + private connectedListener?: Promise; private disconnectedListener?: Promise; private notifyRoomListener?: Promise; private messageReceivedListener?: Promise; @@ -51,8 +51,8 @@ export default class RoomSubscription { } this.promises = sdk.subscribeRoom(this.rid); - this.loginListener = sdk.onStreamData('login', this.handleLogin); - this.disconnectedListener = sdk.onStreamData('close', this.handleClose); + this.connectedListener = sdk.onStreamData('connected', this.handleConnection); + this.disconnectedListener = sdk.onStreamData('close', this.handleConnection); this.notifyRoomListener = sdk.onStreamData('stream-notify-room', this.handleNotifyRoomReceived); this.messageReceivedListener = sdk.onStreamData('stream-room-messages', this.handleMessageReceived); if (!this.isAlive) { @@ -70,13 +70,13 @@ export default class RoomSubscription { if (this.promises) { try { const subscriptions = (await this.promises) || []; - subscriptions.forEach(sub => sub?.unsubscribe?.().catch(() => console.log('unsubscribeRoom'))); + subscriptions.forEach(sub => sub.unsubscribe().catch(() => console.log('unsubscribeRoom'))); } catch (e) { // do nothing } } reduxStore.dispatch(clearUserTyping()); - this.removeListener(this.loginListener); + this.removeListener(this.connectedListener); this.removeListener(this.disconnectedListener); this.removeListener(this.notifyRoomListener); this.removeListener(this.messageReceivedListener); @@ -93,39 +93,16 @@ export default class RoomSubscription { } }; - handleLogin = async () => { - if (!this.isAlive) { - return; - } + handleConnection = async () => { try { - if (this.promises) { - const oldSubs = await this.promises; - oldSubs?.forEach(sub => sub?.unsubscribe?.().catch(() => {})); - } - this.promises = sdk.subscribeRoom(this.rid); - await this.promises; - if (!this.isAlive) { - return; - } reduxStore.dispatch(clearUserTyping()); await loadMissedMessages({ rid: this.rid }); - if (!this.isAlive) { - return; - } this.read(); } catch (e) { log(e); } }; - handleClose = () => { - try { - reduxStore.dispatch(clearUserTyping()); - } catch (e) { - log(e); - } - }; - handleNotifyRoomReceived = protectedFunction(async (ddpMessage: IDDPMessage) => { const [_rid, ev] = ddpMessage.fields.eventName.split('/'); if (this.rid !== _rid) { diff --git a/app/lib/services/connect.test.ts b/app/lib/services/connect.test.ts index d3ac06b24ed..7f1ad5edd3b 100644 --- a/app/lib/services/connect.test.ts +++ b/app/lib/services/connect.test.ts @@ -1,7 +1,6 @@ import { connect, determineAuthType, disconnect } from './connect'; import { mediaSessionInstance } from './voip/MediaSessionInstance'; import { pendingHangups } from './voip/pendingHangups'; -import { unsubscribeRooms } from '../methods/subscribeRooms'; import { setUser } from '../../actions/login'; import database from '../database'; @@ -52,7 +51,8 @@ const mockStoreGetState = jest.fn(() => ({ settings: {} })); const mockStoreDispatch = jest.fn(); -const mockStoreSubscribe = jest.fn<() => void, [() => void]>(() => () => undefined); +const noopUnsubscribe = () => () => {}; +const mockStoreSubscribe = jest.fn<() => void, [() => void]>(noopUnsubscribe); jest.mock('../store/auxStore', () => ({ store: { getState: () => mockStoreGetState(), @@ -98,6 +98,21 @@ const flushMicrotasks = async (): Promise => { const getHandlersByEvent = (event: string): Array<(...args: unknown[]) => void> => mockOnStreamData.mock.calls.filter(([e]) => e === event).map(([, cb]) => cb); +// The drain listener is the `connected` one registered after `close`, since `close` is what arms it. +const getPendingHangupsDrainRegistrationIndex = (): number => { + const closeIndex = mockOnStreamData.mock.calls.findIndex(([event]) => event === 'close'); + const indexes = mockOnStreamData.mock.calls + .map(([event], index) => (event === 'connected' && index > closeIndex ? index : -1)) + .filter(index => index !== -1); + if (indexes.length !== 1) { + throw new Error(`expected exactly one pendingHangups drain listener, found ${indexes.length}`); + } + return indexes[0]; +}; + +const getPendingHangupsDrainHandler = (): ((...args: unknown[]) => void) => + mockOnStreamData.mock.calls[getPendingHangupsDrainRegistrationIndex()][1]; + interface IServices { [index: string]: string | boolean; name: string; @@ -404,6 +419,7 @@ describe('connect — pendingHangups drain on reconnect', () => { beforeEach(() => { jest.clearAllMocks(); mockOnStreamDataStops.length = 0; + mockStoreSubscribe.mockImplementation(noopUnsubscribe); pendingHangups.clear(); mockStoreGetState.mockReturnValue({ meteor: { connected: false }, @@ -422,10 +438,8 @@ describe('connect — pendingHangups drain on reconnect', () => { await connect({ server: 'https://example.com' }); - const connectedHandlers = getHandlersByEvent('connected'); - const closeHandlers = getHandlersByEvent('close'); - const drainHandler = connectedHandlers[1]; - const closeHandler = closeHandlers[0]; + const drainHandler = getPendingHangupsDrainHandler(); + const closeHandler = getHandlersByEvent('close')[0]; closeHandler(); drainHandler(); @@ -434,6 +448,49 @@ describe('connect — pendingHangups drain on reconnect', () => { expect(mediaSessionInstance.drainPendingHangups).toHaveBeenCalledTimes(1); }); + it('waits for login to become ready before draining', async () => { + const subscribeCallbacks: Array<() => void> = []; + mockStoreSubscribe.mockImplementation(cb => { + subscribeCallbacks.push(cb); + return () => { + const index = subscribeCallbacks.indexOf(cb); + if (index !== -1) { + subscribeCallbacks.splice(index, 1); + } + }; + }); + pendingHangups.record('call-a'); + let state: MockStoreState = { + meteor: { connected: false }, + login: { user: null, isAuthenticated: false }, + settings: {} + }; + mockStoreGetState.mockImplementation(() => state); + + await connect({ server: 'https://example.com' }); + + const drainHandler = getPendingHangupsDrainHandler(); + const closeHandler = getHandlersByEvent('close')[0]; + + closeHandler(); + drainHandler(); + await flushMicrotasks(); + + // Not ready yet — subscribed but not drained. + expect(mediaSessionInstance.drainPendingHangups).not.toHaveBeenCalled(); + + // Transition to authenticated + connected and notify subscribers. + state = { + meteor: { connected: true }, + login: { user: null, isAuthenticated: true }, + settings: {} + }; + subscribeCallbacks.forEach(cb => cb()); + await flushMicrotasks(); + + expect(mediaSessionInstance.drainPendingHangups).toHaveBeenCalledTimes(1); + }); + it('does not drain when "connected" fires without a prior "close"', async () => { pendingHangups.record('call-a'); mockStoreGetState.mockReturnValue({ @@ -444,7 +501,7 @@ describe('connect — pendingHangups drain on reconnect', () => { await connect({ server: 'https://example.com' }); - const drainHandler = getHandlersByEvent('connected')[1]; + const drainHandler = getPendingHangupsDrainHandler(); drainHandler(); await flushMicrotasks(); @@ -461,7 +518,7 @@ describe('connect — pendingHangups drain on reconnect', () => { await connect({ server: 'https://example.com' }); - const drainHandler = getHandlersByEvent('connected')[1]; + const drainHandler = getPendingHangupsDrainHandler(); const closeHandler = getHandlersByEvent('close')[0]; closeHandler(); @@ -473,9 +530,8 @@ describe('connect — pendingHangups drain on reconnect', () => { it('stops the previous pendingHangups connected listener when connect runs again', async () => { await connect({ server: 'https://example.com' }); - // onStreamData call order: 'connecting', 'connected', 'close', 'connected' (drain), ... - // pendingHangupsConnectedListener is the second 'connected' registration — index 3. - const firstDrainStop = mockOnStreamDataStops[3]; + // Stops are pushed in registration order, so the drain listener's stop shares its registration index. + const firstDrainStop = mockOnStreamDataStops[getPendingHangupsDrainRegistrationIndex()]; await connect({ server: 'https://example.com' }); await flushMicrotasks(); @@ -484,36 +540,6 @@ describe('connect — pendingHangups drain on reconnect', () => { }); }); -describe('connect — rooms subscription guard reset on close', () => { - beforeEach(() => { - jest.clearAllMocks(); - mockOnStreamDataStops.length = 0; - mockStoreGetState.mockReturnValue({ - meteor: { connected: false }, - login: { user: null, isAuthenticated: false }, - settings: {} - }); - }); - - // Regression: a long background marks the DDP socket stale, so foregrounding triggers - // `checkAndReopen` → `forceReopen`, which wipes the SDK subscriptions and emits 'close' while - // bypassing `connect()`. The rooms-list `stream-notify-user` feed only re-subscribes when the - // module-level guard in `subscribeRooms` is clear, and `unsubscribeRooms()` is what clears it. - // If the 'close' handler stops calling `unsubscribeRooms()`, the guard stays set after reconnect - // and the rooms list silently stops updating (subscriptions/favorites/reads). - it('calls unsubscribeRooms when the socket "close" fires', async () => { - await connect({ server: 'https://example.com' }); - - // connect() itself calls unsubscribeRooms() once while tearing down prior listeners; ignore it. - (unsubscribeRooms as jest.Mock).mockClear(); - - const closeHandler = getHandlersByEvent('close')[0]; - closeHandler(); - - expect(unsubscribeRooms).toHaveBeenCalledTimes(1); - }); -}); - describe('connect — stream-notify-logged updateAvatar', () => { const mockUserUpdate = jest.fn, [(u: { avatarETag: string }) => void]>(fn => { fn({ avatarETag: '' }); diff --git a/app/lib/services/connect.ts b/app/lib/services/connect.ts index dce7f343396..603677ff87b 100644 --- a/app/lib/services/connect.ts +++ b/app/lib/services/connect.ts @@ -40,6 +40,13 @@ interface IServices { service: string; } +// Reads redux rather than `ddp.loggedIn`: `close` clears `meteor.connected`, while `ddp.loggedIn` survives it. +// Neither survives a silent background death, so callers must bound their wait. +function isLoginReady(): boolean { + const state = store.getState(); + return state.login.isAuthenticated && state.meteor.connected; +} + let connectingListener: any; let connectedListener: any; let closeListener: any; @@ -133,22 +140,34 @@ function connect({ server, logoutOnError = false }: { server: string; logoutOnEr let pendingHangupsDrainArmed = false; closeListener = sdk.current.onStreamData('close', () => { - // Reset the rooms-subscription guard on every socket close. `forceReopen` (triggered by - // `checkAndReopen` after a long background) wipes the SDK subscriptions and emits 'close' - // but bypasses `connect()`, so without this the guard in `subscribeRooms` stays set and - // `stream-notify-user` is never re-subscribed — the rooms list silently stops updating. - unsubscribeRooms(); pendingHangupsDrainArmed = true; store.dispatch(disconnectAction()); }); - pendingHangupsConnectedListener = sdk.current.onStreamData('connected', () => { + pendingHangupsConnectedListener = sdk.current.onStreamData('connected', async () => { if (!pendingHangupsDrainArmed) return; pendingHangupsDrainArmed = false; if (pendingHangups.size === 0) return; - awaitDdpLoggedIn(5000) - .then(() => mediaSessionInstance.drainPendingHangups()) - .catch(error => log(error)); + try { + if (!isLoginReady()) { + await new Promise(resolve => { + const unsub = store.subscribe(() => { + if (isLoginReady()) { + clearTimeout(timer); + unsub(); + resolve(); + } + }); + const timer = setTimeout(() => { + unsub(); + resolve(); + }, 5000); + }); + } + await mediaSessionInstance.drainPendingHangups(); + } catch (error) { + log(error); + } }); usersListener = sdk.current.onStreamData( @@ -444,36 +463,6 @@ function checkAndReopen() { return sdk.current.checkAndReopen(); } -/** - * Resolves when the current session is fully logged in (or `timeoutMs` elapses). - * Trusts redux state rather than `ddp.loggedIn`, which isn't cleared on socket - * close and can read true for a stale session. Redux resets to - * `isAuthenticated=false` on `LOGIN.REQUEST` (dispatched by the connectedListener) - * and back to true on `LOGIN.SUCCESS`; `meteor.connected` covers the handshake. - */ -async function awaitDdpLoggedIn(timeoutMs: number = 5000): Promise { - const isReady = () => { - const s = store.getState(); - return s.login.isAuthenticated && s.meteor.connected; - }; - if (isReady()) { - return; - } - await new Promise(resolve => { - const unsub = store.subscribe(() => { - if (isReady()) { - clearTimeout(timer); - unsub(); - resolve(); - } - }); - const timer = setTimeout(() => { - unsub(); - resolve(); - }, timeoutMs); - }); -} - function disconnect() { const result = sdk.disconnect(); mediaSessionInstance.reset(); @@ -566,7 +555,6 @@ export { loginWithPassword, loginOAuthOrSso, checkAndReopen, - awaitDdpLoggedIn, abort, connect, disconnect, diff --git a/app/lib/services/ddpSocket.test.ts b/app/lib/services/ddpSocket.test.ts deleted file mode 100644 index 1fe4a165697..00000000000 --- a/app/lib/services/ddpSocket.test.ts +++ /dev/null @@ -1,183 +0,0 @@ -// eslint-disable-next-line @typescript-eslint/no-var-requires -const { Socket } = require('@rocket.chat/sdk/lib/drivers/ddp'); - -const buildSocket = () => { - const socket = new Socket({ - logger: { debug: jest.fn(), info: jest.fn(), error: jest.fn(), warn: jest.fn() }, - timeout: 10000 - }); - const send = jest.fn(); - const close = jest.fn(); - socket.connection = { - send, - close, - readyState: 1, - onopen: jest.fn(), - onmessage: jest.fn(), - onerror: jest.fn(), - onclose: jest.fn() - }; - return { socket, send, close }; -}; - -describe('Socket.probe', () => { - afterEach(() => { - jest.useRealTimers(); - }); - - it('resolves true when pong arrives within deadline', async () => { - const { socket } = buildSocket(); - const probePromise = socket.probe(); - socket.emit('pong'); - await expect(probePromise).resolves.toBe(true); - }); - - it('resolves false when no pong arrives within 2s deadline', async () => { - jest.useFakeTimers(); - const { socket } = buildSocket(); - const probePromise = socket.probe(); - await jest.advanceTimersByTimeAsync(2000); - await expect(probePromise).resolves.toBe(false); - }); - - it('resolves false when raw connection.send throws', async () => { - const { socket, send } = buildSocket(); - send.mockImplementation(() => { - throw new Error('boom'); - }); - await expect(socket.probe()).resolves.toBe(false); - }); -}); - -describe('Socket.forceReopen awaitability', () => { - it('returned promise resolves only after open() resolves', async () => { - const { socket } = buildSocket(); - let resolveOpen: () => void = () => undefined; - const openPromise = new Promise(res => { - resolveOpen = res; - }); - socket.open = jest.fn(() => openPromise); - - let settled = false; - const result = socket.forceReopen(); - result.then(() => { - settled = true; - }); - - await Promise.resolve(); - await Promise.resolve(); - expect(settled).toBe(false); - - resolveOpen(); - await result; - expect(settled).toBe(true); - }); - - it('concurrent invocations share the same in-flight reconnect', async () => { - const { socket } = buildSocket(); - let resolveOpen: () => void = () => undefined; - const openPromise = new Promise(res => { - resolveOpen = res; - }); - const openMock = jest.fn(() => openPromise); - socket.open = openMock; - - const a = socket.forceReopen(); - const b = socket.forceReopen(); - - expect(openMock).toHaveBeenCalledTimes(1); - - resolveOpen(); - await Promise.all([a, b]); - expect(openMock).toHaveBeenCalledTimes(1); - }); - - it("emits 'close' synchronously before opening so app-level Redux disconnect dispatches", async () => { - const { socket } = buildSocket(); - socket.open = jest.fn(() => Promise.resolve()); - const closeListener = jest.fn(); - socket.on('close', closeListener); - const reopenPromise = socket.forceReopen(); - // Synchronous emit: must have fired by the time forceReopen returns. - expect(closeListener).toHaveBeenCalledTimes(1); - const [event] = closeListener.mock.calls[0]; - expect(event).toEqual({ code: 4000 }); - await reopenPromise; - }); -}); - -describe('Socket.checkAndReopen bucket dispatch', () => { - const buildWithSpies = () => { - const { socket } = buildSocket(); - const { ping } = socket.config; - const forceReopen = jest.fn(); - const probe = jest.fn(); - socket.forceReopen = forceReopen; - socket.probe = probe; - return { socket, forceReopen, probe, ping }; - }; - - it('stale (elapsed > ping*2) calls forceReopen and skips probe', async () => { - const { socket, forceReopen, probe, ping } = buildWithSpies(); - socket.lastPing = Date.now() - ping * 2 - 1000; - await socket.checkAndReopen(); - expect(forceReopen).toHaveBeenCalledTimes(1); - expect(probe).not.toHaveBeenCalled(); - }); - - it('fresh (elapsed < 2000ms) is a no-op', async () => { - const { socket, forceReopen, probe } = buildWithSpies(); - socket.lastPing = Date.now() - 500; - await socket.checkAndReopen(); - expect(forceReopen).not.toHaveBeenCalled(); - expect(probe).not.toHaveBeenCalled(); - }); - - it('gray-zone with successful probe does not call forceReopen', async () => { - const { socket, forceReopen, probe } = buildWithSpies(); - probe.mockResolvedValue(true); - socket.lastPing = Date.now() - 5000; - await socket.checkAndReopen(); - expect(probe).toHaveBeenCalledTimes(1); - expect(forceReopen).not.toHaveBeenCalled(); - }); - - it('gray-zone with failed probe calls forceReopen', async () => { - const { socket, forceReopen, probe } = buildWithSpies(); - probe.mockResolvedValue(false); - socket.lastPing = Date.now() - 5000; - await socket.checkAndReopen(); - expect(probe).toHaveBeenCalledTimes(1); - expect(forceReopen).toHaveBeenCalledTimes(1); - }); - - it('stale and probe-fail buckets resolve only after forceReopen resolves', async () => { - const tryBucket = async (setup: (s: any) => void) => { - const { socket } = buildSocket(); - let resolveReopen: () => void = () => undefined; - const reopenPromise = new Promise(res => { - resolveReopen = res; - }); - socket.forceReopen = jest.fn(() => reopenPromise); - socket.probe = jest.fn().mockResolvedValue(false); - setup(socket); - let settled = false; - const p = socket.checkAndReopen().then(() => { - settled = true; - }); - await Promise.resolve(); - await Promise.resolve(); - expect(settled).toBe(false); - resolveReopen(); - await p; - expect(settled).toBe(true); - }; - - await tryBucket(socket => { - socket.lastPing = Date.now() - socket.config.ping * 2 - 1000; - }); - await tryBucket(socket => { - socket.lastPing = Date.now() - 5000; - }); - }); -}); diff --git a/app/lib/services/voip/MediaCallEvents.ios.test.ts b/app/lib/services/voip/MediaCallEvents.ios.test.ts index 23c71120ae7..deff5504327 100644 --- a/app/lib/services/voip/MediaCallEvents.ios.test.ts +++ b/app/lib/services/voip/MediaCallEvents.ios.test.ts @@ -88,10 +88,6 @@ jest.mock('../restApi', () => ({ registerPushToken: jest.fn(() => Promise.resolve()) })); -jest.mock('../connect', () => require('./MediaCallEvents.testHelpers').createConnectMock()); - -jest.mock('../sdk', () => require('./MediaCallEvents.testHelpers').createSdkMock()); - jest.mock('./MediaCallLogger', () => { const log = jest.fn(); const debug = jest.fn(); diff --git a/app/lib/services/voip/MediaCallEvents.test.ts b/app/lib/services/voip/MediaCallEvents.test.ts index e31188acbfd..3113774c0d5 100644 --- a/app/lib/services/voip/MediaCallEvents.test.ts +++ b/app/lib/services/voip/MediaCallEvents.test.ts @@ -69,10 +69,6 @@ jest.mock('../restApi', () => ({ registerPushToken: jest.fn(() => Promise.resolve()) })); -jest.mock('../connect', () => require('./MediaCallEvents.testHelpers').createConnectMock()); - -jest.mock('../sdk', () => require('./MediaCallEvents.testHelpers').createSdkMock()); - jest.mock('./MediaCallLogger', () => ({ MediaCallLogger: class { log = jest.fn(); @@ -148,7 +144,7 @@ describe('MediaCallEvents cross-server accept (slice 3)', () => { }); }); - it('skips deep link open and replays REST state signals once after re-login when host matches active workspace', async () => { + it('skips deep link open and replays REST state signals when host matches active workspace', () => { const { mediaSessionInstance } = jest.requireMock('./MediaSessionInstance'); mockServerSelector.mockReturnValueOnce('https://workspace-a.example.com'); const payload = buildIncomingPayload({ @@ -159,7 +155,6 @@ describe('MediaCallEvents cross-server accept (slice 3)', () => { DeviceEventEmitter.emit('VoipAcceptSucceeded', payload); expect(mockSetNativeAcceptedCallId).toHaveBeenCalledWith('same-ws-call'); - await new Promise(res => setImmediate(res)); expect(mediaSessionInstance.applyRestStateSignals).toHaveBeenCalledTimes(1); expect(mockOnOpenDeepLink).not.toHaveBeenCalled(); }); @@ -187,97 +182,6 @@ describe('MediaCallEvents cross-server accept (slice 3)', () => { expect(mockSetNativeAcceptedCallId).toHaveBeenCalledTimes(1); expect(mockOnOpenDeepLink).toHaveBeenCalledTimes(1); }); - - it('awaits checkAndReopen before applyRestStateSignals on same-workspace accept', async () => { - const { checkAndReopen } = jest.requireMock('../connect'); - const { mediaSessionInstance } = jest.requireMock('./MediaSessionInstance'); - let resolveReopen: () => void = () => undefined; - const reopenPromise = new Promise(res => { - resolveReopen = res; - }); - (checkAndReopen as jest.Mock).mockReturnValueOnce(reopenPromise); - mockServerSelector.mockReturnValueOnce('https://workspace-a.example.com'); - - DeviceEventEmitter.emit( - 'VoipAcceptSucceeded', - buildIncomingPayload({ - callId: 'same-ws-await', - host: 'https://workspace-a.example.com' - }) - ); - - await Promise.resolve(); - await Promise.resolve(); - expect(checkAndReopen).toHaveBeenCalledTimes(1); - expect(mediaSessionInstance.applyRestStateSignals).not.toHaveBeenCalled(); - - resolveReopen(); - await new Promise(res => setImmediate(res)); - expect(mediaSessionInstance.applyRestStateSignals).toHaveBeenCalledTimes(1); - }); - - it('still calls applyRestStateSignals and warns when checkAndReopen rejects', async () => { - const { checkAndReopen } = jest.requireMock('../connect'); - const { mediaSessionInstance } = jest.requireMock('./MediaSessionInstance'); - (checkAndReopen as jest.Mock).mockRejectedValueOnce(new Error('reconnect-failed')); - mockServerSelector.mockReturnValueOnce('https://workspace-a.example.com'); - - DeviceEventEmitter.emit( - 'VoipAcceptSucceeded', - buildIncomingPayload({ - callId: 'same-ws-reject', - host: 'https://workspace-a.example.com' - }) - ); - - await new Promise(res => setImmediate(res)); - expect(checkAndReopen).toHaveBeenCalledTimes(1); - expect(mediaSessionInstance.applyRestStateSignals).toHaveBeenCalledTimes(1); - }); - - it('runs applyRestStateSignals only after awaitDdpLoggedIn resolves on same-workspace accept', async () => { - const { awaitDdpLoggedIn } = jest.requireMock('../connect'); - const { mediaSessionInstance } = jest.requireMock('./MediaSessionInstance'); - let resolveLogin: () => void = () => undefined; - const loginPromise = new Promise(res => { - resolveLogin = res; - }); - (awaitDdpLoggedIn as jest.Mock).mockReturnValueOnce(loginPromise); - mockServerSelector.mockReturnValueOnce('https://workspace-a.example.com'); - - DeviceEventEmitter.emit( - 'VoipAcceptSucceeded', - buildIncomingPayload({ - callId: 'same-ws-late', - host: 'https://workspace-a.example.com' - }) - ); - - await new Promise(res => setImmediate(res)); - expect(awaitDdpLoggedIn).toHaveBeenCalledTimes(1); - expect(mediaSessionInstance.applyRestStateSignals).not.toHaveBeenCalled(); - - resolveLogin(); - await new Promise(res => setImmediate(res)); - expect(mediaSessionInstance.applyRestStateSignals).toHaveBeenCalledTimes(1); - }); - - it('does not call checkAndReopen when accept is for a different workspace', async () => { - const { checkAndReopen } = jest.requireMock('../connect'); - mockServerSelector.mockReturnValueOnce('https://workspace-a.example.com'); - - DeviceEventEmitter.emit( - 'VoipAcceptSucceeded', - buildIncomingPayload({ - callId: 'cross-ws', - host: 'https://workspace-b.example.com' - }) - ); - - await new Promise(res => setImmediate(res)); - expect(checkAndReopen).not.toHaveBeenCalled(); - expect(mockOnOpenDeepLink).toHaveBeenCalledTimes(1); - }); }); describe('VoipAcceptFailed', () => { diff --git a/app/lib/services/voip/MediaCallEvents.testHelpers.ts b/app/lib/services/voip/MediaCallEvents.testHelpers.ts deleted file mode 100644 index 8ed8f4721a6..00000000000 --- a/app/lib/services/voip/MediaCallEvents.testHelpers.ts +++ /dev/null @@ -1,15 +0,0 @@ -/* eslint-env jest */ -/** Shared jest factories used by MediaCallEvents.test.ts and MediaCallEvents.ios.test.ts. */ -export const createConnectMock = () => ({ - checkAndReopen: jest.fn(() => Promise.resolve()), - awaitDdpLoggedIn: jest.fn(() => Promise.resolve()) -}); - -export const createSdkMock = () => ({ - __esModule: true, - default: { - current: { - subscribeNotifyUser: jest.fn(() => Promise.resolve()) - } - } -}); diff --git a/app/lib/services/voip/MediaCallEvents.ts b/app/lib/services/voip/MediaCallEvents.ts index cd139885999..a20dddf0c34 100644 --- a/app/lib/services/voip/MediaCallEvents.ts +++ b/app/lib/services/voip/MediaCallEvents.ts @@ -6,9 +6,7 @@ import { useCallStore } from './useCallStore'; import { mediaSessionInstance } from './MediaSessionInstance'; import type { VoipPayload } from '../../../definitions/Voip'; import NativeVoipModule from '../../native/NativeVoip'; -import { awaitDdpLoggedIn, checkAndReopen } from '../connect'; import { registerPushToken } from '../restApi'; -import sdk from '../sdk'; import { MediaCallLogger } from './MediaCallLogger'; const Emitter = isIOS ? new NativeEventEmitter(NativeVoipModule) : DeviceEventEmitter; @@ -80,33 +78,6 @@ function dispatchVoipAcceptFailureFromNative( }); } -/** - * Warm-locked CallKit accept path: heal a zombie DDP socket then replay missed - * signals. Order matters — replay must run AFTER re-login + `subscribeNotifyUser`. - * If we replay earlier, the answer SDP fires through a half-open session: methodCall - * resolves while the server's stream-notify-user route still points at the torn-down - * sub, the caller never receives the answer, and the call hits - * `TIMEOUT_TO_PROGRESS_SIGNALING` (10s) → hangup. - */ -async function runReconnectAndReplaySignals(): Promise { - try { - await checkAndReopen(); - } catch (error) { - mediaCallLogger.warn(`${TAG} checkAndReopen failed:`, error); - } - await awaitDdpLoggedIn(5000); - try { - await sdk.current?.subscribeNotifyUser?.(); - } catch (error) { - mediaCallLogger.warn(`${TAG} subscribeNotifyUser failed:`, error); - } - try { - await mediaSessionInstance.applyRestStateSignals(); - } catch (error) { - mediaCallLogger.error(`${TAG} applyRestStateSignals failed:`, error); - } -} - function handleVoipAcceptSucceededFromNative(data: VoipPayload, adapters: MediaCallEventsAdapters) { const { callId } = data; if (callId && lastHandledVoipAcceptSucceededCallId === callId) { @@ -123,8 +94,8 @@ function handleVoipAcceptSucceededFromNative(data: VoipPayload, adapters: MediaC NativeVoipModule.clearInitialEvents(); useCallStore.getState().setNativeAcceptedCallId(data.callId); if (data.host && isVoipIncomingHostCurrentWorkspace(data.host, adapters.getActiveServerUrl)) { - runReconnectAndReplaySignals().catch(error => { - mediaCallLogger.error(`${TAG} runReconnectAndReplaySignals threw:`, error); + mediaSessionInstance.applyRestStateSignals().catch(error => { + mediaCallLogger.error(`${TAG} applyRestStateSignals failed:`, error); }); return; } diff --git a/app/lib/services/voip/MediaCallLogger.test.ts b/app/lib/services/voip/MediaCallLogger.test.ts index a1c3ac20a8b..be2a36ee71d 100644 --- a/app/lib/services/voip/MediaCallLogger.test.ts +++ b/app/lib/services/voip/MediaCallLogger.test.ts @@ -27,7 +27,7 @@ describe('MediaCallLogger', () => { const logger = new MediaCallLogger(); logger.log('sensitive data', { token: 'secret', callId: 'abc' }); - expect(spy).not.toHaveBeenCalledWith(expect.stringMatching(/^\[Media Call/)); + expect(spy).not.toHaveBeenCalled(); spy.mockRestore(); // @ts-expect-error restoring @@ -61,7 +61,7 @@ describe('MediaCallLogger', () => { const logger = new MediaCallLogger(); logger.debug('debug message'); - expect(spy).not.toHaveBeenCalledWith(expect.stringMatching(/^\[Media Call/)); + expect(spy).not.toHaveBeenCalled(); spy.mockRestore(); // @ts-expect-error restoring diff --git a/app/lib/services/voip/MediaSessionInstance.test.ts b/app/lib/services/voip/MediaSessionInstance.test.ts index 8cc39b63f12..8c10f6e2f92 100644 --- a/app/lib/services/voip/MediaSessionInstance.test.ts +++ b/app/lib/services/voip/MediaSessionInstance.test.ts @@ -188,8 +188,8 @@ jest.mock('@rocket.chat/media-signaling', () => ({ const STREAM_NOTIFY_USER = 'stream-notify-user'; -function getStreamNotifyHandler(): (ddpMessage: IDDPMessage) => Promise { - const calls = mockOnStreamData.mock.calls as unknown as [string, (m: IDDPMessage) => Promise][]; +function getStreamNotifyHandler(): (ddpMessage: IDDPMessage) => void { + const calls = mockOnStreamData.mock.calls as unknown as [string, (m: IDDPMessage) => void][]; for (let i = calls.length - 1; i >= 0; i--) { const [eventName, handler] = calls[i]; if (eventName === STREAM_NOTIFY_USER && typeof handler === 'function') { @@ -519,7 +519,7 @@ describe('MediaSessionInstance', () => { }); await mediaSessionInstance.init('user-1'); const streamHandler = getStreamNotifyHandler(); - await streamHandler({ + streamHandler({ msg: 'changed', fields: { eventName: 'uid/media-signal', @@ -553,7 +553,7 @@ describe('MediaSessionInstance', () => { }); await mediaSessionInstance.init('user-1'); const streamHandler = getStreamNotifyHandler(); - await streamHandler({ + streamHandler({ msg: 'changed', fields: { eventName: 'uid/media-signal', diff --git a/app/lib/services/voip/MediaSessionInstance.ts b/app/lib/services/voip/MediaSessionInstance.ts index 3d466c7e56b..e9075cfa0d9 100644 --- a/app/lib/services/voip/MediaSessionInstance.ts +++ b/app/lib/services/voip/MediaSessionInstance.ts @@ -57,11 +57,8 @@ class MediaSessionInstance { nativeAcceptedCallId === signal.callId && call == null ) { - // Defer past the lib's `updatingInputTrack` re-entrancy lock held during the - // current `processSignal` tick — otherwise `accept()`'s state change is dropped - // inside the lock, mic never opens, call hangs in `waiting-for-offer` for 10s. - queueMicrotask(() => { - this.answerCall(signal.callId).catch(error => log(error)); + this.answerCall(signal.callId).catch(error => { + log(error); }); } } @@ -122,7 +119,12 @@ class MediaSessionInstance { return; } const signal = ddpMessage.fields.args[0]; - await this.instance.processSignal(signal); + try { + // `tryAnswerIfNativeAcceptedNotification` reads the state this signal mutates. + await this.instance.processSignal(signal); + } catch (error) { + log(error); + } this.tryAnswerIfNativeAcceptedNotification(signal as ServerMediaSignal); }); diff --git a/app/sagas/__tests__/deepLinking.test.ts b/app/sagas/__tests__/deepLinking.test.ts index 632d05f369f..e6ceccde958 100644 --- a/app/sagas/__tests__/deepLinking.test.ts +++ b/app/sagas/__tests__/deepLinking.test.ts @@ -98,9 +98,7 @@ import createSagaMiddleware from 'redux-saga'; import { deepLinkingOpen, deepLinkingClickCallPush } from '../../actions/deepLinking'; import { loginSuccess } from '../../actions/login'; import { selectServerSuccess } from '../../actions/server'; -import { connectSuccess } from '../../actions/connect'; import { appStart } from '../../actions/app'; -import { LOGIN } from '../../actions/actionsTypes'; import { RootEnum } from '../../definitions'; import reducers from '../../reducers'; import deepLinkingRoot from '../deepLinking'; @@ -112,8 +110,8 @@ import { goRoom, navigateToRoom } from '../../lib/methods/helpers/goRoom'; import { waitForNavigationReady } from '../../lib/navigation/appNavigation'; import { loginOAuthOrSso } from '../../lib/services/connect'; import sdk from '../../lib/services/sdk'; -import EventEmitter from '../../lib/methods/helpers/events'; import database from '../../lib/database'; +import EventEmitter from '../../lib/methods/helpers/events'; // ─── Helpers ────────────────────────────────────────────────────────────────── @@ -132,19 +130,6 @@ function setupStore(preloadedState?: PreloadedState) { return store; } -/** setupStore that also records dispatched actions (incl. saga puts) for assertions. */ -function setupRecordingStore(preloadedState?: PreloadedState) { - const actions: { type: string }[] = []; - const recorder = () => (next: (a: any) => any) => (action: any) => { - actions.push(action); - return next(action); - }; - const sagaMiddleware = createSagaMiddleware(); - const store = createStore(reducers, preloadedState, applyMiddleware(recorder, sagaMiddleware)); - sagaMiddleware.run(deepLinkingRoot); - return { store, actions }; -} - // ─── Factories ──────────────────────────────────────────────────────────────── const HOST = 'https://open.rocket.chat'; @@ -232,11 +217,6 @@ describe('deepLinking saga — Regression race (new server + token + room path)' store.dispatch(selectServerSuccess({ ...makeServerRecord(), name: 'open.rocket.chat', server: HOST })); await flushSagaMicrotasks(); - // The saga waits for METEOR.SUCCESS ('connected') before dispatching - // loginRequest, so it never logs in on a still-connecting socket. - store.dispatch(connectSuccess()); - await flushSagaMicrotasks(); - // Saga is now waiting for LOGIN.SUCCESS expect(jest.mocked(goRoom)).not.toHaveBeenCalled(); @@ -256,56 +236,6 @@ describe('deepLinking saga — Regression race (new server + token + room path)' expect(jest.mocked(goRoom)).toHaveBeenCalledTimes(1); }); - // Ordering race: socket connects before SERVER.SELECT_SUCCESS; the guard must - // skip the already-fired METEOR.SUCCESS take instead of hanging. - it('completes the chain when METEOR.SUCCESS fires before SERVER.SELECT_SUCCESS', async () => { - const store = setupStore(); - - store.dispatch(deepLinkingOpen(makeParamsWithToken())); - await flushSagaMicrotasks(); - await jest.advanceTimersByTimeAsync(1000); - await flushSagaMicrotasks(); - - // Socket connects first — before SERVER.SELECT_SUCCESS is dispatched. - store.dispatch(connectSuccess()); - await flushSagaMicrotasks(); - - store.dispatch(selectServerSuccess({ ...makeServerRecord(), name: 'open.rocket.chat', server: HOST })); - await flushSagaMicrotasks(); - - store.dispatch(loginSuccess({ id: 'user-1', token: makeStoredUser() } as any)); - await flushSagaMicrotasks(); - - store.dispatch(appStart({ root: RootEnum.ROOT_INSIDE })); - await flushSagaMicrotasks(); - await flushSagaMicrotasks(); - - expect(jest.mocked(goRoom)).toHaveBeenCalledTimes(1); - }); - - // loginRequest must not fire until the socket is connected (locks the gate). - it('does not dispatch loginRequest until METEOR.SUCCESS', async () => { - const { store, actions } = setupRecordingStore(); - const loginRequested = () => actions.some(a => a.type === LOGIN.REQUEST); - - store.dispatch(deepLinkingOpen(makeParamsWithToken())); - await flushSagaMicrotasks(); - await jest.advanceTimersByTimeAsync(1000); - await flushSagaMicrotasks(); - - store.dispatch(selectServerSuccess({ ...makeServerRecord(), name: 'open.rocket.chat', server: HOST })); - await flushSagaMicrotasks(); - - // Server selected but socket not connected yet → still parked at the gate. - expect(loginRequested()).toBe(false); - - store.dispatch(connectSuccess()); - await flushSagaMicrotasks(); - - // Socket connected → gate released, loginRequest dispatched. - expect(loginRequested()).toBe(true); - }); - /** * Regression negative: dispatch SERVER.SELECT_SUCCESS, LOGIN.SUCCESS. * Flush microtasks. Assert goRoom NOT yet called. @@ -323,11 +253,6 @@ describe('deepLinking saga — Regression race (new server + token + room path)' store.dispatch(selectServerSuccess({ ...makeServerRecord(), name: 'open.rocket.chat', server: HOST })); await flushSagaMicrotasks(); - // The saga waits for METEOR.SUCCESS ('connected') before dispatching - // loginRequest, so it never logs in on a still-connecting socket. - store.dispatch(connectSuccess()); - await flushSagaMicrotasks(); - store.dispatch(loginSuccess({ id: 'user-1', token: makeStoredUser() } as any)); await flushSagaMicrotasks(); @@ -360,11 +285,6 @@ describe('deepLinking saga — Regression race (new server + token + room path)' store.dispatch(selectServerSuccess({ ...makeServerRecord(), name: 'open.rocket.chat', server: HOST })); await flushSagaMicrotasks(); - // The saga waits for METEOR.SUCCESS ('connected') before dispatching - // loginRequest, so it never logs in on a still-connecting socket. - store.dispatch(connectSuccess()); - await flushSagaMicrotasks(); - // Dispatch LOGIN.SUCCESS AND APP.START(ROOT_INSIDE) synchronously before any flush. // The reducer processes both dispatches before the saga's select runs, // so the select sees ROOT_INSIDE and skips the take. @@ -394,11 +314,6 @@ describe('deepLinking saga — Regression race (new server + token + room path)' store.dispatch(selectServerSuccess({ ...makeServerRecord(), name: 'open.rocket.chat', server: HOST })); await flushSagaMicrotasks(); - // The saga waits for METEOR.SUCCESS ('connected') before dispatching - // loginRequest, so it never logs in on a still-connecting socket. - store.dispatch(connectSuccess()); - await flushSagaMicrotasks(); - store.dispatch(loginSuccess({ id: 'user-1', token: makeStoredUser() } as any)); await flushSagaMicrotasks(); @@ -434,11 +349,6 @@ describe('deepLinking saga — Regression race (new server + token + room path)' store.dispatch(selectServerSuccess({ ...makeServerRecord(), name: 'open.rocket.chat', server: HOST })); await flushSagaMicrotasks(); - // The saga waits for METEOR.SUCCESS ('connected') before dispatching - // loginRequest, so it never logs in on a still-connecting socket. - store.dispatch(connectSuccess()); - await flushSagaMicrotasks(); - store.dispatch(loginSuccess({ id: 'user-1', token: makeStoredUser() } as any)); await flushSagaMicrotasks(); @@ -555,7 +465,7 @@ describe('deepLinking saga — server already connected, should skip changing se }); }); -// ─── handleClickCallPush (OPEN_VIDEO_CONF) — new server + token ────────────────── +// ─── handleClickCallPush (OPEN_VIDEO_CONF) — new server + token ─────────────── describe('deepLinking saga — handleClickCallPush (new server + token + call room)', () => { /** Call-push params: host + token + the rid handleNavigateCallRoom looks up. */ @@ -570,7 +480,7 @@ describe('deepLinking saga — handleClickCallPush (new server + token + call ro jest.mocked(navigateToRoom).mockReset(); jest.mocked(database.active.get).mockReset(); - // Unknown server with a token → reaches the SELECT_SUCCESS/METEOR.SUCCESS gate. + // Unknown server with a token → reaches the SELECT_SUCCESS gate. jest.mocked(UserPreferences.getString).mockImplementation((key: string) => { if (key === 'currentServer') return 'https://other.server.com'; return null; @@ -588,32 +498,7 @@ describe('deepLinking saga — handleClickCallPush (new server + token + call ro jest.useRealTimers(); }); - // Ordering race (call-push path): socket connects before SERVER.SELECT_SUCCESS; - // the guard must skip the already-fired METEOR.SUCCESS take instead of hanging. - it('completes the call-room chain when METEOR.SUCCESS fires before SERVER.SELECT_SUCCESS', async () => { - const store = setupStore(); - - store.dispatch(deepLinkingClickCallPush(makeCallParams())); - await flushSagaMicrotasks(); - await jest.advanceTimersByTimeAsync(1000); - await flushSagaMicrotasks(); - - // Socket connects first — before SERVER.SELECT_SUCCESS is dispatched. - store.dispatch(connectSuccess()); - await flushSagaMicrotasks(); - - store.dispatch(selectServerSuccess({ ...makeServerRecord(), name: 'open.rocket.chat', server: HOST })); - await flushSagaMicrotasks(); - - store.dispatch(loginSuccess({ id: 'user-1', token: makeStoredUser() } as any)); - await flushSagaMicrotasks(); - await flushSagaMicrotasks(); - - expect(jest.mocked(navigateToRoom)).toHaveBeenCalledTimes(1); - }); - - // Happy path: full chain in normal order navigates once. - it('navigates to the call room once after the full chain completes', async () => { + it('navigates to the call room once after SELECT_SUCCESS and LOGIN.SUCCESS', async () => { const store = setupStore(); store.dispatch(deepLinkingClickCallPush(makeCallParams())); @@ -621,13 +506,9 @@ describe('deepLinking saga — handleClickCallPush (new server + token + call ro await jest.advanceTimersByTimeAsync(1000); await flushSagaMicrotasks(); - store.dispatch(selectServerSuccess({ ...makeServerRecord(), name: 'open.rocket.chat', server: HOST })); - await flushSagaMicrotasks(); - - // Still parked at the METEOR.SUCCESS gate — no navigation yet. expect(jest.mocked(navigateToRoom)).not.toHaveBeenCalled(); - store.dispatch(connectSuccess()); + store.dispatch(selectServerSuccess({ ...makeServerRecord(), name: 'open.rocket.chat', server: HOST })); await flushSagaMicrotasks(); store.dispatch(loginSuccess({ id: 'user-1', token: makeStoredUser() } as any)); @@ -636,29 +517,6 @@ describe('deepLinking saga — handleClickCallPush (new server + token + call ro expect(jest.mocked(navigateToRoom)).toHaveBeenCalledTimes(1); }); - - // loginRequest must not fire until the socket is connected (locks the gate). - it('does not dispatch loginRequest until METEOR.SUCCESS', async () => { - const { store, actions } = setupRecordingStore(); - const loginRequested = () => actions.some(a => a.type === LOGIN.REQUEST); - - store.dispatch(deepLinkingClickCallPush(makeCallParams())); - await flushSagaMicrotasks(); - await jest.advanceTimersByTimeAsync(1000); - await flushSagaMicrotasks(); - - store.dispatch(selectServerSuccess({ ...makeServerRecord(), name: 'open.rocket.chat', server: HOST })); - await flushSagaMicrotasks(); - - // Server selected but socket not connected yet → still parked at the gate. - expect(loginRequested()).toBe(false); - - store.dispatch(connectSuccess()); - await flushSagaMicrotasks(); - - // Socket connected → gate released, loginRequest dispatched. - expect(loginRequested()).toBe(true); - }); }); // ─── handleOAuth — single-use credentialToken dedup guard ──────────────────── diff --git a/app/sagas/deepLinking.js b/app/sagas/deepLinking.js index b6892f18b39..fac9e3292db 100644 --- a/app/sagas/deepLinking.js +++ b/app/sagas/deepLinking.js @@ -244,11 +244,6 @@ const handleOpen = function* handleOpen({ params }) { if (params.token) { if (!hostAlreadyConnected) { yield take(types.SERVER.SELECT_SUCCESS); - // SERVER.SELECT_SUCCESS doesn't mean 'connected'; skip the take if it already is. - const connected = yield select(state => state.meteor.connected); - if (!connected) { - yield take(types.METEOR.SUCCESS); - } } yield put(loginRequest({ resume: params.token }, true)); yield take(types.LOGIN.SUCCESS); @@ -344,11 +339,6 @@ const handleClickCallPush = function* handleClickCallPush({ params }) { EventEmitter.emit('NewServer', { server: host }); if (params.token) { yield take(types.SERVER.SELECT_SUCCESS); - // SERVER.SELECT_SUCCESS doesn't mean 'connected'; skip the take if it already is. - const connected = yield select(state => state.meteor.connected); - if (!connected) { - yield take(types.METEOR.SUCCESS); - } yield put(loginRequest({ resume: params.token }, true)); yield take(types.LOGIN.SUCCESS); yield handleNavigateCallRoom({ params }); diff --git a/app/sagas/state.js b/app/sagas/state.js index 44f0b1430fa..20272cfeb73 100644 --- a/app/sagas/state.js +++ b/app/sagas/state.js @@ -19,18 +19,14 @@ const appHasComeBackToForeground = function* appHasComeBackToForeground() { if (appRoot !== RootEnum.ROOT_INSIDE) { return; } - const login = yield select(state => state.login); - if (!login.isAuthenticated) { + const isReady = yield isAuthAndConnected(); + if (!isReady) { return; } try { const server = yield select(state => state.server.server); yield localAuthenticate(server); - // Always reconnect on foreground; gating on meteor.connected would skip - // the call precisely when the socket died silently in background. - checkAndReopen().catch(e => { - log('[state.js] checkAndReopen failed:', e); - }); + checkAndReopen(); // Check for pending notification when app comes to foreground (Android - notification tap while in background) checkPendingNotification().catch(e => { log('[state.js] Error checking pending notification:', e); diff --git a/jest.config.js b/jest.config.js index aa5e3a720c6..2c66372994e 100644 --- a/jest.config.js +++ b/jest.config.js @@ -2,7 +2,7 @@ module.exports = { modulePathIgnorePatterns: ['/.*worktrees/'], testPathIgnorePatterns: ['e2e', 'node_modules', '/.*worktrees/', '/__tests__/testHelpers\\.tsx$'], transformIgnorePatterns: [ - 'node_modules/(?!((jest-)?react-native|@react-native(-community)?)|expo(nent)?|@expo(nent)?/.*|@expo-google-fonts/.*|react-navigation|@react-navigation/.*|@unimodules/.*|unimodules|sentry-expo|native-base|react-native-svg|@rocket.chat/ui-kit|@rocket.chat/sdk|tiny-events)' + 'node_modules/(?!((jest-)?react-native|@react-native(-community)?)|expo(nent)?|@expo(nent)?/.*|@expo-google-fonts/.*|react-navigation|@react-navigation/.*|@unimodules/.*|unimodules|sentry-expo|native-base|react-native-svg|@rocket.chat/ui-kit)' ], preset: './jest.preset.js', cacheDirectory: '/.jest-cache', diff --git a/patches/@rocket.chat+sdk+1.3.3-mobile.patch b/patches/@rocket.chat+sdk+1.3.3-mobile.patch index 3cac42dab33..d13d1d4c749 100644 --- a/patches/@rocket.chat+sdk+1.3.3-mobile.patch +++ b/patches/@rocket.chat+sdk+1.3.3-mobile.patch @@ -1,5 +1,5 @@ diff --git a/node_modules/@rocket.chat/sdk/lib/drivers/ddp.ts b/node_modules/@rocket.chat/sdk/lib/drivers/ddp.ts -index 19d31ae..07bda5f 100644 +index 19d31ae..9f752df 100644 --- a/node_modules/@rocket.chat/sdk/lib/drivers/ddp.ts +++ b/node_modules/@rocket.chat/sdk/lib/drivers/ddp.ts @@ -101,9 +101,25 @@ export class Socket extends EventEmitter { @@ -10,7 +10,7 @@ index 19d31ae..07bda5f 100644 + // The `this.connected` early-return above means we only reach here when the + // existing socket isn't healthy, so detaching its handlers and closing it stops + // a stale or still-connecting socket from later firing onClose and clobbering the -+ // live connection. Mirrors forceReopen's teardown. ++ // live connection. + if (this.connection) { + try { + this.connection.onopen = null as any @@ -45,130 +45,7 @@ index 19d31ae..07bda5f 100644 this.emit('close', e) try { if (e?.code !== userDisconnectCloseCode) { -@@ -175,15 +198,115 @@ export class Socket extends EventEmitter { - return Promise.resolve() - } - -- // Call open directly, so it skips openTimeout -- checkAndReopen = () => { -- if (!this.connected) { -- if (this.openTimeout) { -- clearTimeout(this.openTimeout as any) -- delete this.openTimeout -+ /** -+ * Probe an existing connection by sending a raw `ping` and waiting for `pong`. -+ * Bypasses the SDK send() pipeline (which would await `pong` indefinitely on -+ * a zombie socket). Resolves true if a pong arrives within 2s; false on -+ * timeout or synchronous send throw. -+ */ -+ probe = (): Promise => { -+ return new Promise(resolve => { -+ let settled = false -+ const finish = (alive: boolean) => { -+ if (settled) return -+ settled = true -+ this.off('pong', onPong) -+ clearTimeout(timer) -+ resolve(alive) -+ } -+ const onPong = () => finish(true) -+ const timer = setTimeout(() => finish(false), 2000) -+ this.once('pong', onPong) -+ try { -+ if (!this.connection) return finish(false) -+ this.connection.send(JSON.stringify({ msg: 'ping' })) -+ } catch { -+ finish(false) -+ } -+ }) -+ } -+ -+ _reopenInFlight: Promise | null = null -+ -+ /** -+ * Tear down the existing connection and start a fresh one. Used when the -+ * socket is known or suspected dead. Emits `disconnected` so any in-flight -+ * send() promises (which register `once('disconnected', reject)`) reject -+ * visibly instead of hanging forever, and emits `close` synchronously so -+ * app-level `close` listeners (e.g. Redux `disconnectAction` dispatch in -+ * `connect.ts`) flip to disconnected before the new socket opens; otherwise -+ * the next `'connected'` event would short-circuit on stale Redux state and -+ * the loginRequest → subscribeNotifyUser chain would never re-run, leaving -+ * the fresh socket without `${userId}/media-signal` or any other sub. -+ * Concurrent invocations join the same in-flight reconnect promise so the -+ * parallel callers don't tear each other down. Resolves only after the new -+ * socket has reached `OPEN` and the DDP `connect` handshake has been acked. -+ */ -+ forceReopen = (): Promise => { -+ if (this._reopenInFlight) return this._reopenInFlight -+ this.lastPing = 0 -+ if (this.openTimeout) { -+ clearTimeout(this.openTimeout as any) -+ delete this.openTimeout -+ } -+ if (this.pingTimeout) { -+ clearTimeout(this.pingTimeout as any) -+ delete this.pingTimeout -+ } -+ this.emit('disconnected') -+ this.emit('close', { code: userDisconnectCloseCode }) -+ this.subscriptions = {} -+ if (this.connection) { -+ try { -+ this.connection.onopen = null as any -+ this.connection.onmessage = null as any -+ this.connection.onerror = null as any -+ this.connection.onclose = null as any -+ this.connection.close(userDisconnectCloseCode) -+ } catch (e) { -+ this.logger.debug(`[ddp] forceReopen close error: ${(e as Error).message}`) - } -- this.open() -+ this.connection = undefined -+ } -+ // Mark socket fresh so the next checkAndReopen doesn't see lastPing=0 -+ // and falsely classify the brand-new connection as stale. -+ const p = Promise.resolve(this.open()).then(() => { this.lastPing = Date.now() }) -+ this._reopenInFlight = p -+ const clear = () => { if (this._reopenInFlight === p) this._reopenInFlight = null } -+ p.then(clear, clear) -+ return p -+ } -+ -+ /** -+ * Decide whether the socket needs reopening when the app foregrounds. -+ * Three buckets keyed on time since last incoming message (`lastPing`): -+ * - stale (> ping*2): force reopen, no probe (already past alive()) -+ * - fresh (< 2000ms): no-op (recent traffic confirms liveness) -+ * - gray (in between): probe; if no pong in 2s, force reopen -+ */ -+ checkAndReopen = async (): Promise => { -+ const elapsed = Date.now() - this.lastPing -+ if (!this.connection || this.connection.readyState !== 1) { -+ this.logger.info(`[ddp] checkAndReopen bucket=stale elapsed=${elapsed}`) -+ await this.forceReopen() -+ return -+ } -+ if (elapsed > this.config.ping * 2) { -+ this.logger.info(`[ddp] checkAndReopen bucket=stale elapsed=${elapsed}`) -+ await this.forceReopen() -+ return -+ } -+ if (elapsed < 2000) { -+ this.logger.info(`[ddp] checkAndReopen bucket=fresh elapsed=${elapsed}`) -+ return -+ } -+ const alive = await this.probe() -+ if (alive) { -+ this.logger.info(`[ddp] checkAndReopen bucket=probe-pass elapsed=${elapsed}`) -+ return - } -+ this.logger.info(`[ddp] checkAndReopen bucket=probe-fail elapsed=${elapsed}`) -+ await this.forceReopen() - } - - /** Clear connection and try to connect again. */ -@@ -549,7 +672,9 @@ export class DDPDriver extends EventEmitter implements ISocket, IDriver { +@@ -549,7 +572,9 @@ export class DDPDriver extends EventEmitter implements ISocket, IDriver { 'uiInteraction', 'e2ekeyRequest', 'userData',