Skip to content

Commit 2af4bda

Browse files
authored
fix(voip,connect): probe Meteor Connect on foreground and lock-screen accept (#7298)
1 parent 0800b6b commit 2af4bda

14 files changed

Lines changed: 521 additions & 32 deletions

.github/actions/setup-node/action.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,9 @@ runs:
1212
uses: actions/cache@0057852bfaa89a56745cba8c7296529d2fc39830 # v4.3.0
1313
with:
1414
path: node_modules
15-
key: ${{ runner.os }}-node-modules-${{ hashFiles('**/yarn.lock') }}
15+
key: ${{ runner.os }}-node-modules-${{ hashFiles('**/yarn.lock', 'patches/**') }}
1616
restore-keys: |
17-
${{ runner.os }}-node-modules-${{ hashFiles('**/yarn.lock') }}
17+
${{ runner.os }}-node-modules-${{ hashFiles('**/yarn.lock', 'patches/**') }}
1818
1919
- name: Install JS dependencies
2020
shell: bash

UBIQUITOUS_LANGUAGE.md

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -80,15 +80,15 @@
8080

8181
## Video & Voice
8282

83-
| Term | Definition | Aliases to avoid |
84-
| --------------------------- | --------------------------------------------------------------------------------------------------- | ---------------------- |
85-
| **Video Conference** | A video/voice call session with status lifecycle (calling, started, expired, ended, declined) | Video call, meeting |
86-
| **Direct Video Conference** | A 1-on-1 Video Conference ||
87-
| **Group Video Conference** | A multi-participant Video Conference with title and anonymous user support ||
88-
| **VOIP** | Voice-over-IP phone-style call, separate from Video Conference — uses ICE servers and media streams | Phone call, voice call |
89-
| **Native Accept** | An incoming VOIP call answered by native code (CallKit on iOS, Telecom on Android) before the JS runtime is available; native issues the REST accept and JS reconciles state on launch via initial events | JS accept, app accept |
90-
| **Per-call DDP** | A short-lived DDP client opened by native code per incoming VOIP call so accept and signaling land before JS boots; separate from the main app DDP session | Native socket, side socket |
91-
| **Media Signal** | A typed event on the `@rocket.chat/media-signaling` wire protocol (offer, answer, ICE candidate, state update) carried over DDP `stream-notify-user` and replayable via REST `media-calls.stateSignals` | Signal, RTC event |
83+
| Term | Definition | Aliases to avoid |
84+
| --------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -------------------------- |
85+
| **Video Conference** | A video/voice call session with status lifecycle (calling, started, expired, ended, declined) | Video call, meeting |
86+
| **Direct Video Conference** | A 1-on-1 Video Conference | |
87+
| **Group Video Conference** | A multi-participant Video Conference with title and anonymous user support | |
88+
| **VOIP** | Voice-over-IP phone-style call, separate from Video Conference — uses ICE servers and media streams | Phone call, voice call |
89+
| **Native Accept** | An incoming VOIP call answered by native code (CallKit on iOS, Telecom on Android) before the JS runtime is available; native issues the REST accept and JS reconciles state on launch via initial events | JS accept, app accept |
90+
| **Per-call DDP** | A short-lived DDP client opened by native code per incoming VOIP call so accept and signaling land before JS boots; separate from the main app DDP session | Native socket, side socket |
91+
| **Media Signal** | A typed event on the `@rocket.chat/media-signaling` wire protocol (offer, answer, ICE candidate, state update) carried over DDP `stream-notify-user` and replayable via REST `media-calls.stateSignals` | Signal, RTC event |
9292

9393
## Server & Connection
9494

app/lib/services/connect.ts

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -407,6 +407,36 @@ function checkAndReopen() {
407407
return sdk.current.checkAndReopen();
408408
}
409409

410+
/**
411+
* Resolves when the current session is fully logged in (or `timeoutMs` elapses).
412+
* Trusts redux state rather than `ddp.loggedIn`, which isn't cleared on socket
413+
* close and can read true for a stale session. Redux resets to
414+
* `isAuthenticated=false` on `LOGIN.REQUEST` (dispatched by the connectedListener)
415+
* and back to true on `LOGIN.SUCCESS`; `meteor.connected` covers the handshake.
416+
*/
417+
async function awaitDdpLoggedIn(timeoutMs: number = 5000): Promise<void> {
418+
const isReady = () => {
419+
const s = store.getState();
420+
return s.login.isAuthenticated && s.meteor.connected;
421+
};
422+
if (isReady()) {
423+
return;
424+
}
425+
await new Promise<void>(resolve => {
426+
const unsub = store.subscribe(() => {
427+
if (isReady()) {
428+
clearTimeout(timer);
429+
unsub();
430+
resolve();
431+
}
432+
});
433+
const timer = setTimeout(() => {
434+
unsub();
435+
resolve();
436+
}, timeoutMs);
437+
});
438+
}
439+
410440
function disconnect() {
411441
const result = sdk.disconnect();
412442
mediaSessionInstance.reset();
@@ -499,6 +529,7 @@ export {
499529
loginWithPassword,
500530
loginOAuthOrSso,
501531
checkAndReopen,
532+
awaitDdpLoggedIn,
502533
abort,
503534
connect,
504535
disconnect,

app/lib/services/ddpSocket.test.ts

Lines changed: 183 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,183 @@
1+
// eslint-disable-next-line @typescript-eslint/no-var-requires
2+
const { Socket } = require('@rocket.chat/sdk/lib/drivers/ddp');
3+
4+
const buildSocket = () => {
5+
const socket = new Socket({
6+
logger: { debug: jest.fn(), info: jest.fn(), error: jest.fn(), warn: jest.fn() },
7+
timeout: 10000
8+
});
9+
const send = jest.fn();
10+
const close = jest.fn();
11+
socket.connection = {
12+
send,
13+
close,
14+
readyState: 1,
15+
onopen: jest.fn(),
16+
onmessage: jest.fn(),
17+
onerror: jest.fn(),
18+
onclose: jest.fn()
19+
};
20+
return { socket, send, close };
21+
};
22+
23+
describe('Socket.probe', () => {
24+
afterEach(() => {
25+
jest.useRealTimers();
26+
});
27+
28+
it('resolves true when pong arrives within deadline', async () => {
29+
const { socket } = buildSocket();
30+
const probePromise = socket.probe();
31+
socket.emit('pong');
32+
await expect(probePromise).resolves.toBe(true);
33+
});
34+
35+
it('resolves false when no pong arrives within 2s deadline', async () => {
36+
jest.useFakeTimers();
37+
const { socket } = buildSocket();
38+
const probePromise = socket.probe();
39+
await jest.advanceTimersByTimeAsync(2000);
40+
await expect(probePromise).resolves.toBe(false);
41+
});
42+
43+
it('resolves false when raw connection.send throws', async () => {
44+
const { socket, send } = buildSocket();
45+
send.mockImplementation(() => {
46+
throw new Error('boom');
47+
});
48+
await expect(socket.probe()).resolves.toBe(false);
49+
});
50+
});
51+
52+
describe('Socket.forceReopen awaitability', () => {
53+
it('returned promise resolves only after open() resolves', async () => {
54+
const { socket } = buildSocket();
55+
let resolveOpen: () => void = () => undefined;
56+
const openPromise = new Promise<void>(res => {
57+
resolveOpen = res;
58+
});
59+
socket.open = jest.fn(() => openPromise);
60+
61+
let settled = false;
62+
const result = socket.forceReopen();
63+
result.then(() => {
64+
settled = true;
65+
});
66+
67+
await Promise.resolve();
68+
await Promise.resolve();
69+
expect(settled).toBe(false);
70+
71+
resolveOpen();
72+
await result;
73+
expect(settled).toBe(true);
74+
});
75+
76+
it('concurrent invocations share the same in-flight reconnect', async () => {
77+
const { socket } = buildSocket();
78+
let resolveOpen: () => void = () => undefined;
79+
const openPromise = new Promise<void>(res => {
80+
resolveOpen = res;
81+
});
82+
const openMock = jest.fn(() => openPromise);
83+
socket.open = openMock;
84+
85+
const a = socket.forceReopen();
86+
const b = socket.forceReopen();
87+
88+
expect(openMock).toHaveBeenCalledTimes(1);
89+
90+
resolveOpen();
91+
await Promise.all([a, b]);
92+
expect(openMock).toHaveBeenCalledTimes(1);
93+
});
94+
95+
it("emits 'close' synchronously before opening so app-level Redux disconnect dispatches", async () => {
96+
const { socket } = buildSocket();
97+
socket.open = jest.fn(() => Promise.resolve());
98+
const closeListener = jest.fn();
99+
socket.on('close', closeListener);
100+
const reopenPromise = socket.forceReopen();
101+
// Synchronous emit: must have fired by the time forceReopen returns.
102+
expect(closeListener).toHaveBeenCalledTimes(1);
103+
const [event] = closeListener.mock.calls[0];
104+
expect(event).toEqual({ code: 4000 });
105+
await reopenPromise;
106+
});
107+
});
108+
109+
describe('Socket.checkAndReopen bucket dispatch', () => {
110+
const buildWithSpies = () => {
111+
const { socket } = buildSocket();
112+
const { ping } = socket.config;
113+
const forceReopen = jest.fn();
114+
const probe = jest.fn();
115+
socket.forceReopen = forceReopen;
116+
socket.probe = probe;
117+
return { socket, forceReopen, probe, ping };
118+
};
119+
120+
it('stale (elapsed > ping*2) calls forceReopen and skips probe', async () => {
121+
const { socket, forceReopen, probe, ping } = buildWithSpies();
122+
socket.lastPing = Date.now() - ping * 2 - 1000;
123+
await socket.checkAndReopen();
124+
expect(forceReopen).toHaveBeenCalledTimes(1);
125+
expect(probe).not.toHaveBeenCalled();
126+
});
127+
128+
it('fresh (elapsed < 2000ms) is a no-op', async () => {
129+
const { socket, forceReopen, probe } = buildWithSpies();
130+
socket.lastPing = Date.now() - 500;
131+
await socket.checkAndReopen();
132+
expect(forceReopen).not.toHaveBeenCalled();
133+
expect(probe).not.toHaveBeenCalled();
134+
});
135+
136+
it('gray-zone with successful probe does not call forceReopen', async () => {
137+
const { socket, forceReopen, probe } = buildWithSpies();
138+
probe.mockResolvedValue(true);
139+
socket.lastPing = Date.now() - 5000;
140+
await socket.checkAndReopen();
141+
expect(probe).toHaveBeenCalledTimes(1);
142+
expect(forceReopen).not.toHaveBeenCalled();
143+
});
144+
145+
it('gray-zone with failed probe calls forceReopen', async () => {
146+
const { socket, forceReopen, probe } = buildWithSpies();
147+
probe.mockResolvedValue(false);
148+
socket.lastPing = Date.now() - 5000;
149+
await socket.checkAndReopen();
150+
expect(probe).toHaveBeenCalledTimes(1);
151+
expect(forceReopen).toHaveBeenCalledTimes(1);
152+
});
153+
154+
it('stale and probe-fail buckets resolve only after forceReopen resolves', async () => {
155+
const tryBucket = async (setup: (s: any) => void) => {
156+
const { socket } = buildSocket();
157+
let resolveReopen: () => void = () => undefined;
158+
const reopenPromise = new Promise<void>(res => {
159+
resolveReopen = res;
160+
});
161+
socket.forceReopen = jest.fn(() => reopenPromise);
162+
socket.probe = jest.fn().mockResolvedValue(false);
163+
setup(socket);
164+
let settled = false;
165+
const p = socket.checkAndReopen().then(() => {
166+
settled = true;
167+
});
168+
await Promise.resolve();
169+
await Promise.resolve();
170+
expect(settled).toBe(false);
171+
resolveReopen();
172+
await p;
173+
expect(settled).toBe(true);
174+
};
175+
176+
await tryBucket(socket => {
177+
socket.lastPing = Date.now() - socket.config.ping * 2 - 1000;
178+
});
179+
await tryBucket(socket => {
180+
socket.lastPing = Date.now() - 5000;
181+
});
182+
});
183+
});

app/lib/services/voip/MediaCallEvents.ios.test.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,10 @@ jest.mock('../restApi', () => ({
8888
registerPushToken: jest.fn(() => Promise.resolve())
8989
}));
9090

91+
jest.mock('../connect', () => require('./MediaCallEvents.testHelpers').createConnectMock());
92+
93+
jest.mock('../sdk', () => require('./MediaCallEvents.testHelpers').createSdkMock());
94+
9195
jest.mock('./MediaCallLogger', () => {
9296
const log = jest.fn();
9397
const debug = jest.fn();

0 commit comments

Comments
 (0)