Skip to content

Commit 2009c5b

Browse files
committed
fix(sdk): gate transport overrides in-place to preserve boot timing
The previous gate moved the 5 SDK-transport overrides out of `overrides/index.ts` into a dynamically imported `sdkTransport.ts`, and chained `meteor/login` after its resolution. That changed two things at boot relative to the parent commit: - the override side effects (Meteor.connection wraps, _stream stub, queue drains) ran one microtask later than ddpSdk.ts and the always-on overrides - meteor/login was deferred until the dynamic chunk fetched The shift was enough to flake every UI shard (logout flows, avatar upload, admin rooms, settings reset, file upload, message edit, etc.) because the post-relogin and bootstrap subscriptions raced the wraps that they depend on. Restore the parent commit's exact boot order: `overrides/index.ts` imports the 5 SDK-transport modules synchronously again. The runtime gate moves *inside* each module — `if (isSdkTransportEnabled()) { ... }` around the module's body — so with the flag off the wraps don't apply, and with the flag on the timing is identical to develop. Drop `overrides/sdkTransport.ts` (no longer needed) and revert main.ts to the parent's chain (no `sdkTransportReady` prepend). The boot banner moves to the gated side-effect block in `ddpSdk.ts`.
1 parent fd15892 commit 2009c5b

9 files changed

Lines changed: 273 additions & 334 deletions

File tree

apps/meteor/client/lib/sdk/ddpSdk.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,11 @@ declare global {
202202
}
203203

204204
if (typeof window !== 'undefined' && isSdkTransportEnabled()) {
205+
// eslint-disable-next-line no-console
206+
console.info(
207+
'%c[Rocket.Chat] SDK-over-DDP transport enabled (experimental)',
208+
'color:#fff;background:#f5455c;padding:2px 6px;border-radius:3px;font-weight:bold',
209+
);
205210
const sdk = getDdpSdk();
206211
window.__rocketChatSdk = sdk;
207212

apps/meteor/client/main.ts

Lines changed: 1 addition & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -2,27 +2,8 @@ import './meteor/overrides';
22
import './meteor/startup';
33
import './lib/sdk/ddpSdk';
44
import './serviceWorker';
5-
import { isSdkTransportEnabled } from './lib/sdk/sdkTransportEnabled';
65

7-
// Defer the rest of the boot chain until the SDK-transport overrides are
8-
// actually installed (when the flag is on). They mutate
9-
// `Meteor.connection._send`, `_subscribe`, and `_stream` at module init —
10-
// loading them in parallel with `./meteor/login` would race the first
11-
// `Meteor.loginWithToken` call against the wrap, leaving the login frame on
12-
// the un-wrapped transport. With the flag off, the conditional resolves to a
13-
// resolved Promise immediately.
14-
const sdkTransportEnabled = isSdkTransportEnabled();
15-
if (sdkTransportEnabled) {
16-
// eslint-disable-next-line no-console
17-
console.info(
18-
'%c[Rocket.Chat] SDK-over-DDP transport enabled (experimental)',
19-
'color:#fff;background:#f5455c;padding:2px 6px;border-radius:3px;font-weight:bold',
20-
);
21-
}
22-
const sdkTransportReady = sdkTransportEnabled ? import('./meteor/overrides/sdkTransport') : Promise.resolve();
23-
24-
void sdkTransportReady
25-
.then(() => import('./meteor/login'))
6+
import('./meteor/login')
267
.then(() => import('./importPackages'))
278
.then(() => import('./startup'))
289
.then(() =>

apps/meteor/client/meteor/overrides/ddpOverREST.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { DDPCommon } from 'meteor/ddp-common';
22
import { Meteor } from 'meteor/meteor';
33

44
import { sdk } from '../../../app/utils/client/lib/SDKClient';
5+
import { isSdkTransportEnabled } from '../../lib/sdk/sdkTransportEnabled';
56
import { getUserId } from '../../lib/user';
67

78
const bypassMethods: string[] = ['setUserStatus', 'logout'];
@@ -128,4 +129,6 @@ const withDDPOverREST = (_send: (this: Meteor.IMeteorConnection, message: Meteor
128129
};
129130
};
130131

131-
Meteor.connection._send = withDDPOverREST(Meteor.connection._send);
132+
if (isSdkTransportEnabled()) {
133+
Meteor.connection._send = withDDPOverREST(Meteor.connection._send);
134+
}

apps/meteor/client/meteor/overrides/ddpSdkCollectionBridge.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { DDPCommon } from 'meteor/ddp-common';
22
import { Meteor } from 'meteor/meteor';
33

44
import { getDdpSdk } from '../../lib/sdk/ddpSdk';
5+
import { isSdkTransportEnabled } from '../../lib/sdk/sdkTransportEnabled';
56

67
/**
78
* Bridge incoming DDPSDK frames into Meteor.connection's collection dispatch.
@@ -93,4 +94,6 @@ export const installDdpSdkCollectionBridge = (): void => {
9394
});
9495
};
9596

96-
installDdpSdkCollectionBridge();
97+
if (isSdkTransportEnabled()) {
98+
installDdpSdkCollectionBridge();
99+
}

apps/meteor/client/meteor/overrides/index.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
1-
// Always-on Meteor patches that aren't tied to the DDP transport choice.
2-
// The 5 SDK-transport overrides (ddpOverREST, killMeteorStream, subscribeViaSDK,
3-
// stubMeteorStream, ddpSdkCollectionBridge) live in `./sdkTransport.ts` and
4-
// are loaded conditionally from main.ts via `isSdkTransportEnabled()`.
1+
import './ddpOverREST';
2+
import './ddpSdkCollectionBridge';
3+
import './subscribeViaSDK';
4+
import './stubMeteorStream';
5+
import './killMeteorStream';
56
import './desktopInjection';
67
import './oauthRedirectUri';
78
import './settings';
Lines changed: 28 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { Accounts } from 'meteor/accounts-base';
22
import { Meteor } from 'meteor/meteor';
33

4+
import { isSdkTransportEnabled } from '../../lib/sdk/sdkTransportEnabled';
45
import { userIdStore } from '../../lib/user';
56

67
/**
@@ -27,84 +28,33 @@ import { userIdStore } from '../../lib/user';
2728
* Meteor's WS now stays connected — invokers reach `_send`, which
2829
* ddpOverREST intercepts and routes to REST (or DDPSDK for `login`).
2930
*/
30-
const conn = Meteor.connection as unknown as {
31-
_subsBeingRevived: Record<string, unknown>;
32-
_methodsBlockingQuiescence: Record<string, unknown>;
33-
_messagesBufferedUntilQuiescence: unknown[];
34-
_outstandingMethodBlocks: unknown[];
35-
_methodInvokers: Record<string, unknown>;
36-
};
31+
if (isSdkTransportEnabled()) {
32+
const conn = Meteor.connection as unknown as {
33+
_subsBeingRevived: Record<string, unknown>;
34+
_methodsBlockingQuiescence: Record<string, unknown>;
35+
_messagesBufferedUntilQuiescence: unknown[];
36+
_outstandingMethodBlocks: unknown[];
37+
_methodInvokers: Record<string, unknown>;
38+
};
3739

38-
conn._subsBeingRevived = Object.create(null);
39-
conn._methodsBlockingQuiescence = Object.create(null);
40-
conn._messagesBufferedUntilQuiescence = [];
41-
42-
/**
43-
* Force-clear Accounts._loggingIn once a uid lands in userIdStore.
44-
*
45-
* Meteor's loggedInAndDataReadyCallback flips _loggingIn back to false
46-
* from inside a Tracker.autorun that awaits Meteor.userAsync(). Our
47-
* findOneAsync await boundary breaks Tracker dep propagation, and the
48-
* same autorun is where Accounts.onLogin would normally fire — so
49-
* neither hook fires when synchronizeUserData later writes the user
50-
* into the store, and the UI stays on "Connecting..." with
51-
* Meteor.loggingIn() pinned to true.
52-
*
53-
* userIdStore is updated by the userAndUsers Tracker.autorun the moment
54-
* Accounts.connection.userId() flips, which happens inside
55-
* makeClientLoggedIn before the broken autorun is even installed. So
56-
* subscribing here gives a reliable "login completed" signal — just
57-
* skip the synchronous boot snapshot and react only to subsequent
58-
* transitions.
59-
*/
60-
let saw: string | undefined = userIdStore.getState();
61-
userIdStore.subscribe((next) => {
62-
if (next === saw) return;
63-
saw = next;
64-
if (next) {
65-
(Accounts as unknown as { _setLoggingIn?: (v: boolean) => void })._setLoggingIn?.(false);
66-
}
67-
});
68-
69-
/**
70-
* Drain Meteor's outstanding-method queue on logout.
71-
*
72-
* Accounts.logout's `applyAsync` resolves immediately via Meteor's
73-
* fire-and-forget client path (the actual server response is awaited
74-
* only by the MethodInvoker callback). Inside that resolved `.then`,
75-
* makeClientLoggedOut clears userId, which fires our userIdStore
76-
* subscriber to teardown DDPSDK. By the time the server's logout result
77-
* frame would arrive, the DDPSDK socket is already closed — so the
78-
* logout MethodInvoker stays in `_outstandingMethodBlocks` with
79-
* `sentMessage=true` forever.
80-
*
81-
* The next `_addOutstandingMethod` call (e.g. token-resume right after
82-
* logout) checks `_outstandingMethodBlocks.length === 1` to decide
83-
* whether to send immediately. With the orphaned logout block ahead of
84-
* it, the new method is silently queued and never sent. Visible
85-
* failure: re-login after logout never produces a login `_send` and the
86-
* UI hangs on PageLoading.
87-
*
88-
* Drain inside Accounts.onLogout because it fires synchronously from
89-
* makeClientLoggedOut *before* setUserId(null) and before any
90-
* subsequent applyAsync can enqueue. Doing the same drain on the
91-
* userIdStore transition would race the test's `_pollStoredLoginToken`
92-
* call, which can enqueue the new login between makeClientLoggedOut and
93-
* Tracker's deferred autorun re-run — we'd then wipe the new login
94-
* along with the dead logout.
95-
*/
96-
Accounts.onLogout(() => {
97-
conn._outstandingMethodBlocks = [];
98-
conn._methodInvokers = Object.create(null);
99-
// Also wipe _methodsBlockingQuiescence: the logout method's wait-flag
100-
// is still here because its server response never landed (DDPSDK was
101-
// torn down by makeClientLoggedOut). With it left in place,
102-
// _waitingForQuiescence() stays true and _livedata_data buffers every
103-
// subsequent frame — including the synthetic `updated` we inject via
104-
// processResult to trigger dataVisible on the next method invoker. The
105-
// invoker would then sit with _methodResult set, _dataVisible false,
106-
// and the login callback never fires.
40+
conn._subsBeingRevived = Object.create(null);
10741
conn._methodsBlockingQuiescence = Object.create(null);
10842
conn._messagesBufferedUntilQuiescence = [];
109-
conn._subsBeingRevived = Object.create(null);
110-
});
43+
44+
let saw: string | undefined = userIdStore.getState();
45+
userIdStore.subscribe((next) => {
46+
if (next === saw) return;
47+
saw = next;
48+
if (next) {
49+
(Accounts as unknown as { _setLoggingIn?: (v: boolean) => void })._setLoggingIn?.(false);
50+
}
51+
});
52+
53+
Accounts.onLogout(() => {
54+
conn._outstandingMethodBlocks = [];
55+
conn._methodInvokers = Object.create(null);
56+
conn._methodsBlockingQuiescence = Object.create(null);
57+
conn._messagesBufferedUntilQuiescence = [];
58+
conn._subsBeingRevived = Object.create(null);
59+
});
60+
}

apps/meteor/client/meteor/overrides/sdkTransport.ts

Lines changed: 0 additions & 12 deletions
This file was deleted.

0 commit comments

Comments
 (0)