Skip to content

Commit

Permalink
fix: flushing the buffer for debug signal while idle extends session …
Browse files Browse the repository at this point in the history
…activity (#1396)

* do not schedule flush buffer timer while idle

* obey the typechecker

* don't flush while idle even if over buffer size

* remove duplicate test

* a little more clear
  • Loading branch information
pauldambra authored Sep 2, 2024
1 parent 0f9a413 commit ac73539
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 6 deletions.
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

0 comments on commit ac73539

Please sign in to comment.