From 2a706906e6fc9d7a06b674956f2f19cc92a4f2af Mon Sep 17 00:00:00 2001 From: Steven Luscher Date: Mon, 18 Sep 2023 20:02:07 -0700 Subject: [PATCH] refactor(experimental): teach the auto-pinger *not* to ping the connection when you're offline (#1604) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Summary There is zero point to slamming ping messages down a socket if the server can't hear them. At most we should send a ping the moment we come back online, then resume as normal. # Test Plan ``` cd packages/library pnpm test:unit:browser pnpm test:unit:node ``` 1. Paste [this](https://gist.github.com/steveluscher/4254aba293222daded59bdceafbdaa4c) code into a Chrome console. 2. Toggle the network between ‘no throttling’ and ‘offline’ 3. Observe the ping behavior, as described in the unit tests --------- Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .../rpc-websocket-autopinger-test.ts | 53 +++++++++++++++++++ .../library/src/rpc-websocket-autopinger.ts | 30 +++++++++-- 2 files changed, 79 insertions(+), 4 deletions(-) diff --git a/packages/library/src/__tests__/rpc-websocket-autopinger-test.ts b/packages/library/src/__tests__/rpc-websocket-autopinger-test.ts index af55948550f2..d1dde4a2855d 100644 --- a/packages/library/src/__tests__/rpc-websocket-autopinger-test.ts +++ b/packages/library/src/__tests__/rpc-websocket-autopinger-test.ts @@ -142,4 +142,57 @@ describe('getWebSocketTransportWithAutoping', () => { jest.advanceTimersByTime(MOCK_INTERVAL_MS); expect(send).not.toHaveBeenCalled(); }); + if (__BROWSER__) { + it('stops pinging the connection when it goes offline', async () => { + expect.assertions(1); + await transport({ payload: 'hi', signal: new AbortController().signal }); + globalThis.window.dispatchEvent(new Event('offline')); + jest.advanceTimersByTime(MOCK_INTERVAL_MS); + expect(send).not.toHaveBeenCalled(); + }); + describe('when the network connection is offline to start', () => { + beforeEach(() => { + const originalNavigator = globalThis.navigator; + jest.spyOn(globalThis, 'navigator', 'get').mockImplementation(() => ({ + ...originalNavigator, + onLine: false, + })); + }); + it('does not ping the connection', async () => { + expect.assertions(1); + await transport({ payload: 'hi', signal: new AbortController().signal }); + jest.advanceTimersByTime(MOCK_INTERVAL_MS); + expect(send).not.toHaveBeenCalled(); + }); + it('pings the connection immediately when the connection comes back online', async () => { + expect.assertions(1); + await transport({ payload: 'hi', signal: new AbortController().signal }); + jest.advanceTimersByTime(500); + globalThis.window.dispatchEvent(new Event('online')); + expect(send).toHaveBeenCalledWith( + expect.objectContaining({ + jsonrpc: '2.0', + method: 'ping', + }) + ); + }); + it('pings the connection interval milliseconds after the connection comes back online', async () => { + expect.assertions(3); + await transport({ payload: 'hi', signal: new AbortController().signal }); + jest.advanceTimersByTime(500); + globalThis.window.dispatchEvent(new Event('online')); + send.mockClear(); + expect(send).not.toHaveBeenCalled(); + jest.advanceTimersByTime(MOCK_INTERVAL_MS - 1); + expect(send).not.toHaveBeenCalled(); + jest.advanceTimersByTime(1); + expect(send).toHaveBeenCalledWith( + expect.objectContaining({ + jsonrpc: '2.0', + method: 'ping', + }) + ); + }); + }); + } }); diff --git a/packages/library/src/rpc-websocket-autopinger.ts b/packages/library/src/rpc-websocket-autopinger.ts index 9a8f1e78c26e..4b25cb52a145 100644 --- a/packages/library/src/rpc-websocket-autopinger.ts +++ b/packages/library/src/rpc-websocket-autopinger.ts @@ -18,11 +18,12 @@ export function getWebSocketTransportWithAutoping({ intervalMs, transport }: Con return async (...args) => { const connection = await transport(...args); let intervalId: string | number | NodeJS.Timeout | undefined; + function sendPing() { + connection.send_DO_NOT_USE_OR_YOU_WILL_BE_FIRED(PING_PAYLOAD); + } function restartPingTimer() { clearInterval(intervalId); - intervalId = setInterval(() => { - connection.send_DO_NOT_USE_OR_YOU_WILL_BE_FIRED(PING_PAYLOAD); - }, intervalMs); + intervalId = setInterval(sendPing, intervalMs); } if (pingableConnections.has(connection) === false) { pingableConnections.set(connection, { @@ -45,9 +46,30 @@ export function getWebSocketTransportWithAutoping({ intervalMs, transport }: Con } finally { pingableConnections.delete(connection); clearInterval(intervalId); + if (handleOffline) { + globalThis.window.removeEventListener('offline', handleOffline); + } + if (handleOnline) { + globalThis.window.removeEventListener('online', handleOnline); + } } })(); - restartPingTimer(); + if (!__BROWSER__ || globalThis.navigator.onLine) { + restartPingTimer(); + } + let handleOffline; + let handleOnline; + if (__BROWSER__) { + handleOffline = () => { + clearInterval(intervalId); + }; + handleOnline = () => { + sendPing(); + restartPingTimer(); + }; + globalThis.window.addEventListener('offline', handleOffline); + globalThis.window.addEventListener('online', handleOnline); + } } return pingableConnections.get(connection)!; };