Conversation
6109625 to
7eb55bb
Compare
c6bb75d to
a769852
Compare
5c56e10 to
eae1ff4
Compare
a769852 to
b9221c2
Compare
eae1ff4 to
3f88511
Compare
7cc0255 to
0466712
Compare
7396a8f to
128bac6
Compare
065c1b6 to
3b021c2
Compare
| * } | ||
| * ``` | ||
| */ | ||
| flagsConfiguration: string; |
There was a problem hiding this comment.
Opted to deviate from our client JS SDK and follow in the footsteps of python and iOS using the more user-friendly and information-rich wire format.
| /** | ||
| * Configuration settings for the event dispatcher. | ||
| * @deprecated Eppo has discontinued eventing support. Event tracking will be handled by Datadog SDKs. | ||
| */ |
There was a problem hiding this comment.
May as well start flagging this as deprecated so we can rip out in a future version! 🪓
| /** | ||
| * @deprecated Eppo has discontinued eventing support. Event tracking will be handled by Datadog SDKs. | ||
| */ |
There was a problem hiding this comment.
Deprecating this event-related method too! 🪓
| * @returns the initialized client instance | ||
| * @public | ||
| */ | ||
| export function offlineInit(config: IOfflineClientConfig): EppoClient { |
There was a problem hiding this comment.
🚀 The main new method!
There was a problem hiding this comment.
Pull request overview
This PR adds an offlineInit() function to enable synchronous SDK initialization using pre-fetched configuration, allowing the SDK to operate without network access or achieve faster startup times.
Changes:
- Introduces
offlineInit()function that parses and loads flag and bandit configurations from JSON strings - Adds
IOfflineClientConfiginterface for offline initialization configuration - Extracts
DEFAULT_ASSIGNMENT_CACHE_SIZEconstant to share betweeninit()andofflineInit()
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/index.ts | Adds offlineInit() function, IOfflineClientConfig export, and DEFAULT_ASSIGNMENT_CACHE_SIZE constant |
| src/index.spec.ts | Comprehensive test suite for offlineInit() covering initialization, assignment logging, error handling, and bandit support |
| src/i-client-config.ts | Defines IOfflineClientConfig interface with documentation for offline initialization parameters |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/index.ts
Outdated
| // Note: setEntries is async but MemoryOnlyConfigurationStore performs synchronous operations internally, | ||
| // so there's no race condition. We add .catch() for defensive error handling, matching JS client SDK pattern. |
There was a problem hiding this comment.
The comment claims setEntries is async but MemoryOnlyConfigurationStore performs synchronous operations internally. This is confusing and potentially misleading. If the operations are truly synchronous, the method shouldn't return a promise. Consider clarifying whether this is actually asynchronous or if the promise is just for API consistency.
| // Note: setEntries is async but MemoryOnlyConfigurationStore performs synchronous operations internally, | |
| // so there's no race condition. We add .catch() for defensive error handling, matching JS client SDK pattern. | |
| // Note: setEntries returns a Promise for API consistency, but MemoryOnlyConfigurationStore | |
| // performs its work synchronously, so there is no race condition. We still attach .catch() | |
| // for defensive error handling in case the Promise is ever rejected. |
src/index.ts
Outdated
| .catch((err) => | ||
| applicationLogger.warn(`Error setting flags for memory-only configuration store: ${err}`), | ||
| ); |
There was a problem hiding this comment.
The error message uses string interpolation with ${err} which will result in "[object Object]" for Error objects. Use err.message or handle the error object properly to provide a meaningful error message.
| .catch((err) => | |
| applicationLogger.warn(`Error setting flags for memory-only configuration store: ${err}`), | |
| ); | |
| .catch((err) => { | |
| const message = err instanceof Error ? err.message : String(err); | |
| applicationLogger.warn( | |
| `Error setting flags for memory-only configuration store: ${message}`, | |
| ); | |
| }); |
src/index.ts
Outdated
| banditModelConfigurationStore | ||
| .setEntries(banditsConfigResponse.bandits ?? {}) | ||
| .catch((err) => | ||
| applicationLogger.warn( | ||
| `Error setting bandit models for memory-only configuration store: ${err}`, | ||
| ), | ||
| ); |
There was a problem hiding this comment.
The error message uses string interpolation with ${err} which will result in "[object Object]" for Error objects. Use err.message or handle the error object properly to provide a meaningful error message.
src/index.ts
Outdated
| const flagsConfigResponse = JSON.parse(flagsConfiguration) as { | ||
| createdAt?: string; | ||
| format?: string; | ||
| environment?: { name: string }; | ||
| flags: Record<string, Flag>; | ||
| banditReferences?: Record< | ||
| string, | ||
| { | ||
| modelVersion: string; | ||
| flagVariations: BanditVariation[]; | ||
| } | ||
| >; | ||
| }; |
There was a problem hiding this comment.
There should never be a situation where a configuration object is missing a key, so why not just validate that each field exists and throw or log an "invalid configuration" error if it's missing anything? That way you can cast to IUniversalFlagConfigResponse for valid configurations and you can remove the guards below. The main benefit here is that you reduce the chance that an invalid configuration causes issues in other parts of the SDK that expect certain fields to be set.
There was a problem hiding this comment.
Great call! Will go beyond just parsing to checking expected keys are all there.
greghuels
left a comment
There was a problem hiding this comment.
Main issue is that I don't think we shouldn't allow invalid configurations
1eb8a4e to
87e6424
Compare
4bd2bc5 to
ae372fa
Compare
b235e30 to
c35ea62
Compare
96ca311 to
5a34532
Compare
| try { | ||
| // Parse and validate the flags configuration JSON | ||
| const parsedFlagsConfig = JSON.parse(flagsConfiguration); | ||
| const flagsValidationErrors = validateFlagsConfiguration(parsedFlagsConfig); |
There was a problem hiding this comment.
@greghuels Claude whipped this up to ensure we meet the type so we don't need to be super defensive with checking field existence per your suggestion
There was a problem hiding this comment.
Nice, thanks for updating!
291bc6f to
f3ee984
Compare
Adds the ability to initialize the SDK with pre-fetched configuration JSON, enabling: - Synchronous initialization without network requests - Use cases where configuration is bootstrapped from another source - Edge/serverless environments where polling isn't desired Also includes: - IOfflineClientConfig interface for offline initialization options - DEFAULT_ASSIGNMENT_CACHE_SIZE constant for consistent cache sizing - Deprecation annotations on event tracking (discontinued feature) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Use realistic bandit model coefficients from bandit-models-v1.json and test subject "alice" from test-case-banner-bandit.json to verify that getBanditAction returns the expected variation and action after offline initialization. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Rename "makes client available via getInstance()" to "can request assignment" - Move "no network requests" test into "basic initialization" section - Add assignment verification to "empty flags configuration" test Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add validateFlagsConfiguration() to ensure all required fields exist
- Add validateBanditsConfiguration() to validate bandit config structure
- Update offlineInit() to validate configs before loading:
- If validation fails and throwOnFailedInitialization=true: throw error
- If validation fails and throwOnFailedInitialization=false: log warning
and use empty config (all assignments return defaults)
- Update test helper to include banditReferences field
This ensures invalid configurations are caught early and provides clear
error messages about what fields are missing or invalid.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Reorder validation functions to follow clean code principles (high-level methods first)
- Fix ${err} string interpolation to properly extract error messages
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
75ac5ee to
6111973
Compare
Summary
offlineInit()function to initialize the SDK with pre-fetched configurationStacked PRs
getFlagsConfiguration()getBanditsConfiguration()offlineInit()(this PR)Test plan
offlineInit()initializes the client with provided configurationgetInstance()returns the client after offline init🤖 Generated with Claude Code