From 3ea0d613fbcd6daba669f93d9ccc84ae6345f613 Mon Sep 17 00:00:00 2001 From: Diego Mello Date: Mon, 24 Aug 2026 15:01:20 -0300 Subject: [PATCH 1/5] fix: resume the Login on a reopened connection --- lib/drivers/__tests__/socket.relogin.spec.ts | 113 +++++++++++++++++++ lib/drivers/socket.ts | 27 ++++- 2 files changed, 136 insertions(+), 4 deletions(-) create mode 100644 lib/drivers/__tests__/socket.relogin.spec.ts diff --git a/lib/drivers/__tests__/socket.relogin.spec.ts b/lib/drivers/__tests__/socket.relogin.spec.ts new file mode 100644 index 0000000..68ccef7 --- /dev/null +++ b/lib/drivers/__tests__/socket.relogin.spec.ts @@ -0,0 +1,113 @@ +import { Socket } from '../socket' +import { ILogger, ILoginResult } from '../../../interfaces' +import { createSilentLogger } from '../../../test/createSilentLogger' +import { + driveToHandshake, + FakeWebSocket, + fakeSockets, + flushMicrotasks, + openFakeConnection, + useFakeClockAndSocketRegistry +} from '../../../test/fakeTransport' + +jest.mock('universal-websocket-client', () => require('../../../test/fakeTransport').fakeTransportModule) + +useFakeClockAndSocketRegistry() + +const REOPEN_DELAY = 3000 +const TIMEOUT = 7000 + +const resumeToken: ILoginResult = { + id: 'user-id', + token: 'resume-token', + tokenExpires: { $date: Date.now() + 60000 } +} as ILoginResult + +const createSocket = (logger: ILogger, resume: ILoginResult | null = null) => new Socket({ + host: 'localhost:3000', + logger, + reopen: REOPEN_DELAY, + timeout: TIMEOUT, + ping: 10 * 60 * 1000 +}, resume) + +const loginFrames = (transport: FakeWebSocket) => + transport.sent + .map((frame) => JSON.parse(frame)) + .filter((frame) => frame.msg === 'method' && frame.method === 'login') + +/** + * A Reopen builds a new connection with the same DDP session gone, so the + * Login has to be made again on it. The token the Socket holds is the only + * thing that makes that possible, and it is not itself proof of a Login. + */ +describe('Resume on reopen', () => { + let logger: ILogger + + beforeEach(() => { + logger = createSilentLogger() + }) + + it('sends a login method call on the reopened connection', async () => { + const socket = createSocket(logger, resumeToken) + const transport = await openFakeConnection(socket) + + transport.close(1006) + await jest.advanceTimersByTimeAsync(REOPEN_DELAY) + + const reopened = fakeSockets[1] + expect(reopened).toBeDefined() + await driveToHandshake(reopened) + await flushMicrotasks() + + expect(loginFrames(reopened)).toHaveLength(1) + expect(loginFrames(reopened)[0].params).toEqual([{ resume: resumeToken.token }]) + }) + + it('sends no login method call with no token held', async () => { + const socket = createSocket(logger) + const transport = await openFakeConnection(socket) + + transport.close(1006) + await jest.advanceTimersByTimeAsync(REOPEN_DELAY) + + const reopened = fakeSockets[1] + await driveToHandshake(reopened) + await flushMicrotasks() + + expect(loginFrames(reopened)).toHaveLength(0) + }) + + it('resolves the open and emits open without waiting on the login response', async () => { + const socket = createSocket(logger, resumeToken) + const opened = jest.fn() + socket.on('open', opened) + + const opening = socket.open() + const transport = fakeSockets[0] + await driveToHandshake(transport) + + await expect(opening).resolves.toBe(transport) + expect(opened).toHaveBeenCalled() + expect(loginFrames(transport)).toHaveLength(1) + }) + + it('reports logged in only once the relogin has its result', async () => { + const socket = createSocket(logger, resumeToken) + const transport = await openFakeConnection(socket) + + transport.close(1006) + await jest.advanceTimersByTimeAsync(REOPEN_DELAY) + + const reopened = fakeSockets[1] + await driveToHandshake(reopened) + await flushMicrotasks() + + expect(socket.loggedIn).toBe(false) + + reopened.receive({ msg: 'result', id: loginFrames(reopened)[0].id, result: resumeToken }) + await flushMicrotasks() + + expect(socket.loggedIn).toBe(true) + }) +}) diff --git a/lib/drivers/socket.ts b/lib/drivers/socket.ts index 80c64ac..7f964a2 100644 --- a/lib/drivers/socket.ts +++ b/lib/drivers/socket.ts @@ -92,6 +92,7 @@ export class Socket extends SDKEventEmitter { session?: string logger: ILogger reopenPromise?: Promise + private authenticated = false private settleReopen?: () => void private pendingOpenRejects = new WeakMap void>() private subscriptionRequests: { [id: string]: Promise } = {} @@ -152,6 +153,7 @@ export class Socket extends SDKEventEmitter { } } this.connection = connection + this.authenticated = false this.connection.onmessage = this.onMessage.bind(this) this.connection.onclose = (ev: any) => this.onClose(ev, connection) // pass closing socket so onClose can compare identity this.connection.onopen = this.onOpen.bind(this, resolve, reject) @@ -162,7 +164,6 @@ export class Socket extends SDKEventEmitter { /** * Open websocket connection. * Stores connection, setting up handlers for open/close/message events. - * Resumes login if given token. */ open = async (): Promise => { if (this.connected) { @@ -181,7 +182,21 @@ export class Socket extends SDKEventEmitter { return this.connection } - /** Send handshake message to confirm connection, start pinging. */ + /** + * Resume the Login on this connection when a token is held, without the open + * waiting on it. A websocket callback has nowhere to put a throw. + */ + private resumeLogin = () => { + if (!this.resume) return + this.login(this.resume).catch((err) => + this.logger.error(`[ddp] the resume login did not complete: ${(err as Error).message}`) + ) + } + + /** + * Send handshake message to confirm connection, start pinging, and resume the + * Login if a token is held. + */ onOpen = async (resolve: Function, reject: Function) => { this.lastPing = Date.now() @@ -200,7 +215,8 @@ export class Socket extends SDKEventEmitter { this.session = connected.session this.ping().catch((err) => this.logger.error(`[ddp] Unable to ping server: ${err.message}`)) this.emit('open') - return resolve(this.connection) + resolve(this.connection) + return this.resumeLogin() } onClose = (e: any, closedConnection?: WebSocket) => { @@ -208,6 +224,7 @@ export class Socket extends SDKEventEmitter { if (closedConnection && closedConnection !== this.connection) { return } + this.authenticated = false this.emit('close', e) try { if (e?.code !== userDisconnectCloseCode) { @@ -483,7 +500,7 @@ export class Socket extends SDKEventEmitter { } get loggedIn () { - return (this.connected && !!this.resume) + return (this.connected && this.authenticated) } /** @@ -645,6 +662,7 @@ export class Socket extends SDKEventEmitter { login = async (credentials: IRealtimeCredentials) => { const params = this.loginParams(credentials) this.resume = (await this.call('login', params) as ILoginResult) + this.authenticated = true this.subscribeAll().catch(console.log) this.emit('login', this.resume) return this.resume @@ -678,6 +696,7 @@ export class Socket extends SDKEventEmitter { /** Logout the current User from the server via Socket. */ logout = () => { this.resume = null + this.authenticated = false return this.unsubscribeAll() .then(() => this.call('logout')) } From 34d41b07897cd117ec07335d2320414c41c723b1 Mon Sep 17 00:00:00 2001 From: Diego Mello Date: Mon, 24 Aug 2026 15:28:18 -0300 Subject: [PATCH 2/5] refactor: name the resumed-login flag after the Login it confirms --- lib/drivers/socket.ts | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/lib/drivers/socket.ts b/lib/drivers/socket.ts index 2062de0..c282f2c 100644 --- a/lib/drivers/socket.ts +++ b/lib/drivers/socket.ts @@ -92,7 +92,7 @@ export class Socket extends SDKEventEmitter { session?: string logger: ILogger reopenPromise?: Promise - private authenticated = false + private loginConfirmed = false private settleReopen?: () => void private pendingOpenRejects = new WeakMap void>() private subscriptionRequests: { [id: string]: Promise } = {} @@ -153,7 +153,7 @@ export class Socket extends SDKEventEmitter { } } this.connection = connection - this.authenticated = false + this.loginConfirmed = false this.connection.onmessage = this.onMessage.bind(this) this.connection.onclose = (ev: any) => this.onClose(ev, connection) // pass closing socket so onClose can compare identity this.connection.onopen = this.onOpen.bind(this, resolve, reject) @@ -186,17 +186,14 @@ export class Socket extends SDKEventEmitter { * Resume the Login on this connection when a token is held, without the open * waiting on it. A websocket callback has nowhere to put a throw. */ - private resumeLogin = () => { + private resumeLoginInBackground = () => { if (!this.resume) return this.login(this.resume).catch((err) => - this.logger.error(`[ddp] the resume login did not complete: ${(err as Error).message}`) + this.logger.error(`[ddp] Resume login did not complete: ${(err as Error).message}`) ) } - /** - * Send handshake message to confirm connection, start pinging, and resume the - * Login if a token is held. - */ + /** Send handshake message to confirm connection, start pinging. */ onOpen = async (resolve: Function, reject: Function) => { this.lastPing = Date.now() @@ -216,7 +213,7 @@ export class Socket extends SDKEventEmitter { this.ping().catch((err) => this.logger.error(`[ddp] Unable to ping server: ${err.message}`)) this.emit('open') resolve(this.connection) - return this.resumeLogin() + return this.resumeLoginInBackground() } onClose = (e: any, closedConnection?: WebSocket) => { @@ -224,7 +221,7 @@ export class Socket extends SDKEventEmitter { if (closedConnection && closedConnection !== this.connection) { return } - this.authenticated = false + this.loginConfirmed = false this.emit('close', e) try { if (e?.code !== userDisconnectCloseCode) { @@ -500,7 +497,7 @@ export class Socket extends SDKEventEmitter { } get loggedIn () { - return (this.connected && this.authenticated) + return (this.connected && this.loginConfirmed) } /** @@ -662,7 +659,7 @@ export class Socket extends SDKEventEmitter { login = async (credentials: IRealtimeCredentials) => { const params = this.loginParams(credentials) this.resume = (await this.call('login', params) as ILoginResult) - this.authenticated = true + this.loginConfirmed = true this.subscribeAll().catch((err) => { this.logger.error(`[ddp] Resubscribe after login failed: ${err.message}`) this.emit('resubscribe-error', err) @@ -699,7 +696,7 @@ export class Socket extends SDKEventEmitter { /** Logout the current User from the server via Socket. */ logout = () => { this.resume = null - this.authenticated = false + this.loginConfirmed = false return this.unsubscribeAll() .then(() => this.call('logout')) } From a8b27bfd89dc7a8cc6eeb550ad1f44ce8589b6aa Mon Sep 17 00:00:00 2001 From: Diego Mello Date: Mon, 24 Aug 2026 15:29:51 -0300 Subject: [PATCH 3/5] refactor: factor the reopen drive out of the relogin spec --- lib/drivers/__tests__/socket.relogin.spec.ts | 37 +++++++++----------- lib/drivers/socket.ts | 7 ++-- 2 files changed, 18 insertions(+), 26 deletions(-) diff --git a/lib/drivers/__tests__/socket.relogin.spec.ts b/lib/drivers/__tests__/socket.relogin.spec.ts index 9280679..e659e85 100644 --- a/lib/drivers/__tests__/socket.relogin.spec.ts +++ b/lib/drivers/__tests__/socket.relogin.spec.ts @@ -15,7 +15,6 @@ jest.mock('universal-websocket-client', () => require('../../../test/fakeTranspo useFakeClockAndSocketRegistry() const REOPEN_DELAY = 3000 -const TIMEOUT = 7000 const resumeToken: ILoginResult = { id: 'user-id', @@ -27,10 +26,22 @@ const createSocket = (logger: ILogger, resume: ILoginResult | null = null) => ne host: 'localhost:3000', logger, reopen: REOPEN_DELAY, - timeout: TIMEOUT, + timeout: 7000, ping: 10 * 60 * 1000 }, resume) +const reopenAfterDrop = async (transport: FakeWebSocket) => { + transport.close(1006) + await jest.advanceTimersByTimeAsync(REOPEN_DELAY) + + const reopened = fakeSockets[1] + expect(reopened).toBeDefined() + await driveToHandshake(reopened) + await flushMicrotasks() + + return reopened +} + const loginFrames = (transport: FakeWebSocket) => transport.sent .map((frame) => JSON.parse(frame)) @@ -52,13 +63,7 @@ describe('Resume on reopen', () => { const socket = createSocket(logger, resumeToken) const transport = await openFakeConnection(socket) - transport.close(1006) - await jest.advanceTimersByTimeAsync(REOPEN_DELAY) - - const reopened = fakeSockets[1] - expect(reopened).toBeDefined() - await driveToHandshake(reopened) - await flushMicrotasks() + const reopened = await reopenAfterDrop(transport) expect(loginFrames(reopened)).toHaveLength(1) expect(loginFrames(reopened)[0].params).toEqual([{ resume: resumeToken.token }]) @@ -68,12 +73,7 @@ describe('Resume on reopen', () => { const socket = createSocket(logger) const transport = await openFakeConnection(socket) - transport.close(1006) - await jest.advanceTimersByTimeAsync(REOPEN_DELAY) - - const reopened = fakeSockets[1] - await driveToHandshake(reopened) - await flushMicrotasks() + const reopened = await reopenAfterDrop(transport) expect(loginFrames(reopened)).toHaveLength(0) }) @@ -96,12 +96,7 @@ describe('Resume on reopen', () => { const socket = createSocket(logger, resumeToken) const transport = await openFakeConnection(socket) - transport.close(1006) - await jest.advanceTimersByTimeAsync(REOPEN_DELAY) - - const reopened = fakeSockets[1] - await driveToHandshake(reopened) - await flushMicrotasks() + const reopened = await reopenAfterDrop(transport) expect(socket.loggedIn).toBe(false) diff --git a/lib/drivers/socket.ts b/lib/drivers/socket.ts index c282f2c..017463d 100644 --- a/lib/drivers/socket.ts +++ b/lib/drivers/socket.ts @@ -182,10 +182,7 @@ export class Socket extends SDKEventEmitter { return this.connection } - /** - * Resume the Login on this connection when a token is held, without the open - * waiting on it. A websocket callback has nowhere to put a throw. - */ + /** Not awaited: a websocket callback has nowhere to put a throw. */ private resumeLoginInBackground = () => { if (!this.resume) return this.login(this.resume).catch((err) => @@ -213,7 +210,7 @@ export class Socket extends SDKEventEmitter { this.ping().catch((err) => this.logger.error(`[ddp] Unable to ping server: ${err.message}`)) this.emit('open') resolve(this.connection) - return this.resumeLoginInBackground() + this.resumeLoginInBackground() } onClose = (e: any, closedConnection?: WebSocket) => { From 91762302aff885a83efb6a9b1304a840afe80bd4 Mon Sep 17 00:00:00 2001 From: Diego Mello Date: Mon, 24 Aug 2026 15:31:35 -0300 Subject: [PATCH 4/5] docs: record the reopened-connection Login decision as ADR-0012 --- ...reopened-connection-makes-its-own-login.md | 47 +++++++++++++++++++ lib/drivers/__tests__/socket.relogin.spec.ts | 2 +- lib/drivers/socket.ts | 2 +- 3 files changed, 49 insertions(+), 2 deletions(-) create mode 100644 docs/adr/0012-a-reopened-connection-makes-its-own-login.md diff --git a/docs/adr/0012-a-reopened-connection-makes-its-own-login.md b/docs/adr/0012-a-reopened-connection-makes-its-own-login.md new file mode 100644 index 0000000..f9fbd27 --- /dev/null +++ b/docs/adr/0012-a-reopened-connection-makes-its-own-login.md @@ -0,0 +1,47 @@ +# ADR-0012: A reopened connection makes its own Login + +**Status:** Accepted + +## Context + +A Reopen builds a new connection, and the DDP session the previous one carried +is gone with it. The Login has to be made again on the new connection, and the +Resume token the Socket holds is the only thing that makes that possible. + +The Socket stopped doing so years ago, and nothing else inside the SDK does it +either, so a reopened connection stayed anonymous until the consuming app logged +in again. `loggedIn` did not report that: it read the token, which survives every +connection, so an anonymous reopened session claimed to be logged in. A `sub` +sent in that window is refused, and under ADR-0004 a refused resubscribe forgets +the entry it was meant to restore. + +## Decision + +The Socket makes the Login itself when a connection opens holding a token, and +`loggedIn` reports the Login rather than the token. + +- The Resume is made from `onOpen`, after the handshake, so it runs on a + connection the server has answered. +- `open` does not wait on it. A websocket callback has nowhere to put a throw, and + a Login the server never answers would otherwise stall every caller of `open` — + a Reopen among them — behind a wait no Deadline in ADR-0003 covers. A Resume + that fails is logged and nothing else; the connection stands. +- `loggedIn` is `connected && loginConfirmed`, where the flag is set when a Login + resolves and cleared wherever the identity it stands for ends: a new connection, + a close, a logout. It therefore reports the connection in front of it, not the + token, and reads `false` for the window between an open and its Resume. + +## Consequences + +- `loggedIn` reports `false` during the Reopen window, where it previously + reported `true`. Nothing in the SDK reads it, but ADR-0007 notes that + Rocket.Chat.ReactNative reaches into the Socket and types it loosely, so a gate + built there on the old meaning changes behaviour without failing to compile. +- `open` resolves and `open` is emitted before the Resume lands, so the window + ADR-0004 penalises is narrowed rather than closed. A caller that resubscribes + on `open` is still resubscribing anonymously; closing that gap means gating the + subscribe on `loggedIn`, which this decision does not do. +- A consuming app that dispatches its own login from a connected handler now + makes a second Login on the same connection. The server accepts both. +- The Login carries `subscribeAll` with it, so the SDK's own resubscribe after a + Reopen runs behind a confirmed Login rather than in front of one. diff --git a/lib/drivers/__tests__/socket.relogin.spec.ts b/lib/drivers/__tests__/socket.relogin.spec.ts index e659e85..f166ee5 100644 --- a/lib/drivers/__tests__/socket.relogin.spec.ts +++ b/lib/drivers/__tests__/socket.relogin.spec.ts @@ -19,7 +19,7 @@ const REOPEN_DELAY = 3000 const resumeToken: ILoginResult = { id: 'user-id', token: 'resume-token', - createCipher: { $date: Date.now() + 60000 } + createCipher: { $date: 0 } } const createSocket = (logger: ILogger, resume: ILoginResult | null = null) => new Socket({ diff --git a/lib/drivers/socket.ts b/lib/drivers/socket.ts index 017463d..aba500f 100644 --- a/lib/drivers/socket.ts +++ b/lib/drivers/socket.ts @@ -186,7 +186,7 @@ export class Socket extends SDKEventEmitter { private resumeLoginInBackground = () => { if (!this.resume) return this.login(this.resume).catch((err) => - this.logger.error(`[ddp] Resume login did not complete: ${(err as Error).message}`) + this.logger.error(`[ddp] Resume did not complete: ${(err as Error).message}`) ) } From 56cd9ccae4e9932165190d9f0c0963529f6719a9 Mon Sep 17 00:00:00 2001 From: Diego Mello Date: Mon, 24 Aug 2026 15:32:56 -0300 Subject: [PATCH 5/5] refactor(test): name the resume spec after the domain term --- ....relogin.spec.ts => socket.resume.spec.ts} | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) rename lib/drivers/__tests__/{socket.relogin.spec.ts => socket.resume.spec.ts} (80%) diff --git a/lib/drivers/__tests__/socket.relogin.spec.ts b/lib/drivers/__tests__/socket.resume.spec.ts similarity index 80% rename from lib/drivers/__tests__/socket.relogin.spec.ts rename to lib/drivers/__tests__/socket.resume.spec.ts index f166ee5..aeffd26 100644 --- a/lib/drivers/__tests__/socket.relogin.spec.ts +++ b/lib/drivers/__tests__/socket.resume.spec.ts @@ -16,7 +16,7 @@ useFakeClockAndSocketRegistry() const REOPEN_DELAY = 3000 -const resumeToken: ILoginResult = { +const loginResult: ILoginResult = { id: 'user-id', token: 'resume-token', createCipher: { $date: 0 } @@ -47,11 +47,6 @@ const loginFrames = (transport: FakeWebSocket) => .map((frame) => JSON.parse(frame)) .filter((frame) => frame.msg === 'method' && frame.method === 'login') -/** - * A Reopen builds a new connection with the same DDP session gone, so the - * Login has to be made again on it. The token the Socket holds is the only - * thing that makes that possible, and it is not itself proof of a Login. - */ describe('Resume on reopen', () => { let logger: ILogger @@ -60,13 +55,13 @@ describe('Resume on reopen', () => { }) it('sends a login method call on the reopened connection', async () => { - const socket = createSocket(logger, resumeToken) + const socket = createSocket(logger, loginResult) const transport = await openFakeConnection(socket) const reopened = await reopenAfterDrop(transport) expect(loginFrames(reopened)).toHaveLength(1) - expect(loginFrames(reopened)[0].params).toEqual([{ resume: resumeToken.token }]) + expect(loginFrames(reopened)[0].params).toEqual([{ resume: loginResult.token }]) }) it('sends no login method call with no token held', async () => { @@ -79,7 +74,7 @@ describe('Resume on reopen', () => { }) it('resolves the open and emits open without waiting on the login response', async () => { - const socket = createSocket(logger, resumeToken) + const socket = createSocket(logger, loginResult) const opened = jest.fn() socket.on('open', opened) @@ -92,15 +87,15 @@ describe('Resume on reopen', () => { expect(loginFrames(transport)).toHaveLength(1) }) - it('reports logged in only once the relogin has its result', async () => { - const socket = createSocket(logger, resumeToken) + it('reports logged in only once the Resume has its result', async () => { + const socket = createSocket(logger, loginResult) const transport = await openFakeConnection(socket) const reopened = await reopenAfterDrop(transport) expect(socket.loggedIn).toBe(false) - reopened.receive({ msg: 'result', id: loginFrames(reopened)[0].id, result: resumeToken }) + reopened.receive({ msg: 'result', id: loginFrames(reopened)[0].id, result: loginResult }) await flushMicrotasks() expect(socket.loggedIn).toBe(true)