Skip to content

Commit 42a6987

Browse files
ggazzoclaude
andcommitted
fix(sdk): revert to full fire('reset') — _reconnectStopper is needed
The 'skip _reconnectStopper' approach broke force-logout coverage: e2ee-key-reset, account-manage-devices, admin-device-management all failed because, even with the ddp-streamer ws.close() server fix, the notify-user/<uid>/force_logout stream message still races with the close in microservices (it travels rocketchat-main → broker → ddp-streamer → WS, while the close listener fires directly on ddp-streamer). The graceful close only flushes what's already in the WS buffer at that moment — if the stream message is still in transit, it's still lost. So the per-call _reconnectStopper from callLoginMethod IS the load-bearing failsafe in microservices: it retries login with the latest stored token and calls makeClientLoggedOut on auth failure, which is what drives the user to /login when the stream message is lost. Restore the full fire('reset'). :87 is the only remaining EE 2/5 regression and a separate problem to solve. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 8861751 commit 42a6987

1 file changed

Lines changed: 21 additions & 35 deletions

File tree

apps/meteor/client/meteor/overrides/stubMeteorStream.ts

Lines changed: 21 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -213,29 +213,27 @@ queueMicrotask(() => {
213213
});
214214

215215
// When the underlying SDK socket reconnects (e.g. after a server-side
216-
// ws.terminate / ws.close from force-logout in microservices), Meteor's
216+
// ws.close / ws.terminate from force-logout in microservices), Meteor's
217217
// connection sees no transport event because the stub keeps reporting
218-
// 'connected'. Methods that were sent but not yet completed on the prior
219-
// SDK session are stranded with sentMessage=true, so any test that fires a
220-
// method right when the socket churns wedges (message-actions /
221-
// report-message / e2ee-encryption-decryption all timed out before this).
218+
// 'connected'. Without help, both the in-flight method machinery and
219+
// accounts-base's reconnect-time login retry stay dormant — methods sent
220+
// on the prior SDK session are stranded with sentMessage=true, and the
221+
// per-call _reconnectStopper from callLoginMethod (accounts_client.js:292)
222+
// never runs. Force-logout flows then leave the user with stale
223+
// credentials.
222224
//
223-
// Resend the in-flight method blocks on every subsequent SDK 'connected'
224-
// event. Mirror everything Meteor's `_streamHandlers.onReset` does EXCEPT
225-
// invoke `DDP._reconnectHook` callbacks: the per-call `_reconnectStopper`
226-
// that callLoginMethod registers (accounts_client.js:292) retries login
227-
// with the captured `result.token` from the *original* successful login,
228-
// which is now the stale token the server just invalidated. That retry
229-
// runs on a `wait:true` block which queues *ahead of* the test's own
230-
// loginByUserState login; if it fails (it will), the stopper's userCallback
231-
// calls `makeClientLoggedOut`, which clobbers the credentials the test
232-
// just injected — and the test's queued login then never sends a frame
233-
// because the stopper's wait block drains in a way that confuses the
234-
// outstanding queue. Force-logout cleanup is now handled by the
235-
// `useForceLogout` client hook (notify-user/<uid>/force_logout stream
236-
// message) which arrives reliably since DDPStreamer.ts:62 was switched to
237-
// ws.close(). The first connect is handled by the queueMicrotask above;
238-
// skip it here.
225+
// Fire `reset` on every subsequent SDK 'connected' event: this drives
226+
// _streamHandlers.onReset → _handleOutstandingMethodsOnReset (resends
227+
// pending methods so message-actions / report-message tests don't wedge)
228+
// AND _callOnReconnectAndSendAppropriateOutstandingMethods → DDP._reconnectHook
229+
// callbacks → the _reconnectStopper that retries login with the latest
230+
// stored token and calls makeClientLoggedOut on failure (so the
231+
// account-manage-devices / admin-device-management / e2ee-key-reset
232+
// force-logout tests recover). The first connect is handled by the
233+
// queueMicrotask above; skip it here. The "method result but no methods
234+
// outstanding" / "No callback invoker" warnings the resent blocks
235+
// occasionally generate are caught by the bridge's async catch in
236+
// ddpSdkCollectionBridge.
239237
const sdk = getDdpSdk();
240238
let firstConnectHandled = false;
241239
sdk.connection.on('connected', () => {
@@ -244,20 +242,8 @@ sdk.connection.on('connected', () => {
244242
return;
245243
}
246244
try {
247-
const c = conn as unknown as {
248-
_outstandingMethodBlocks: Array<{ wait: boolean; methods: unknown[] }>;
249-
_sendOutstandingMethodBlocksMessages(blocks: Array<{ wait: boolean; methods: unknown[] }>): void;
250-
_streamHandlers: {
251-
_handleOutstandingMethodsOnReset?: () => void;
252-
_resendSubscriptions?: () => void;
253-
};
254-
};
255-
c._streamHandlers._handleOutstandingMethodsOnReset?.();
256-
const oldBlocks = c._outstandingMethodBlocks;
257-
c._outstandingMethodBlocks = [];
258-
c._sendOutstandingMethodBlocksMessages(oldBlocks);
259-
c._streamHandlers._resendSubscriptions?.();
245+
fire('reset');
260246
} catch (err) {
261-
console.warn('[stubMeteorStream] custom reset on SDK reconnect failed', err);
247+
console.warn('[stubMeteorStream] reset on SDK reconnect failed', err);
262248
}
263249
});

0 commit comments

Comments
 (0)