Merge pull request #382 from johanneswuerbach/fix-error-handling

fix(humanitec): continue polling on error

.changeset/green-coats-march.md

@@ -0,0 +1,5 @@
+---
+'@frontside/backstage-plugin-humanitec-backend': patch
+---
+
+Continue polling when an error is returned.

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

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<string, Promise<any>> = {};
-    private timeouts: Record<string, NodeJS.Timeout> = {};
-    private lastData: Record<string, AppInfoUpdate> = {};
+    private pending: Map<string, Promise<any>> = new Map();
+    private timeouts: Map<string, NodeJS.Timeout> = new Map();
+    private lastData: Map<string, AppInfoUpdate> = 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,49 +34,48 @@ 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<any> {
+    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;
                 } else {
                     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];
+        })());
     }
 }

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