Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: flushing the buffer for debug signal while idle extends session activity #1396

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 35 additions & 2 deletions src/__tests__/extensions/replay/sessionrecording.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import {
EventType,
eventWithTime,
fullSnapshotEvent,
incrementalData,
incrementalSnapshotEvent,
IncrementalSource,
metaEvent,
Expand Down Expand Up @@ -84,7 +85,7 @@ const createIncrementalSnapshot = (event = {}): incrementalSnapshotEvent => ({
type: INCREMENTAL_SNAPSHOT_EVENT_TYPE,
data: {
source: 1,
},
} as Partial<incrementalData> as incrementalData,
...event,
})

Expand Down Expand Up @@ -1032,6 +1033,7 @@ describe('SessionRecording', () => {
// we always take a full snapshot when there hasn't been one
// and use _fullSnapshotTimer to track that
// we want to avoid that behavior here, so we set it to any value
// @ts-expect-error -- detected as Timeout because the test is picking up `NodeJS.Timeout` as the type not `number` as in the browser
sessionRecording['_fullSnapshotTimer'] = 1

sessionIdGeneratorMock.mockImplementation(() => 'old-session-id')
Expand Down Expand Up @@ -1362,7 +1364,7 @@ describe('SessionRecording', () => {
})
})

it('emits custom events even when idle', () => {
it('buffers custom events without capturing while idle', () => {
// force idle state
sessionRecording['isIdle'] = true
// buffer is empty
Expand All @@ -1389,6 +1391,37 @@ describe('SessionRecording', () => {
size: 47,
windowId: 'windowId',
})
emitInactiveEvent(startingTimestamp + 100, true)
expect(posthog.capture).not.toHaveBeenCalled()

expect(sessionRecording['flushBufferTimer']).toBeUndefined()
})

it('does not emit buffered custom events while idle even when over buffer max size', () => {
// force idle state
sessionRecording['isIdle'] = true
// buffer is empty
expect(sessionRecording['buffer']).toEqual({
...EMPTY_BUFFER,
sessionId: sessionId,
windowId: 'windowId',
})

// ensure buffer isn't empty
sessionRecording.onRRwebEmit(createCustomSnapshot({}) as eventWithTime)

// fake having a large buffer
// in reality we would need a very long idle period emitting custom events to reach 1MB of buffer data
// particularly since we flush the buffer on entering idle
sessionRecording['buffer'].size = RECORDING_MAX_EVENT_SIZE - 1
sessionRecording.onRRwebEmit(createCustomSnapshot({}) as eventWithTime)

// we're still idle
expect(sessionRecording['isIdle']).toBe(true)
// return from idle

// we did not capture
expect(posthog.capture).not.toHaveBeenCalled()
})

it('drops full snapshots when idle - so we must make sure not to take them while idle!', () => {
Expand Down
10 changes: 6 additions & 4 deletions src/extensions/replay/sessionrecording.ts
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,7 @@ export class SessionRecording {
} else {
this.stopRecording()
this.clearBuffer()
clearInterval(this._fullSnapshotTimer)
}
}

Expand Down Expand Up @@ -516,7 +517,7 @@ export class SessionRecording {
if (event.timestamp - this._lastActivityTimestamp > RECORDING_IDLE_ACTIVITY_TIMEOUT_MS) {
this.isIdle = true
// don't take full snapshots while idle
clearTimeout(this._fullSnapshotTimer)
clearInterval(this._fullSnapshotTimer)
// proactively flush the buffer in case the session is idle for a long time
this._flushBuffer()
}
Expand Down Expand Up @@ -893,16 +894,17 @@ export class SessionRecording {
private _captureSnapshotBuffered(properties: Properties) {
const additionalBytes = 2 + (this.buffer?.data.length || 0) // 2 bytes for the array brackets and 1 byte for each comma
if (
this.buffer.size + properties.$snapshot_bytes + additionalBytes > RECORDING_MAX_EVENT_SIZE ||
this.buffer.sessionId !== this.sessionId
!this.isIdle && // we never want to flush when idle
(this.buffer.size + properties.$snapshot_bytes + additionalBytes > RECORDING_MAX_EVENT_SIZE ||
this.buffer.sessionId !== this.sessionId)
) {
this.buffer = this._flushBuffer()
}

this.buffer.size += properties.$snapshot_bytes
this.buffer.data.push(properties.$snapshot_data)

if (!this.flushBufferTimer) {
if (!this.flushBufferTimer && !this.isIdle) {
this.flushBufferTimer = setTimeout(() => {
this._flushBuffer()
}, RECORDING_BUFFER_TIMEOUT)
Expand Down
Loading