From fc748dd2d3e1c3293b82ac52e5a020831e21e8b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johannes=20W=C3=BCrbach?= Date: Thu, 8 Feb 2024 21:35:46 +0100 Subject: [PATCH] fix(humanitec-backend): handle disconnects while fetching --- .changeset/strong-kings-pay.md | 5 +++ .../src/service/app-info-service.test.ts | 32 +++++++++++++++-- .../src/service/app-info-service.ts | 34 +++++++++---------- 3 files changed, 51 insertions(+), 20 deletions(-) create mode 100644 .changeset/strong-kings-pay.md diff --git a/.changeset/strong-kings-pay.md b/.changeset/strong-kings-pay.md new file mode 100644 index 0000000000..3622ff0725 --- /dev/null +++ b/.changeset/strong-kings-pay.md @@ -0,0 +1,5 @@ +--- +'@frontside/backstage-plugin-humanitec-backend': patch +--- + +Handle disconnects while fetching app info diff --git a/plugins/humanitec-backend/src/service/app-info-service.test.ts b/plugins/humanitec-backend/src/service/app-info-service.test.ts index 3aae45f611..37263e95f3 100644 --- a/plugins/humanitec-backend/src/service/app-info-service.test.ts +++ b/plugins/humanitec-backend/src/service/app-info-service.test.ts @@ -4,8 +4,10 @@ import * as common from '@frontside/backstage-plugin-humanitec-common'; import { AppInfoService } from './app-info-service'; const fetchInterval = 50; +const slowFetchTimeout = 100; let returnError = false; +let slowFetch = false const fakeAppInfo = { fake: 'res' } const fakeError = new Error('fake error'); @@ -16,6 +18,10 @@ jest.mock('@frontside/backstage-plugin-humanitec-common', () => ({ throw fakeError; } + if (slowFetch) { + await setTimeout(100); + } + return fakeAppInfo; }), })) @@ -24,6 +30,7 @@ describe('AppInfoService', () => { afterEach(() => { jest.clearAllMocks(); returnError = false; + slowFetch = false; }); it('single subscriber', async () => { @@ -52,7 +59,7 @@ describe('AppInfoService', () => { expect(common.createHumanitecClient).toHaveBeenCalledTimes(2); }); - it('single subscriber, recovers after an erro', async () => { + it('single subscriber, recovers after an error', async () => { returnError = true const service = new AppInfoService('token', fetchInterval); @@ -82,6 +89,27 @@ describe('AppInfoService', () => { expect(common.createHumanitecClient).toHaveBeenCalledTimes(2); }); + it('single subscriber, disconnects with slow fetch', async () => { + slowFetch = true + + const service = new AppInfoService('token', fetchInterval); + const subscriber = jest.fn(); + + const close = service.addSubscriber('orgId', 'appId', subscriber); + + await setTimeout(slowFetchTimeout / 2); + + expect(subscriber).toHaveBeenCalledTimes(0); + expect(common.createHumanitecClient).toHaveBeenCalledTimes(1); + + close(); + + // Wait for two cycles to ensure that the fetch is not retried. + await setTimeout((slowFetchTimeout + fetchInterval) * 2); + + expect(common.createHumanitecClient).toHaveBeenCalledTimes(1); + }); + it('two subscribers', async () => { const service = new AppInfoService('token', fetchInterval); const subscriber1 = jest.fn(); @@ -90,7 +118,7 @@ describe('AppInfoService', () => { const close1 = service.addSubscriber('orgId', 'appId', subscriber1); const close2 = service.addSubscriber('orgId', 'appId', subscriber2); - await setTimeout(10); + await setTimeout(fetchInterval); expect(subscriber1).toHaveBeenCalledTimes(1); expect(subscriber2).toHaveBeenCalledTimes(1); diff --git a/plugins/humanitec-backend/src/service/app-info-service.ts b/plugins/humanitec-backend/src/service/app-info-service.ts index 4ec3b64edf..8882a24146 100644 --- a/plugins/humanitec-backend/src/service/app-info-service.ts +++ b/plugins/humanitec-backend/src/service/app-info-service.ts @@ -16,7 +16,6 @@ export interface AppInfoUpdate { // export class AppInfoService { private emitter: EventEmitter = new EventEmitter(); - private pending: Map> = new Map(); private timeouts: Map = new Map(); private lastData: Map = new Map(); @@ -34,8 +33,8 @@ export class AppInfoService { this.emitter.on(key, subscriber); // Only fetch app info if a fetch is not pending. - if (!this.pending.has(key)) { - this.fetchAppInfo(orgId, appId); + if (!this.timeouts.has(key)) { + this.timeouts.set(key, setTimeout(() => this.fetchAppInfo(orgId, appId), 1)); } else { if (this.lastData.has(key)) { subscriber(this.lastData.get(key)!); @@ -48,34 +47,33 @@ export class AppInfoService { if (this.emitter.listenerCount(key) === 0 && this.timeouts.has(key)) { clearTimeout(this.timeouts.get(key)!); this.timeouts.delete(key); - this.pending.delete(key); this.lastData.delete(key); } }; } - private fetchAppInfo(orgId: string, appId: string): void { + private async fetchAppInfo(orgId: string, appId: string): Promise { const key = `${orgId}:${appId}`; const client = createHumanitecClient({ token: this.token, orgId }); const id = this.lastData.has(key) ? this.lastData.get(key)!.id + 1 : 0; - this.pending.set(key, (async () => { - const update: AppInfoUpdate = { id: id }; - try { - const data = await fetchAppInfo({ client }, appId); - update.data = data; - } catch (error) { - if (error instanceof Error) { - update.error = error; - } else { - update.error = new Error(`${error}`); - } - } finally { + const update: AppInfoUpdate = { id: id }; + try { + const data = await fetchAppInfo({ client }, appId); + update.data = data; + } catch (error) { + if (error instanceof Error) { + update.error = error; + } else { + update.error = new Error(`${error}`); + } + } finally { + if (this.emitter.listenerCount(key) > 0) { this.timeouts.set(key, setTimeout(() => this.fetchAppInfo(orgId, appId), this.fetchInterval)); this.lastData.set(key, update); this.emitter.emit(key, update); } - })()); + } } }