Forráskód Böngészése

feat: return ready state from socket hook and attempt reconnection on close

Fela Maslen 4 éve
szülő
commit
4bb8794515

+ 75 - 13
gmus-web/src/hooks/socket.spec.tsx

@@ -1,4 +1,4 @@
-import { act, fireEvent, render, RenderResult } from '@testing-library/react';
+import { act, fireEvent, render, RenderResult, waitFor } from '@testing-library/react';
 import WS from 'jest-websocket-mock';
 import React, { Dispatch } from 'react';
 import * as storageHooks from 'react-storage-hooks';
@@ -167,9 +167,10 @@ describe(useSocket.name, () => {
   it.each`
     testCase                   | key             | expectedValue
     ${'the name'}              | ${'name'}       | ${''}
-    ${'the error status'}      | ${'error'}      | ${false}
     ${'the connecting status'} | ${'connecting'} | ${false}
-    ${'the connected status'}  | ${'connected'}  | ${false}
+    ${'the error status'}      | ${'error'}      | ${false}
+    ${'the ready status'}      | ${'ready'}      | ${false}
+    ${'the identified status'} | ${'identified'} | ${false}
   `('should return $testCase', ({ key, expectedValue }) => {
     expect.assertions(1);
     const { getByTestId } = render(<TestComponent />);
@@ -223,9 +224,19 @@ describe(useSocket.name, () => {
       expect(server).toHaveReceivedMessages(['Hello world!']);
     });
 
-    it('should set the connected state to true', async () => {
-      expect.assertions(1);
+    it('should set the connecting, ready and identified state', async () => {
+      expect.assertions(2);
       const { getByTestId } = setupIdentify();
+
+      expect(JSON.parse(getByTestId('hook-result').innerHTML)).toStrictEqual(
+        expect.objectContaining({
+          connecting: true,
+          error: false,
+          ready: false,
+          identified: true,
+        }),
+      );
+
       await act(async () => {
         await server.connected;
       });
@@ -233,7 +244,9 @@ describe(useSocket.name, () => {
       expect(JSON.parse(getByTestId('hook-result').innerHTML)).toStrictEqual(
         expect.objectContaining({
           connecting: false,
-          connected: true,
+          error: false,
+          ready: true,
+          identified: true,
         }),
       );
     });
@@ -285,14 +298,16 @@ describe(useSocket.name, () => {
         .mockReturnValue(['my-stored-name' as unknown, saveName, undefined]);
     });
 
-    it('should set connecting to true', () => {
+    it('should set the status', () => {
       expect.assertions(1);
       const { getByTestId } = render(<TestComponent />);
 
       expect(JSON.parse(getByTestId('hook-result').innerHTML)).toStrictEqual(
         expect.objectContaining({
           connecting: true,
-          connected: false,
+          error: false,
+          identified: true,
+          ready: false,
         }),
       );
     });
@@ -322,7 +337,9 @@ describe(useSocket.name, () => {
       expect(JSON.parse(getByTestId('hook-result').innerHTML)).toStrictEqual(
         expect.objectContaining({
           connecting: false,
-          connected: true,
+          error: false,
+          identified: true,
+          ready: true,
         }),
       );
     });
@@ -356,11 +373,10 @@ describe(useSocket.name, () => {
       server = new WS('ws://my-api.url:1234/pubsub');
     });
 
-    it('should reconnect automatically', async () => {
-      expect.assertions(1);
-      const { getByText } = render(<TestComponent />);
+    const setupError = async (): Promise<RenderResult> => {
+      const utils = render(<TestComponent />);
       act(() => {
-        fireEvent.click(getByText('Identify!'));
+        fireEvent.click(utils.getByText('Identify!'));
       });
 
       await server.connected;
@@ -368,13 +384,59 @@ describe(useSocket.name, () => {
       act(() => {
         server.error();
       });
+
       server = new WS('ws://my-api.url:1234/pubsub');
 
+      return utils;
+    };
+
+    it('should reconnect automatically', async () => {
+      expect.assertions(1);
+      await setupError();
+
       await server.connected;
 
       server.send('foo');
 
       expect(onMessage).toHaveBeenCalledTimes(1);
     });
+
+    it('should set error to true but keep the identified state', async () => {
+      expect.hasAssertions();
+      const { getByTestId } = await setupError();
+
+      expect(JSON.parse(getByTestId('hook-result').innerHTML)).toStrictEqual(
+        expect.objectContaining({
+          ready: false,
+          error: true,
+          connecting: false,
+          identified: true,
+        }),
+      );
+
+      await waitFor(() => {
+        expect(JSON.parse(getByTestId('hook-result').innerHTML)).toStrictEqual(
+          expect.objectContaining({
+            ready: false,
+            error: true,
+            connecting: true,
+            identified: true,
+          }),
+        );
+      });
+
+      await server.connected;
+
+      await waitFor(() => {
+        expect(JSON.parse(getByTestId('hook-result').innerHTML)).toStrictEqual(
+          expect.objectContaining({
+            ready: true,
+            error: false,
+            connecting: false,
+            identified: true,
+          }),
+        );
+      });
+    });
   });
 });

+ 36 - 27
gmus-web/src/hooks/socket.ts

@@ -7,6 +7,7 @@ import { errorOccurred } from '../actions/error';
 import { socketKeepaliveTimeout } from '../constants/system';
 import { globalEffects } from '../effects';
 import { GlobalState } from '../reducer';
+import { noop } from '../utils/noop';
 import { getPubsubUrl } from '../utils/url';
 
 const getUniqueName = (name: string): string => (name.length ? `${name}-${nanoid(5)}` : '');
@@ -73,17 +74,17 @@ type SocketState = {
   connectAttemptNumber: number;
 };
 
-export function useSocket(
-  onMessage: OnMessage,
-  onLogin: (name: string) => void,
-): {
+type SocketHookResult = {
   name: string | null;
+  identified: boolean;
   onIdentify: (name: string) => void;
-  socket: WebSocket | null;
-  error: boolean;
   connecting: boolean;
-  connected: boolean;
-} {
+  error: boolean;
+  ready: boolean;
+  socket: WebSocket | null;
+};
+
+export function useSocket(onMessage: OnMessage, onLogin: (name: string) => void): SocketHookResult {
   const [storedName, saveName] = useStorageState<string>(localStorage, clientNameKey, '');
   const [{ uniqueName, tempName, socket, error, connecting }, setState] = useState<SocketState>({
     uniqueName: getUniqueName(storedName),
@@ -97,17 +98,15 @@ export function useSocket(
   const connectAttemptTimer = useRef<number>();
 
   useEffect(() => {
-    if (!tempName) {
-      setState((last) => ({ ...last, connecting: false, error: false }));
-      return (): void => {
-        // pass
-      };
-    }
-
     let cancelled = false;
     let ws: WebSocket | undefined;
 
-    const connect = (): void => {
+    const connectIfPossible = (): void => {
+      if (!tempName) {
+        setState((last) => ({ ...last, connecting: false, error: false }));
+        return;
+      }
+
       setState((last) => ({ ...last, connecting: true }));
 
       const uniqueTempName = getUniqueName(tempName);
@@ -132,23 +131,28 @@ export function useSocket(
       ws.onmessage = onMessage;
 
       ws.onclose = (): void => {
-        setState((last) => ({ ...last, error: false, socket: null }));
-      };
-
-      ws.onerror = (): void => {
         setState((last) => {
           clearTimeout(connectAttemptTimer.current);
           connectAttemptTimer.current = setTimeout(
-            connect,
+            connectIfPossible,
             getConnectAttemptDelayMs(last.connectAttemptNumber),
           );
 
-          return { ...last, error: true, connectAttemptNumber: last.connectAttemptNumber + 1 };
+          return {
+            ...last,
+            socket: null,
+            connecting: false,
+            connectAttemptNumber: last.connectAttemptNumber + 1,
+          };
         });
       };
+
+      ws.onerror = (): void => {
+        setState((last) => ({ ...last, error: true }));
+      };
     };
 
-    connect();
+    connectIfPossible();
 
     return (): void => {
       clearTimeout(connectAttemptTimer.current);
@@ -163,18 +167,23 @@ export function useSocket(
 
   return {
     name: uniqueName,
+    identified: !!tempName,
     onIdentify,
-    socket,
-    error,
     connecting,
-    connected: !!socket && socket?.readyState === socket?.OPEN,
+    error,
+    ready: !!(socket && socket.readyState === socket.OPEN),
+    socket,
   };
 }
 
-export function useKeepalive(socket: WebSocket): void {
+export function useKeepalive(socket: WebSocket | null): void {
   const keepalive = useRef<number>();
 
   useEffect(() => {
+    if (!socket) {
+      return noop;
+    }
+
     keepalive.current = window.setInterval(() => {
       if (socket.readyState === socket.OPEN) {
         socket.send(JSON.stringify({ type: 'PING' }));

+ 3 - 0
gmus-web/src/utils/noop.ts

@@ -0,0 +1,3 @@
+export const noop = (): void => {
+  /* pass */
+};