Skip to content

Commit

Permalink
fix: do not start recording buffer without id (#1215)
Browse files Browse the repository at this point in the history
  • Loading branch information
@pauldambra
pauldambra authored Jun 3, 2024
1 parent ff555b3 commit 6d384a0
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 23 deletions.
61 changes: 46 additions & 15 deletions src/__tests__/extensions/replay/sessionrecording.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,11 @@ describe('SessionRecording', () => {
sessionRecording.startIfEnabledOrStop()
expect(loadScript).toHaveBeenCalled()
expect(sessionRecording['status']).toBe('buffering')
expect(sessionRecording['buffer']).toEqual(EMPTY_BUFFER)
expect(sessionRecording['buffer']).toEqual({
...EMPTY_BUFFER,
sessionId: sessionId,
windowId: 'windowId',
})

const metaSnapshot = createMetaSnapshot({ data: { href: 'https://example.com' } })
_emit(metaSnapshot)
Expand All @@ -290,7 +294,11 @@ describe('SessionRecording', () => {
sessionRecording.startIfEnabledOrStop()
expect(loadScript).toHaveBeenCalled()
expect(sessionRecording['status']).toBe('buffering')
expect(sessionRecording['buffer']).toEqual(EMPTY_BUFFER)
expect(sessionRecording['buffer']).toEqual({
...EMPTY_BUFFER,
sessionId: sessionId,
windowId: 'windowId',
})

const fullSnapshot = createFullSnapshot()
_emit(fullSnapshot)
Expand All @@ -306,7 +314,11 @@ describe('SessionRecording', () => {
sessionRecording.startIfEnabledOrStop()
expect(loadScript).toHaveBeenCalled()
expect(sessionRecording['status']).toBe('buffering')
expect(sessionRecording['buffer']).toEqual(EMPTY_BUFFER)
expect(sessionRecording['buffer']).toEqual({
...EMPTY_BUFFER,
sessionId: sessionId,
windowId: 'windowId',
})

const incrementalSnapshot = createIncrementalSnapshot({ data: { source: 1 } })
_emit(incrementalSnapshot)
Expand All @@ -322,7 +334,11 @@ describe('SessionRecording', () => {
sessionRecording.startIfEnabledOrStop()
expect(loadScript).toHaveBeenCalled()
expect(sessionRecording['status']).toBe('buffering')
expect(sessionRecording['buffer']).toEqual(EMPTY_BUFFER)
expect(sessionRecording['buffer']).toEqual({
...EMPTY_BUFFER,
sessionId: sessionId,
windowId: 'windowId',
})

const incrementalSnapshot = createIncrementalSnapshot({ data: { source: 1 } })
_emit(incrementalSnapshot)
Expand Down Expand Up @@ -717,7 +733,7 @@ describe('SessionRecording', () => {
sessionRecording.afterDecideResponse(makeDecideResponse({ sessionRecording: { endpoint: '/s/' } }))
sessionRecording.startIfEnabledOrStop()

expect(sessionRecording['buffer']?.sessionId).toEqual(null)
expect(sessionRecording['buffer']?.sessionId).toEqual(sessionId)

_emit(createIncrementalSnapshot({ emit: 1 }))

Expand Down Expand Up @@ -883,6 +899,11 @@ describe('SessionRecording', () => {
it('does not send a full snapshot if there is not a new session or window id', () => {
assignableWindow.rrweb.record.takeFullSnapshot.mockClear()

// 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
sessionRecording['_fullSnapshotTimer'] = 1

sessionIdGeneratorMock.mockImplementation(() => 'old-session-id')
windowIdGeneratorMock.mockImplementation(() => 'old-window-id')
sessionManager.resetSessionId()
Expand Down Expand Up @@ -1153,9 +1174,9 @@ describe('SessionRecording', () => {
// the buffer starts out empty
expect(sessionRecording['buffer']).toEqual({
data: [],
sessionId: null,
sessionId: sessionId,
size: 0,
windowId: null,
windowId: 'windowId',
})

// options will have been emitted
Expand All @@ -1164,22 +1185,32 @@ describe('SessionRecording', () => {
})

it('does not emit when idle', () => {
const emptyBuffer = {
...EMPTY_BUFFER,
sessionId: sessionId,
windowId: 'windowId',
}

// force idle state
sessionRecording['isIdle'] = true
// buffer is empty
expect(sessionRecording['buffer']).toEqual(EMPTY_BUFFER)
expect(sessionRecording['buffer']).toEqual(emptyBuffer)
// a plugin event doesn't count as returning from idle
sessionRecording.onRRwebEmit(createPluginSnapshot({}) as unknown as eventWithTime)

// buffer is still empty
expect(sessionRecording['buffer']).toEqual(EMPTY_BUFFER)
expect(sessionRecording['buffer']).toEqual(emptyBuffer)
})

it('emits custom events even when idle', () => {
// force idle state
sessionRecording['isIdle'] = true
// buffer is empty
expect(sessionRecording['buffer']).toEqual(EMPTY_BUFFER)
expect(sessionRecording['buffer']).toEqual({
...EMPTY_BUFFER,
sessionId: sessionId,
windowId: 'windowId',
})

sessionRecording.onRRwebEmit(createCustomSnapshot({}) as unknown as eventWithTime)

Expand All @@ -1194,9 +1225,9 @@ describe('SessionRecording', () => {
type: 5,
},
],
sessionId: null,
sessionId: sessionId,
size: 47,
windowId: null,
windowId: 'windowId',
})
})

Expand Down Expand Up @@ -1609,12 +1640,12 @@ describe('SessionRecording', () => {
expect(sessionRecording['_fullSnapshotTimer']).toBe(undefined)
})

it('schedules a snapshot on start', () => {
it('does not schedule a snapshot on start', () => {
sessionRecording.startIfEnabledOrStop()
expect(sessionRecording['_fullSnapshotTimer']).not.toBe(undefined)
expect(sessionRecording['_fullSnapshotTimer']).toBe(undefined)
})

it('reschedules a snapshot, when we take a full snapshot', () => {
it('schedules a snapshot, when we take a full snapshot', () => {
sessionRecording.startIfEnabledOrStop()
const startTimer = sessionRecording['_fullSnapshotTimer']

Expand Down
16 changes: 8 additions & 8 deletions src/extensions/replay/sessionrecording.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,8 +129,8 @@ export class SessionRecording {

private _linkedFlagSeen: boolean = false
private _lastActivityTimestamp: number = Date.now()
private windowId: string | null = null
private sessionId: string | null = null
private windowId: string
private sessionId: string
private _linkedFlag: string | FlagVariant | null = null

private _fullSnapshotTimer?: ReturnType<typeof setInterval>
Expand Down Expand Up @@ -281,6 +281,11 @@ export class SessionRecording {
throw new Error(LOGGER_PREFIX + ' started without valid sessionManager. This is a bug.')
}

// we know there's a sessionManager, so don't need to start without a session id
const { sessionId, windowId } = this.sessionManager.checkAndGetSessionAndWindowId()
this.sessionId = sessionId
this.windowId = windowId

this.buffer = this.clearBuffer()

// on reload there might be an already sampled session that should be continued before decide response,
Expand Down Expand Up @@ -554,7 +559,7 @@ export class SessionRecording {
if (
returningFromIdle ||
([FULL_SNAPSHOT_EVENT_TYPE, META_EVENT_TYPE].indexOf(event.type) === -1 &&
(windowIdChanged || sessionIdChanged))
(windowIdChanged || sessionIdChanged || isUndefined(this._fullSnapshotTimer)))
) {
this._tryTakeFullSnapshot()
}
Expand Down Expand Up @@ -645,11 +650,6 @@ export class SessionRecording {
},
})

// rrweb takes a snapshot on initialization,
// we want to take one in five minutes
// if nothing else happens to reset the timer
this._scheduleFullSnapshot()

const activePlugins = this._gatherRRWebPlugins()
this.stopRrweb = this.rrwebRecord({
emit: (event) => {
Expand Down

0 comments on commit 6d384a0

Please sign in to comment.