fix: concurrency issue when running multiple requests (#3442)

## About the changes

Fix issue when running multiple calls to the /frontend endpoint concurrently, which ends up creating many instances of unleash SDK client.

src/lib/routes/admin-api/api-token.ts

@@ -200,7 +200,7 @@ export class ApiTokenController extends Controller {
         const { token } = req.params;
 
         await this.apiTokenService.delete(token, extractUsername(req));
-        this.proxyService.deleteClientForProxyToken(token);
+        await this.proxyService.deleteClientForProxyToken(token);
         res.status(200).end();
     }
 

src/lib/routes/admin-api/project/api-token.ts

@@ -187,7 +187,7 @@ export class ProjectApiTokenController extends Controller {
                     storedToken.project[0] === projectId))
         ) {
             await this.apiTokenService.delete(token, extractUsername(req));
-            this.proxyService.deleteClientForProxyToken(token);
+            await this.proxyService.deleteClientForProxyToken(token);
             res.status(200).end();
         }
     }

src/lib/services/proxy-service.ts

@@ -42,7 +42,13 @@ export class ProxyService {
 
     private readonly services: Services;
 
-    private readonly clients: Map<ApiUser['secret'], Unleash> = new Map();
+    /**
+     * This is intentionally a Promise becasue we want to be able to await
+     * until the client (which might be being created by a different request) is ready
+     * Check this test that fails if we don't use a Promise: src/test/e2e/api/proxy/proxy.concurrency.e2e.test.ts
+     */
+    private readonly clients: Map<ApiUser['secret'], Promise<Unleash>> =
+        new Map();
 
     private cachedFrontendSettings?: FrontendSettings;
 
@@ -99,14 +105,13 @@ export class ProxyService {
     private async clientForProxyToken(token: ApiUser): Promise<Unleash> {
         ProxyService.assertExpectedTokenType(token);
 
-        if (!this.clients.has(token.secret)) {
-            this.clients.set(
-                token.secret,
-                await this.createClientForProxyToken(token),
-            );
+        let client = this.clients.get(token.secret);
+        if (!client) {
+            client = this.createClientForProxyToken(token);
+            this.clients.set(token.secret, client);
         }
 
-        return this.clients.get(token.secret);
+        return client;
     }
 
     private async createClientForProxyToken(token: ApiUser): Promise<Unleash> {
@@ -134,13 +139,17 @@ export class ProxyService {
         return client;
     }
 
-    deleteClientForProxyToken(secret: string): void {
-        this.clients.get(secret)?.destroy();
-        this.clients.delete(secret);
+    async deleteClientForProxyToken(secret: string): Promise<void> {
+        let clientPromise = this.clients.get(secret);
+        if (clientPromise) {
+            const client = await clientPromise;
+            client.destroy();
+            this.clients.delete(secret);
+        }
     }
 
     stopAll(): void {
-        this.clients.forEach((client) => client.destroy());
+        this.clients.forEach((promise) => promise.then((c) => c.destroy()));
     }
 
     private static assertExpectedTokenType({ type }: ApiUser) {

src/test/e2e/api/proxy/proxy.concurrency.e2e.test.ts

@@ -0,0 +1,64 @@
+import { IUnleashTest, setupAppWithAuth } from '../../helpers/test-helper';
+import dbInit, { ITestDb } from '../../helpers/database-init';
+import getLogger from '../../../fixtures/no-logger';
+import { randomId } from '../../../../lib/util';
+import { ApiTokenType } from '../../../../lib/types/models/api-token';
+
+let app: IUnleashTest;
+let db: ITestDb;
+let appErrorLogs: string[] = [];
+
+beforeAll(async () => {
+    db = await dbInit(`proxy_concurrency`, getLogger);
+    const baseLogger = getLogger();
+    const appLogger = {
+        ...baseLogger,
+        error: (msg: string, ...args: any[]) => {
+            appErrorLogs.push(msg);
+            baseLogger.error(msg, ...args);
+        },
+    };
+    app = await setupAppWithAuth(db.stores, {
+        frontendApiOrigins: ['https://example.com'],
+        getLogger: () => appLogger,
+    });
+});
+
+afterEach(() => {
+    app.services.proxyService.stopAll();
+    jest.clearAllMocks();
+});
+
+afterAll(async () => {
+    await app.destroy();
+    await db.destroy();
+});
+
+beforeEach(async () => {
+    appErrorLogs = [];
+});
+
+/**
+ * This test needs to run on a new instance of the application and a clean DB
+ * which is why it should be the only test of this file
+ */
+test('multiple parallel calls to api/frontend should not create multiple instances', async () => {
+    const frontendTokenDefault =
+        await app.services.apiTokenService.createApiTokenWithProjects({
+            type: ApiTokenType.FRONTEND,
+            projects: ['default'],
+            environment: 'default',
+            username: `test-token-${randomId()}`,
+        });
+
+    await Promise.all(
+        Array.from(Array(15).keys()).map(() =>
+            app.request
+                .get('/api/frontend')
+                .set('Authorization', frontendTokenDefault.secret)
+                .expect('Content-Type', /json/)
+                .expect(200),
+        ),
+    );
+    expect(appErrorLogs).toHaveLength(0);
+});