-
Notifications
You must be signed in to change notification settings - Fork 1
refactor: JS SDK v5 #256
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
refactor: JS SDK v5 #256
Changes from 14 commits
60fafe1
68c8b2f
b51b576
a491d0b
9325c88
c2a07c1
6df8767
4d3b534
2f70a51
d4c6f06
60c8617
51f4abb
34ff6c3
a7da43d
6b4725c
311f61f
e5e5b9f
9ee4834
3b79e68
bef0393
311364d
9647e30
4c9a8f8
896ffb8
a8866fb
6bea66d
badd643
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
{ | ||
"singleQuote": true, | ||
"trailingComma": "all", | ||
"printWidth": 100 | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,70 @@ | ||
# Configuration Lifecycle | ||
|
||
This document explains how configuration is managed throughout its lifecycle in the Eppo SDK. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔥 |
||
|
||
## Components Overview | ||
|
||
The SDK's configuration management is built around several key components that work together: | ||
|
||
- **ConfigurationFeed**: A broadcast channel that serves as the central communication point between components | ||
- **ConfigurationStore**: Maintains the currently active configuration used for all evaluations | ||
- **ConfigurationPoller**: Periodically fetches new configurations from the Eppo API | ||
- **PersistentConfigurationCache**: Persists configuration between application restarts | ||
|
||
## Communication Flow | ||
|
||
The ConfigurationFeed acts as a central hub through which different components communicate: | ||
|
||
 | ||
|
||
When a new configuration is received (either from network or cache), it's broadcast through the ConfigurationFeed. Components subscribe to this feed to react to configuration changes. Importantly, configurations broadcast on the ConfigurationFeed are not necessarily activated - they may never be activated at all, as they represent only the latest discovered configurations. For components interested in the currently active configuration, the ConfigurationStore provides its own broadcast channel that only emits when configurations become active. | ||
Comment on lines
+14
to
+20
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a really exciting new interface - something I want to support in the near future is a concept that the requested configuration is not going to be "complete": we might want to deliver updates via a websocket or the edge JSON will be paged. Either way having a separation between the network response being the canonical source of truth and the configuration store is very important. Do you feel like the interface you are creating makes us read to push updated in this way? I am wondering if the broadcaster should have a more verbose API that tells listeners whether flags are being added or removed. |
||
|
||
## Initialization Process | ||
|
||
During initialization, the client: | ||
|
||
1. **Configuration Loading Strategy**: | ||
- `stale-while-revalidate`: Uses cached config if within `maxStaleSeconds`, while fetching fresh data | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
- `only-if-cached`: Uses cached config without network requests | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. even if stale? If so we should put that in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. oh, no, |
||
- `no-cache`: Always fetches fresh configuration | ||
- `none`: Uses only initial configuration without loading/fetching | ||
|
||
2. **Loading cached configuration**: | ||
- If `initialConfiguration` is provided, uses it immediately | ||
- Otherwise, tries to load cached configuration | ||
|
||
3. **Network Fetching**: | ||
- If fetching is needed, attempts to fetch until success or timeout | ||
- Applies backoff with jitter between retry attempts (with shorter period than normal polling) | ||
- Broadcasts fetched configuration via ConfigurationFeed | ||
|
||
4. **Completion**: | ||
- Initialization completes when either: | ||
- Fresh configuration is fetched (for network strategies) | ||
- Cache is loaded (for cache-only strategies) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. or stale-while-revalidate right? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For stale-while-revalidate, the initialization completes when a fresh configuration is fetched There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh I thought that strategy means serve stale assignments and fetch and background? I think that would mean initialization should finish prior to fetch. |
||
- Timeout is reached (using best available configuration) | ||
|
||
## Ongoing Configuration Management | ||
|
||
After initialization: | ||
|
||
1. **Polling** (if enabled): | ||
- ConfigurationPoller periodically fetches new configurations | ||
- Uses exponential backoff with jitter for retries on failure | ||
- Broadcasts new configurations through ConfigurationFeed | ||
|
||
2. **Configuration Activation**: | ||
- When ConfigurationStore receives new configurations, it activates them based on strategy: | ||
- `always`: Activate immediately | ||
- `stale`: Activate if current config exceeds `maxStaleSeconds` | ||
- `empty`: Activate if current config is empty | ||
- `next-load`: Store for next initialization | ||
|
||
3. **Persistent Storage**: | ||
- PersistentConfigurationCache listens to ConfigurationFeed | ||
- Automatically stores new configurations to persistent storage | ||
- Provides cached configurations on initialization | ||
|
||
## Evaluation | ||
|
||
For all feature flag evaluations, EppoClient always uses the currently active configuration from ConfigurationStore. This ensures consistent behavior even as configurations are updated in the background. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,32 @@ | ||
export type Listener<T extends unknown[]> = (...args: T) => void; | ||
|
||
/** | ||
* A broadcast channel for dispatching events to multiple listeners. | ||
* | ||
* @internal | ||
*/ | ||
export class BroadcastChannel<T extends unknown[]> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Any particular reason you're rolling your own vs. using a multi-js-environment third party library like This seems simple enough and keeps dependencies down, which are advantages so I'm cool rolling with this just curious if you had thoughts about the pros and cons. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wanted it to have proper typescript support and be lightweight ( There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. makes sense! |
||
private listeners: Array<Listener<T>> = []; | ||
|
||
public addListener(listener: Listener<T>): () => void { | ||
this.listeners.push(listener); | ||
return () => this.removeListener(listener); | ||
} | ||
|
||
public removeListener(listener: Listener<T>): void { | ||
const idx = this.listeners.indexOf(listener); | ||
if (idx !== -1) { | ||
this.listeners.splice(idx, 1); | ||
} | ||
} | ||
|
||
public broadcast(...args: T): void { | ||
for (const listener of this.listeners) { | ||
try { | ||
listener(...args); | ||
} catch { | ||
// ignore | ||
} | ||
} | ||
} | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: newline |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,7 @@ import * as fs from 'fs'; | |
import { | ||
IAssignmentTestCase, | ||
MOCK_UFC_RESPONSE_FILE, | ||
readMockUfcConfiguration, | ||
readMockUFCResponse, | ||
} from '../../test/testHelpers'; | ||
import { MemoryOnlyConfigurationStore } from '../configuration-store/memory.store'; | ||
|
@@ -13,28 +14,25 @@ import { AttributeType } from '../types'; | |
|
||
import EppoClient, { IAssignmentDetails } from './eppo-client'; | ||
import { initConfiguration } from './test-utils'; | ||
import { read } from 'fs'; | ||
|
||
describe('EppoClient get*AssignmentDetails', () => { | ||
const testStart = Date.now(); | ||
|
||
global.fetch = jest.fn(() => { | ||
const ufc = readMockUFCResponse(MOCK_UFC_RESPONSE_FILE); | ||
let client: EppoClient; | ||
|
||
return Promise.resolve({ | ||
ok: true, | ||
status: 200, | ||
json: () => Promise.resolve(ufc), | ||
beforeEach(() => { | ||
client = new EppoClient({ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🙌 |
||
sdkKey: 'dummy', | ||
sdkName: 'js-client-sdk-common', | ||
sdkVersion: '1.0.0', | ||
baseUrl: 'http://127.0.0.1:4000', | ||
configuration: { initialConfiguration: readMockUfcConfiguration() }, | ||
}); | ||
}) as jest.Mock; | ||
const storage = new MemoryOnlyConfigurationStore<Flag | ObfuscatedFlag>(); | ||
|
||
beforeAll(async () => { | ||
await initConfiguration(storage); | ||
client.setIsGracefulFailureMode(false); | ||
}); | ||
|
||
it('should set the details for a matched rule', () => { | ||
const client = new EppoClient({ flagConfigurationStore: storage }); | ||
client.setIsGracefulFailureMode(false); | ||
const subjectAttributes = { email: '[email protected]', country: 'US' }; | ||
const result = client.getIntegerAssignmentDetails( | ||
'integer-flag', | ||
|
@@ -85,8 +83,6 @@ describe('EppoClient get*AssignmentDetails', () => { | |
}); | ||
|
||
it('should set the details for a matched split', () => { | ||
const client = new EppoClient({ flagConfigurationStore: storage }); | ||
client.setIsGracefulFailureMode(false); | ||
const subjectAttributes = { email: '[email protected]', country: 'Brazil' }; | ||
const result = client.getIntegerAssignmentDetails( | ||
'integer-flag', | ||
|
@@ -128,8 +124,6 @@ describe('EppoClient get*AssignmentDetails', () => { | |
}); | ||
|
||
it('should handle matching a split allocation with a matched rule', () => { | ||
const client = new EppoClient({ flagConfigurationStore: storage }); | ||
client.setIsGracefulFailureMode(false); | ||
const subjectAttributes = { id: 'alice', email: '[email protected]', country: 'Brazil' }; | ||
const result = client.getStringAssignmentDetails( | ||
'new-user-onboarding', | ||
|
@@ -190,8 +184,6 @@ describe('EppoClient get*AssignmentDetails', () => { | |
}); | ||
|
||
it('should handle unrecognized flags', () => { | ||
const client = new EppoClient({ flagConfigurationStore: storage }); | ||
client.setIsGracefulFailureMode(false); | ||
const result = client.getIntegerAssignmentDetails('asdf', 'alice', {}, 0); | ||
expect(result).toEqual({ | ||
variation: 0, | ||
|
@@ -215,7 +207,6 @@ describe('EppoClient get*AssignmentDetails', () => { | |
}); | ||
|
||
it('should handle type mismatches with graceful failure mode enabled', () => { | ||
const client = new EppoClient({ flagConfigurationStore: storage }); | ||
client.setIsGracefulFailureMode(true); | ||
const result = client.getBooleanAssignmentDetails('integer-flag', 'alice', {}, true); | ||
expect(result).toEqual({ | ||
|
@@ -252,7 +243,6 @@ describe('EppoClient get*AssignmentDetails', () => { | |
}); | ||
|
||
it('should throw an error for type mismatches with graceful failure mode disabled', () => { | ||
const client = new EppoClient({ flagConfigurationStore: storage }); | ||
client.setIsGracefulFailureMode(false); | ||
expect(() => client.getBooleanAssignmentDetails('integer-flag', 'alice', {}, true)).toThrow(); | ||
}); | ||
|
@@ -277,22 +267,6 @@ describe('EppoClient get*AssignmentDetails', () => { | |
} | ||
}; | ||
|
||
beforeAll(async () => { | ||
global.fetch = jest.fn(() => { | ||
return Promise.resolve({ | ||
ok: true, | ||
status: 200, | ||
json: () => Promise.resolve(readMockUFCResponse(MOCK_UFC_RESPONSE_FILE)), | ||
}); | ||
}) as jest.Mock; | ||
|
||
await initConfiguration(storage); | ||
}); | ||
|
||
afterAll(() => { | ||
jest.restoreAllMocks(); | ||
}); | ||
|
||
describe.each(getTestFilePaths())('for file: %s', (testFilePath: string) => { | ||
const testCase = parseJSON(testFilePath); | ||
describe.each(testCase.subjects.map(({ subjectKey }) => subjectKey))( | ||
|
@@ -302,9 +276,6 @@ describe('EppoClient get*AssignmentDetails', () => { | |
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion | ||
const subject = subjects.find((subject) => subject.subjectKey === subjectKey)!; | ||
|
||
const client = new EppoClient({ flagConfigurationStore: storage }); | ||
client.setIsGracefulFailureMode(false); | ||
|
||
const focusOn = { | ||
testFilePath: '', // focus on test file paths (don't forget to set back to empty string!) | ||
subjectKey: '', // focus on subject (don't forget to set back to empty string!) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹