From c4b0c0efb9150d5f62d23f4b309b04187b1bfe7d Mon Sep 17 00:00:00 2001 From: Haven Barnes Date: Wed, 4 Dec 2024 13:19:30 -0800 Subject: [PATCH] Add $used_bootstrap_value evt property, update tests --- cypress/e2e/capture.cy.ts | 20 +++++++++++++++++++- src/__tests__/featureflags.ts | 19 ++++++++++--------- src/decide.ts | 4 +++- src/posthog-core.ts | 2 ++ src/posthog-featureflags.ts | 11 +++++++---- 5 files changed, 41 insertions(+), 15 deletions(-) diff --git a/cypress/e2e/capture.cy.ts b/cypress/e2e/capture.cy.ts index 133370f52..bc11604d1 100644 --- a/cypress/e2e/capture.cy.ts +++ b/cypress/e2e/capture.cy.ts @@ -157,7 +157,14 @@ describe('Event capture', () => { cy.get('[data-cy-feature-flag-button]').click() - cy.phCaptures().should('include', '$feature_flag_called') + cy.phCaptures({ full: true }).then((captures) => { + const flagCallEvents = captures.filter((capture) => capture.event === '$feature_flag_called') + expect(flagCallEvents.length).to.eq(1) + const flagCallEvent = flagCallEvents[0] + expect(flagCallEvent.properties.$feature_flag_bootstrapped_response).to.not.exist + expect(flagCallEvent.properties.$feature_flag_bootstrapped_payload).to.not.exist + expect(flagCallEvent.properties.$used_bootstrap_value).to.equal(false) + }) }) it('captures $feature_flag_called with bootstrapped value properties', () => { @@ -171,9 +178,19 @@ describe('Event capture', () => { 'some-feature': 'some-payload', }, }, + advanced_disable_feature_flags: true, }, + waitForDecide: false, }) + cy.intercept( + '/decide/*', + { times: 1 }, + { + forceNetworkError: true, + } + ) + cy.get('[data-cy-feature-flag-button]').click() cy.phCaptures({ full: true }).then((captures) => { @@ -182,6 +199,7 @@ describe('Event capture', () => { const flagCallEvent = flagCallEvents[0] expect(flagCallEvent.properties.$feature_flag_bootstrapped_response).to.equal('some-value') expect(flagCallEvent.properties.$feature_flag_bootstrapped_payload).to.equal('some-payload') + expect(flagCallEvent.properties.$used_bootstrap_value).to.equal(true) }) }) diff --git a/src/__tests__/featureflags.ts b/src/__tests__/featureflags.ts index 7dd295c6c..0de1ee386 100644 --- a/src/__tests__/featureflags.ts +++ b/src/__tests__/featureflags.ts @@ -32,6 +32,7 @@ describe('featureflags', () => { get_property: (key) => instance.persistence.props[key], capture: () => {}, decideEndpointWasHit: false, + receivedFlagValues: false, _send_request: jest.fn().mockImplementation(({ callback }) => callback({ statusCode: 200, @@ -67,7 +68,7 @@ describe('featureflags', () => { }) it('should return flags from persistence even if decide endpoint was not hit', () => { - featureFlags.instance.decideEndpointWasHit = false + featureFlags.instance.receivedFlagValues = false expect(featureFlags.getFlags()).toEqual([ 'beta-feature', @@ -80,7 +81,7 @@ describe('featureflags', () => { it('should warn if decide endpoint was not hit and no flags exist', () => { ;(window as any).POSTHOG_DEBUG = true - featureFlags.instance.decideEndpointWasHit = false + featureFlags.instance.receivedFlagValues = false instance.persistence.unregister('$enabled_feature_flags') instance.persistence.unregister('$active_feature_flags') @@ -101,7 +102,7 @@ describe('featureflags', () => { }) it('should return the right feature flag and call capture', () => { - featureFlags.instance.decideEndpointWasHit = false + featureFlags.instance.receivedFlagValues = false expect(featureFlags.getFlags()).toEqual([ 'beta-feature', @@ -132,7 +133,7 @@ describe('featureflags', () => { }) it('should call capture for every different flag response', () => { - featureFlags.instance.decideEndpointWasHit = true + featureFlags.instance.receivedFlagValues = true instance.persistence.register({ $enabled_feature_flags: { @@ -156,13 +157,13 @@ describe('featureflags', () => { instance.persistence.register({ $enabled_feature_flags: {}, }) - featureFlags.instance.decideEndpointWasHit = false + featureFlags.instance.receivedFlagValues = false expect(featureFlags.getFlagVariants()).toEqual({}) expect(featureFlags.isFeatureEnabled('beta-feature')).toEqual(undefined) // no extra capture call because flags haven't loaded yet. expect(instance.capture).toHaveBeenCalledTimes(1) - featureFlags.instance.decideEndpointWasHit = true + featureFlags.instance.receivedFlagValues = true instance.persistence.register({ $enabled_feature_flags: { x: 'y' }, }) @@ -185,7 +186,7 @@ describe('featureflags', () => { }) it('should return the right feature flag and not call capture', () => { - featureFlags.instance.decideEndpointWasHit = true + featureFlags.instance.receivedFlagValues = true expect(featureFlags.isFeatureEnabled('beta-feature', { send_event: false })).toEqual(true) expect(instance.capture).not.toHaveBeenCalled() @@ -315,7 +316,7 @@ describe('featureflags', () => { }) it('onFeatureFlags callback should be called immediately if feature flags were loaded', () => { - featureFlags.instance.decideEndpointWasHit = true + featureFlags.instance.receivedFlagValues = true let called = false featureFlags.onFeatureFlags(() => (called = true)) expect(called).toEqual(true) @@ -324,7 +325,7 @@ describe('featureflags', () => { }) it('onFeatureFlags should not return flags that are off', () => { - featureFlags.instance.decideEndpointWasHit = true + featureFlags.instance.receivedFlagValues = true let _flags = [] let _variants = {} featureFlags.onFeatureFlags((flags, variants) => { diff --git a/src/decide.ts b/src/decide.ts index 88c7ab92d..ced733962 100644 --- a/src/decide.ts +++ b/src/decide.ts @@ -8,7 +8,7 @@ import { document } from './utils/globals' export class Decide { constructor(private readonly instance: PostHog) { // don't need to wait for `decide` to return if flags were provided on initialisation - this.instance.decideEndpointWasHit = this.instance._hasBootstrappedFeatureFlags() + this.instance.receivedFlagValues = this.instance._hasBootstrappedFeatureFlags() } call(): void { @@ -51,6 +51,8 @@ export class Decide { this.instance.featureFlags.receivedFeatureFlags(response ?? {}, errorsLoading) } + this.instance.decideEndpointWasHit = !errorsLoading + if (errorsLoading) { logger.error('Failed to fetch feature flags from PostHog.') return diff --git a/src/posthog-core.ts b/src/posthog-core.ts index 49e21a25f..5e1f9afac 100644 --- a/src/posthog-core.ts +++ b/src/posthog-core.ts @@ -276,6 +276,7 @@ export class PostHog { compression?: Compression __request_queue: QueuedRequestOptions[] decideEndpointWasHit: boolean + receivedFlagValues: boolean analyticsDefaultEndpoint: string version = Config.LIB_VERSION _initialPersonProfilesConfig: 'always' | 'never' | 'identified_only' | null @@ -295,6 +296,7 @@ export class PostHog { this.config = defaultConfig() this.decideEndpointWasHit = false + this.receivedFlagValues = false this.SentryIntegration = SentryIntegration this.sentryIntegration = (options?: SentryIntegrationOptions) => sentryIntegration(this, options) this.__request_queue = [] diff --git a/src/posthog-featureflags.ts b/src/posthog-featureflags.ts index 53d6d5142..04f62c4c8 100644 --- a/src/posthog-featureflags.ts +++ b/src/posthog-featureflags.ts @@ -209,6 +209,7 @@ export class PostHogFeatureFlags { // This is because we don't want to block clients waiting for flags to load. // It's possible they're waiting for the callback to render the UI, but it never occurs. this.receivedFeatureFlags(response.json ?? {}, errorsLoading) + this.instance.decideEndpointWasHit = !errorsLoading // :TRICKY: Reload - start another request if queued! this._startReloadTimer() @@ -227,7 +228,7 @@ export class PostHogFeatureFlags { * @param {Object|String} options (optional) If {send_event: false}, we won't send an $feature_flag_call event to PostHog. */ getFeatureFlag(key: string, options: { send_event?: boolean } = {}): boolean | string | undefined { - if (!this.instance.decideEndpointWasHit && !(this.getFlags() && this.getFlags().length > 0)) { + if (!this.instance.receivedFlagValues && !(this.getFlags() && this.getFlags().length > 0)) { logger.warn('getFeatureFlag for key "' + key + '" failed. Feature flags didn\'t load in time.') return undefined } @@ -251,6 +252,8 @@ export class PostHogFeatureFlags { $feature_flag_bootstrapped_response: this.instance.config.bootstrap?.featureFlags?.[key] || null, $feature_flag_bootstrapped_payload: this.instance.config.bootstrap?.featureFlagPayloads?.[key] || null, + // If we haven't yet received a response from the /decide endpoint, we must have used the bootstrapped value + $used_bootstrap_value: !this.instance.decideEndpointWasHit, }) } } @@ -273,7 +276,7 @@ export class PostHogFeatureFlags { * @param {Object|String} options (optional) If {send_event: false}, we won't send an $feature_flag_call event to PostHog. */ isFeatureEnabled(key: string, options: { send_event?: boolean } = {}): boolean | undefined { - if (!this.instance.decideEndpointWasHit && !(this.getFlags() && this.getFlags().length > 0)) { + if (!this.instance.receivedFlagValues && !(this.getFlags() && this.getFlags().length > 0)) { logger.warn('isFeatureEnabled for key "' + key + '" failed. Feature flags didn\'t load in time.') return undefined } @@ -292,7 +295,7 @@ export class PostHogFeatureFlags { if (!this.instance.persistence) { return } - this.instance.decideEndpointWasHit = true + this.instance.receivedFlagValues = true const currentFlags = this.getFlagVariants() const currentFlagPayloads = this.getFlagPayloads() parseFeatureFlagDecideResponse(response, this.instance.persistence, currentFlags, currentFlagPayloads) @@ -346,7 +349,7 @@ export class PostHogFeatureFlags { */ onFeatureFlags(callback: FeatureFlagsCallback): () => void { this.addFeatureFlagsHandler(callback) - if (this.instance.decideEndpointWasHit) { + if (this.instance.receivedFlagValues) { const { flags, flagVariants } = this._prepareFeatureFlagsForCallbacks() callback(flags, flagVariants) }