Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
36abbfa
fix: make sdk.current nullable and guard its call sites
diegolmello Aug 20, 2026
a6cec3d
Merge branch 'new-sdk' into diegolmello/finding-09-sdk-current-nullable
diegolmello Aug 20, 2026
c4359c3
refactor: close the Sdk facade and drop sdk.current
diegolmello Aug 20, 2026
3df507b
Merge remote-tracking branch 'origin/new-sdk' into diegolmello/findin…
diegolmello Aug 21, 2026
222bc93
fix: drop stream frames and skip logout when the client is absent
diegolmello Aug 21, 2026
2feecf3
refactor: make the absent-client outcome explicit at each guard
diegolmello Aug 21, 2026
02f517e
test: tighten the host-guard assertions
diegolmello Aug 21, 2026
2444269
fix: forget the cached push tokens even with no client to delete them…
diegolmello Aug 21, 2026
3cc3018
fix: forget the cached push tokens unconditionally
diegolmello Aug 21, 2026
041bfe2
Merge remote-tracking branch 'origin/new-sdk' into diegolmello/findin…
diegolmello Aug 21, 2026
2685cb2
fix: forget the cached push tokens only when a device token exists
diegolmello Aug 21, 2026
f510e6e
fix: drop media signals produced after the client is gone
diegolmello Aug 21, 2026
48f1c48
refactor: ask for client presence through a single predicate
diegolmello Aug 21, 2026
e739ec2
test: build driver-shaped doubles from the shared SDK harness
diegolmello Aug 21, 2026
0390bcf
test: cover the absent-client logout and the matching-host frame
diegolmello Aug 21, 2026
b2836f3
refactor: name the absent connection in the triggerAction failure
diegolmello Aug 21, 2026
69f6e38
refactor: name the facade predicate for what it reports
diegolmello Aug 21, 2026
e2683de
test: drop the sdk double's getter for a removed property
diegolmello Aug 21, 2026
f5a9592
test: drop the unreachable rejection from the media-signal double
diegolmello Aug 21, 2026
588ed43
refactor: keep the sdk client and its driver behind the facade
diegolmello Aug 24, 2026
cf0e933
fix: read the subscribed host once when starting the rooms subscription
diegolmello Aug 24, 2026
b18d920
refactor: name the driver contract for what it is and check it agains…
diegolmello Aug 24, 2026
d2653fe
fix: drop rooms frames that arrive with no subscribed host
diegolmello Aug 24, 2026
6ca5377
refactor: annotate the abort exits like the disconnect beside them
diegolmello Aug 24, 2026
79a721b
test: drop the unread client predicate from the rooms host-guard double
diegolmello Aug 24, 2026
0582ad7
refactor: read the subscribed host the same way in both rooms guards
diegolmello Aug 24, 2026
d39a54a
refactor: name the sdk predicate for the initialization it reports
diegolmello Aug 24, 2026
24ac770
test: build the inline sdk doubles from the shared mock
diegolmello Aug 24, 2026
458f54b
refactor: gate rooms frames on the subscribed server itself
diegolmello Aug 24, 2026
87f771f
docs: say what triggerAction needs and stop naming a gone accessor
diegolmello Aug 24, 2026
492a9be
refactor: read the subscribed host directly and restore the no-socket…
diegolmello Aug 24, 2026
9d12bd3
refactor: name the subscribed host and the mock driver for what they are
diegolmello Aug 24, 2026
dd06ba5
refactor: inline the sdk mock's member constraint
diegolmello Aug 24, 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
1 change: 1 addition & 0 deletions app/definitions/rest/v1/push.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ export type PushEndpoints = {
userId: string;
};
};
DELETE: (params: { token: string }) => void;
};
'push.info': {
GET: () => TPushInfo;
Expand Down
8 changes: 6 additions & 2 deletions app/lib/methods/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,12 +108,16 @@ export async function triggerAction({
const payload = rest.payload ?? rest.value;

try {
const { currentLogin } = sdk.current;
const client = sdk.current;
if (!client) {
return;
}
const { currentLogin } = client;
if (!currentLogin) {
throw new Error('triggerAction requires an authenticated session');
}
const { userId, authToken } = currentLogin;
const { host } = sdk.current.client;
const { host } = client.client;
const interaction = toUserInteraction({
type,
actionId,
Expand Down
8 changes: 6 additions & 2 deletions app/lib/methods/getSettings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -157,10 +157,14 @@ export async function getSettings(): Promise<void> {
let offset = 0;
let remaining;
let settings: IData[] = [];
const client = sdk.current;
if (!client) {
return;
}
const serverVersion = reduxStore.getState().server.version;
const url = compareServerVersion(serverVersion, 'greaterThanOrEqualTo', '7.0.0')
? `${sdk.current.client.host}/api/v1/settings.public?_id=${settingsParams.join(',')}`
: `${sdk.current.client.host}/api/v1/settings.public?query={"_id":{"$in":${JSON.stringify(settingsParams)}}}`;
? `${client.client.host}/api/v1/settings.public?_id=${settingsParams.join(',')}`
: `${client.client.host}/api/v1/settings.public?query={"_id":{"$in":${JSON.stringify(settingsParams)}}}`;
// Iterate over paginated results to retrieve all settings
do {
// TODO: why is no-await-in-loop enforced in the first place?
Expand Down
2 changes: 1 addition & 1 deletion app/lib/methods/logout.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ export async function logout({ server }: { server: string }): Promise<void> {

try {
// RC 0.60.0
await sdk.current.logout();
await sdk.logout();
} catch (e) {
log(e);
}
Expand Down
11 changes: 8 additions & 3 deletions app/lib/methods/subscriptions/rooms.ts
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,7 @@ export default function subscribeRooms() {
const db = database.active;

// check if the server from variable is the same as the js sdk client
if (sdk && sdk.current.client && sdk.current.client.host !== subServer) {
if (sdk.current?.client && sdk.current.client.host !== subServer) {
return;
}
if (ddpMessage.msg === 'added') {
Expand Down Expand Up @@ -435,12 +435,17 @@ export default function subscribeRooms() {
roomsSubscription = null;
};

const client = sdk.current;
if (!client) {
return null;
}

streamListener = sdk.onStreamData('stream-notify-user', handleStreamMessageReceived);

try {
// set the server that started this task
subServer = sdk.current.client.host;
sdk.current.subscribeNotifyUser().catch((e: unknown) => console.log(e));
subServer = client.client.host;
client.subscribeNotifyUser().catch((e: unknown) => console.log(e));
roomsSubscription = { stop: () => stop() };
return null;
} catch (e) {
Expand Down
2 changes: 1 addition & 1 deletion app/lib/services/connect.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ const mockOnStreamData = jest.fn<Promise<{ stop: jest.Mock }>, [string, (...args
const mockSdkConnect = jest.fn<Promise<void>, []>(() => Promise.resolve());
const mockSdkAbort = jest.fn<void, []>();
const mockSdkDisconnect = jest.fn<void, []>();
const mockSdkInitialize = jest.fn<void, [string]>();
const mockSdkLogin = jest.fn<Promise<void>, [unknown]>(() => Promise.resolve());
const mockSdkCurrent: Record<string, unknown> = {
onStreamData: (event: string, cb: (...args: any[]) => void) => mockOnStreamData(event, cb),
Expand All @@ -32,6 +31,7 @@ const mockSdkCurrent: Record<string, unknown> = {
login: (credentials: unknown) => mockSdkLogin(credentials),
currentLogin: undefined
};
const mockSdkInitialize = jest.fn<typeof mockSdkCurrent, [string]>(() => mockSdkCurrent);
jest.mock('./sdk', () => ({
__esModule: true,
default: {
Expand Down
30 changes: 17 additions & 13 deletions app/lib/services/connect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,10 @@ function connect({ server, logoutOnError = false }: { server: string; logoutOnEr

EventEmitter.emit('INQUIRY_UNSUBSCRIBE');

sdk.initialize(server);
const client = sdk.initialize(server);
getSettings();

sdk.current
client
.connect()
.then(() => {
console.log('connected');
Expand All @@ -97,11 +97,11 @@ function connect({ server, logoutOnError = false }: { server: string; logoutOnEr
console.log('connect error', err);
});

connectingListener = sdk.current.onStreamData('connecting', () => {
connectingListener = client.onStreamData('connecting', () => {
store.dispatch(connectRequest());
});

connectedListener = sdk.current.onStreamData('connected', () => {
connectedListener = client.onStreamData('connected', () => {
const { connected } = store.getState().meteor;
if (connected) {
return;
Expand All @@ -117,12 +117,12 @@ function connect({ server, logoutOnError = false }: { server: string; logoutOnEr
// the WebSocket was unhealthy. Local to the closure so it resets per `connect()` call.
let pendingHangupsDrainArmed = false;

closeListener = sdk.current.onStreamData('close', () => {
closeListener = client.onStreamData('close', () => {
pendingHangupsDrainArmed = true;
store.dispatch(disconnectAction());
});

pendingHangupsConnectedListener = sdk.current.onStreamData('connected', async () => {
pendingHangupsConnectedListener = client.onStreamData('connected', async () => {
if (!pendingHangupsDrainArmed) return;
pendingHangupsDrainArmed = false;
if (pendingHangups.size === 0) return;
Expand All @@ -134,12 +134,12 @@ function connect({ server, logoutOnError = false }: { server: string; logoutOnEr
}
});

usersListener = sdk.current.onStreamData(
usersListener = client.onStreamData(
'users',
protectedFunction((ddpMessage: any) => _setUser(ddpMessage))
);

notifyAllListener = sdk.current.onStreamData(
notifyAllListener = client.onStreamData(
'stream-notify-all',
protectedFunction(async (ddpMessage: { fields: { args?: any; eventName: string } }) => {
const { eventName } = ddpMessage.fields;
Expand Down Expand Up @@ -177,7 +177,7 @@ function connect({ server, logoutOnError = false }: { server: string; logoutOnEr
})
);

rolesListener = sdk.current.onStreamData(
rolesListener = client.onStreamData(
'stream-roles',
protectedFunction((ddpMessage: any) => onRolesChanged(ddpMessage))
);
Expand All @@ -199,7 +199,7 @@ function connect({ server, logoutOnError = false }: { server: string; logoutOnEr
}
});

notifyLoggedListener = sdk.current.onStreamData(
notifyLoggedListener = client.onStreamData(
'stream-notify-logged',
protectedFunction(async (ddpMessage: { fields: { args?: any; eventName?: any } }) => {
const { eventName } = ddpMessage.fields;
Expand Down Expand Up @@ -290,7 +290,7 @@ function connect({ server, logoutOnError = false }: { server: string; logoutOnEr
})
);

logoutListener = sdk.current.onStreamData('stream-force_logout', () => store.dispatch(logout(true)));
logoutListener = client.onStreamData('stream-force_logout', () => store.dispatch(logout(true)));

resolve();
});
Expand All @@ -301,10 +301,14 @@ function stopListener(listener: any): void {
}

async function login(credentials: ILoginCredentials): Promise<ILoggedUser> {
const client = sdk.current;
if (!client) {
throw new Error('Cannot login without an active connection');
}
// RC 0.64.0
await sdk.current.login(credentials);
await client.login(credentials);
const serverVersion = store.getState().server.version;
const result = sdk.current.currentLogin?.result;
const result = client.currentLogin?.result;
if (!result) {
throw new Error('Login failed: missing login result');
}
Expand Down
5 changes: 2 additions & 3 deletions app/lib/services/restApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,15 @@
import { TEAM_TYPE } from '../../definitions/ITeam';
import { type OperationParams, type ResultFor } from '../../definitions/rest/helpers';
import { type SubscriptionsEndpoints } from '../../definitions/rest/v1/subscriptions';
import { Encryption } from '../encryption';

Check warning on line 25 in app/lib/services/restApi.ts

View workflow job for this annotation

GitHub Actions / ESLint and Test / run-eslint-and-test

import(no-cycle)

Dependency cycle detected
import { type RoomTypes, roomTypeToApiType } from '../methods/roomTypeToApiType';
import { uploadUserAvatarMultipart } from '../methods/uploadAvatar/uploadAvatar';
import { unsubscribeRooms } from '../methods/subscribeRooms';

Check warning on line 28 in app/lib/services/restApi.ts

View workflow job for this annotation

GitHub Actions / ESLint and Test / run-eslint-and-test

import(no-cycle)

Dependency cycle detected
import { compareServerVersion, getBundleId, isIOS } from '../methods/helpers';

Check warning on line 29 in app/lib/services/restApi.ts

View workflow job for this annotation

GitHub Actions / ESLint and Test / run-eslint-and-test

import(no-cycle)

Dependency cycle detected
import { getDeviceToken } from '../notifications';

Check warning on line 30 in app/lib/services/restApi.ts

View workflow job for this annotation

GitHub Actions / ESLint and Test / run-eslint-and-test

import(no-cycle)

Dependency cycle detected
import NativeVoipModule from '../native/NativeVoip';
import { store as reduxStore } from '../store/auxStore';
import sdk from './sdk';

Check warning on line 33 in app/lib/services/restApi.ts

View workflow job for this annotation

GitHub Actions / ESLint and Test / run-eslint-and-test

import(no-cycle)

Dependency cycle detected
import fetch from '../methods/helpers/fetch';
import log from '../methods/helpers/log';

Expand Down Expand Up @@ -1175,15 +1175,14 @@
};

// TODO: add voip token removal
export const removePushToken = (): Promise<boolean | void> => {
export const removePushToken = async (): Promise<void> => {
const token = getDeviceToken();
if (token) {
lastToken = '';
lastVoipToken = '';
// RC 0.60.0
return sdk.current.del('push.token', { token });
await sdk.del('push.token', { token });
}
return Promise.resolve();
};

// RC 6.6.0
Expand Down
49 changes: 34 additions & 15 deletions app/lib/services/sdk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,33 +24,34 @@ export interface IStreamDataListener {
}

class Sdk {
private sdk!: Rocketchat;
private sdk: Rocketchat | null = null;
private code: any;

private get activeSdk(): Rocketchat {
if (!this.sdk) {
throw new Error('Sdk is not initialized');
}
return this.sdk;
}

private initializeSdk(server: string): Rocketchat {
// The app can't reconnect if reopen interval is 5s while in development
return new Rocketchat({ host: server, protocol: 'ddp', useSsl: isSsl(server), reopen: __DEV__ ? 20000 : 5000 });
}

// TODO: We need to stop returning the SDK after all methods are dehydrated
initialize(server: string) {
this.code = null;
this.sdk = this.initializeSdk(server);
return this.sdk;
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
}

get current(): Rocketchat {
get current(): Rocketchat | null {
return this.sdk;
}

/**
* TODO: evaluate the need for assigning "null" to this.sdk
* I'm returning "null" because we need to remove both instances of this.sdk here and on rocketchat.js
*/
disconnect() {
if (this.sdk) {
this.sdk.disconnect();
// @ts-expect-error
this.sdk = null;
}
return null;
Expand All @@ -67,7 +68,7 @@ class Sdk {
? void
: Serialized<OperationParams<'GET', MatchPathPattern<TPath>>>
): Promise<Serialized<ResultFor<'GET', MatchPathPattern<TPath>>>> {
return this.current.get(endpoint, params);
return this.activeSdk.get(endpoint, params);
}

post<TPath extends PathFor<'POST'>>(
Expand All @@ -84,7 +85,7 @@ class Sdk {
return new Promise(async (resolve, reject) => {
const isMethodCall = endpoint?.startsWith('method.call/');
try {
const result = await this.current.post(endpoint, params);
const result = await this.activeSdk.post(endpoint, params);

/**
* if API_Use_REST_For_DDP_Calls is enabled and it's a method call,
Expand Down Expand Up @@ -117,13 +118,31 @@ class Sdk {
});
}

del<TPath extends PathFor<'DELETE'>>(
endpoint: TPath,
params: void extends OperationParams<'DELETE', MatchPathPattern<TPath>>
? void
: Serialized<OperationParams<'DELETE', MatchPathPattern<TPath>>> = undefined as void extends OperationParams<
'DELETE',
MatchPathPattern<TPath>
>
? void
: Serialized<OperationParams<'DELETE', MatchPathPattern<TPath>>>
): Promise<Serialized<ResultFor<'DELETE', MatchPathPattern<TPath>>>> {
return this.activeSdk.del(endpoint, params);
}

logout() {
return this.activeSdk.logout();
}

methodCall(method: string, ...args: any[]): Promise<any> {
return new Promise(async (resolve, reject) => {
try {
// Clear the 2FA code after use — a stale trailing arg breaks typed method signatures
const { code } = this;
this.code = null;
const result = await this.current.methodCall(method, ...args, ...(code ? [code] : []));
const result = await this.activeSdk.methodCall(method, ...args, ...(code ? [code] : []));
return resolve(result);
} catch (e: any) {
if (e.error && (e.error === 'totp-required' || e.error === 'totp-invalid')) {
Expand Down Expand Up @@ -161,11 +180,11 @@ class Sdk {
}

subscribe(topic: string, eventName?: string, ...args: any[]): Promise<ISubscription | undefined> {
return this.current.subscribe(topic, eventName as string, ...args);
return this.activeSdk.subscribe(topic, eventName as string, ...args);
}

subscribeRaw(name: string, params: any[]): Promise<ISubscription | undefined> {
return this.current.subscribeRaw(name, params);
return this.activeSdk.subscribeRaw(name, params);
}

subscribeRoom(...args: any[]) {
Expand All @@ -190,11 +209,11 @@ class Sdk {
}

unsubscribe(subscription: ISubscription) {
return this.current.unsubscribe(subscription);
return this.activeSdk.unsubscribe(subscription);
}

onStreamData(event: string, callback: TStreamDataCallback): Promise<IStreamDataListener> {
return this.current.onStreamData(event, callback as ICallback);
return this.activeSdk.onStreamData(event, callback as ICallback);
}
}

Expand Down
Loading