From b9e57e3311bd5036e784586fa646ad39f682040d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johannes=20W=C3=BCrbach?= Date: Thu, 1 Feb 2024 10:14:31 +0100 Subject: [PATCH] fix(humanitec): continue polling on error --- .changeset/green-coats-march.md | 5 + .../src/service/app-info-service.test.ts | 126 ++++++++++++++++++ .../src/service/app-info-service.ts | 45 ++++--- .../humanitec-backend/src/service/router.ts | 8 +- 4 files changed, 156 insertions(+), 28 deletions(-) create mode 100644 .changeset/green-coats-march.md create mode 100644 plugins/humanitec-backend/src/service/app-info-service.test.ts diff --git a/.changeset/green-coats-march.md b/.changeset/green-coats-march.md new file mode 100644 index 0000000000..d1a5eff712 --- /dev/null +++ b/.changeset/green-coats-march.md @@ -0,0 +1,5 @@ +--- +'@frontside/backstage-plugin-humanitec-backend': patch +--- + +Continue polling when an error is returned. diff --git a/plugins/humanitec-backend/src/service/app-info-service.test.ts b/plugins/humanitec-backend/src/service/app-info-service.test.ts new file mode 100644 index 0000000000..3aae45f611 --- /dev/null +++ b/plugins/humanitec-backend/src/service/app-info-service.test.ts @@ -0,0 +1,126 @@ +import { setTimeout } from 'node:timers/promises'; +import * as common from '@frontside/backstage-plugin-humanitec-common'; + +import { AppInfoService } from './app-info-service'; + +const fetchInterval = 50; + +let returnError = false; +const fakeAppInfo = { fake: 'res' } +const fakeError = new Error('fake error'); + +jest.mock('@frontside/backstage-plugin-humanitec-common', () => ({ + createHumanitecClient: jest.fn(), + fetchAppInfo: jest.fn(async () => { + if (returnError) { + throw fakeError; + } + + return fakeAppInfo; + }), +})) + +describe('AppInfoService', () => { + afterEach(() => { + jest.clearAllMocks(); + returnError = false; + }); + + it('single subscriber', async () => { + const service = new AppInfoService('token', fetchInterval); + const subscriber = jest.fn(); + + const close = service.addSubscriber('orgId', 'appId', subscriber); + + await setTimeout(50); + + expect(subscriber).toHaveBeenCalledTimes(1); + expect(subscriber).toHaveBeenLastCalledWith({ id: 0, data: fakeAppInfo }); + expect(common.createHumanitecClient).toHaveBeenCalledTimes(1); + + await setTimeout(fetchInterval); + + expect(subscriber).toHaveBeenCalledTimes(2); + expect(subscriber).toHaveBeenLastCalledWith({ id: 1, data: fakeAppInfo }); + expect(common.createHumanitecClient).toHaveBeenCalledTimes(2); + + close(); + + await setTimeout(fetchInterval * 2); + + expect(subscriber).toHaveBeenCalledTimes(2); + expect(common.createHumanitecClient).toHaveBeenCalledTimes(2); + }); + + it('single subscriber, recovers after an erro', async () => { + returnError = true + + const service = new AppInfoService('token', fetchInterval); + const subscriber = jest.fn(); + + const close = service.addSubscriber('orgId', 'appId', subscriber); + + await setTimeout(50); + + expect(subscriber).toHaveBeenCalledTimes(1); + expect(subscriber).toHaveBeenLastCalledWith({ id: 0, error: fakeError }); + expect(common.createHumanitecClient).toHaveBeenCalledTimes(1); + + returnError = false; + + await setTimeout(fetchInterval); + + expect(subscriber).toHaveBeenCalledTimes(2); + expect(subscriber).toHaveBeenLastCalledWith({ id: 1, data: fakeAppInfo }); + expect(common.createHumanitecClient).toHaveBeenCalledTimes(2); + + close(); + + await setTimeout(fetchInterval * 2); + + expect(subscriber).toHaveBeenCalledTimes(2); + expect(common.createHumanitecClient).toHaveBeenCalledTimes(2); + }); + + it('two subscribers', async () => { + const service = new AppInfoService('token', fetchInterval); + const subscriber1 = jest.fn(); + const subscriber2 = jest.fn(); + + const close1 = service.addSubscriber('orgId', 'appId', subscriber1); + const close2 = service.addSubscriber('orgId', 'appId', subscriber2); + + await setTimeout(10); + + expect(subscriber1).toHaveBeenCalledTimes(1); + expect(subscriber2).toHaveBeenCalledTimes(1); + expect(subscriber1).toHaveBeenLastCalledWith({ id: 0, data: fakeAppInfo }); + expect(subscriber2).toHaveBeenLastCalledWith({ id: 0, data: fakeAppInfo }); + expect(common.createHumanitecClient).toHaveBeenCalledTimes(1); + + await setTimeout(fetchInterval); + + expect(subscriber1).toHaveBeenCalledTimes(2); + expect(subscriber1).toHaveBeenLastCalledWith({ id: 1, data: fakeAppInfo }); + expect(subscriber2).toHaveBeenCalledTimes(2); + expect(subscriber2).toHaveBeenLastCalledWith({ id: 1, data: fakeAppInfo }); + expect(common.createHumanitecClient).toHaveBeenCalledTimes(2); + + close1(); + + await setTimeout(fetchInterval); + + expect(subscriber1).toHaveBeenCalledTimes(2); + expect(subscriber2).toHaveBeenCalledTimes(3); + expect(subscriber2).toHaveBeenLastCalledWith({ id: 2, data: fakeAppInfo }); + expect(common.createHumanitecClient).toHaveBeenCalledTimes(3); + + close2(); + + await setTimeout(fetchInterval); + + expect(subscriber1).toHaveBeenCalledTimes(2); + expect(subscriber2).toHaveBeenCalledTimes(3); + expect(common.createHumanitecClient).toHaveBeenCalledTimes(3); + }); +}); diff --git a/plugins/humanitec-backend/src/service/app-info-service.ts b/plugins/humanitec-backend/src/service/app-info-service.ts index 378787c5dc..4ec3b64edf 100644 --- a/plugins/humanitec-backend/src/service/app-info-service.ts +++ b/plugins/humanitec-backend/src/service/app-info-service.ts @@ -2,7 +2,7 @@ import { EventEmitter } from 'events'; import { createHumanitecClient, fetchAppInfo } from '@frontside/backstage-plugin-humanitec-common'; -const fetchInterval = 10000; +const defaultFetchInterval = 10000; export interface AppInfoUpdate { id: number; @@ -16,14 +16,16 @@ export interface AppInfoUpdate { // export class AppInfoService { private emitter: EventEmitter = new EventEmitter(); - private pending: Record> = {}; - private timeouts: Record = {}; - private lastData: Record = {}; + private pending: Map> = new Map(); + private timeouts: Map = new Map(); + private lastData: Map = new Map(); private token: string; + private fetchInterval: number; - constructor(token: string) { + constructor(token: string, fetchInterval = defaultFetchInterval) { this.token = token; + this.fetchInterval = fetchInterval; } addSubscriber(orgId: string, appId: string, subscriber: (data: AppInfoUpdate) => void): () => void { @@ -32,38 +34,37 @@ export class AppInfoService { this.emitter.on(key, subscriber); // Only fetch app info if a fetch is not pending. - if (!this.pending[key]) { + if (!this.pending.has(key)) { this.fetchAppInfo(orgId, appId); } else { - if (this.lastData[key]) { - subscriber(this.lastData[key]); + if (this.lastData.has(key)) { + subscriber(this.lastData.get(key)!); } } // Return a function that removes this subscriber when it's no longer interested. return () => { this.emitter.off(key, subscriber); - if (this.emitter.listenerCount(key) === 0 && this.timeouts[key]) { - clearTimeout(this.timeouts[key]); - delete this.pending[key]; - delete this.timeouts[key]; - delete this.lastData[key]; + 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): Promise { + private fetchAppInfo(orgId: string, appId: string): void { const key = `${orgId}:${appId}`; const client = createHumanitecClient({ token: this.token, orgId }); - let id = 0; - this.pending[key] = (async () => { - const update: AppInfoUpdate = { id: id++ }; + 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; - - this.timeouts[key] = setTimeout(()=> this.fetchAppInfo(orgId, appId), fetchInterval); } catch (error) { if (error instanceof Error) { update.error = error; @@ -71,10 +72,10 @@ export class AppInfoService { update.error = new Error(`${error}`); } } finally { + this.timeouts.set(key, setTimeout(() => this.fetchAppInfo(orgId, appId), this.fetchInterval)); + this.lastData.set(key, update); this.emitter.emit(key, update); - this.lastData[key] = update; } - })(); - return this.pending[key]; + })()); } } diff --git a/plugins/humanitec-backend/src/service/router.ts b/plugins/humanitec-backend/src/service/router.ts index 947765a5be..99a3966094 100644 --- a/plugins/humanitec-backend/src/service/router.ts +++ b/plugins/humanitec-backend/src/service/router.ts @@ -61,15 +61,11 @@ export async function createRouter( const unsubscribe = appInfoService.addSubscriber(orgId, appId, (data) => { if (data.error) { response.write(`event: update-failure\ndata: ${data.error.message}\nid: ${data.id}\n\n`); - flush(response); logger.error(`Error encountered trying to update environment`, data.error); - response.end(); - unsubscribe(); - - return + } else { + response.write(`event: update-success\ndata: ${JSON.stringify(data.data)}\nid: ${data.id}\n\n`); } - response.write(`event: update-success\ndata: ${JSON.stringify(data.data)}\nid: ${data.id}\n\n`); flush(response); });