Skip to content

revert: roll back SDK socket probing (#7298) and dependent reconnect fixes #7521

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
diegolmello merged 13 commits into from
Jul 31, 2026
Merged

revert: roll back SDK socket probing (#7298) and dependent reconnect fixes #7521

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
f07798a
Revert "fix: re-subscribe room streams after re-authentication on rec…
diegolmello Jul 26, 2026
5efd229
Revert "fix: re-subscribe room streams on DDP reconnect (#7426)"
diegolmello Jul 26, 2026
fa3278a
Revert "fix: re-subscribe rooms stream after forced socket reopen (#7…
diegolmello Jul 26, 2026
635b153
Revert "fix: keep live socket on deeplink login (prevent orphan WebSo…
diegolmello Jul 26, 2026
187acf2
Revert "fix(voip,connect): probe Meteor Connect on foreground and loc…
diegolmello Jul 26, 2026
095b455
fix: rewire pendingHangups drain without awaitDdpLoggedIn
diegolmello Jul 26, 2026
b5d6064
fix: remove 7380 orphan-socket gate tests from deepLinking
diegolmello Jul 26, 2026
1b4c174
test: cover handleClickCallPush new-server path and pendingHangups lo…
diegolmello Jul 26, 2026
8b45556
refactor: extract isLoginReady helper in connect.ts
diegolmello Jul 26, 2026
83e878f
fix: restore orphan-socket guard in SDK patch
diegolmello Jul 27, 2026
13e1afa
test: restore updateMessage concurrency and subscribe coverage
diegolmello Jul 27, 2026
36fb8aa
refactor: address review nits in connect and MediaSessionInstance
diegolmello Jul 27, 2026
8af1086
fix(voip): await processSignal before native-accept replay
diegolmello Jul 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
161 changes: 4 additions & 157 deletions app/lib/methods/subscriptions/room.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -17,14 +15,10 @@ jest.mock('../../services/sdk', () => ({
}
}));

const mockStoreGetState = jest.fn<{ meteor: { connected: boolean } }, []>(() => ({
meteor: { connected: false }
}));
const mockStoreDispatch = jest.fn<unknown, [unknown]>();
jest.mock('../../store/auxStore', () => ({
store: {
getState: () => mockStoreGetState(),
dispatch: (action: unknown) => mockStoreDispatch(action)
getState: jest.fn(() => ({})),
dispatch: jest.fn()
}
}));

Expand Down Expand Up @@ -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))
}
}));

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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<Promise<unknown>>(
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);
Expand Down
35 changes: 6 additions & 29 deletions app/lib/methods/subscriptions/room.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ export default class RoomSubscription {
private rid: string;
private isAlive: boolean;
private promises?: Promise<TSubscriptionModel[]>;
private loginListener?: Promise<any>;
private connectedListener?: Promise<any>;
private disconnectedListener?: Promise<any>;
private notifyRoomListener?: Promise<any>;
private messageReceivedListener?: Promise<any>;
Expand All @@ -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);
Comment thread
diegolmello marked this conversation as resolved.
Resolved
this.notifyRoomListener = sdk.onStreamData('stream-notify-room', this.handleNotifyRoomReceived);
this.messageReceivedListener = sdk.onStreamData('stream-room-messages', this.handleMessageReceived);
if (!this.isAlive) {
Expand All @@ -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);
Expand All @@ -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) {
Expand Down
Loading
Loading