refactor(experimental): teach the auto-pinger *not* to ping the conne…

…ction when you're offline (#1604) # 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] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
solana-labs · Sep 19, 2023 · 2a70690 · 2a70690
1 parent a8f1f88
commit 2a70690
Show file tree

Hide file tree

Showing 2 changed files with 79 additions and 4 deletions.
diff --git 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
@@ -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)!;
     };