From 231a5f03ba0d7d07c2a0a919101cc4ddbe2d1d3c Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 10 May 2023 14:17:07 +0200 Subject: [PATCH 1/7] feat(replay): Capture slow clicks (experimental) --- .../replay/slowClick/clickTargets/test.ts | 139 +++++++++++++++ .../suites/replay/slowClick/ignore/test.ts | 107 +++++++++++ .../suites/replay/slowClick/init.js | 24 +++ .../suites/replay/slowClick/mutation/test.ts | 166 ++++++++++++++++++ .../suites/replay/slowClick/scroll/test.ts | 110 ++++++++++++ .../suites/replay/slowClick/template.html | 83 +++++++++ .../suites/replay/slowClick/timeout/test.ts | 114 ++++++++++++ packages/replay/src/coreHandlers/handleDom.ts | 32 +++- .../src/coreHandlers/handleSlowClick.ts | 144 +++++++++++++++ packages/replay/src/types.ts | 13 ++ 10 files changed, 928 insertions(+), 4 deletions(-) create mode 100644 packages/browser-integration-tests/suites/replay/slowClick/clickTargets/test.ts create mode 100644 packages/browser-integration-tests/suites/replay/slowClick/ignore/test.ts create mode 100644 packages/browser-integration-tests/suites/replay/slowClick/init.js create mode 100644 packages/browser-integration-tests/suites/replay/slowClick/mutation/test.ts create mode 100644 packages/browser-integration-tests/suites/replay/slowClick/scroll/test.ts create mode 100644 packages/browser-integration-tests/suites/replay/slowClick/template.html create mode 100644 packages/browser-integration-tests/suites/replay/slowClick/timeout/test.ts create mode 100644 packages/replay/src/coreHandlers/handleSlowClick.ts diff --git a/packages/browser-integration-tests/suites/replay/slowClick/clickTargets/test.ts b/packages/browser-integration-tests/suites/replay/slowClick/clickTargets/test.ts new file mode 100644 index 000000000000..dfa1581d4704 --- /dev/null +++ b/packages/browser-integration-tests/suites/replay/slowClick/clickTargets/test.ts @@ -0,0 +1,139 @@ +import { expect } from '@playwright/test'; + +import { sentryTest } from '../../../../utils/fixtures'; +import { getCustomRecordingEvents, shouldSkipReplayTest, waitForReplayRequest } from '../../../../utils/replayHelpers'; + +[ + { + id: 'link', + slowClick: true, + }, + { + id: 'linkExternal', + slowClick: false, + }, + { + id: 'linkDownload', + slowClick: false, + }, + { + id: 'inputButton', + slowClick: true, + }, + { + id: 'inputSubmit', + slowClick: true, + }, + { + id: 'inputText', + slowClick: false, + }, +].forEach(({ id, slowClick }) => { + if (slowClick) { + sentryTest(`slow click is captured for ${id}`, async ({ getLocalTestUrl, 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 getLocalTestUrl({ testDir: __dirname }); + + await page.goto(url); + await reqPromise0; + + const reqPromise1 = waitForReplayRequest(page, (event, res) => { + const { breadcrumbs } = getCustomRecordingEvents(res); + + return breadcrumbs.some(breadcrumb => breadcrumb.category === 'ui.slowClickDetected'); + }); + + await page.click(`#${id}`); + + const { breadcrumbs } = getCustomRecordingEvents(await reqPromise1); + + const slowClickBreadcrumbs = breadcrumbs.filter(breadcrumb => breadcrumb.category === 'ui.slowClickDetected'); + + expect(slowClickBreadcrumbs).toEqual([ + { + category: 'ui.slowClickDetected', + data: { + endReason: 'timeout', + node: { + attributes: expect.objectContaining({ + id, + }), + id: expect.any(Number), + tagName: expect.any(String), + textContent: expect.any(String), + }, + nodeId: expect.any(Number), + timeAfterClickMs: expect.any(Number), + url: expect.any(String), + }, + message: expect.any(String), + timestamp: expect.any(Number), + }, + ]); + }); + } else { + sentryTest(`slow click is not captured for ${id}`, async ({ getLocalTestUrl, 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 getLocalTestUrl({ testDir: __dirname }); + + await page.goto(url); + await reqPromise0; + + const reqPromise1 = waitForReplayRequest(page, (event, res) => { + const { breadcrumbs } = getCustomRecordingEvents(res); + + return breadcrumbs.some(breadcrumb => breadcrumb.category === 'ui.click'); + }); + + await page.click(`#${id}`); + + const { breadcrumbs } = getCustomRecordingEvents(await reqPromise1); + + expect(breadcrumbs).toEqual([ + { + category: 'ui.click', + data: { + node: { + attributes: expect.objectContaining({ + id, + }), + id: expect.any(Number), + tagName: expect.any(String), + textContent: expect.any(String), + }, + nodeId: expect.any(Number), + }, + message: expect.any(String), + timestamp: expect.any(Number), + type: 'default', + }, + ]); + }); + } +}); diff --git a/packages/browser-integration-tests/suites/replay/slowClick/ignore/test.ts b/packages/browser-integration-tests/suites/replay/slowClick/ignore/test.ts new file mode 100644 index 000000000000..750e2cd45838 --- /dev/null +++ b/packages/browser-integration-tests/suites/replay/slowClick/ignore/test.ts @@ -0,0 +1,107 @@ +import { expect } from '@playwright/test'; + +import { sentryTest } from '../../../../utils/fixtures'; +import { getCustomRecordingEvents, shouldSkipReplayTest, waitForReplayRequest } from '../../../../utils/replayHelpers'; + +sentryTest('click is ignored on div', async ({ getLocalTestUrl, 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 getLocalTestUrl({ testDir: __dirname }); + + await page.goto(url); + await reqPromise0; + + const reqPromise1 = waitForReplayRequest(page, (event, res) => { + const { breadcrumbs } = getCustomRecordingEvents(res); + + return breadcrumbs.some(breadcrumb => breadcrumb.category === 'ui.click'); + }); + + await page.click('#mutationDiv'); + + const { breadcrumbs } = getCustomRecordingEvents(await reqPromise1); + + expect(breadcrumbs).toEqual([ + { + category: 'ui.click', + data: { + node: { + attributes: { + id: 'mutationDiv', + }, + id: expect.any(Number), + tagName: 'div', + textContent: '******* ********', + }, + nodeId: expect.any(Number), + }, + message: 'body > div#mutationDiv', + timestamp: expect.any(Number), + type: 'default', + }, + ]); +}); + +sentryTest('click is ignored on ignoreSelectors', async ({ getLocalTestUrl, 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 getLocalTestUrl({ testDir: __dirname }); + + await page.goto(url); + await reqPromise0; + + const reqPromise1 = waitForReplayRequest(page, (event, res) => { + const { breadcrumbs } = getCustomRecordingEvents(res); + + return breadcrumbs.some(breadcrumb => breadcrumb.category === 'ui.click'); + }); + + await page.click('#mutationIgnoreButton'); + + const { breadcrumbs } = getCustomRecordingEvents(await reqPromise1); + + expect(breadcrumbs).toEqual([ + { + category: 'ui.click', + data: { + node: { + attributes: { + class: 'ignore-class', + id: 'mutationIgnoreButton', + }, + id: expect.any(Number), + tagName: 'button', + textContent: '******* ****** ****', + }, + nodeId: expect.any(Number), + }, + message: 'body > button#mutationIgnoreButton.ignore-class', + timestamp: expect.any(Number), + type: 'default', + }, + ]); +}); diff --git a/packages/browser-integration-tests/suites/replay/slowClick/init.js b/packages/browser-integration-tests/suites/replay/slowClick/init.js new file mode 100644 index 000000000000..2fc5dab81aea --- /dev/null +++ b/packages/browser-integration-tests/suites/replay/slowClick/init.js @@ -0,0 +1,24 @@ +import * as Sentry from '@sentry/browser'; + +window.Sentry = Sentry; +window.Replay = new Sentry.Replay({ + flushMinDelay: 500, + flushMaxDelay: 500, + _experiments: { + slowClicks: { + threshold: 300, + scrollThreshold: 300, + timeout: 2000, + ignoreSelectors: ['.ignore-class', '[ignore-attribute]'], + }, + }, +}); + +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/slowClick/mutation/test.ts b/packages/browser-integration-tests/suites/replay/slowClick/mutation/test.ts new file mode 100644 index 000000000000..12078eac5250 --- /dev/null +++ b/packages/browser-integration-tests/suites/replay/slowClick/mutation/test.ts @@ -0,0 +1,166 @@ +import { expect } from '@playwright/test'; + +import { sentryTest } from '../../../../utils/fixtures'; +import { getCustomRecordingEvents, shouldSkipReplayTest, waitForReplayRequest } from '../../../../utils/replayHelpers'; + +sentryTest('mutation after threshold results in slow click', async ({ getLocalTestUrl, 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 getLocalTestUrl({ testDir: __dirname }); + + await page.goto(url); + await reqPromise0; + + const reqPromise1 = waitForReplayRequest(page, (event, res) => { + const { breadcrumbs } = getCustomRecordingEvents(res); + + return breadcrumbs.some(breadcrumb => breadcrumb.category === 'ui.slowClickDetected'); + }); + + // Trigger this twice, sometimes this was flaky otherwise... + await page.click('#mutationButton'); + await page.click('#mutationButton'); + + const { breadcrumbs } = getCustomRecordingEvents(await reqPromise1); + + const slowClickBreadcrumbs = breadcrumbs.filter(breadcrumb => breadcrumb.category === 'ui.slowClickDetected'); + + expect(slowClickBreadcrumbs).toEqual([ + { + category: 'ui.slowClickDetected', + data: { + endReason: 'mutation', + node: { + attributes: { + id: 'mutationButton', + }, + id: expect.any(Number), + tagName: 'button', + textContent: '******* ********', + }, + nodeId: expect.any(Number), + timeAfterClickMs: expect.any(Number), + url: 'http://sentry-test.io/index.html', + }, + message: 'body > button#mutationButton', + timestamp: expect.any(Number), + }, + ]); + + expect(slowClickBreadcrumbs[0]?.data?.timeAfterClickMs).toBeGreaterThan(300); + expect(slowClickBreadcrumbs[0]?.data?.timeAfterClickMs).toBeLessThan(2000); +}); + +sentryTest('immediate mutation does not trigger slow click', async ({ getLocalTestUrl, 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 getLocalTestUrl({ testDir: __dirname }); + + await page.goto(url); + await reqPromise0; + + const reqPromise1 = waitForReplayRequest(page, (event, res) => { + const { breadcrumbs } = getCustomRecordingEvents(res); + + return breadcrumbs.some(breadcrumb => breadcrumb.category === 'ui.click'); + }); + + await page.click('#mutationButtonImmediately'); + + const { breadcrumbs } = getCustomRecordingEvents(await reqPromise1); + + expect(breadcrumbs).toEqual([ + { + category: 'ui.click', + data: { + node: { + attributes: { + id: 'mutationButtonImmediately', + }, + id: expect.any(Number), + tagName: 'button', + textContent: '******* ******** ***********', + }, + nodeId: expect.any(Number), + }, + message: 'body > button#mutationButtonImmediately', + timestamp: expect.any(Number), + type: 'default', + }, + ]); +}); + +sentryTest('inline click handler does not trigger slow click', async ({ getLocalTestUrl, 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 getLocalTestUrl({ testDir: __dirname }); + + await page.goto(url); + await reqPromise0; + + const reqPromise1 = waitForReplayRequest(page, (event, res) => { + const { breadcrumbs } = getCustomRecordingEvents(res); + + return breadcrumbs.some(breadcrumb => breadcrumb.category === 'ui.click'); + }); + + await page.click('#mutationButtonInline'); + + const { breadcrumbs } = getCustomRecordingEvents(await reqPromise1); + + expect(breadcrumbs).toEqual([ + { + category: 'ui.click', + data: { + node: { + attributes: { + id: 'mutationButtonInline', + }, + id: expect.any(Number), + tagName: 'button', + textContent: '******* ******** ***********', + }, + nodeId: expect.any(Number), + }, + message: 'body > button#mutationButtonInline', + timestamp: expect.any(Number), + type: 'default', + }, + ]); +}); diff --git a/packages/browser-integration-tests/suites/replay/slowClick/scroll/test.ts b/packages/browser-integration-tests/suites/replay/slowClick/scroll/test.ts new file mode 100644 index 000000000000..f7f705ce5670 --- /dev/null +++ b/packages/browser-integration-tests/suites/replay/slowClick/scroll/test.ts @@ -0,0 +1,110 @@ +import { expect } from '@playwright/test'; + +import { sentryTest } from '../../../../utils/fixtures'; +import { getCustomRecordingEvents, shouldSkipReplayTest, waitForReplayRequest } from '../../../../utils/replayHelpers'; + +sentryTest('immediate scroll does not trigger slow click', async ({ getLocalTestUrl, 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 getLocalTestUrl({ testDir: __dirname }); + + await page.goto(url); + await reqPromise0; + + const reqPromise1 = waitForReplayRequest(page, (event, res) => { + const { breadcrumbs } = getCustomRecordingEvents(res); + + return breadcrumbs.some(breadcrumb => breadcrumb.category === 'ui.click'); + }); + + await page.click('#scrollButton'); + + const { breadcrumbs } = getCustomRecordingEvents(await reqPromise1); + + expect(breadcrumbs).toEqual([ + { + category: 'ui.click', + data: { + node: { + attributes: { + id: 'scrollButton', + }, + id: expect.any(Number), + tagName: 'button', + textContent: '******* ******', + }, + nodeId: expect.any(Number), + }, + message: 'body > button#scrollButton', + timestamp: expect.any(Number), + type: 'default', + }, + ]); +}); + +sentryTest('late scroll triggers slow click', async ({ getLocalTestUrl, 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 getLocalTestUrl({ testDir: __dirname }); + + await page.goto(url); + await reqPromise0; + + const reqPromise1 = waitForReplayRequest(page, (event, res) => { + const { breadcrumbs } = getCustomRecordingEvents(res); + + return breadcrumbs.some(breadcrumb => breadcrumb.category === 'ui.slowClickDetected'); + }); + + await page.click('#scrollLateButton'); + + const { breadcrumbs } = getCustomRecordingEvents(await reqPromise1); + + const slowClickBreadcrumbs = breadcrumbs.filter(breadcrumb => breadcrumb.category === 'ui.slowClickDetected'); + + expect(slowClickBreadcrumbs).toEqual([ + { + category: 'ui.slowClickDetected', + data: { + endReason: 'timeout', + node: { + attributes: { + id: 'scrollLateButton', + }, + id: expect.any(Number), + tagName: 'button', + textContent: '******* ****** ****', + }, + nodeId: expect.any(Number), + timeAfterClickMs: expect.any(Number), + url: 'http://sentry-test.io/index.html', + }, + message: 'body > button#scrollLateButton', + timestamp: expect.any(Number), + }, + ]); +}); diff --git a/packages/browser-integration-tests/suites/replay/slowClick/template.html b/packages/browser-integration-tests/suites/replay/slowClick/template.html new file mode 100644 index 000000000000..07e12cc088f3 --- /dev/null +++ b/packages/browser-integration-tests/suites/replay/slowClick/template.html @@ -0,0 +1,83 @@ + + + + + + + +
Trigger mutation
+ + + + + + + + + Link + Link external + Link download + + + + +

Heading

+ +
+ +

Bottom

+ + + + diff --git a/packages/browser-integration-tests/suites/replay/slowClick/timeout/test.ts b/packages/browser-integration-tests/suites/replay/slowClick/timeout/test.ts new file mode 100644 index 000000000000..e3fba57cd2b9 --- /dev/null +++ b/packages/browser-integration-tests/suites/replay/slowClick/timeout/test.ts @@ -0,0 +1,114 @@ +import { expect } from '@playwright/test'; + +import { sentryTest } from '../../../../utils/fixtures'; +import { getCustomRecordingEvents, shouldSkipReplayTest, waitForReplayRequest } from '../../../../utils/replayHelpers'; + +sentryTest('mutation after timeout results in slow click', async ({ getLocalTestUrl, 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 getLocalTestUrl({ testDir: __dirname }); + + await page.goto(url); + await reqPromise0; + + const reqPromise1 = waitForReplayRequest(page, (event, res) => { + const { breadcrumbs } = getCustomRecordingEvents(res); + + return breadcrumbs.some(breadcrumb => breadcrumb.category === 'ui.slowClickDetected'); + }); + + await page.click('#mutationButtonLate'); + + const { breadcrumbs } = getCustomRecordingEvents(await reqPromise1); + + const slowClickBreadcrumbs = breadcrumbs.filter(breadcrumb => breadcrumb.category === 'ui.slowClickDetected'); + + expect(slowClickBreadcrumbs).toEqual([ + { + category: 'ui.slowClickDetected', + data: { + endReason: 'timeout', + node: { + attributes: { + id: 'mutationButtonLate', + }, + id: expect.any(Number), + tagName: 'button', + textContent: '******* ******** ****', + }, + nodeId: expect.any(Number), + timeAfterClickMs: 2000, + url: 'http://sentry-test.io/index.html', + }, + message: 'body > button#mutationButtonLate', + timestamp: expect.any(Number), + }, + ]); +}); + +sentryTest('console.log results in slow click', async ({ getLocalTestUrl, 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 getLocalTestUrl({ testDir: __dirname }); + + await page.goto(url); + await reqPromise0; + + const reqPromise1 = waitForReplayRequest(page, (event, res) => { + const { breadcrumbs } = getCustomRecordingEvents(res); + + return breadcrumbs.some(breadcrumb => breadcrumb.category === 'ui.slowClickDetected'); + }); + + await page.click('#consoleLogButton'); + + const { breadcrumbs } = getCustomRecordingEvents(await reqPromise1); + + const slowClickBreadcrumbs = breadcrumbs.filter(breadcrumb => breadcrumb.category === 'ui.slowClickDetected'); + + expect(slowClickBreadcrumbs).toEqual([ + { + category: 'ui.slowClickDetected', + data: { + endReason: 'timeout', + node: { + attributes: { + id: 'consoleLogButton', + }, + id: expect.any(Number), + tagName: 'button', + textContent: '******* ******* ***', + }, + nodeId: expect.any(Number), + timeAfterClickMs: 2000, + url: 'http://sentry-test.io/index.html', + }, + message: 'body > button#consoleLogButton', + timestamp: expect.any(Number), + }, + ]); +}); diff --git a/packages/replay/src/coreHandlers/handleDom.ts b/packages/replay/src/coreHandlers/handleDom.ts index 00d274760511..65e7eb4d88a8 100644 --- a/packages/replay/src/coreHandlers/handleDom.ts +++ b/packages/replay/src/coreHandlers/handleDom.ts @@ -3,8 +3,9 @@ import { NodeType } from '@sentry-internal/rrweb-snapshot'; import type { Breadcrumb } from '@sentry/types'; import { htmlTreeAsString } from '@sentry/utils'; -import type { ReplayContainer } from '../types'; +import type { ReplayContainer, SlowClickConfig } from '../types'; import { createBreadcrumb } from '../util/createBreadcrumb'; +import { detectSlowClick } from './handleSlowClick'; import { addBreadcrumbEvent } from './util/addBreadcrumbEvent'; import { getAttributesToRecord } from './util/getAttributesToRecord'; @@ -13,9 +14,21 @@ export interface DomHandlerData { event: Node | { target: EventTarget }; } -export const handleDomListener: (replay: ReplayContainer) => (handlerData: DomHandlerData) => void = - (replay: ReplayContainer) => - (handlerData: DomHandlerData): void => { +export const handleDomListener: (replay: ReplayContainer) => (handlerData: DomHandlerData) => void = ( + replay: ReplayContainer, +) => { + const slowClickExperiment = replay.getOptions()._experiments.slowClicks; + + const slowClickConfig: SlowClickConfig | undefined = slowClickExperiment + ? { + threshold: slowClickExperiment.threshold, + timeout: slowClickExperiment.timeout, + scrollTimeout: slowClickExperiment.scrollTimeout, + ignoreSelector: slowClickExperiment.ignoreSelectors ? slowClickExperiment.ignoreSelectors.join(',') : '', + } + : undefined; + + return (handlerData: DomHandlerData): void => { if (!replay.isEnabled()) { return; } @@ -26,8 +39,19 @@ export const handleDomListener: (replay: ReplayContainer) => (handlerData: DomHa return; } + const isClick = handlerData.name === 'click'; + if (isClick && slowClickConfig) { + detectSlowClick( + replay, + slowClickConfig, + result as Breadcrumb & { timestamp: number }, + getClickTargetNode(handlerData.event) as HTMLElement, + ); + } + addBreadcrumbEvent(replay, result); }; +}; /** Get the base DOM breadcrumb. */ export function getBaseDomBreadcrumb(target: Node | INode | null, message: string): Breadcrumb { diff --git a/packages/replay/src/coreHandlers/handleSlowClick.ts b/packages/replay/src/coreHandlers/handleSlowClick.ts new file mode 100644 index 000000000000..d14c73206f1c --- /dev/null +++ b/packages/replay/src/coreHandlers/handleSlowClick.ts @@ -0,0 +1,144 @@ +import type { Breadcrumb } from '@sentry/types'; + +import { WINDOW } from '../constants'; +import type { ReplayContainer, SlowClickConfig } from '../types'; +import { addBreadcrumbEvent } from './util/addBreadcrumbEvent'; + +type ClickBreadcrumb = Breadcrumb & { + timestamp: number; +}; + +/** + * Detect a slow click on a button/a tag, + * and potentially create a corresponding breadcrumb. + */ +export function detectSlowClick( + replay: ReplayContainer, + config: SlowClickConfig, + clickBreadcrumb: ClickBreadcrumb, + node: HTMLElement, +): void { + if (ignoreElement(node, config)) { + return; + } + + /* + We consider a slow click a click on a button/a, which does not trigger one of: + - DOM mutation + - Scroll (within 100ms) + Within the given threshold time. + After time timeout time, we stop listening and mark it as a slow click anyhow. + */ + + let cleanup: () => void = () => { + // replaced further down + }; + + // After timeout time, def. consider this a slow click, and stop watching for mutations + const timeout = setTimeout(() => { + handleSlowClick(replay, clickBreadcrumb, config.timeout, 'timeout'); + cleanup(); + }, config.timeout); + + const mutationHandler = (): void => { + maybeHandleSlowClick(replay, clickBreadcrumb, config.threshold, config.timeout, 'mutation'); + cleanup(); + }; + + const scrollHandler = (): void => { + maybeHandleSlowClick(replay, clickBreadcrumb, config.scrollTimeout, config.timeout, 'scroll'); + cleanup(); + }; + + const obs = new MutationObserver(mutationHandler); + + obs.observe(WINDOW.document.documentElement, { + attributes: true, + characterData: true, + childList: true, + subtree: true, + }); + + WINDOW.addEventListener('scroll', scrollHandler); + + // Stop listening to scroll timeouts early + const scrollTimeout = setTimeout(() => { + WINDOW.removeEventListener('scroll', scrollHandler); + }, config.scrollTimeout); + + cleanup = (): void => { + clearTimeout(timeout); + clearTimeout(scrollTimeout); + obs.disconnect(); + WINDOW.removeEventListener('scroll', scrollHandler); + }; +} + +function maybeHandleSlowClick( + replay: ReplayContainer, + clickBreadcrumb: ClickBreadcrumb, + threshold: number, + timeout: number, + endReason: string, +): boolean { + const now = Date.now(); + const timeAfterClickMs = now - clickBreadcrumb.timestamp * 1000; + + if (timeAfterClickMs > threshold) { + handleSlowClick(replay, clickBreadcrumb, Math.min(timeAfterClickMs, timeout), endReason); + return true; + } + + return false; +} + +function handleSlowClick( + replay: ReplayContainer, + clickBreadcrumb: ClickBreadcrumb, + timeAfterClickMs: number, + endReason: string, +): void { + const breadcrumb = { + message: clickBreadcrumb.message, + timestamp: clickBreadcrumb.timestamp, + category: 'ui.slowClickDetected', + data: { + ...clickBreadcrumb.data, + url: WINDOW.location.href, + // TODO FN: add parametrized route, when possible + timeAfterClickMs, + endReason, + }, + }; + + addBreadcrumbEvent(replay, breadcrumb); +} + +const SLOW_CLICK_POSSIBLE_TAGS = ['BUTTON', 'A', 'INPUT']; + +function ignoreElement(node: HTMLElement, config: SlowClickConfig): boolean { + if (!SLOW_CLICK_POSSIBLE_TAGS.includes(node.tagName)) { + return true; + } + + // If tag, detect special variants that may not lead to an action + // If target !== _self, we may open the link somewhere else, which would lead to no action + // Also, when downloading a file, we may not leave the page, but still not trigger an action + if ( + node.tagName === 'A' && + (node.hasAttribute('download') || (node.hasAttribute('target') && node.getAttribute('target') !== '_self')) + ) { + return true; + } + + // If tag, we only want to consider input[type='submit'] & input[type='button'] + if (node.tagName === 'INPUT' && !['submit', 'button'].includes(node.getAttribute('type') || '')) { + return true; + } + + if (config.ignoreSelector && node.matches(config.ignoreSelector)) { + return true; + } + + return false; +} diff --git a/packages/replay/src/types.ts b/packages/replay/src/types.ts index aef61f57a07a..12900dc22e74 100644 --- a/packages/replay/src/types.ts +++ b/packages/replay/src/types.ts @@ -279,6 +279,12 @@ export interface ReplayPluginOptions extends ReplayNetworkOptions { traceInternals: boolean; mutationLimit: number; mutationBreadcrumbLimit: number; + slowClicks: { + threshold: number; + timeout: number; + scrollTimeout: number; + ignoreSelectors: string[]; + }; }>; } @@ -583,3 +589,10 @@ export type ReplayNetworkRequestData = { request?: ReplayNetworkRequestOrResponse; response?: ReplayNetworkRequestOrResponse; }; + +export interface SlowClickConfig { + threshold: number; + timeout: number; + scrollTimeout: number; + ignoreSelector: string; +} From c76b8724bd5e8b37b9d84b2f0bd113c95551c4d5 Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Thu, 11 May 2023 15:19:11 -0400 Subject: [PATCH 2/7] broaden allowed tags --- .../replay/src/coreHandlers/handleSlowClick.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/replay/src/coreHandlers/handleSlowClick.ts b/packages/replay/src/coreHandlers/handleSlowClick.ts index d14c73206f1c..c8a867209ffd 100644 --- a/packages/replay/src/coreHandlers/handleSlowClick.ts +++ b/packages/replay/src/coreHandlers/handleSlowClick.ts @@ -114,10 +114,15 @@ function handleSlowClick( addBreadcrumbEvent(replay, breadcrumb); } -const SLOW_CLICK_POSSIBLE_TAGS = ['BUTTON', 'A', 'INPUT']; +const SLOW_CLICK_IGNORE_TAGS = ['SELECT', 'OPTION']; function ignoreElement(node: HTMLElement, config: SlowClickConfig): boolean { - if (!SLOW_CLICK_POSSIBLE_TAGS.includes(node.tagName)) { + // If tag, we only want to consider input[type='submit'] & input[type='button'] + if (node.tagName === 'INPUT' && !['submit', 'button'].includes(node.getAttribute('type') || '')) { + return true; + } + + if (SLOW_CLICK_IGNORE_TAGS.includes(node.tagName)) { return true; } @@ -131,11 +136,6 @@ function ignoreElement(node: HTMLElement, config: SlowClickConfig): boolean { return true; } - // If tag, we only want to consider input[type='submit'] & input[type='button'] - if (node.tagName === 'INPUT' && !['submit', 'button'].includes(node.getAttribute('type') || '')) { - return true; - } - if (config.ignoreSelector && node.matches(config.ignoreSelector)) { return true; } From bdb6c921dcb60d8cf88983d69e6ccd08ab153a9c Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Thu, 11 May 2023 15:28:01 -0400 Subject: [PATCH 3/7] ignore clicks with meta keys --- packages/replay/src/coreHandlers/handleDom.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/replay/src/coreHandlers/handleDom.ts b/packages/replay/src/coreHandlers/handleDom.ts index 65e7eb4d88a8..1e46a18864d5 100644 --- a/packages/replay/src/coreHandlers/handleDom.ts +++ b/packages/replay/src/coreHandlers/handleDom.ts @@ -40,7 +40,9 @@ export const handleDomListener: (replay: ReplayContainer) => (handlerData: DomHa } const isClick = handlerData.name === 'click'; - if (isClick && slowClickConfig) { + const event = isClick && (handlerData.event as PointerEvent); + // Ignore clicks if ctrl/alt/meta keys are held down as they alter behavior of clicks (e.g. open in new tab) + if (isClick && slowClickConfig && event && !event.altKey && !event.metaKey && !event.ctrlKey) { detectSlowClick( replay, slowClickConfig, From c17fe1d702d9e297026ae88f43347e0d4483b084 Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Fri, 12 May 2023 11:41:53 -0400 Subject: [PATCH 4/7] update test --- .../suites/replay/slowClick/ignore/test.ts | 21 ++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/packages/browser-integration-tests/suites/replay/slowClick/ignore/test.ts b/packages/browser-integration-tests/suites/replay/slowClick/ignore/test.ts index 750e2cd45838..98b8b9a90cba 100644 --- a/packages/browser-integration-tests/suites/replay/slowClick/ignore/test.ts +++ b/packages/browser-integration-tests/suites/replay/slowClick/ignore/test.ts @@ -3,7 +3,7 @@ import { expect } from '@playwright/test'; import { sentryTest } from '../../../../utils/fixtures'; import { getCustomRecordingEvents, shouldSkipReplayTest, waitForReplayRequest } from '../../../../utils/replayHelpers'; -sentryTest('click is ignored on div', async ({ getLocalTestUrl, page }) => { +sentryTest('click is not ignored on div', async ({ getLocalTestUrl, page }) => { if (shouldSkipReplayTest()) { sentryTest.skip(); } @@ -51,6 +51,25 @@ sentryTest('click is ignored on div', async ({ getLocalTestUrl, page }) => { timestamp: expect.any(Number), type: 'default', }, + { + category: 'ui.slowClickDetected', + data: { + endReason: 'mutation', + node: { + attributes: { + id: 'mutationDiv', + }, + id: expect.any(Number), + tagName: 'div', + textContent: '******* ********', + }, + nodeId: expect.any(Number), + timeAfterClickMs: expect.any(Number), + url: 'http://sentry-test.io/index.html', + }, + message: 'body > div#mutationDiv', + timestamp: expect.any(Number), + }, ]); }); From 72a8f1b15980da85d6db09f84071db715687e494 Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Fri, 12 May 2023 11:42:45 -0400 Subject: [PATCH 5/7] actually move test --- .../suites/replay/slowClick/ignore/test.ts | 70 ------------------ .../suites/replay/slowClick/mutation/test.ts | 71 +++++++++++++++++++ 2 files changed, 71 insertions(+), 70 deletions(-) diff --git a/packages/browser-integration-tests/suites/replay/slowClick/ignore/test.ts b/packages/browser-integration-tests/suites/replay/slowClick/ignore/test.ts index 98b8b9a90cba..c3cdb6e35c65 100644 --- a/packages/browser-integration-tests/suites/replay/slowClick/ignore/test.ts +++ b/packages/browser-integration-tests/suites/replay/slowClick/ignore/test.ts @@ -3,76 +3,6 @@ import { expect } from '@playwright/test'; import { sentryTest } from '../../../../utils/fixtures'; import { getCustomRecordingEvents, shouldSkipReplayTest, waitForReplayRequest } from '../../../../utils/replayHelpers'; -sentryTest('click is not ignored on div', async ({ getLocalTestUrl, 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 getLocalTestUrl({ testDir: __dirname }); - - await page.goto(url); - await reqPromise0; - - const reqPromise1 = waitForReplayRequest(page, (event, res) => { - const { breadcrumbs } = getCustomRecordingEvents(res); - - return breadcrumbs.some(breadcrumb => breadcrumb.category === 'ui.click'); - }); - - await page.click('#mutationDiv'); - - const { breadcrumbs } = getCustomRecordingEvents(await reqPromise1); - - expect(breadcrumbs).toEqual([ - { - category: 'ui.click', - data: { - node: { - attributes: { - id: 'mutationDiv', - }, - id: expect.any(Number), - tagName: 'div', - textContent: '******* ********', - }, - nodeId: expect.any(Number), - }, - message: 'body > div#mutationDiv', - timestamp: expect.any(Number), - type: 'default', - }, - { - category: 'ui.slowClickDetected', - data: { - endReason: 'mutation', - node: { - attributes: { - id: 'mutationDiv', - }, - id: expect.any(Number), - tagName: 'div', - textContent: '******* ********', - }, - nodeId: expect.any(Number), - timeAfterClickMs: expect.any(Number), - url: 'http://sentry-test.io/index.html', - }, - message: 'body > div#mutationDiv', - timestamp: expect.any(Number), - }, - ]); -}); - sentryTest('click is ignored on ignoreSelectors', async ({ getLocalTestUrl, page }) => { if (shouldSkipReplayTest()) { sentryTest.skip(); diff --git a/packages/browser-integration-tests/suites/replay/slowClick/mutation/test.ts b/packages/browser-integration-tests/suites/replay/slowClick/mutation/test.ts index 12078eac5250..b7973e43ebfb 100644 --- a/packages/browser-integration-tests/suites/replay/slowClick/mutation/test.ts +++ b/packages/browser-integration-tests/suites/replay/slowClick/mutation/test.ts @@ -164,3 +164,74 @@ sentryTest('inline click handler does not trigger slow click', async ({ getLocal }, ]); }); + +sentryTest('click is not ignored on div', async ({ getLocalTestUrl, 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 getLocalTestUrl({ testDir: __dirname }); + + await page.goto(url); + await reqPromise0; + + const reqPromise1 = waitForReplayRequest(page, (event, res) => { + const { breadcrumbs } = getCustomRecordingEvents(res); + + return breadcrumbs.some(breadcrumb => breadcrumb.category === 'ui.click'); + }); + + await page.click('#mutationDiv'); + + const { breadcrumbs } = getCustomRecordingEvents(await reqPromise1); + + expect(breadcrumbs).toEqual([ + { + category: 'ui.click', + data: { + node: { + attributes: { + id: 'mutationDiv', + }, + id: expect.any(Number), + tagName: 'div', + textContent: '******* ********', + }, + nodeId: expect.any(Number), + }, + message: 'body > div#mutationDiv', + timestamp: expect.any(Number), + type: 'default', + }, + { + category: 'ui.slowClickDetected', + data: { + endReason: 'mutation', + node: { + attributes: { + id: 'mutationDiv', + }, + id: expect.any(Number), + tagName: 'div', + textContent: '******* ********', + }, + nodeId: expect.any(Number), + timeAfterClickMs: expect.any(Number), + url: 'http://sentry-test.io/index.html', + }, + message: 'body > div#mutationDiv', + timestamp: expect.any(Number), + }, + ]); +}); + From d35aa125b871c1ca08258d1ee4e06af9ed65bfec Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Fri, 12 May 2023 12:10:49 -0400 Subject: [PATCH 6/7] lint --- .../suites/replay/slowClick/mutation/test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/browser-integration-tests/suites/replay/slowClick/mutation/test.ts b/packages/browser-integration-tests/suites/replay/slowClick/mutation/test.ts index b7973e43ebfb..b349570ca6d4 100644 --- a/packages/browser-integration-tests/suites/replay/slowClick/mutation/test.ts +++ b/packages/browser-integration-tests/suites/replay/slowClick/mutation/test.ts @@ -234,4 +234,3 @@ sentryTest('click is not ignored on div', async ({ getLocalTestUrl, page }) => { }, ]); }); - From f56e9214920f6bf40035e4fafeae835e1a199c5c Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Fri, 12 May 2023 16:28:31 -0400 Subject: [PATCH 7/7] fix flakeyness --- .../suites/replay/slowClick/mutation/test.ts | 21 ++----------------- 1 file changed, 2 insertions(+), 19 deletions(-) diff --git a/packages/browser-integration-tests/suites/replay/slowClick/mutation/test.ts b/packages/browser-integration-tests/suites/replay/slowClick/mutation/test.ts index b349570ca6d4..2ed458213955 100644 --- a/packages/browser-integration-tests/suites/replay/slowClick/mutation/test.ts +++ b/packages/browser-integration-tests/suites/replay/slowClick/mutation/test.ts @@ -188,31 +188,14 @@ sentryTest('click is not ignored on div', async ({ getLocalTestUrl, page }) => { const reqPromise1 = waitForReplayRequest(page, (event, res) => { const { breadcrumbs } = getCustomRecordingEvents(res); - return breadcrumbs.some(breadcrumb => breadcrumb.category === 'ui.click'); + return breadcrumbs.some(breadcrumb => breadcrumb.category === 'ui.slowClickDetected'); }); await page.click('#mutationDiv'); const { breadcrumbs } = getCustomRecordingEvents(await reqPromise1); - expect(breadcrumbs).toEqual([ - { - category: 'ui.click', - data: { - node: { - attributes: { - id: 'mutationDiv', - }, - id: expect.any(Number), - tagName: 'div', - textContent: '******* ********', - }, - nodeId: expect.any(Number), - }, - message: 'body > div#mutationDiv', - timestamp: expect.any(Number), - type: 'default', - }, + expect(breadcrumbs.filter(({ category }) => category === 'ui.slowClickDetected')).toEqual([ { category: 'ui.slowClickDetected', data: {