From 3be550688573f277fa36c0439028a876110f9240 Mon Sep 17 00:00:00 2001
From: Vincenzo Scamporlino <vincenzos@spotify.com>
Date: Wed, 22 Oct 2025 20:47:35 +0200
Subject: [PATCH] signals: fix subscribing twice on error

Signed-off-by: Vincenzo Scamporlino <vincenzos@spotify.com>
---
 plugins/signals/package.json                  |  3 +-
 plugins/signals/src/api/SignalClient.ts       |  5 +-
 plugins/signals/src/api/SignalsClient.test.ts | 64 +++++++++++++------
 yarn.lock                                     |  1 +
 4 files changed, 53 insertions(+), 20 deletions(-)

diff --git a/plugins/signals/package.json b/plugins/signals/package.json
index 9069a64387..89e5c11bc0 100644
--- a/plugins/signals/package.json
+++ b/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",
diff --git a/plugins/signals/src/api/SignalClient.ts b/plugins/signals/src/api/SignalClient.ts
index 15948236c5..5a4362fdfd 100644
--- a/plugins/signals/src/api/SignalClient.ts
+++ b/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) => {
diff --git a/plugins/signals/src/api/SignalsClient.test.ts b/plugins/signals/src/api/SignalsClient.test.ts
index b6944cfdc1..04b0e0c437 100644
--- a/plugins/signals/src/api/SignalsClient.test.ts
+++ b/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' },
+      ]),
+    );
   });
 });
diff --git a/yarn.lock b/yarn.lock
index 04970afcde..2087c4aa58 100644
--- a/yarn.lock
+++ b/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