Skip to content

Commit 91f0965

Browse files
Kim DuganKim Dugan
authored andcommitted
replay: wait for new config before recording decision
1 parent 755c2d7 commit 91f0965

13 files changed

+107
-42
lines changed

.changeset/tiny-glasses-pump.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'posthog-js': patch
3+
---
4+
5+
wait for fresh config before recording start decision

packages/browser/playwright/mocked/session-recording/lazy-session-recording-sampling.spec.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { expect, test, WindowWithPostHog } from '../utils/posthog-playwright-test-base'
2-
import { start } from '../utils/setup'
2+
import { start, waitForSessionRecordingToStart } from '../utils/setup'
33

44
const startOptions = {
55
options: {
@@ -34,6 +34,7 @@ test.describe('Session recording - sampling', () => {
3434
await start(startOptions, page, context)
3535
},
3636
})
37+
await waitForSessionRecordingToStart(page)
3738

3839
await page.expectCapturedEventsToBe(['$pageview'])
3940
await page.resetCapturedEvents()

packages/browser/playwright/mocked/session-recording/lazy-session-recording.spec.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { expect, test, WindowWithPostHog } from '../utils/posthog-playwright-test-base'
2-
import { start } from '../utils/setup'
2+
import { start, waitForSessionRecordingToStart } from '../utils/setup'
33
import { Page } from '@playwright/test'
44
import { isUndefined } from '@posthog/core'
55

@@ -99,6 +99,7 @@ test.describe('Session recording - array.js', () => {
9999
await start(startOptions, page, context)
100100
},
101101
})
102+
await waitForSessionRecordingToStart(page)
102103
await page.expectCapturedEventsToBe(['$pageview'])
103104
await page.resetCapturedEvents()
104105
})

packages/browser/playwright/mocked/session-recording/session-linking.spec.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { expect, test, WindowWithPostHog } from '../utils/posthog-playwright-test-base'
2-
import { start } from '../utils/setup'
2+
import { start, waitForSessionRecordingToStart } from '../utils/setup'
33

44
const startOptions = {
55
options: {
@@ -25,6 +25,7 @@ test.describe('Session Recording - Session Linking', () => {
2525
await start(startOptions, page, context)
2626
},
2727
})
28+
await waitForSessionRecordingToStart(page)
2829
await page.expectCapturedEventsToBe(['$pageview'])
2930
await page.resetCapturedEvents()
3031
})

packages/browser/playwright/mocked/session-recording/session-recording-idle-timeout.spec.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { expect, test, WindowWithPostHog } from '../utils/posthog-playwright-test-base'
2-
import { start } from '../utils/setup'
2+
import { start, waitForSessionRecordingToStart } from '../utils/setup'
33
import { Page } from '@playwright/test'
44

55
async function ensureRecordingIsStopped(page: Page) {
@@ -88,6 +88,7 @@ test.describe('Session recording - idle timeout behavior', () => {
8888
await start(startOptions, page, context)
8989
},
9090
})
91+
await waitForSessionRecordingToStart(page)
9192
await page.expectCapturedEventsToBe(['$pageview'])
9293
await page.resetCapturedEvents()
9394
})

packages/browser/playwright/mocked/session-recording/session-recording-network-recorder.spec.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { test, expect } from '../utils/posthog-playwright-test-base'
2-
import { start } from '../utils/setup'
2+
import { start, waitForSessionRecordingToStart } from '../utils/setup'
33
import { Page } from '@playwright/test'
44

55
test.beforeEach(async ({ context }) => {
@@ -90,6 +90,7 @@ test.beforeEach(async ({ context }) => {
9090
)
9191
},
9292
})
93+
await waitForSessionRecordingToStart(page)
9394

9495
// also wrap after posthog is loaded
9596
await page.evaluate((isBadlyBehaved) => {

packages/browser/playwright/mocked/session-recording/session-recording-sampling.spec.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { expect, test, WindowWithPostHog } from '../utils/posthog-playwright-test-base'
2-
import { start } from '../utils/setup'
2+
import { start, waitForSessionRecordingToStart } from '../utils/setup'
33

44
const startOptions = {
55
options: {
@@ -36,6 +36,7 @@ test.describe('Session recording - sampling', () => {
3636
await start(startOptions, page, context)
3737
},
3838
})
39+
await waitForSessionRecordingToStart(page)
3940

4041
await page.expectCapturedEventsToBe(['$pageview'])
4142
await page.resetCapturedEvents()

packages/browser/playwright/mocked/session-recording/session-recording.spec.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { expect, test, WindowWithPostHog } from '../utils/posthog-playwright-test-base'
2-
import { start } from '../utils/setup'
2+
import { start, waitForSessionRecordingToStart } from '../utils/setup'
33
import { Page } from '@playwright/test'
44
import { isUndefined } from '@posthog/core'
55

@@ -101,6 +101,7 @@ test.describe('Session recording - array.js', () => {
101101
await start(startOptions, page, context)
102102
},
103103
})
104+
await waitForSessionRecordingToStart(page)
104105
await page.expectCapturedEventsToBe(['$pageview'])
105106
await page.resetCapturedEvents()
106107
})

packages/browser/playwright/mocked/utils/setup.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,3 +175,17 @@ export async function start(
175175
await flagsMock
176176
}
177177
}
178+
179+
/**
180+
* Wait for session recording to actually start after remote config is received
181+
* This is needed because session recording now waits for fresh remote config before starting
182+
*/
183+
export async function waitForSessionRecordingToStart(page: Page, timeout = 5000): Promise<void> {
184+
await page.waitForFunction(
185+
() => {
186+
const ph = (window as any).posthog
187+
return ph?.sessionRecording?.started === true
188+
},
189+
{ timeout }
190+
)
191+
}

packages/browser/src/__tests__/extensions/replay/lazy-sessionrecording.test.ts

Lines changed: 2 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -3819,49 +3819,16 @@ describe('Lazy SessionRecording', () => {
38193819
})
38203820
})
38213821

3822-
describe('stale config retry on script loaded', () => {
3823-
const FIVE_MINUTES_IN_MS = 5 * 60 * 1000
3824-
3822+
describe('wait for fresh config before starting', () => {
38253823
beforeEach(() => {
38263824
addRRwebToWindow()
38273825
})
38283826

3829-
it('requests fresh config when persisted config is stale', () => {
3830-
posthog.persistence?.register({
3831-
[SESSION_RECORDING_REMOTE_CONFIG]: {
3832-
enabled: true,
3833-
endpoint: '/s/',
3834-
cache_timestamp: Date.now() - FIVE_MINUTES_IN_MS - 1000,
3835-
},
3836-
})
3837-
3838-
sessionRecording.startIfEnabledOrStop()
3839-
3840-
expect(mockRemoteConfigLoad).toHaveBeenCalledTimes(1)
3841-
expect(sessionRecording.started).toBe(false)
3842-
})
3843-
3844-
it('does not request fresh config more than once', () => {
3845-
posthog.persistence?.register({
3846-
[SESSION_RECORDING_REMOTE_CONFIG]: {
3847-
enabled: true,
3848-
endpoint: '/s/',
3849-
cache_timestamp: Date.now() - FIVE_MINUTES_IN_MS - 1000,
3850-
},
3851-
})
3852-
3853-
sessionRecording.startIfEnabledOrStop()
3854-
sessionRecording.startIfEnabledOrStop()
3855-
3856-
expect(mockRemoteConfigLoad).toHaveBeenCalledTimes(1)
3857-
})
3858-
3859-
it('starts recording after fresh config arrives', () => {
3827+
it('does not start recording until fresh config arrives', () => {
38603828
posthog.persistence?.register({
38613829
[SESSION_RECORDING_REMOTE_CONFIG]: {
38623830
enabled: true,
38633831
endpoint: '/s/',
3864-
cache_timestamp: Date.now() - FIVE_MINUTES_IN_MS - 1000,
38653832
},
38663833
})
38673834

0 commit comments

Comments
 (0)