Skip to content

Commit

Permalink
fix: Resume Page Focus Now Checks Session State (#961)
Browse files Browse the repository at this point in the history
  • Loading branch information
metal-messiah authored Apr 10, 2024
1 parent 9f85f6c commit e48af6b
Show file tree
Hide file tree
Showing 8 changed files with 94 additions and 63 deletions.
3 changes: 2 additions & 1 deletion src/common/session/session-entity.js
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,8 @@ export class SessionEntity {
this.write(getModeledObject(this.state, model))
},
ee: this.ee,
refreshEvents: ['click', 'keydown', 'scroll']
refreshEvents: ['click', 'keydown', 'scroll'],
readStorage: () => this.storage.get(this.lookupKey)
}, this.state.inactiveAt - Date.now())
} else {
this.state.inactiveAt = Infinity
Expand Down
19 changes: 17 additions & 2 deletions src/common/timer/interaction-timer.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ export class InteractionTimer extends Timer {
this.onRefresh = typeof opts.onRefresh === 'function' ? opts.onRefresh : () => { /* noop */ }
this.onResume = typeof opts.onResume === 'function' ? opts.onResume : () => { /* noop */ }

/** used to double-check LS state at resume time */
this.readStorage = opts.readStorage

// used by pause/resume
this.remainingMs = undefined

Expand Down Expand Up @@ -67,8 +70,20 @@ export class InteractionTimer extends Timer {
}

resume () {
this.refresh()
this.onResume() // emit resume event after state updated
try {
const lsData = this.readStorage()
const obj = typeof lsData === 'string' ? JSON.parse(lsData) : lsData
if (isExpired(obj.expiresAt) || isExpired(obj.inactiveAt)) this.end()
else {
this.refresh()
this.onResume() // emit resume event after state updated
}
} catch (err) {
this.end()
}
function isExpired (timestamp) {
return Date.now() > timestamp
}
}

refresh (cb, ms) {
Expand Down
1 change: 0 additions & 1 deletion src/features/session_replay/aggregate/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ export class Aggregate extends AggregateBase {

// The SessionEntity class can emit a message indicating the session was cleared and reset (expiry, inactivity). This feature must abort and never resume if that occurs.
this.ee.on(SESSION_EVENTS.RESET, () => {
this.scheduler.runHarvest()
this.abort(ABORT_REASONS.RESET)
})

Expand Down
31 changes: 28 additions & 3 deletions tests/components/interaction-timer.test.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
import { InteractionTimer } from '../../src/common/timer/interaction-timer'
import { LocalMemory, model } from './session-helpers'
import { PREFIX } from '../../src/common/session/constants'

jest.useFakeTimers()

let now
const key = 'test_key'
const value = 'test_value'

beforeEach(() => {
now = Date.now()
Expand Down Expand Up @@ -133,7 +137,8 @@ describe('pause()', () => {

describe('resume()', () => {
test('resume allows the callback continue firing', () => {
const timer = new InteractionTimer({ onEnd: jest.fn() }, 100)
const storage = new LocalMemory({ [`${PREFIX}_${key}`]: { ...model, value, expiresAt: now + 5000, inactiveAt: Infinity, updatedAt: now } })
const timer = new InteractionTimer({ onEnd: jest.fn(), readStorage: () => storage.get(`${PREFIX}_${key}`) }, 100)
expect(timer.onEnd).toHaveBeenCalledTimes(0)
timer.pause()
jest.advanceTimersByTime(150)
Expand All @@ -143,11 +148,31 @@ describe('resume()', () => {
expect(timer.onEnd).toHaveBeenCalledTimes(1)
})

test('resume fires the refresh callback', () => {
const timer = new InteractionTimer({ onEnd: jest.fn(), onRefresh: jest.fn() }, 100)
test('resume fires the refresh callback if session is valid', () => {
const storage = new LocalMemory({ [`${PREFIX}_${key}`]: { ...model, value, expiresAt: now + 5000, inactiveAt: Infinity, updatedAt: now } })
const timer = new InteractionTimer({ onEnd: jest.fn(), onRefresh: jest.fn(), readStorage: () => storage.get(`${PREFIX}_${key}`) }, 100)
timer.pause()
timer.resume()
expect(timer.onRefresh).toHaveBeenCalledTimes(1)
expect(timer.onEnd).toHaveBeenCalledTimes(0)
})

test('resume calls onEnd if storage is already invalid on resume -- expired', () => {
const storage = new LocalMemory({ [`${PREFIX}_${key}`]: { ...model, value, expiresAt: now - 5000, inactiveAt: Infinity, updatedAt: now } })
const timer = new InteractionTimer({ onEnd: jest.fn(), onRefresh: jest.fn(), readStorage: () => storage.get(`${PREFIX}_${key}`) }, 100)
timer.pause()
timer.resume()
expect(timer.onRefresh).toHaveBeenCalledTimes(0)
expect(timer.onEnd).toHaveBeenCalledTimes(1)
})

test('resume calls onEnd if storage is already invalid on resume -- inactive', () => {
const storage = new LocalMemory({ [`${PREFIX}_${key}`]: { ...model, value, expiresAt: now + 5000, inactiveAt: now - 5000, updatedAt: now } })
const timer = new InteractionTimer({ onEnd: jest.fn(), onRefresh: jest.fn(), readStorage: () => storage.get(`${PREFIX}_${key}`) }, 100)
timer.pause()
timer.resume()
expect(timer.onRefresh).toHaveBeenCalledTimes(0)
expect(timer.onEnd).toHaveBeenCalledTimes(1)
})
})

Expand Down
43 changes: 1 addition & 42 deletions tests/components/session-entity.test.js
Original file line number Diff line number Diff line change
@@ -1,51 +1,10 @@
import { PREFIX } from '../../src/common/session/constants'
import { SessionEntity } from '../../src/common/session/session-entity'
import { LocalMemory, model } from './session-helpers'

const agentIdentifier = 'test_agent_identifier'
const key = 'test_key'
const value = 'test_value'
class LocalMemory {
constructor (initialState = {}) {
this.state = initialState
}

get (key) {
try {
return this.state[key]
} catch (err) {
// Error is ignored
return ''
}
}

set (key, value) {
try {
if (value === undefined || value === null) return this.remove(key)
this.state[key] = value
} catch (err) {
// Error is ignored
}
}

remove (key) {
try {
delete this.state[key]
} catch (err) {
// Error is ignored
}
}
}
const model = {
value: '',
inactiveAt: 0,
expiresAt: 0,
updatedAt: Date.now(),
sessionReplayMode: 0,
sessionReplaySentFirstChunk: false,
sessionTraceMode: 0,
traceHarvestStarted: false,
custom: {}
}

jest.mock('../../src/common/timer/timer')
jest.mock('../../src/common/timer/interaction-timer')
Expand Down
42 changes: 42 additions & 0 deletions tests/components/session-helpers.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
export class LocalMemory {
constructor (initialState = {}) {
this.state = initialState
}

get (key) {
try {
return this.state[key]
} catch (err) {
// Error is ignored
return ''
}
}

set (key, value) {
try {
if (value === undefined || value === null) return this.remove(key)
this.state[key] = value
} catch (err) {
// Error is ignored
}
}

remove (key) {
try {
delete this.state[key]
} catch (err) {
// Error is ignored
}
}
}
export const model = {
value: '',
inactiveAt: 0,
expiresAt: 0,
updatedAt: Date.now(),
sessionReplayMode: 0,
sessionReplaySentFirstChunk: false,
sessionTraceMode: 0,
traceHarvestStarted: false,
custom: {}
}
2 changes: 1 addition & 1 deletion tests/specs/session-replay/mode.e2e.js
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ describe.withBrowsersMatching(notIE)('Session Replay Sample Mode Validation', ()

it('ERROR (seen before init) => ERROR', async () => {
await browser.url(await browser.testHandle.assetURL('rrweb-split-errors.html', config({ session_replay: { preload: false, sampling_rate: 0, error_sampling_rate: 100 } })))
.then(() => browser.waitForFeatureAggregate('session_replay'))
.then(() => browser.waitForSessionReplayRecording('session_replay'))

await expect(getSR()).resolves.toMatchObject({
recording: true,
Expand Down
16 changes: 3 additions & 13 deletions tests/specs/session-replay/session-state.e2e.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ describe.withBrowsersMatching(notIE)('session manager state behavior', () => {
})

describe('When session ends', () => {
it('should end recording and unload', async () => {
await browser.url(await browser.testHandle.assetURL('instrumented.html', config({ session: { expiresMs: 7500 }, session_replay: { harvestTimeSeconds: 10 } })))
it('should end recording but not unload', async () => {
await browser.url(await browser.testHandle.assetURL('instrumented.html', config({ session: { expiresMs: 5000 }, session_replay: { harvestTimeSeconds: 10 } })))
.then(() => browser.waitForSessionReplayRecording())

// session has started, replay should have set mode to "FULL"
Expand All @@ -53,16 +53,11 @@ describe.withBrowsersMatching(notIE)('session manager state behavior', () => {
expect(oldSessionClass.sessionReplayMode).toEqual(MODE.FULL)

await Promise.all([
browser.testHandle.expectBlob(),
browser.testHandle.expectBlob(10000, true),
browser.execute(function () {
document.querySelector('body').click()
})
])

// session has ended, replay should have set mode to "OFF"
const { agentSessions: newSession } = await browser.getAgentSessionInfo()
const newSessionClass = Object.values(newSession)[0]
expect(newSessionClass.sessionReplayMode).toEqual(MODE.OFF)
})
})

Expand All @@ -78,11 +73,6 @@ describe.withBrowsersMatching(notIE)('session manager state behavior', () => {
// type 2 payloads are snapshots
expect(payload.body.filter(x => x.type === RRWEB_EVENT_TYPES.FullSnapshot).length).toEqual(1)

/** This should fire when the tab changes, it's easier to stage it this way before hand, and allows for the super early staging for the next expect */
browser.testHandle.expectBlob(15000).then(({ request: page1UnloadContents }) => {
testExpectedReplay({ data: page1UnloadContents, session: localStorage.value, hasError: false, hasMeta: false, hasSnapshot: false, isFirstChunk: false })
})

/** This is scoped out this way to guarantee we have it staged in time since preload can harvest super early, sometimes earlier than wdio can expect normally */
/** see next `testExpectedReplay` */
browser.testHandle.expectBlob(15000).then(async ({ request: page2Contents }) => {
Expand Down

0 comments on commit e48af6b

Please sign in to comment.