Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -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": [
Expand Down
40 changes: 36 additions & 4 deletions src/client/eppo-client.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import {
import { IAssignmentLogger } from '../assignment-logger';
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 { Flag, ObfuscatedFlag, VariationType } from '../interfaces';

import EppoClient, { FlagConfigurationRequestParameters, checkTypeMatch } from './eppo-client';
Expand Down Expand Up @@ -554,7 +554,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(() => {
Expand Down Expand Up @@ -630,6 +630,38 @@ describe('EppoClient E2E test', () => {
expect(variation).toBe(pi);
});

describe('Poll after successful start', () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙌

it('Continues to poll when cache has not expired', async () => {
class MockStore<T> extends MemoryOnlyConfigurationStore<T> {
public static expired = false;

async isExpired(): Promise<boolean> {
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<T> extends MemoryOnlyConfigurationStore<T> {
async isExpired(): Promise<boolean> {
Expand Down Expand Up @@ -698,7 +730,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);
});
Expand Down Expand Up @@ -760,7 +792,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);
Expand Down
50 changes: 25 additions & 25 deletions src/client/eppo-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -60,6 +60,7 @@ export type FlagConfigurationRequestParameters = {
sdkName: string;
baseUrl?: string;
requestTimeoutMs?: number;
pollingIntervalMs?: number;
numInitialRequestRetries?: number;
numPollRequestRetries?: number;
pollAfterSuccessfulInitialization?: boolean;
Expand Down Expand Up @@ -128,19 +129,9 @@ 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,
Expand All @@ -154,6 +145,13 @@ export default class EppoClient {
throwOnFailedInitialization = false,
skipInitialPoll = false,
} = this.configurationRequestParameters;

let { pollingIntervalMs = DEFAULT_POLL_INTERVAL_MS } = this.configurationRequestParameters;
if (pollingIntervalMs <= 0) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's a reasonable guard/error-action to take here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can still log the error, and then perhaps set it to the default? Alternatively, we could use an invalid polling interval to mean don't poll, which would be consistent with other SDKs

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think setting to default is the way to go. Since there are pollAfterSuccessfulInitialization and pollAfterFailedInitialization, it doesn't make sense for an invalid argument to implicitly override the variables explicitly controlling this behaviour.

logger.error('pollingIntervalMs must be greater than 0. Using default');
pollingIntervalMs = DEFAULT_POLL_INTERVAL_MS;
}

// todo: Inject the chain of dependencies below
const apiEndpoints = new ApiEndpoints({
baseUrl,
Expand All @@ -167,18 +165,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()) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Polling = frequent checking of the cache and, if necessary, a fetch.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love it!

return configurationRequestor.fetchAndStoreConfigurations();
}
};

this.requestPoller = initPoller(pollingIntervalMs, pollingCallback, {
maxStartRetries: numInitialRequestRetries,
maxPollRetries: numPollRequestRetries,
pollAfterSuccessfulStart: pollAfterSuccessfulInitialization,
pollAfterFailedStart: pollAfterFailedInitialization,
errorOnFailedStart: throwOnFailedInitialization,
skipInitialPoll: skipInitialPoll,
});

await this.requestPoller.start();
}
Expand Down
2 changes: 1 addition & 1 deletion src/constants.ts
Original file line number Diff line number Diff line change
@@ -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;
Expand Down
4 changes: 2 additions & 2 deletions src/poller.spec.ts
Original file line number Diff line number Diff line change
@@ -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<void>>();

Expand Down
Loading