fix(flags): Enqueue follow up requests without dropping

PostHog · Sep 14, 2023 · c0419c9 · c0419c9
1 parent 233e604
commit c0419c9
Show file tree

Hide file tree

Showing 7 changed files with 34 additions and 20 deletions.
diff --git a/functional_tests/feature-flags.test.ts b/functional_tests/feature-flags.test.ts
@@ -107,10 +107,7 @@ test('person properties set in identify() with the same distinct_id are sent to
     })
 })
 
-test('identify() doesnt trigger new request automatically if first request takes too long', async () => {
-    // TODO: Make this experience nicer, and queue requests, rather than leave
-    // it upto the user to call reloadFeatureFlags() manually.
-
+test('identify() triggers new request in queue after first request', async () => {
     const token = v4()
     const posthog = await createPosthogInstance(token, { advanced_disable_decide: false })
 
@@ -130,12 +127,28 @@ test('identify() doesnt trigger new request automatically if first request takes
     resetRequests(token)
 
     // don't wait for decide callback
-
     posthog.identify('test-id', {
-        email: 'test@email.com',
+        email: 'test2@email.com',
     })
 
     await waitFor(() => {
         expect(getRequests(token)['/decide/']).toEqual([])
     })
+
+    // wait for decide callback
+    // eslint-disable-next-line compat/compat
+    await new Promise((res) => setTimeout(res, 500))
+
+    // now second call should've fired
+    await waitFor(() => {
+        expect(getRequests(token)['/decide/']).toEqual([
+            {
+                $anon_distinct_id: anonymousId,
+                distinct_id: 'test-id',
+                groups: {},
+                person_properties: { email: '[email protected]' },
+                token,
+            },
+        ])
+    })
 })
diff --git a/src/__tests__/compression.js b/src/__tests__/compression.js
@@ -92,8 +92,8 @@ describe('Payload Compression', () => {
                 },
                 featureFlags: {
                     receivedFeatureFlags: jest.fn(),
-                    resetRequestQueue: jest.fn(),
                     setReloadingPaused: jest.fn(),
+                    _startReloadTimer: jest.fn(),
                 },
                 _hasBootstrappedFeatureFlags: jest.fn(),
                 get_property: (property_key) =>

diff --git a/src/__tests__/decide.js b/src/__tests__/decide.js
@@ -27,8 +27,8 @@ describe('Decide', () => {
         },
         featureFlags: {
             receivedFeatureFlags: jest.fn(),
-            resetRequestQueue: jest.fn(),
             setReloadingPaused: jest.fn(),
+            _startReloadTimer: jest.fn(),
         },
         _hasBootstrappedFeatureFlags: jest.fn(),
         getGroups: () => ({ organization: '5' }),

diff --git a/src/__tests__/posthog-core.js b/src/__tests__/posthog-core.js
@@ -892,7 +892,7 @@ describe('_loaded()', () => {
         capture: jest.fn(),
         featureFlags: {
             setReloadingPaused: jest.fn(),
-            resetRequestQueue: jest.fn(),
+            _startReloadTimer: jest.fn(),
         },
         _start_queue_if_opted_in: jest.fn(),
     }))

diff --git a/src/__tests__/posthog-core.loaded.js b/src/__tests__/posthog-core.loaded.js
@@ -17,7 +17,7 @@ describe('loaded() with flags', () => {
         capture: jest.fn(),
         featureFlags: {
             setReloadingPaused: jest.fn(),
-            resetRequestQueue: jest.fn(),
+            _startReloadTimer: jest.fn(),
             receivedFeatureFlags: jest.fn(),
         },
         _start_queue_if_opted_in: jest.fn(),
@@ -48,24 +48,28 @@ describe('loaded() with flags', () => {
         beforeEach(() => {
             jest.spyOn(given.lib.featureFlags, 'setGroupPropertiesForFlags')
             jest.spyOn(given.lib.featureFlags, 'setReloadingPaused')
-            jest.spyOn(given.lib.featureFlags, 'resetRequestQueue')
+            jest.spyOn(given.lib.featureFlags, '_startReloadTimer')
             jest.spyOn(given.lib.featureFlags, '_reloadFeatureFlagsRequest')
         })
 
         it('doesnt call flags while initial load is happening', () => {
             given.subject()
 
-            jest.runAllTimers()
+            jest.runOnlyPendingTimers()
 
             expect(given.lib.featureFlags.setGroupPropertiesForFlags).toHaveBeenCalled() // loaded ph.group() calls setGroupPropertiesForFlags
             expect(given.lib.featureFlags.setReloadingPaused).toHaveBeenCalledWith(true)
-            expect(given.lib.featureFlags.resetRequestQueue).toHaveBeenCalledTimes(1)
+            expect(given.lib.featureFlags._startReloadTimer).toHaveBeenCalled()
             expect(given.lib.featureFlags.setReloadingPaused).toHaveBeenCalledWith(false)
 
-            // even if the decide request returned late, we should not call _reloadFeatureFlagsRequest
+            // we should call _reloadFeatureFlagsRequest for `group` only after the initial load
             // because it ought to be paused until decide returns
             expect(given.overrides._send_request).toHaveBeenCalledTimes(1)
             expect(given.lib.featureFlags._reloadFeatureFlagsRequest).toHaveBeenCalledTimes(0)
+
+            jest.runOnlyPendingTimers()
+            expect(given.overrides._send_request).toHaveBeenCalledTimes(2)
+            expect(given.lib.featureFlags._reloadFeatureFlagsRequest).toHaveBeenCalledTimes(1)
         })
     })
 
@@ -74,7 +78,7 @@ describe('loaded() with flags', () => {
 
         expect(given.overrides.featureFlags.setReloadingPaused).toHaveBeenCalledWith(true)
         expect(given.overrides.featureFlags.setReloadingPaused).toHaveBeenCalledWith(false)
-        expect(given.overrides.featureFlags.resetRequestQueue).toHaveBeenCalledTimes(1)
+        expect(given.overrides.featureFlags._startReloadTimer).toHaveBeenCalled()
         expect(given.overrides.featureFlags.receivedFeatureFlags).toHaveBeenCalledTimes(1)
     })
 })
diff --git a/src/decide.ts b/src/decide.ts
@@ -39,8 +39,9 @@ export class Decide {
     }
 
     parseDecideResponse(response: DecideResponse): void {
-        this.instance.featureFlags.resetRequestQueue()
         this.instance.featureFlags.setReloadingPaused(false)
+        // :TRICKY: Reload - start another request if queued!
+        this.instance.featureFlags._startReloadTimer()
 
         if (response?.status === 0) {
             console.error('Failed to fetch feature flags from PostHog.')

diff --git a/src/posthog-featureflags.ts b/src/posthog-featureflags.ts
@@ -151,10 +151,6 @@ export class PostHogFeatureFlags {
         this.reloadFeatureFlagsInAction = isPaused
     }
 
-    resetRequestQueue(): void {
-        this.reloadFeatureFlagsQueued = false
-    }
-
     _startReloadTimer(): void {
         if (this.reloadFeatureFlagsQueued && !this.reloadFeatureFlagsInAction) {
             setTimeout(() => {