Skip to content

Commit

Permalink
πŸ› fix frustration animation in session replay (#1999)
Browse files Browse the repository at this point in the history
* πŸ› fix frustration animation in session replay

Frustration records used to reference MouseInteraction/Click records.
Since Click Actions are now generated based on mouseup events, we broke
this reference. There is no MouseInteraction/PointerUp record that
Frustration can reference.

This commit is a quick fix. It changes how we are generating
MouseInteraction/MouseUp records: instead of listening to mouseup DOM
events, we now listen to pointerup events, allowing Frustration records
to reference MouseInteraction/MouseUp records.

This works on the player side without any modification.

In the context of supporting Mobile Session Replay, we introduced
`PointerInteraction` records used by the Mobile SDKs in place of
`MouseInteraction`. In the future, it would be great to replace
`MouseInteraction` by `PointerInteraction` in the Browser SDK so we have
an uniform way to convey such interaction. This would cleanly solve the
issue since we would have `PointerInteraction/Up` records that we could
reference from `Frustration` records.

* βœ… adjust unit tests
  • Loading branch information
BenoitZugmeyer authored Feb 7, 2023
1 parent 81296f9 commit 4fd5ebe
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 12 deletions.
22 changes: 21 additions & 1 deletion packages/rum/src/domain/record/observers.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import type {
MousemoveCallBack,
} from './observers'
import {
getRecordIdForEvent,
initStyleSheetObserver,
initFrustrationObserver,
initInputObserver,
Expand Down Expand Up @@ -134,7 +135,7 @@ describe('initFrustrationObserver', () => {
if (isIE()) {
pending('IE not supported')
}
mouseEvent = new MouseEvent('click')
mouseEvent = new MouseEvent('pointerup')
frustrationsCallbackSpy = jasmine.createSpy()

rumData = {
Expand Down Expand Up @@ -169,6 +170,7 @@ describe('initFrustrationObserver', () => {
expect(frustrationRecord.type).toEqual(RecordType.FrustrationRecord)
expect(frustrationRecord.timestamp).toEqual(rumData.rawRumEvent.date)
expect(frustrationRecord.data.frustrationTypes).toEqual(rumData.rawRumEvent.action.frustration!.type)
expect(frustrationRecord.data.recordIds).toEqual([getRecordIdForEvent(mouseEvent)])
})

it('ignores events other than click actions', () => {
Expand Down Expand Up @@ -391,6 +393,24 @@ describe('initMouseInteractionObserver', () => {
})
})

it('should generate mouseup record on pointerup DOM event', () => {
const pointerupEvent = createNewEvent('pointerup', { clientX: 1, clientY: 2 })
a.dispatchEvent(pointerupEvent)

expect(mouseInteractionCallbackSpy).toHaveBeenCalledWith({
id: getRecordIdForEvent(pointerupEvent),
type: RecordType.IncrementalSnapshot,
timestamp: jasmine.any(Number),
data: {
source: IncrementalSource.MouseInteraction,
type: MouseInteractionType.MouseUp,
id: jasmine.any(Number),
x: jasmine.any(Number),
y: jasmine.any(Number),
},
})
})

it('should not generate click record if x/y are missing', () => {
const clickEvent = createNewEvent('click')
a.dispatchEvent(clickEvent)
Expand Down
13 changes: 11 additions & 2 deletions packages/rum/src/domain/record/observers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ const VISUAL_VIEWPORT_OBSERVER_THRESHOLD = 200
const recordIds = new WeakMap<Event, number>()
let nextId = 1

function getRecordIdForEvent(event: Event): number {
export function getRecordIdForEvent(event: Event): number {
if (!recordIds.has(event)) {
recordIds.set(event, nextId++)
}
Expand Down Expand Up @@ -184,7 +184,16 @@ export function initMoveObserver(cb: MousemoveCallBack): ListenerHandler {
}

const eventTypeToMouseInteraction = {
[DOM_EVENT.MOUSE_UP]: MouseInteractionType.MouseUp,
// Listen for pointerup DOM events instead of mouseup for MouseInteraction/MouseUp records. This
// allows to reference such records from Frustration records.
//
// In the context of supporting Mobile Session Replay, we introduced `PointerInteraction` records
// used by the Mobile SDKs in place of `MouseInteraction`. In the future, we should replace
// `MouseInteraction` by `PointerInteraction` in the Browser SDK so we have an uniform way to
// convey such interaction. This would cleanly solve the issue since we would have
// `PointerInteraction/Up` records that we could reference from `Frustration` records.
[DOM_EVENT.POINTER_UP]: MouseInteractionType.MouseUp,

[DOM_EVENT.MOUSE_DOWN]: MouseInteractionType.MouseDown,
[DOM_EVENT.CLICK]: MouseInteractionType.Click,
[DOM_EVENT.CONTEXT_MENU]: MouseInteractionType.ContextMenu,
Expand Down
18 changes: 9 additions & 9 deletions test/e2e/scenario/recorder/recorder.scenario.ts
Original file line number Diff line number Diff line change
Expand Up @@ -719,7 +719,7 @@ describe('recorder', () => {

describe('frustration records', () => {
createTest('should detect a dead click and match it to mouse interaction record')
.withRum({ trackFrustrations: true })
.withRum({ trackFrustrations: true, enableExperimentalFeatures: ['dead_click_fixes'] })
.withRumInit(initRumAndStartRecording)
.withSetup(bundleSetup)
.run(async ({ serverEvents }) => {
Expand All @@ -730,20 +730,20 @@ describe('recorder', () => {
expect(serverEvents.sessionReplay.length).toBe(1)
const { segment } = serverEvents.sessionReplay[0]

const clickRecords = findMouseInteractionRecords(segment.data, MouseInteractionType.Click)
const mouseupRecords = findMouseInteractionRecords(segment.data, MouseInteractionType.MouseUp)
const frustrationRecords = findAllFrustrationRecords(segment.data)

expect(clickRecords.length).toBe(1)
expect(clickRecords[0].id).toBeTruthy('mouse interaction record should have an id')
expect(mouseupRecords.length).toBe(1)
expect(mouseupRecords[0].id).toBeTruthy('mouse interaction record should have an id')
expect(frustrationRecords.length).toBe(1)
expect(frustrationRecords[0].data).toEqual({
frustrationTypes: [FrustrationType.DEAD_CLICK],
recordIds: [clickRecords[0].id!],
recordIds: [mouseupRecords[0].id!],
})
})

createTest('should detect a rage click and match it to mouse interaction records')
.withRum({ trackFrustrations: true })
.withRum({ trackFrustrations: true, enableExperimentalFeatures: ['dead_click_fixes'] })
.withRumInit(initRumAndStartRecording)
.withSetup(bundleSetup)
.withBody(
Expand All @@ -763,14 +763,14 @@ describe('recorder', () => {
expect(serverEvents.sessionReplay.length).toBe(1)
const { segment } = serverEvents.sessionReplay[0]

const clickRecords = findMouseInteractionRecords(segment.data, MouseInteractionType.Click)
const mouseupRecords = findMouseInteractionRecords(segment.data, MouseInteractionType.MouseUp)
const frustrationRecords = findAllFrustrationRecords(segment.data)

expect(clickRecords.length).toBe(4)
expect(mouseupRecords.length).toBe(4)
expect(frustrationRecords.length).toBe(1)
expect(frustrationRecords[0].data).toEqual({
frustrationTypes: [FrustrationType.RAGE_CLICK],
recordIds: clickRecords.map((r) => r.id!),
recordIds: mouseupRecords.map((r) => r.id!),
})
})
})
Expand Down

0 comments on commit 4fd5ebe

Please sign in to comment.