feat: experimentation support - #301
Merged
Merged
Conversation
Parse `variant`, `reason` and `metadata.experiment` from remote identity
evaluations onto `Flag`, and fix `reason` parsing, which read the wrong
nesting level and was therefore always undefined.
Add an opt-in events pipeline behind `enableEvents`: an `EventProcessor`
buffers events, deduplicates exposures per flush window, and posts them
to `{eventsApiUrl}v1/events` every 10s or at 1000 events, retrying a
failed batch once before dropping it.
Add `getExperimentFlag`, `trackEvent`, `trackExposureEvent` and
`flushEvents` on `Flagsmith`; `close()` now flushes.
Local evaluation and offline mode never carry experiment metadata and
never record exposures. The engine is untouched.
The floating `finally()` on an in-flight batch had no rejection handler, so a throwing logger would surface as an unhandled rejection and break the processor's "never throws" contract. The maxBuffer test asserted only after an explicit flush, so it passed identically without any auto-flush; it now asserts before flushing. Also pin the stringification of falsy event values.
`flush()` looped until the in-flight set was empty, so under sustained traffic that kept reaching maxBuffer it never resolved and `close()` waited for traffic to stop. It now awaits a snapshot of the batches in flight at the time of the call, which still covers every event tracked before it, via `Promise.allSettled` so one failed batch neither rejects the promise nor stops it from waiting for the others. Correct the `getExperimentFlag` return doc: a missing feature with no default flag handler yields a disabled plain object, not a `DefaultFlag`. Pin the flush semantics, the throwing-logger path, the identity-cache exposure path and the `eventProcessorConfig` wiring with tests.
Zaimwa9
requested review from
emyller
and removed request for
a team and
emyller
September 23, 2026 12:51
Zaimwa9
marked this pull request as draft
September 23, 2026 12:51
Contributor
Author
|
@themis-blindfold review |
![themis-blindfold[bot]](https://avatars.githubusercontent.com/in/4278417?s=60&v=4){kind=small}
⚖️ Themis review: 🟠 Fix before mergeTL;DR: The new experimentation flow is well covered for the standard endpoint, but its event requests bypass the client-wide transport configuration. This breaks the opted-in feature for deployments that use a dispatcher or required custom headers. CI completed successfully on Node 20, 22, and 24; the local test command could not start because test dependencies are not installed.
🟠 Majors
📝 Walkthrough
🧪 How to verify
Product take: A solid experimentation capability, but it is unusable for a meaningful set of self-hosted and proxied deployments until event transport inherits client configuration. 🧭 Assumptions & unverified claimsNo unverified assumptions or claims. A fine event pipeline, once it takes the same route as the rest of the client · reviewed at 5a8486d |
Flag and identity requests go through the configured `agent` and `customHeaders`, but the event processor received only `fetch`, so a client behind a proxy would evaluate flags and silently drop every event. Custom headers are applied first so the SDK's own headers, which the events pipeline parses for language and version, cannot be overridden.
Contributor
Author
|
@themis-blindfold review |
⚖️ Themis review: 🧹 Ship it, nits insideTL;DR: The opt-in event and experiment flow is covered across buffering, retries, shutdown, cache use, and local/offline gates. The completed Node 20, 22, and 24 build-and-test checks passed; one header-casing edge case remains.
🧹 Nits
📝 Walkthrough
🧪 How to verify
Product take: This is a solid experimentation capability for Node services, with delivery and shutdown behaviour that matters for short-lived workloads. 🧭 Assumptions & unverified claimsNo unverified assumptions or claims. Event batches are ready for their close-up, once headers agree on casing. · reviewed at 87b3e16 |
Header names are case-insensitive, so a custom `x-environment-key` would be combined with the SDK's `X-Environment-Key` rather than overridden by it. Reserved names are now filtered out of custom headers regardless of case.
Zaimwa9
marked this pull request as ready for review
September 24, 2026 08:23
gagantrivedi
approved these changes
Sep 24, 2026
gagantrivedi
left a comment
gagantrivedi
left a comment
Member
There was a problem hiding this comment.
lgtm apart from one comment
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Thanks for submitting a PR! Please check the boxes below:
docs/if required so people know about the feature.Changes
Experimentation support, opt-in via
enableEvents: true. Nothing changes for existing users.Flaggainsvariantandexperiment { id, name, inExperiment }from remote identity evaluation. Fixesreason, which readfeature.reasonand was alwaysundefined.EventProcessor: buffers events, dedupes exposures per flush window, POSTs{ events }to{eventsApiUrl}v1/eventsevery 10 s or at 1000 events. Retries a failed batch once, then drops it; never re-queues.Flagsmithmethods:getExperimentFlag,trackEvent,trackExposureEvent,flushEvents.close()now flushes.Docs live at docs.flagsmith.com; the README defers to them and needs no change.
How did you test this code?
npm test: 452 passed.npm run test:esm-build: 442 passed, 10 skipped (instanceofassertions, as existing tests do). Engine e2e suite untouched and green.events.test.ts(buffering, dedupe matrix, headers/body, maxBuffer, bounded flush under in-flight batches, retry-then-drop, timerunref),flagsmith-experiments.test.ts(fullgetExperimentFlaggate matrix, identity-cache path, offline/local evaluation,close()),models.test.ts.