From 4fc8fefd1d6ccdcde28c6f3780103ba6b5130228 Mon Sep 17 00:00:00 2001 From: Ty Potter Date: Wed, 6 Nov 2024 09:24:00 -0700 Subject: [PATCH 1/3] polling interval param, keep polling and check cache expiration in callback --- src/client/eppo-client.spec.ts | 40 +++++++++++++++++++++++++--- src/client/eppo-client.ts | 48 ++++++++++++++++------------------ src/constants.ts | 2 +- src/poller.spec.ts | 4 +-- 4 files changed, 62 insertions(+), 32 deletions(-) diff --git a/src/client/eppo-client.spec.ts b/src/client/eppo-client.spec.ts index d5d91f9..4ddf902 100644 --- a/src/client/eppo-client.spec.ts +++ b/src/client/eppo-client.spec.ts @@ -17,7 +17,7 @@ import { IAssignmentLogger } from '../assignment-logger'; import ConfigurationRequestor from '../configuration-requestor'; import { IConfigurationStore } from '../configuration-store/configuration-store'; import { MemoryOnlyConfigurationStore } from '../configuration-store/memory.store'; -import { MAX_EVENT_QUEUE_SIZE, POLL_INTERVAL_MS, POLL_JITTER_PCT } from '../constants'; +import { MAX_EVENT_QUEUE_SIZE, DEFAULT_POLL_INTERVAL_MS, POLL_JITTER_PCT } from '../constants'; import FetchHttpClient from '../http-client'; import { Flag, ObfuscatedFlag, VariationType } from '../interfaces'; @@ -576,7 +576,7 @@ describe('EppoClient E2E test', () => { const subject = 'alice'; const pi = 3.1415926; - const maxRetryDelay = POLL_INTERVAL_MS * POLL_JITTER_PCT; + const maxRetryDelay = DEFAULT_POLL_INTERVAL_MS * POLL_JITTER_PCT; beforeAll(async () => { global.fetch = jest.fn(() => { @@ -652,6 +652,38 @@ describe('EppoClient E2E test', () => { expect(variation).toBe(pi); }); + describe('Poll after successful start', () => { + it('Continues to poll when cache has not expired', async () => { + class MockStore extends MemoryOnlyConfigurationStore { + public static expired = false; + + async isExpired(): Promise { + return MockStore.expired; + } + } + + client = new EppoClient(new MockStore(), undefined, undefined, { + ...requestConfiguration, + pollAfterSuccessfulInitialization: true, + }); + client.setIsGracefulFailureMode(false); + // no configuration loaded + let variation = client.getNumericAssignment(flagKey, subject, {}, 0.0); + expect(variation).toBe(0.0); + + // have client fetch configurations; cache is not expired so assignment stays + await client.fetchFlagConfigurations(); + variation = client.getNumericAssignment(flagKey, subject, {}, 0.0); + expect(variation).toBe(0.0); + + // Expire the cache and advance time until a reload should happen + MockStore.expired = true; + await jest.advanceTimersByTimeAsync(DEFAULT_POLL_INTERVAL_MS * 1.5); + + variation = client.getNumericAssignment(flagKey, subject, {}, 0.0); + expect(variation).toBe(pi); + }); + }); it('Does not fetch configurations if the configuration store is unexpired', async () => { class MockStore extends MemoryOnlyConfigurationStore { async isExpired(): Promise { @@ -720,7 +752,7 @@ describe('EppoClient E2E test', () => { expect(variation).toBe(pi); expect(callCount).toBe(2); - await jest.advanceTimersByTimeAsync(POLL_INTERVAL_MS); + await jest.advanceTimersByTimeAsync(DEFAULT_POLL_INTERVAL_MS); // By default, no more polling expect(callCount).toBe(pollAfterSuccessfulInitialization ? 3 : 2); }); @@ -782,7 +814,7 @@ describe('EppoClient E2E test', () => { expect(client.getNumericAssignment(flagKey, subject, {}, 10.0)).toBe(10.0); // Advance timers so a post-init poll can take place - await jest.advanceTimersByTimeAsync(POLL_INTERVAL_MS * 1.5); + await jest.advanceTimersByTimeAsync(DEFAULT_POLL_INTERVAL_MS * 1.5); // if pollAfterFailedInitialization = true, we will poll later and get a config, otherwise not expect(callCount).toBe(pollAfterFailedInitialization ? 2 : 1); diff --git a/src/client/eppo-client.ts b/src/client/eppo-client.ts index 3927af7..3232c7a 100644 --- a/src/client/eppo-client.ts +++ b/src/client/eppo-client.ts @@ -15,7 +15,7 @@ import { DEFAULT_POLL_CONFIG_REQUEST_RETRIES, DEFAULT_REQUEST_TIMEOUT_MS, MAX_EVENT_QUEUE_SIZE, - POLL_INTERVAL_MS, + DEFAULT_POLL_INTERVAL_MS, } from '../constants'; import { decodeFlag } from '../decoding'; import { EppoValue } from '../eppo_value'; @@ -60,6 +60,7 @@ export type FlagConfigurationRequestParameters = { sdkName: string; baseUrl?: string; requestTimeoutMs?: number; + pollingIntervalMs?: number; numInitialRequestRetries?: number; numPollRequestRetries?: number; pollAfterSuccessfulInitialization?: boolean; @@ -121,25 +122,16 @@ export default class EppoClient { 'Eppo SDK unable to fetch flag configurations without configuration request parameters', ); } + // if fetchFlagConfigurations() was previously called, stop any polling process from that call + this.requestPoller?.stop(); - if (this.requestPoller) { - // if fetchFlagConfigurations() was previously called, stop any polling process from that call - this.requestPoller.stop(); - } - - const isExpired = await this.flagConfigurationStore.isExpired(); - if (!isExpired) { - logger.info( - '[Eppo SDK] Configuration store is not expired. Skipping fetching flag configurations', - ); - return; - } const { apiKey, sdkName, sdkVersion, baseUrl, // Default is set in ApiEndpoints constructor if undefined requestTimeoutMs = DEFAULT_REQUEST_TIMEOUT_MS, + pollingIntervalMs = DEFAULT_POLL_INTERVAL_MS, numInitialRequestRetries = DEFAULT_INITIAL_CONFIG_REQUEST_RETRIES, numPollRequestRetries = DEFAULT_POLL_CONFIG_REQUEST_RETRIES, pollAfterSuccessfulInitialization = false, @@ -147,6 +139,10 @@ export default class EppoClient { throwOnFailedInitialization = false, skipInitialPoll = false, } = this.configurationRequestParameters; + if (pollingIntervalMs <= 0) { + logger.error('PollingIntervalMs must be greater than 0'); + } + // todo: Inject the chain of dependencies below const apiEndpoints = new ApiEndpoints({ baseUrl, @@ -160,18 +156,20 @@ export default class EppoClient { this.banditModelConfigurationStore ?? null, ); - this.requestPoller = initPoller( - POLL_INTERVAL_MS, - configurationRequestor.fetchAndStoreConfigurations.bind(configurationRequestor), - { - maxStartRetries: numInitialRequestRetries, - maxPollRetries: numPollRequestRetries, - pollAfterSuccessfulStart: pollAfterSuccessfulInitialization, - pollAfterFailedStart: pollAfterFailedInitialization, - errorOnFailedStart: throwOnFailedInitialization, - skipInitialPoll: skipInitialPoll, - }, - ); + const pollingCallback = async () => { + if (await this.flagConfigurationStore.isExpired()) { + return configurationRequestor.fetchAndStoreConfigurations(); + } + }; + + this.requestPoller = initPoller(pollingIntervalMs, pollingCallback, { + maxStartRetries: numInitialRequestRetries, + maxPollRetries: numPollRequestRetries, + pollAfterSuccessfulStart: pollAfterSuccessfulInitialization, + pollAfterFailedStart: pollAfterFailedInitialization, + errorOnFailedStart: throwOnFailedInitialization, + skipInitialPoll: skipInitialPoll, + }); await this.requestPoller.start(); } diff --git a/src/constants.ts b/src/constants.ts index 6221255..4546722 100644 --- a/src/constants.ts +++ b/src/constants.ts @@ -1,6 +1,6 @@ export const DEFAULT_REQUEST_TIMEOUT_MS = 5000; export const REQUEST_TIMEOUT_MILLIS = DEFAULT_REQUEST_TIMEOUT_MS; // for backwards compatibility -export const POLL_INTERVAL_MS = 30000; +export const DEFAULT_POLL_INTERVAL_MS = 30000; export const POLL_JITTER_PCT = 0.1; export const DEFAULT_INITIAL_CONFIG_REQUEST_RETRIES = 1; export const DEFAULT_POLL_CONFIG_REQUEST_RETRIES = 7; diff --git a/src/poller.spec.ts b/src/poller.spec.ts index 1ef8099..d45d083 100644 --- a/src/poller.spec.ts +++ b/src/poller.spec.ts @@ -1,10 +1,10 @@ import * as td from 'testdouble'; -import { POLL_INTERVAL_MS, POLL_JITTER_PCT } from './constants'; +import { DEFAULT_POLL_INTERVAL_MS, POLL_JITTER_PCT } from './constants'; import initPoller from './poller'; describe('poller', () => { - const testIntervalMs = POLL_INTERVAL_MS; + const testIntervalMs = DEFAULT_POLL_INTERVAL_MS; const maxRetryDelay = testIntervalMs * POLL_JITTER_PCT; const noOpCallback = td.func<() => Promise>(); From 27da4ffe1078c681fc4f1fbbce79e5a2d3acf688 Mon Sep 17 00:00:00 2001 From: Ty Potter Date: Wed, 6 Nov 2024 14:33:26 -0700 Subject: [PATCH 2/3] set to default --- src/client/eppo-client.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/client/eppo-client.ts b/src/client/eppo-client.ts index 39c0392..05d76c2 100644 --- a/src/client/eppo-client.ts +++ b/src/client/eppo-client.ts @@ -138,7 +138,6 @@ export default class EppoClient { sdkVersion, baseUrl, // Default is set in ApiEndpoints constructor if undefined requestTimeoutMs = DEFAULT_REQUEST_TIMEOUT_MS, - pollingIntervalMs = DEFAULT_POLL_INTERVAL_MS, numInitialRequestRetries = DEFAULT_INITIAL_CONFIG_REQUEST_RETRIES, numPollRequestRetries = DEFAULT_POLL_CONFIG_REQUEST_RETRIES, pollAfterSuccessfulInitialization = false, @@ -146,8 +145,11 @@ export default class EppoClient { throwOnFailedInitialization = false, skipInitialPoll = false, } = this.configurationRequestParameters; + + let { pollingIntervalMs = DEFAULT_POLL_INTERVAL_MS } = this.configurationRequestParameters; if (pollingIntervalMs <= 0) { - logger.error('PollingIntervalMs must be greater than 0'); + logger.error('pollingIntervalMs must be greater than 0. Using default'); + pollingIntervalMs = DEFAULT_POLL_INTERVAL_MS; } // todo: Inject the chain of dependencies below From a142854a9dc30c72b6dd254d68eaece4ece4b671 Mon Sep 17 00:00:00 2001 From: Ty Potter Date: Wed, 6 Nov 2024 14:36:05 -0700 Subject: [PATCH 3/3] version bump --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 7f2ba4b..2574db8 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@eppo/js-client-sdk-common", - "version": "4.2.0", + "version": "4.3.0", "description": "Eppo SDK for client-side JavaScript applications (base for both web and react native)", "main": "dist/index.js", "files": [