diff --git a/.changeset/quiet-singers-pick.md b/.changeset/quiet-singers-pick.md new file mode 100644 index 0000000000..cc66db1bcd --- /dev/null +++ b/.changeset/quiet-singers-pick.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-signals': patch +--- + +Fixes a bug where the `SignalClient` would try to subscribe to the same channel twice after an error, instead of just once. diff --git a/plugins/signals/package.json b/plugins/signals/package.json index 1699786785..74ce452495 100644 --- a/plugins/signals/package.json +++ b/plugins/signals/package.json @@ -71,7 +71,8 @@ "jest-websocket-mock": "^2.5.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 1535b4d5e1..b93e8b7011 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7421,6 +7421,7 @@ __metadata: react-dom: "npm:^18.0.2" react-router-dom: "npm:^6.3.0" 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