diff --git a/posthog-node/CHANGELOG.md b/posthog-node/CHANGELOG.md index afc5b59a..b2d22417 100644 --- a/posthog-node/CHANGELOG.md +++ b/posthog-node/CHANGELOG.md @@ -1,5 +1,9 @@ # Next +# 4.3.1 - 2024-11-26 + +1. Fix bug where this SDK incorrectly sent `$feature_flag_called` events with null values when using `getFeatureFlagPayload`. + # 4.3.0 - 2024-11-25 1. Add Sentry v8 support to the Sentry integration diff --git a/posthog-node/package.json b/posthog-node/package.json index 87ecec5f..aa82b826 100644 --- a/posthog-node/package.json +++ b/posthog-node/package.json @@ -1,6 +1,6 @@ { "name": "posthog-node", - "version": "4.3.0", + "version": "4.3.1", "description": "PostHog Node.js integration", "repository": { "type": "git", diff --git a/posthog-node/src/posthog-node.ts b/posthog-node/src/posthog-node.ts index cff93a28..36fd1f7b 100644 --- a/posthog-node/src/posthog-node.ts +++ b/posthog-node/src/posthog-node.ts @@ -294,59 +294,80 @@ export class PostHog extends PostHogCoreStateless implements PostHogNodeV1 { disableGeoip?: boolean } ): Promise { - const { groups, disableGeoip } = options || {} - let { onlyEvaluateLocally, sendFeatureFlagEvents, personProperties, groupProperties } = options || {} + const { groups, disableGeoip, onlyEvaluateLocally = false, personProperties, groupProperties } = options || {} - const adjustedProperties = this.addLocalPersonAndGroupProperties( + const { allPersonProperties, allGroupProperties } = this.addLocalPersonAndGroupProperties( distinctId, groups, personProperties, groupProperties ) - personProperties = adjustedProperties.allPersonProperties - groupProperties = adjustedProperties.allGroupProperties - - let response = undefined - - // Try to get match value locally if not provided - if (!matchValue) { + if (matchValue === undefined) { matchValue = await this.getFeatureFlag(key, distinctId, { ...options, onlyEvaluateLocally: true, + sendFeatureFlagEvents: false, }) } - if (matchValue) { - response = await this.featureFlagsPoller?.computeFeatureFlagPayloadLocally(key, matchValue) - } + let response: string | boolean | undefined + let payload: JsonType | undefined - // set defaults - if (onlyEvaluateLocally == undefined) { - onlyEvaluateLocally = false - } - if (sendFeatureFlagEvents == undefined) { - sendFeatureFlagEvents = true + if (matchValue) { + response = matchValue + payload = await this.featureFlagsPoller?.computeFeatureFlagPayloadLocally(key, matchValue) + } else { + response = undefined + payload = undefined } - // set defaults - if (onlyEvaluateLocally == undefined) { - onlyEvaluateLocally = false - } + // Determine if the payload was evaluated locally + const payloadWasLocallyEvaluated = payload !== undefined - const payloadWasLocallyEvaluated = response !== undefined + // Fetch final flags and payloads either locally or from the remote server + let fetchedOrLocalFlags: Record | undefined + let fetchedOrLocalPayloads: Record | undefined - if (!payloadWasLocallyEvaluated && !onlyEvaluateLocally) { - response = await super.getFeatureFlagPayloadStateless( - key, + if (payloadWasLocallyEvaluated || onlyEvaluateLocally) { + if (response !== undefined) { + fetchedOrLocalFlags = { [key]: response } + fetchedOrLocalPayloads = { [key]: payload } + } else { + fetchedOrLocalFlags = {} + fetchedOrLocalPayloads = {} + } + } else { + const fetchedData = await super.getFeatureFlagsAndPayloadsStateless( distinctId, groups, - personProperties, - groupProperties, + allPersonProperties, + allGroupProperties, disableGeoip ) + fetchedOrLocalFlags = fetchedData.flags || {} + fetchedOrLocalPayloads = fetchedData.payloads || {} } - return response + + const finalResponse = fetchedOrLocalFlags[key] + const finalPayload = fetchedOrLocalPayloads[key] + const finalLocallyEvaluated = payloadWasLocallyEvaluated + + this.capture({ + distinctId, + event: '$feature_flag_called', + properties: { + $feature_flag: key, + $feature_flag_response: finalResponse, + $feature_flag_payload: finalPayload, + locally_evaluated: finalLocallyEvaluated, + [`$feature/${key}`]: finalResponse, + }, + groups, + disableGeoip, + }) + + return finalPayload } async isFeatureEnabled( diff --git a/posthog-node/test/posthog-node.spec.ts b/posthog-node/test/posthog-node.spec.ts index cf785e45..c54d9267 100644 --- a/posthog-node/test/posthog-node.spec.ts +++ b/posthog-node/test/posthog-node.spec.ts @@ -897,13 +897,18 @@ describe('PostHog Node.js', () => { rollout_percentage: 100, }, ], + payloads: { true: { variant: 'A' } }, }, }, ], } mockedFetch.mockImplementation( - apiImplementation({ localFlags: flags, decideFlags: { 'decide-flag': 'decide-value' } }) + apiImplementation({ + localFlags: flags, + decideFlags: { 'decide-flag': 'decide-value' }, + decideFlagPayloads: { 'beta-feature': { variant: 'A' } }, + }) ) posthog = new PostHog('TEST_API_KEY', { @@ -945,6 +950,34 @@ describe('PostHog Node.js', () => { ) mockedFetch.mockClear() + expect( + await posthog.getFeatureFlagPayload('beta-feature', 'some-distinct-id', undefined, { + personProperties: { region: 'USA', name: 'Aloha' }, + }) + ).toEqual({ variant: 'A' }) + + // TRICKY: There's now an extra step before events are queued, so need to wait for that to resolve + jest.runOnlyPendingTimers() + await waitForPromises() + await posthog.flush() + + expect(mockedFetch).toHaveBeenCalledWith('http://example.com/batch/', expect.any(Object)) + + expect(getLastBatchEvents()?.[0]).toEqual( + expect.objectContaining({ + distinct_id: 'some-distinct-id', + event: '$feature_flag_called', + properties: expect.objectContaining({ + $feature_flag: 'beta-feature', + $feature_flag_response: true, + $feature_flag_payload: { variant: 'A' }, + locally_evaluated: true, + [`$feature/${'beta-feature'}`]: true, + }), + }) + ) + mockedFetch.mockClear() + // # called again for same user, shouldn't call capture again expect( await posthog.getFeatureFlag('beta-feature', 'some-distinct-id', { @@ -1081,7 +1114,7 @@ describe('PostHog Node.js', () => { expect(mockedFetch).toHaveBeenCalledTimes(0) await expect(posthog.getFeatureFlagPayload('false-flag', '123', true)).resolves.toEqual(300) - expect(mockedFetch).toHaveBeenCalledTimes(0) + expect(mockedFetch).toHaveBeenCalledTimes(1) // this now calls the server, because in this case the flag is not locally evaluated but we have a payload that we need to calculate }) it('should not double parse json with getFeatureFlagPayloads and server eval', async () => {