Merge pull request #389 from johanneswuerbach/humanitec-backend-slow-fix

fix(humanitec-backend): handle disconnects while fetching

.changeset/strong-kings-pay.md

@@ -0,0 +1,5 @@
+---
+'@frontside/backstage-plugin-humanitec-backend': patch
+---
+
+Handle disconnects while fetching app info

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);

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<string, Promise<any>> = new Map();
     private timeouts: Map<string, NodeJS.Timeout> = new Map();
     private lastData: Map<string, AppInfoUpdate> = 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<void> {
         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);
             }
-        })());
+        }
     }
 }