signals: fix subscribing twice on error

Signed-off-by: Vincenzo Scamporlino <vincenzos@spotify.com>

plugins/signals/package.json

@@ -77,7 +77,8 @@
     "msw": "^1.0.0",
     "react": "^18.0.2",
     "react-dom": "^18.0.2",
-    "react-router-dom": "^6.3.0"
+    "react-router-dom": "^6.3.0",
+    "wait-for-expect": "^3.0.2"
   },
   "peerDependencies": {
     "@types/react": "^17.0.0 || ^18.0.0",

plugins/signals/src/api/SignalClient.ts

@@ -176,7 +176,10 @@ export class SignalClient implements SignalApi {
     };
 
     this.ws.onerror = () => {
-      this.reconnect();
+      if (this.ws) {
+        this.ws.close();
+      }
+      this.ws = null;
     };
 
     this.ws.onclose = (ev: CloseEvent) => {

plugins/signals/src/api/SignalsClient.test.ts

@@ -17,8 +17,9 @@
 import { mockApis } from '@backstage/test-utils';
 import WS from 'jest-websocket-mock';
 import { SignalClient } from './SignalClient';
+import waitForExpect from 'wait-for-expect';
 
-describe('SignalsClient', () => {
+describe('SignalClient', () => {
   const identity = mockApis.identity({ token: '12345' });
   const discoveryApi = mockApis.discovery({ baseUrl: 'http://localhost:1234' });
 
@@ -72,25 +73,51 @@ describe('SignalsClient', () => {
 
     await server.connected;
 
-    await expect(server).toReceiveMessage({
-      action: 'subscribe',
-      channel: 'channel',
-    });
+    await waitForExpect(() =>
+      expect(server).toHaveReceivedMessages([
+        {
+          action: 'subscribe',
+          channel: 'channel',
+        },
+        {
+          action: 'subscribe',
+          channel: 'channel',
+        },
+      ]),
+    );
     server.send({ channel: 'channel', message: { hello: 'world' } });
     expect(messageMock1).toHaveBeenCalledWith({ hello: 'world' });
     expect(messageMock2).toHaveBeenCalledWith({ hello: 'world' });
 
     await unsubscribe1();
-    await expect(server).not.toReceiveMessage({
-      action: 'unsubscribe',
-      channel: 'channel',
-    });
+    await waitForExpect(() =>
+      expect(server).toReceiveMessage({
+        action: 'unsubscribe',
+        channel: 'channel',
+      }),
+    );
 
     await unsubscribe2();
-    await expect(server).toReceiveMessage({
-      action: 'unsubscribe',
-      channel: 'channel',
-    });
+    await waitForExpect(() =>
+      expect(server.messages).toEqual([
+        {
+          action: 'subscribe',
+          channel: 'channel',
+        },
+        {
+          action: 'subscribe',
+          channel: 'channel',
+        },
+        {
+          action: 'unsubscribe',
+          channel: 'channel',
+        },
+        {
+          action: 'unsubscribe',
+          channel: 'channel',
+        },
+      ]),
+    );
   });
 
   it('should reconnect on error', async () => {
@@ -111,10 +138,11 @@ describe('SignalsClient', () => {
 
     await server.server.emit('error', null);
 
-    await new Promise(r => setTimeout(r, 50));
-    await expect(server).toReceiveMessage({
-      action: 'subscribe',
-      channel: 'channel',
-    });
+    await waitForExpect(() =>
+      expect(server.messages).toEqual([
+        { action: 'subscribe', channel: 'channel' },
+        { action: 'subscribe', channel: 'channel' },
+      ]),
+    );
   });
 });

yarn.lock

@@ -7442,6 +7442,7 @@ __metadata:
     react-router-dom: "npm:^6.3.0"
     react-use: "npm:^17.2.4"
     uuid: "npm:^11.0.0"
+    wait-for-expect: "npm:^3.0.2"
   peerDependencies:
     "@types/react": ^17.0.0 || ^18.0.0
     react: ^17.0.0 || ^18.0.0