diff --git a/src/lib/proxy/proxy-repository.ts b/src/lib/proxy/proxy-repository.ts index 141374ddd9..a3c185c5e1 100644 --- a/src/lib/proxy/proxy-repository.ts +++ b/src/lib/proxy/proxy-repository.ts @@ -42,7 +42,9 @@ export class ProxyRepository private interval: number; - private timer: NodeJS.Timer; + private timer: NodeJS.Timer | null; + + private running: boolean; constructor( config: Config, @@ -73,6 +75,7 @@ export class ProxyRepository } async start(): Promise { + this.running = true; await this.dataPolling(); // Reload cached token data whenever something relevant has changed. @@ -85,11 +88,19 @@ export class ProxyRepository stop(): void { this.stores.eventStore.off(ANY_EVENT, this.onAnyEvent); - clearTimeout(this.timer); + this.running = false; } private async dataPolling() { this.timer = setTimeout(async () => { + if (!this.running) { + clearTimeout(this.timer!); + this.timer = null; + this.logger.debug( + 'Shutting down data polling for proxy repository', + ); + return; + } await this.dataPolling(); }, this.randomizeDelay(this.interval, this.interval * 2)).unref(); diff --git a/src/lib/services/proxy-service.ts b/src/lib/services/proxy-service.ts index ad1314395b..cc9526db93 100644 --- a/src/lib/services/proxy-service.ts +++ b/src/lib/services/proxy-service.ts @@ -46,7 +46,7 @@ export class ProxyService { private cachedFrontendSettings?: FrontendSettings; - private timer: NodeJS.Timeout; + private timer: NodeJS.Timeout | null; constructor(config: Config, stores: Stores, services: Services) { this.config = config; @@ -150,6 +150,7 @@ export class ProxyService { } deleteClientForProxyToken(secret: string): void { + this.clients.get(secret)?.destroy(); this.clients.delete(secret); } @@ -200,7 +201,9 @@ export class ProxyService { } destroy(): void { - clearInterval(this.timer); - this.timer = null; + if (this.timer) { + clearInterval(this.timer); + this.timer = null; + } } } diff --git a/src/test/e2e/api/proxy/proxy.e2e.test.ts b/src/test/e2e/api/proxy/proxy.e2e.test.ts index 3dd7ff93a2..e32dffff82 100644 --- a/src/test/e2e/api/proxy/proxy.e2e.test.ts +++ b/src/test/e2e/api/proxy/proxy.e2e.test.ts @@ -14,6 +14,7 @@ import { IStrategyConfig, } from '../../../../lib/types'; import { ProxyRepository } from '../../../../lib/proxy'; +import { Logger } from '../../../../lib/logger'; let app: IUnleashTest; let db: ITestDb; @@ -27,6 +28,7 @@ beforeAll(async () => { afterEach(() => { app.services.proxyService.stopAll(); + jest.clearAllMocks(); }); afterAll(async () => { @@ -853,6 +855,7 @@ test('Should sync proxy for keys on an interval', async () => { ProxyRepository.prototype as any, 'featuresForToken', ); + expect(user).not.toBeNull(); const proxyRepository = new ProxyRepository( { getLogger, @@ -860,7 +863,7 @@ test('Should sync proxy for keys on an interval', async () => { }, db.stores, app.services, - user, + user!, ); await proxyRepository.start(); @@ -891,7 +894,7 @@ test('Should change fetch interval', async () => { }, db.stores, app.services, - user, + user!, ); await proxyRepository.start(); @@ -919,7 +922,7 @@ test('Should not recursively set off timers on events', async () => { }, db.stores, app.services, - user, + user!, ); await proxyRepository.start(); @@ -934,7 +937,7 @@ test('Should not recursively set off timers on events', async () => { }); test('should return all features when specified', async () => { - app.config.experimental.flags.proxyReturnAllToggles = true; + app.config.experimental!.flags.proxyReturnAllToggles = true; const frontendToken = await createApiToken(ApiTokenType.FRONTEND); await createFeatureToggle({ name: 'enabledFeature1', @@ -991,3 +994,47 @@ test('should return maxAge header on options call', async () => { expect(res.headers['access-control-max-age']).toBe('86400'); }); }); + +test('should terminate data polling when stop is called', async () => { + const frontendToken = await createApiToken(ApiTokenType.FRONTEND); + const user = await app.services.apiTokenService.getUserForToken( + frontendToken.secret, + ); + + const logTrap = []; + const getDebugLogger = (): Logger => { + return { + /* eslint-disable-next-line */ + debug: (message: any, ...args: any[]) => { + logTrap.push(message); + }, + /* eslint-disable-next-line */ + info: (...args: any[]) => {}, + /* eslint-disable-next-line */ + warn: (...args: any[]) => {}, + /* eslint-disable-next-line */ + error: (...args: any[]) => {}, + /* eslint-disable-next-line */ + fatal: (...args: any[]) => {}, + }; + }; + + /* eslint-disable-next-line */ + const proxyRepository = new ProxyRepository( + { + getLogger: getDebugLogger, + frontendApi: { refreshIntervalInMs: 1 }, + }, + db.stores, + app.services, + user, + ); + + await proxyRepository.start(); + proxyRepository.stop(); + // Polling here is an async recursive call, so we gotta give it a bit of time + await new Promise((r) => setTimeout(r, 10)); + expect(logTrap).toContain( + 'Shutting down data polling for proxy repository', + ); +});