Skip to content

Commit

Permalink
feat(feature-flags): Allow upserting decide v3 responses (#53)
Browse files Browse the repository at this point in the history 
* feat(feature-flags): Allow upserting decide v3 responses

* fix tests

* fix lint

* no overrides on local evaluation
  • Loading branch information
@neilkakkar
neilkakkar authored Jan 17, 2023
1 parent 2e92ad8 commit ef11d41
Show file tree
Hide file tree
Showing 6 changed files with 256 additions and 19 deletions.
17 changes: 13 additions & 4 deletions posthog-core/src/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -407,7 +407,7 @@ export abstract class PostHogCore {
}

private async _decideAsync(sendAnonDistinctId: boolean = true): Promise<PostHogDecideResponse> {
const url = `${this.host}/decide/?v=2`
const url = `${this.host}/decide/?v=3`

const distinctId = this.getDistinctId()
const groups = this.props.$groups || {}
Expand All @@ -433,9 +433,16 @@ export abstract class PostHogCore {
.then((r) => r.json() as Promise<PostHogDecideResponse>)
.then((res) => {
if (res.featureFlags) {
this.setKnownFeatureFlags(res.featureFlags)
let newFeatureFlags = res.featureFlags
if (res.errorsWhileComputingFlags) {
// if not all flags were computed, we upsert flags instead of replacing them
const currentFlags = this.getPersistedProperty<PostHogDecideResponse['featureFlags']>(
PostHogPersistedProperty.FeatureFlags
)
newFeatureFlags = { ...currentFlags, ...res.featureFlags }
}
this.setKnownFeatureFlags(newFeatureFlags)
}

return res
})
.finally(() => {
Expand All @@ -461,8 +468,10 @@ export abstract class PostHogCore {
}

let response = featureFlags[key]
// `/decide` v3 returns all flags

if (response === undefined) {
// `/decide` returns nothing for flags which are false.
// For cases where the flag is unknown, return false
response = false
}

Expand Down
1 change: 1 addition & 0 deletions posthog-core/src/types.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,5 +86,6 @@ export type PostHogDecideResponse = {
featureFlags: {
[key: string]: string | boolean
}
errorsWhileComputingFlags: boolean
sessionRecording: boolean
}
242 changes: 231 additions & 11 deletions posthog-core/test/posthog.featureflags.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,19 @@ describe('PostHog Core', () => {
'feature-variant': 'variant',
})

const errorAPIResponse = Promise.resolve({
status: 400,
text: () => Promise.resolve('error'),
json: () =>
Promise.resolve({
status: 'error',
}),
})

beforeEach(() => {
;[posthog, mocks] = createTestClient('TEST_API_KEY', { flushAt: 1 }, (_mocks) => {
_mocks.fetch.mockImplementation((url) => {
if (url.includes('/decide/')) {
if (url.includes('/decide/?v=3')) {
return Promise.resolve({
status: 200,
text: () => Promise.resolve('ok'),
Expand Down Expand Up @@ -76,7 +85,7 @@ describe('PostHog Core', () => {
})

it('should load feature flags on init', async () => {
expect(mocks.fetch).toHaveBeenCalledWith('https://app.posthog.com/decide/?v=2', {
expect(mocks.fetch).toHaveBeenCalledWith('https://app.posthog.com/decide/?v=3', {
body: JSON.stringify({
token: 'TEST_API_KEY',
distinct_id: posthog.getDistinctId(),
Expand Down Expand Up @@ -120,19 +129,30 @@ describe('PostHog Core', () => {
})
}

return Promise.resolve({
status: 400,
text: () => Promise.resolve('ok'),
json: () =>
Promise.resolve({
status: 'ok',
}),
})
return errorAPIResponse
})
})

jest.runOnlyPendingTimers() // trigger init setImmediate
})

it('should return undefined', async () => {
expect(mocks.fetch).toHaveBeenCalledWith('https://app.posthog.com/decide/?v=3', {
body: JSON.stringify({
token: 'TEST_API_KEY',
distinct_id: posthog.getDistinctId(),
$anon_distinct_id: posthog.getAnonymousId(),
groups: {},
person_properties: {},
group_properties: {},
}),
method: 'POST',
headers: {
'Content-Type': 'application/json',
},
signal: expect.anything(),
})

expect(posthog.getFeatureFlag('feature-1')).toEqual(undefined)
expect(posthog.getFeatureFlag('feature-variant')).toEqual(undefined)
expect(posthog.getFeatureFlag('feature-missing')).toEqual(undefined)
Expand All @@ -143,6 +163,206 @@ describe('PostHog Core', () => {
})
})

describe('when subsequent decide calls return partial results', () => {
beforeEach(() => {
;[posthog, mocks] = createTestClient('TEST_API_KEY', { flushAt: 1 }, (_mocks) => {
_mocks.fetch
.mockImplementationOnce((url) => {
if (url.includes('/decide/?v=3')) {
return Promise.resolve({
status: 200,
text: () => Promise.resolve('ok'),
json: () =>
Promise.resolve({
featureFlags: createMockFeatureFlags(),
}),
})
}
return errorAPIResponse
})
.mockImplementationOnce((url) => {
if (url.includes('/decide/?v=3')) {
return Promise.resolve({
status: 200,
text: () => Promise.resolve('ok'),
json: () =>
Promise.resolve({
featureFlags: { 'x-flag': 'x-value', 'feature-1': false },
errorsWhileComputingFlags: true,
}),
})
}

return errorAPIResponse
})
.mockImplementation(() => {
return errorAPIResponse
})
})

jest.runOnlyPendingTimers() // trigger init setImmediate
})

it('should return combined results', async () => {
expect(mocks.fetch).toHaveBeenCalledWith('https://app.posthog.com/decide/?v=3', {
body: JSON.stringify({
token: 'TEST_API_KEY',
distinct_id: posthog.getDistinctId(),
$anon_distinct_id: posthog.getAnonymousId(),
groups: {},
person_properties: {},
group_properties: {},
}),
method: 'POST',
headers: {
'Content-Type': 'application/json',
},
signal: expect.anything(),
})

expect(posthog.getFeatureFlags()).toEqual({
'feature-1': true,
'feature-2': true,
'feature-variant': 'variant',
})

// now second call to feature flags
await posthog.reloadFeatureFlagsAsync(false)

expect(mocks.fetch).toHaveBeenCalledWith('https://app.posthog.com/decide/?v=3', {
body: JSON.stringify({
token: 'TEST_API_KEY',
distinct_id: posthog.getDistinctId(),
$anon_distinct_id: posthog.getAnonymousId(),
groups: {},
person_properties: {},
group_properties: {},
}),
method: 'POST',
headers: {
'Content-Type': 'application/json',
},
signal: expect.anything(),
})

expect(posthog.getFeatureFlags()).toEqual({
'feature-1': false,
'feature-2': true,
'feature-variant': 'variant',
'x-flag': 'x-value',
})

expect(posthog.getFeatureFlag('feature-1')).toEqual(false)
expect(posthog.getFeatureFlag('feature-variant')).toEqual('variant')
expect(posthog.getFeatureFlag('feature-missing')).toEqual(false)
expect(posthog.getFeatureFlag('x-flag')).toEqual('x-value')

expect(posthog.isFeatureEnabled('feature-1')).toEqual(false)
expect(posthog.isFeatureEnabled('feature-variant')).toEqual(true)
expect(posthog.isFeatureEnabled('feature-missing')).toEqual(false)
expect(posthog.isFeatureEnabled('x-flag')).toEqual(true)
})
})

describe('when subsequent decide calls return results without errors', () => {
beforeEach(() => {
;[posthog, mocks] = createTestClient('TEST_API_KEY', { flushAt: 1 }, (_mocks) => {
_mocks.fetch
.mockImplementationOnce((url) => {
if (url.includes('/decide/?v=3')) {
return Promise.resolve({
status: 200,
text: () => Promise.resolve('ok'),
json: () =>
Promise.resolve({
featureFlags: createMockFeatureFlags(),
}),
})
}
return errorAPIResponse
})
.mockImplementationOnce((url) => {
if (url.includes('/decide/?v=3')) {
return Promise.resolve({
status: 200,
text: () => Promise.resolve('ok'),
json: () =>
Promise.resolve({
featureFlags: { 'x-flag': 'x-value', 'feature-1': false },
errorsWhileComputingFlags: false,
}),
})
}

return errorAPIResponse
})
.mockImplementation(() => {
return errorAPIResponse
})
})

jest.runOnlyPendingTimers() // trigger init setImmediate
})

it('should return only latest results', async () => {
expect(mocks.fetch).toHaveBeenCalledWith('https://app.posthog.com/decide/?v=3', {
body: JSON.stringify({
token: 'TEST_API_KEY',
distinct_id: posthog.getDistinctId(),
$anon_distinct_id: posthog.getAnonymousId(),
groups: {},
person_properties: {},
group_properties: {},
}),
method: 'POST',
headers: {
'Content-Type': 'application/json',
},
signal: expect.anything(),
})

expect(posthog.getFeatureFlags()).toEqual({
'feature-1': true,
'feature-2': true,
'feature-variant': 'variant',
})

// now second call to feature flags
await posthog.reloadFeatureFlagsAsync(false)

expect(mocks.fetch).toHaveBeenCalledWith('https://app.posthog.com/decide/?v=3', {
body: JSON.stringify({
token: 'TEST_API_KEY',
distinct_id: posthog.getDistinctId(),
$anon_distinct_id: posthog.getAnonymousId(),
groups: {},
person_properties: {},
group_properties: {},
}),
method: 'POST',
headers: {
'Content-Type': 'application/json',
},
signal: expect.anything(),
})

expect(posthog.getFeatureFlags()).toEqual({
'feature-1': false,
'x-flag': 'x-value',
})

expect(posthog.getFeatureFlag('feature-1')).toEqual(false)
expect(posthog.getFeatureFlag('feature-variant')).toEqual(false)
expect(posthog.getFeatureFlag('feature-missing')).toEqual(false)
expect(posthog.getFeatureFlag('x-flag')).toEqual('x-value')

expect(posthog.isFeatureEnabled('feature-1')).toEqual(false)
expect(posthog.isFeatureEnabled('feature-variant')).toEqual(false)
expect(posthog.isFeatureEnabled('feature-missing')).toEqual(false)
expect(posthog.isFeatureEnabled('x-flag')).toEqual(true)
})
})

it('should return the boolean value of a flag', async () => {
expect(posthog.isFeatureEnabled('feature-1')).toEqual(true)
expect(posthog.isFeatureEnabled('feature-variant')).toEqual(true)
Expand Down Expand Up @@ -280,7 +500,7 @@ describe('PostHog Core', () => {
})

it('should load new feature flags', async () => {
expect(mocks.fetch).toHaveBeenCalledWith('https://app.posthog.com/decide/?v=2', {
expect(mocks.fetch).toHaveBeenCalledWith('https://app.posthog.com/decide/?v=3', {
body: JSON.stringify({
token: 'TEST_API_KEY',
distinct_id: posthog.getDistinctId(),
Expand Down
7 changes: 7 additions & 0 deletions posthog-node/src/feature-flags.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,13 @@ class FeatureFlagsPoller {
`Your personalApiKey is invalid. Are you sure you're not using your Project API key? More information: https://posthog.com/docs/api/overview`
)
}

if (res && res.status !== 200) {
// something else went wrong, or the server is down.
// In this case, don't override existing flags
return
}

const responseJson = await res.json()
if (!('flags' in responseJson)) {
console.error(`Invalid response when getting feature flags: ${JSON.stringify(responseJson)}`)
Expand Down
6 changes: 3 additions & 3 deletions posthog-node/test/feature-flags.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ export const anyLocalEvalCall = [
'http://example.com/api/feature_flag/local_evaluation?token=TEST_API_KEY',
expect.any(Object),
]
export const anyDecideCall = ['http://example.com/decide/?v=2', expect.any(Object)]
export const anyDecideCall = ['http://example.com/decide/?v=3', expect.any(Object)]

describe('local evaluation', () => {
let posthog: PostHog
Expand Down Expand Up @@ -325,7 +325,7 @@ describe('local evaluation', () => {
})
).toEqual('decide-fallback-value')
expect(mockedFetch).toHaveBeenCalledWith(
'http://example.com/decide/?v=2',
'http://example.com/decide/?v=3',
expect.objectContaining({
body: JSON.stringify({
token: 'TEST_API_KEY',
Expand All @@ -343,7 +343,7 @@ describe('local evaluation', () => {
await posthog.getFeatureFlag('complex-flag', 'some-distinct-id', { personProperties: { doesnt_matter: '1' } })
).toEqual('decide-fallback-value')
expect(mockedFetch).toHaveBeenCalledWith(
'http://example.com/decide/?v=2',
'http://example.com/decide/?v=3',
expect.objectContaining({
body: JSON.stringify({
token: 'TEST_API_KEY',
Expand Down
2 changes: 1 addition & 1 deletion posthog-node/test/posthog-node.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ describe('PostHog Node.js', () => {
})

expect(mockedFetch).toHaveBeenCalledWith(
'http://example.com/decide/?v=2',
'http://example.com/decide/?v=3',
expect.objectContaining({ method: 'POST' })
)

Expand Down

0 comments on commit ef11d41

Please sign in to comment.