From eea5df46f6984625f7bd3cf67a984383225c3170 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Mon, 8 May 2023 10:27:09 +0200 Subject: [PATCH 1/4] feat(replay): Capture keyboard presses for special characters --- packages/replay/src/coreHandlers/handleDom.ts | 78 +++++++------ .../src/coreHandlers/handleKeyboardEvent.ts | 64 +++++++++++ packages/replay/src/replay.ts | 5 +- .../coreHandlers/handleKeyboardEvent.test.ts | 104 ++++++++++++++++++ 4 files changed, 218 insertions(+), 33 deletions(-) create mode 100644 packages/replay/src/coreHandlers/handleKeyboardEvent.ts create mode 100644 packages/replay/test/unit/coreHandlers/handleKeyboardEvent.test.ts diff --git a/packages/replay/src/coreHandlers/handleDom.ts b/packages/replay/src/coreHandlers/handleDom.ts index f98a92725861..00d274760511 100644 --- a/packages/replay/src/coreHandlers/handleDom.ts +++ b/packages/replay/src/coreHandlers/handleDom.ts @@ -10,7 +10,7 @@ import { getAttributesToRecord } from './util/getAttributesToRecord'; export interface DomHandlerData { name: string; - event: Node | { target: Node }; + event: Node | { target: EventTarget }; } export const handleDomListener: (replay: ReplayContainer) => (handlerData: DomHandlerData) => void = @@ -29,39 +29,21 @@ export const handleDomListener: (replay: ReplayContainer) => (handlerData: DomHa addBreadcrumbEvent(replay, result); }; -/** - * An event handler to react to DOM events. - * Exported for tests only. - */ -export function handleDom(handlerData: DomHandlerData): Breadcrumb | null { - let target; - let targetNode: Node | INode | undefined; - - const isClick = handlerData.name === 'click'; - - // Accessing event.target can throw (see getsentry/raven-js#838, #768) - try { - targetNode = isClick ? getClickTargetNode(handlerData.event) : getTargetNode(handlerData.event); - target = htmlTreeAsString(targetNode, { maxStringLength: 200 }); - } catch (e) { - target = ''; - } - +/** Get the base DOM breadcrumb. */ +export function getBaseDomBreadcrumb(target: Node | INode | null, message: string): Breadcrumb { // `__sn` property is the serialized node created by rrweb - const serializedNode = - targetNode && '__sn' in targetNode && targetNode.__sn.type === NodeType.Element ? targetNode.__sn : null; + const serializedNode = target && isRrwebNode(target) && target.__sn.type === NodeType.Element ? target.__sn : null; - return createBreadcrumb({ - category: `ui.${handlerData.name}`, - message: target, + return { + message, data: serializedNode ? { nodeId: serializedNode.id, node: { id: serializedNode.id, tagName: serializedNode.tagName, - textContent: targetNode - ? Array.from(targetNode.childNodes) + textContent: target + ? Array.from(target.childNodes) .map( (node: Node | INode) => '__sn' in node && node.__sn.type === NodeType.Text && node.__sn.textContent, ) @@ -73,12 +55,46 @@ export function handleDom(handlerData: DomHandlerData): Breadcrumb | null { }, } : {}, + }; +} + +/** + * An event handler to react to DOM events. + * Exported for tests. + */ +export function handleDom(handlerData: DomHandlerData): Breadcrumb | null { + const { target, message } = getDomTarget(handlerData); + + return createBreadcrumb({ + category: `ui.${handlerData.name}`, + ...getBaseDomBreadcrumb(target, message), }); } -function getTargetNode(event: DomHandlerData['event']): Node { +function getDomTarget(handlerData: DomHandlerData): { target: Node | INode | null; message: string } { + const isClick = handlerData.name === 'click'; + + let message: string | undefined; + let target: Node | INode | null = null; + + // Accessing event.target can throw (see getsentry/raven-js#838, #768) + try { + target = isClick ? getClickTargetNode(handlerData.event) : getTargetNode(handlerData.event); + message = htmlTreeAsString(target, { maxStringLength: 200 }) || ''; + } catch (e) { + message = ''; + } + + return { target, message }; +} + +function isRrwebNode(node: EventTarget): node is INode { + return '__sn' in node; +} + +function getTargetNode(event: Node | { target: EventTarget | null }): Node | INode | null { if (isEventWithTarget(event)) { - return event.target; + return event.target as Node | null; } return event; @@ -90,7 +106,7 @@ const INTERACTIVE_SELECTOR = 'button,a'; // If so, we use this as the target instead // This is useful because if you click on the image in , // The target will be the image, not the button, which we don't want here -function getClickTargetNode(event: DomHandlerData['event']): Node { +function getClickTargetNode(event: DomHandlerData['event']): Node | INode | null { const target = getTargetNode(event); if (!target || !(target instanceof Element)) { @@ -101,6 +117,6 @@ function getClickTargetNode(event: DomHandlerData['event']): Node { return closestInteractive || target; } -function isEventWithTarget(event: unknown): event is { target: Node } { - return !!(event as { target?: Node }).target; +function isEventWithTarget(event: unknown): event is { target: EventTarget | null } { + return typeof event === 'object' && !!event && 'target' in event; } diff --git a/packages/replay/src/coreHandlers/handleKeyboardEvent.ts b/packages/replay/src/coreHandlers/handleKeyboardEvent.ts new file mode 100644 index 000000000000..e50f5e6e3ab5 --- /dev/null +++ b/packages/replay/src/coreHandlers/handleKeyboardEvent.ts @@ -0,0 +1,64 @@ +import type { Breadcrumb } from '@sentry/types'; +import { htmlTreeAsString } from '@sentry/utils'; + +import type { ReplayContainer } from '../types'; +import { createBreadcrumb } from '../util/createBreadcrumb'; +import { getBaseDomBreadcrumb } from './handleDom'; +import { addBreadcrumbEvent } from './util/addBreadcrumbEvent'; + +/** Handle keyboard events & create breadcrumbs. */ +export function handleKeyboardEvent(replay: ReplayContainer, event: KeyboardEvent): void { + if (!replay.isEnabled()) { + return; + } + + replay.triggerUserActivity(); + + const breadcrumb = getKeyboardBreadcrumb(event); + + if (!breadcrumb) { + return; + } + + addBreadcrumbEvent(replay, breadcrumb); +} + +/** exported only for tests */ +export function getKeyboardBreadcrumb(event: KeyboardEvent): Breadcrumb | null { + const { metaKey, shiftKey, ctrlKey, altKey, key, target } = event; + + // never capture for input fields + if (!target || isInputElement(target as HTMLElement)) { + return null; + } + + // Note: We do not consider shift here, as that means "uppercase" + const hasModifierKey = metaKey || ctrlKey || altKey; + const isCharacterKey = key.length === 1; // other keys like Escape, Tab, etc have a longer length + + // Do not capture breadcrumb if only a word key is pressed + // This could leak e.g. user input + if (!hasModifierKey && isCharacterKey) { + return null; + } + + const message = htmlTreeAsString(target, { maxStringLength: 200 }) || ''; + const baseBreadcrumb = getBaseDomBreadcrumb(target as Node, message); + + return createBreadcrumb({ + category: 'ui.keyDown', + message, + data: { + ...baseBreadcrumb.data, + metaKey, + shiftKey, + ctrlKey, + altKey, + key, + }, + }); +} + +function isInputElement(target: HTMLElement): boolean { + return target.tagName === 'INPUT' || target.tagName === 'TEXTAREA' || target.isContentEditable; +} diff --git a/packages/replay/src/replay.ts b/packages/replay/src/replay.ts index 3d2fe4a6d7af..8d89aa0b2653 100644 --- a/packages/replay/src/replay.ts +++ b/packages/replay/src/replay.ts @@ -11,6 +11,7 @@ import { SESSION_IDLE_PAUSE_DURATION, WINDOW, } from './constants'; +import { handleKeyboardEvent } from './coreHandlers/handleKeyboardEvent'; import { setupPerformanceObserver } from './coreHandlers/performanceObserver'; import { createEventBuffer } from './eventBuffer'; import { clearSession } from './session/clearSession'; @@ -701,8 +702,8 @@ export class ReplayContainer implements ReplayContainerInterface { }; /** Ensure page remains active when a key is pressed. */ - private _handleKeyboardEvent: (event: KeyboardEvent) => void = () => { - this.triggerUserActivity(); + private _handleKeyboardEvent: (event: KeyboardEvent) => void = (event: KeyboardEvent) => { + handleKeyboardEvent(this, event); }; /** diff --git a/packages/replay/test/unit/coreHandlers/handleKeyboardEvent.test.ts b/packages/replay/test/unit/coreHandlers/handleKeyboardEvent.test.ts new file mode 100644 index 000000000000..d08f1ef1a800 --- /dev/null +++ b/packages/replay/test/unit/coreHandlers/handleKeyboardEvent.test.ts @@ -0,0 +1,104 @@ +import { getKeyboardBreadcrumb } from '../../../src/coreHandlers/handleKeyboardEvent'; + +describe('Unit | coreHandlers | handleKeyboardEvent', () => { + describe('getKeyboardBreadcrumb', () => { + it('returns null for event on input', function () { + const event = makeKeyboardEvent({ tagName: 'input', key: 'Escape' }); + const actual = getKeyboardBreadcrumb(event); + expect(actual).toBeNull(); + }); + + it('returns null for event on textarea', function () { + const event = makeKeyboardEvent({ tagName: 'textarea', key: 'Escape' }); + const actual = getKeyboardBreadcrumb(event); + expect(actual).toBeNull(); + }); + + it('returns null for event on contenteditable div', function () { + // JSOM does not support contentEditable properly :( + const target = document.createElement('div'); + Object.defineProperty(target, 'isContentEditable', { + get: function () { + return true; + }, + }); + + const event = makeKeyboardEvent({ target, key: 'Escape' }); + const actual = getKeyboardBreadcrumb(event); + expect(actual).toBeNull(); + }); + + it('returns breadcrumb for Escape event on body', function () { + const event = makeKeyboardEvent({ tagName: 'body', key: 'Escape' }); + const actual = getKeyboardBreadcrumb(event); + expect(actual).toEqual({ + category: 'ui.keyDown', + data: { + altKey: false, + ctrlKey: false, + key: 'Escape', + metaKey: false, + shiftKey: false, + }, + message: 'body', + timestamp: expect.any(Number), + type: 'default', + }); + }); + + it.each(['a', '1', '!', '~', ']'])('returns null for %s key on body', key => { + const event = makeKeyboardEvent({ tagName: 'body', key }); + const actual = getKeyboardBreadcrumb(event); + expect(actual).toEqual(null); + }); + + it.each(['a', '1', '!', '~', ']'])('returns null for %s key + Shift on body', key => { + const event = makeKeyboardEvent({ tagName: 'body', key, shiftKey: true }); + const actual = getKeyboardBreadcrumb(event); + expect(actual).toEqual(null); + }); + + it.each(['a', '1', '!', '~', ']'])('returns breadcrumb for %s key + Ctrl on body', key => { + const event = makeKeyboardEvent({ tagName: 'body', key, ctrlKey: true }); + const actual = getKeyboardBreadcrumb(event); + expect(actual).toEqual({ + category: 'ui.keyDown', + data: { + altKey: false, + ctrlKey: true, + key, + metaKey: false, + shiftKey: false, + }, + message: 'body', + timestamp: expect.any(Number), + type: 'default', + }); + }); + }); +}); + +function makeKeyboardEvent({ + metaKey = false, + shiftKey = false, + ctrlKey = false, + altKey = false, + key, + tagName, + target, +}: { + metaKey?: boolean; + shiftKey?: boolean; + ctrlKey?: boolean; + altKey?: boolean; + key: string; + tagName?: string; + target?: HTMLElement; +}): KeyboardEvent { + const event = new KeyboardEvent('keydown', { metaKey, shiftKey, ctrlKey, altKey, key }); + + const element = target || document.createElement(tagName || 'div'); + element.dispatchEvent(event); + + return event; +} From acaf41a2f9c93fe4e12a53cf59b16dc1aae85a2b Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 10 May 2023 10:47:34 +0200 Subject: [PATCH 2/4] add integration test --- .../suites/replay/keyboardEvents/init.js | 16 +++ .../replay/keyboardEvents/template.html | 9 ++ .../suites/replay/keyboardEvents/test.ts | 112 ++++++++++++++++++ 3 files changed, 137 insertions(+) create mode 100644 packages/browser-integration-tests/suites/replay/keyboardEvents/init.js create mode 100644 packages/browser-integration-tests/suites/replay/keyboardEvents/template.html create mode 100644 packages/browser-integration-tests/suites/replay/keyboardEvents/test.ts diff --git a/packages/browser-integration-tests/suites/replay/keyboardEvents/init.js b/packages/browser-integration-tests/suites/replay/keyboardEvents/init.js new file mode 100644 index 000000000000..1b5f4f447543 --- /dev/null +++ b/packages/browser-integration-tests/suites/replay/keyboardEvents/init.js @@ -0,0 +1,16 @@ +import * as Sentry from '@sentry/browser'; + +window.Sentry = Sentry; +window.Replay = new Sentry.Replay({ + flushMinDelay: 1000, + flushMaxDelay: 1000, +}); + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + sampleRate: 0, + replaysSessionSampleRate: 1.0, + replaysOnErrorSampleRate: 0.0, + + integrations: [window.Replay], +}); diff --git a/packages/browser-integration-tests/suites/replay/keyboardEvents/template.html b/packages/browser-integration-tests/suites/replay/keyboardEvents/template.html new file mode 100644 index 000000000000..e35f2650aa72 --- /dev/null +++ b/packages/browser-integration-tests/suites/replay/keyboardEvents/template.html @@ -0,0 +1,9 @@ + + + + + + + + + diff --git a/packages/browser-integration-tests/suites/replay/keyboardEvents/test.ts b/packages/browser-integration-tests/suites/replay/keyboardEvents/test.ts new file mode 100644 index 000000000000..9b3661f3c2e4 --- /dev/null +++ b/packages/browser-integration-tests/suites/replay/keyboardEvents/test.ts @@ -0,0 +1,112 @@ +import { expect } from '@playwright/test'; +import { IncrementalSource } from '@sentry-internal/rrweb'; +import type { inputData } from '@sentry-internal/rrweb/typings/types'; + +import { sentryTest } from '../../../utils/fixtures'; +import { IncrementalRecordingSnapshot, getCustomRecordingEvents } from '../../../utils/replayHelpers'; +import { + getIncrementalRecordingSnapshots, + shouldSkipReplayTest, + waitForReplayRequest, +} from '../../../utils/replayHelpers'; + +function isInputMutation( + snap: IncrementalRecordingSnapshot, +): snap is IncrementalRecordingSnapshot & { data: inputData } { + return snap.data.source == IncrementalSource.Input; +} + +sentryTest('captures keyboard events', async ({ forceFlushReplay, getLocalTestPath, page }) => { + if (shouldSkipReplayTest()) { + sentryTest.skip(); + } + + const reqPromise0 = waitForReplayRequest(page, 0); + + await page.route('https://dsn.ingest.sentry.io/**/*', route => { + return route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ id: 'test-id' }), + }); + }); + + const url = await getLocalTestPath({ testDir: __dirname }); + + await page.goto(url); + await reqPromise0; + await forceFlushReplay(); + + const reqPromise1 = waitForReplayRequest(page, (event, res) => { + return getCustomRecordingEvents(res).breadcrumbs.some(breadcrumb => breadcrumb.category === 'ui.keyDown'); + }); + + // Trigger keyboard unfocused + await page.keyboard.press('a'); + await page.keyboard.press('Control+A'); + + // Type unfocused + await page.keyboard.type('Hello', { delay: 10 }); + + // Type focused + await page.locator('#input').focus(); + + await page.keyboard.press('Control+A'); + await page.keyboard.type('Hello', { delay: 10 }); + + await forceFlushReplay(); + const { breadcrumbs } = getCustomRecordingEvents(await reqPromise1); + + expect(breadcrumbs).toEqual([ + { + timestamp: expect.any(Number), + type: 'default', + category: 'ui.keyDown', + message: 'body', + data: { + nodeId: expect.any(Number), + node: { + attributes: {}, + id: expect.any(Number), + tagName: 'body', + textContent: '', + }, + metaKey: false, + shiftKey: false, + ctrlKey: true, + altKey: false, + key: 'Control', + }, + }, + { + timestamp: expect.any(Number), + type: 'default', + category: 'ui.keyDown', + message: 'body', + data: { + nodeId: expect.any(Number), + node: { attributes: {}, id: expect.any(Number), tagName: 'body', textContent: '' }, + metaKey: false, + shiftKey: false, + ctrlKey: true, + altKey: false, + key: 'A', + }, + }, + { + timestamp: expect.any(Number), + type: 'default', + category: 'ui.input', + message: 'body > input#input', + data: { + nodeId: expect.any(Number), + node: { + attributes: { id: 'input' }, + id: expect.any(Number), + tagName: 'input', + textContent: '', + }, + }, + }, + ]); +}); From 2dac14324de852b1c960af18915ebf8711e117f8 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 10 May 2023 13:02:41 +0200 Subject: [PATCH 3/4] fix lint --- .../suites/replay/keyboardEvents/test.ts | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/packages/browser-integration-tests/suites/replay/keyboardEvents/test.ts b/packages/browser-integration-tests/suites/replay/keyboardEvents/test.ts index 9b3661f3c2e4..bbf4984d6752 100644 --- a/packages/browser-integration-tests/suites/replay/keyboardEvents/test.ts +++ b/packages/browser-integration-tests/suites/replay/keyboardEvents/test.ts @@ -1,20 +1,7 @@ import { expect } from '@playwright/test'; -import { IncrementalSource } from '@sentry-internal/rrweb'; -import type { inputData } from '@sentry-internal/rrweb/typings/types'; import { sentryTest } from '../../../utils/fixtures'; -import { IncrementalRecordingSnapshot, getCustomRecordingEvents } from '../../../utils/replayHelpers'; -import { - getIncrementalRecordingSnapshots, - shouldSkipReplayTest, - waitForReplayRequest, -} from '../../../utils/replayHelpers'; - -function isInputMutation( - snap: IncrementalRecordingSnapshot, -): snap is IncrementalRecordingSnapshot & { data: inputData } { - return snap.data.source == IncrementalSource.Input; -} +import { getCustomRecordingEvents, shouldSkipReplayTest, waitForReplayRequest } from '../../../utils/replayHelpers'; sentryTest('captures keyboard events', async ({ forceFlushReplay, getLocalTestPath, page }) => { if (shouldSkipReplayTest()) { From 0577791296388ce5f00d04162829e5037e83c887 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 10 May 2023 13:33:47 +0200 Subject: [PATCH 4/4] unflake test --- .../suites/replay/keyboardEvents/test.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/packages/browser-integration-tests/suites/replay/keyboardEvents/test.ts b/packages/browser-integration-tests/suites/replay/keyboardEvents/test.ts index bbf4984d6752..edeacb7f2db0 100644 --- a/packages/browser-integration-tests/suites/replay/keyboardEvents/test.ts +++ b/packages/browser-integration-tests/suites/replay/keyboardEvents/test.ts @@ -27,6 +27,9 @@ sentryTest('captures keyboard events', async ({ forceFlushReplay, getLocalTestPa const reqPromise1 = waitForReplayRequest(page, (event, res) => { return getCustomRecordingEvents(res).breadcrumbs.some(breadcrumb => breadcrumb.category === 'ui.keyDown'); }); + const reqPromise2 = waitForReplayRequest(page, (event, res) => { + return getCustomRecordingEvents(res).breadcrumbs.some(breadcrumb => breadcrumb.category === 'ui.input'); + }); // Trigger keyboard unfocused await page.keyboard.press('a'); @@ -43,6 +46,15 @@ sentryTest('captures keyboard events', async ({ forceFlushReplay, getLocalTestPa await forceFlushReplay(); const { breadcrumbs } = getCustomRecordingEvents(await reqPromise1); + const { breadcrumbs: breadcrumbs2 } = getCustomRecordingEvents(await reqPromise2); + + // Combine the two together + // Usually, this should all be in a single request, but it _may_ be split out, so we combine this together here. + breadcrumbs2.forEach(breadcrumb => { + if (!breadcrumbs.some(b => b.category === breadcrumb.category && b.timestamp === breadcrumb.timestamp)) { + breadcrumbs.push(breadcrumb); + } + }); expect(breadcrumbs).toEqual([ {