diff --git a/app/lib/services/ddpSocket.test.ts b/app/lib/services/ddpSocket.test.ts index 44acd6e732..53b424b90c 100644 --- a/app/lib/services/ddpSocket.test.ts +++ b/app/lib/services/ddpSocket.test.ts @@ -206,6 +206,85 @@ describe('Socket.reopenNow', () => { await secondPromise; }); + it('re-arms the reconnect ladder when a forced reopen never opens', async () => { + jest.useFakeTimers(); + const socket = trackSocket( + new Socket({ + logger: { debug: jest.fn(), info: jest.fn(), error: jest.fn(), warn: jest.fn() }, + timeout: 10000 + }) + ); + + const promise = socket.reopenNow(); + expect(mockConnections).toHaveLength(1); + + // The forced reopen never opens — let its own deadline expire. + await jest.advanceTimersByTimeAsync(10000); + await promise; + + // `reopenNow` cancelled the ladder to attempt an immediate reconnect, and that + // attempt failed. The ladder has to be back, or nothing retries and the session + // waits for the next foreground: one `reopen` interval later it tries again. + await jest.advanceTimersByTimeAsync(10000); + + expect(mockConnections).toHaveLength(2); + }); + + it('re-arms the ladder when a ladder tick was already scheduled before the forced reopen', async () => { + jest.useFakeTimers(); + const socket = trackSocket( + new Socket({ + logger: { debug: jest.fn(), info: jest.fn(), error: jest.fn(), warn: jest.fn() }, + timeout: 10000 + }) + ); + + // The real precondition: onClose already armed the ladder before anything forced a + // reopen. reopenNow cancels that tick, so it must not leave the field pointing at it. + socket.reopen(); + expect(socket.openTimeout).toBeTruthy(); + + const promise = socket.reopenNow(); + await jest.advanceTimersByTimeAsync(10000); + await promise; + + await jest.advanceTimersByTimeAsync(socket.config.reopen); + + expect(mockConnections).toHaveLength(2); + }); + + it('re-arms the ladder as soon as the attempt errors, without waiting for the deadline', async () => { + jest.useFakeTimers(); + const socket = trackSocket( + new Socket({ + logger: { debug: jest.fn(), info: jest.fn(), error: jest.fn(), warn: jest.fn() }, + timeout: 10000 + }) + ); + + const promise = socket.reopenNow(); + // How a WebSocket reports a failed connect: onerror is createConnection's reject. + mockConnections[0].onerror(new Error('offline')); + await promise; + + expect(socket.reopenPromise).toBeUndefined(); + + await jest.advanceTimersByTimeAsync(10000); + + expect(mockConnections).toHaveLength(2); + }); + + it('schedules no ladder tick when the forced reopen opens', async () => { + const { socket } = buildSocket(); + + const promise = socket.reopenNow(); + mockConnections[0].onopen(); + await promise; + + // A reopen that produced a live socket must not leave a redundant tick behind. + expect(socket.openTimeout).toBeUndefined(); + }); + it('forces a reconnect on an already healthy socket', async () => { const { socket } = buildSocket(); const initialConnection = socket.connection; diff --git a/patches/@rocket.chat+sdk+1.3.3-mobile.patch b/patches/@rocket.chat+sdk+1.3.3-mobile.patch index e0e2d0b446..fb5cc5b976 100644 --- a/patches/@rocket.chat+sdk+1.3.3-mobile.patch +++ b/patches/@rocket.chat+sdk+1.3.3-mobile.patch @@ -104,7 +104,7 @@ index 19d31ae..068b61e 100644 this.emit('close', e) try { if (e?.code !== userDisconnectCloseCode) { -@@ -201,6 +243,85 @@ export class Socket extends EventEmitter { +@@ -201,6 +243,98 @@ export class Socket extends EventEmitter { }, this.config.reopen); } @@ -112,8 +112,7 @@ index 19d31ae..068b61e 100644 + * Force an immediate reconnect. Shared across concurrent callers so only one + * new WebSocket is created. Emits 'disconnected' to unblock in-flight sends, + * then creates the connection directly so a concurrent open() cannot tear it -+ * down. Unhandled creation errors are swallowed because cleanup already runs -+ * via the open/timeout paths. ++ * down. A reopen that fails to open re-arms the retry ladder it cancelled. + */ + reopenNow = (): Promise => { + if (this.reopenPromise) { @@ -121,25 +120,39 @@ index 19d31ae..068b61e 100644 + } + + this.reopenPromise = new Promise(resolve => { -+ this.openTimeout && clearTimeout(this.openTimeout as any) ++ if (this.openTimeout) { ++ clearTimeout(this.openTimeout as any) ++ // `reopen()` treats a set `openTimeout` as "tick already scheduled", so the id has to ++ // go too or the ladder stays disabled for the rest of the session. ++ delete this.openTimeout ++ } + this.lastPing = 0 + this.emit('disconnected') + + let settled = false -+ const cleanup = () => { ++ let timeout: NodeJS.Timer | number | undefined ++ ++ const cleanup = (opened: boolean) => { + if (settled) return + settled = true -+ this.off('open', cleanup) ++ this.off('open', onOpen) + if (timeout) clearTimeout(timeout as any) ++ // Drop before re-arming, or a ladder tick's open() short-circuits onto a settled reopen. + delete this.reopenPromise ++ // A failed attempt leaves nothing scheduled, so hand retrying back to the ladder. ++ if (!opened && !this.connected) { ++ this.reopen() ++ } + resolve() + } + -+ this.once('open', cleanup) ++ const onOpen = () => cleanup(true) ++ ++ this.once('open', onOpen) + -+ this.createConnection().catch(() => {}) ++ this.createConnection().catch(() => cleanup(false)) + -+ const timeout = setTimeout(() => cleanup(), 10000) ++ timeout = setTimeout(() => cleanup(false), 10000) + }) + + return this.reopenPromise