From 8b97141a16ee6c6f8aba0a7abf6a2eb75d7f306f Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Thu, 9 Mar 2023 15:01:22 -0500 Subject: [PATCH 01/13] feat(replay): Add additional properties for UI clicks For replay click breadcrumbs, add the following properties: * tag name * text content * element attributes --- packages/replay/package.json | 1 + packages/replay/src/coreHandlers/handleDom.ts | 19 ++++++++++++++----- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/packages/replay/package.json b/packages/replay/package.json index 95268a509710..56365f7fac7d 100644 --- a/packages/replay/package.json +++ b/packages/replay/package.json @@ -45,6 +45,7 @@ "@babel/core": "^7.17.5", "@sentry-internal/replay-worker": "7.42.0", "@sentry-internal/rrweb": "1.105.0", + "@sentry-internal/rrweb-snapshot": "1.105.0", "jsdom-worker": "^0.2.1", "tslib": "^1.9.3" }, diff --git a/packages/replay/src/coreHandlers/handleDom.ts b/packages/replay/src/coreHandlers/handleDom.ts index 17dcadc9d4ea..ae06f71a137e 100644 --- a/packages/replay/src/coreHandlers/handleDom.ts +++ b/packages/replay/src/coreHandlers/handleDom.ts @@ -1,4 +1,5 @@ -import { record } from '@sentry-internal/rrweb'; +import type { elementNode, INode} from '@sentry-internal/rrweb-snapshot'; +import { NodeType } from '@sentry-internal/rrweb-snapshot'; import type { Breadcrumb } from '@sentry/types'; import { htmlTreeAsString } from '@sentry/utils'; @@ -33,7 +34,7 @@ export const handleDomListener: (replay: ReplayContainer) => (handlerData: DomHa function handleDom(handlerData: DomHandlerData): Breadcrumb | null { // Taken from https://github.com/getsentry/sentry-javascript/blob/master/packages/browser/src/integrations/breadcrumbs.ts#L112 let target; - let targetNode; + let targetNode: Node | INode | undefined; // Accessing event.target can throw (see getsentry/raven-js#838, #768) try { @@ -47,13 +48,21 @@ function handleDom(handlerData: DomHandlerData): Breadcrumb | null { return null; } + const serializedNode = targetNode && ('__sn' in targetNode) ? targetNode.__sn as elementNode & {id: number} : null; return createBreadcrumb({ category: `ui.${handlerData.name}`, message: target, data: { - // Not sure why this errors, Node should be correct (Argument of type 'Node' is not assignable to parameter of type 'INode') - // eslint-disable-next-line @typescript-eslint/no-explicit-any - ...(targetNode ? { nodeId: record.mirror.getId(targetNode as any) } : {}), + ...(serializedNode ? { + nodeId: serializedNode.id, + node: { + id: serializedNode.id, + tagName: serializedNode.tagName, + textContent: targetNode ? Array.from(targetNode.childNodes).filter((node: Node | INode) => '__sn' in node && node.__sn.type === NodeType.Text).map(node => node.textContent) : '', + attributes: serializedNode.attributes, + }, + } : {}), + }, }); } From 8a726a0c73e6f2ffa1bcebde4dcf4af60ac4b0a3 Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Thu, 9 Mar 2023 15:03:26 -0500 Subject: [PATCH 02/13] add comments --- packages/replay/src/coreHandlers/handleDom.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/replay/src/coreHandlers/handleDom.ts b/packages/replay/src/coreHandlers/handleDom.ts index ae06f71a137e..879fa3d84cd2 100644 --- a/packages/replay/src/coreHandlers/handleDom.ts +++ b/packages/replay/src/coreHandlers/handleDom.ts @@ -48,7 +48,9 @@ function handleDom(handlerData: DomHandlerData): Breadcrumb | null { return null; } + // `__sn` property is the serialized node created by rrweb const serializedNode = targetNode && ('__sn' in targetNode) ? targetNode.__sn as elementNode & {id: number} : null; + return createBreadcrumb({ category: `ui.${handlerData.name}`, message: target, From 8f7d5be9cd4e7e0afa3017f46f3f037cb72a113c Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Thu, 9 Mar 2023 18:23:46 -0500 Subject: [PATCH 03/13] fix tests --- .../suites/replay/errors/errorMode/test.ts | 25 +++++++++- .../replay/errors/errorsInSession/test.ts | 34 +++++++++++++- .../utils/replayEventTemplates.ts | 8 ++++ packages/replay/src/coreHandlers/handleDom.ts | 46 ++++++++++++++----- 4 files changed, 98 insertions(+), 15 deletions(-) diff --git a/packages/integration-tests/suites/replay/errors/errorMode/test.ts b/packages/integration-tests/suites/replay/errors/errorMode/test.ts index b4da8aa4d3e4..7bc4419266aa 100644 --- a/packages/integration-tests/suites/replay/errors/errorMode/test.ts +++ b/packages/integration-tests/suites/replay/errors/errorMode/test.ts @@ -99,6 +99,17 @@ sentryTest( { ...expectedClickBreadcrumb, message: 'body > button#error', + data: { + nodeId: expect.any(Number), + node: { + attributes: { + id: 'error', + }, + id: expect.any(Number), + tagName: 'button', + textContent: 'Throw Error', + }, + }, }, ]), ); @@ -131,7 +142,19 @@ sentryTest( expect(content2.breadcrumbs).toEqual( expect.arrayContaining([ - { ...expectedClickBreadcrumb, message: 'body > button#log' }, + { + ...expectedClickBreadcrumb, + message: 'body > button#log', + data: { + node: { + attributes: { id: 'log' }, + id: expect.any(Number), + tagName: 'button', + textContent: 'Log stuff to the console', + }, + nodeId: expect.any(Number), + }, + }, { ...expectedConsoleBreadcrumb, level: 'log', message: 'Some message' }, ]), ); diff --git a/packages/integration-tests/suites/replay/errors/errorsInSession/test.ts b/packages/integration-tests/suites/replay/errors/errorsInSession/test.ts index 79835eef5771..9920bb8dd328 100644 --- a/packages/integration-tests/suites/replay/errors/errorsInSession/test.ts +++ b/packages/integration-tests/suites/replay/errors/errorsInSession/test.ts @@ -62,7 +62,21 @@ sentryTest( ); expect(content1.breadcrumbs).toEqual( - expect.arrayContaining([{ ...expectedClickBreadcrumb, message: 'body > button#error' }]), + expect.arrayContaining([ + { + ...expectedClickBreadcrumb, + message: 'body > button#error', + data: { + node: { + attributes: { id: 'error' }, + id: expect.any(Number), + tagName: 'button', + textContent: 'Throw Error', + }, + nodeId: expect.any(Number), + }, + }, + ]), ); }, ); @@ -108,7 +122,23 @@ sentryTest( // The button click that triggered the error should still be recorded expect(content1.breadcrumbs).toEqual( - expect.arrayContaining([{ ...expectedClickBreadcrumb, message: 'body > button#drop' }]), + expect.arrayContaining([ + { + ...expectedClickBreadcrumb, + message: 'body > button#drop', + data: { + node: { + attributes: { + id: 'drop', + }, + id: expect.any(Number), + tagName: 'button', + textContent: 'Throw Error but drop it', + }, + nodeId: expect.any(Number), + }, + }, + ]), ); }, ); diff --git a/packages/integration-tests/utils/replayEventTemplates.ts b/packages/integration-tests/utils/replayEventTemplates.ts index cabb27d5d6e9..c417624f9b9a 100644 --- a/packages/integration-tests/utils/replayEventTemplates.ts +++ b/packages/integration-tests/utils/replayEventTemplates.ts @@ -161,6 +161,14 @@ export const expectedClickBreadcrumb = { message: expect.any(String), data: { nodeId: expect.any(Number), + node: { + attributes: { + id: expect.any(String), + }, + id: expect.any(Number), + tagName: expect.any(String), + textContent: expect.any(String), + }, }, }; diff --git a/packages/replay/src/coreHandlers/handleDom.ts b/packages/replay/src/coreHandlers/handleDom.ts index 879fa3d84cd2..fc5b9e0dc608 100644 --- a/packages/replay/src/coreHandlers/handleDom.ts +++ b/packages/replay/src/coreHandlers/handleDom.ts @@ -1,4 +1,4 @@ -import type { elementNode, INode} from '@sentry-internal/rrweb-snapshot'; +import type { INode } from '@sentry-internal/rrweb-snapshot'; import { NodeType } from '@sentry-internal/rrweb-snapshot'; import type { Breadcrumb } from '@sentry/types'; import { htmlTreeAsString } from '@sentry/utils'; @@ -49,22 +49,30 @@ function handleDom(handlerData: DomHandlerData): Breadcrumb | null { } // `__sn` property is the serialized node created by rrweb - const serializedNode = targetNode && ('__sn' in targetNode) ? targetNode.__sn as elementNode & {id: number} : null; + const serializedNode = + targetNode && '__sn' in targetNode && targetNode.__sn.type === NodeType.Element ? targetNode.__sn : null; return createBreadcrumb({ category: `ui.${handlerData.name}`, message: target, data: { - ...(serializedNode ? { - nodeId: serializedNode.id, - node: { - id: serializedNode.id, - tagName: serializedNode.tagName, - textContent: targetNode ? Array.from(targetNode.childNodes).filter((node: Node | INode) => '__sn' in node && node.__sn.type === NodeType.Text).map(node => node.textContent) : '', - attributes: serializedNode.attributes, - }, - } : {}), - + ...(serializedNode + ? { + nodeId: serializedNode.id, + node: { + id: serializedNode.id, + tagName: serializedNode.tagName, + textContent: targetNode + ? Array.from(targetNode.childNodes) + .filter((node: Node | INode) => '__sn' in node && node.__sn.type === NodeType.Text) + .map(node => node.textContent) + .join('') + : '', + // TODO: strict list of attributes + attributes: getAttributesToIndex(serializedNode.attributes), + }, + } + : {}), }, }); } @@ -80,3 +88,17 @@ function getTargetNode(handlerData: DomHandlerData): Node { function isEventWithTarget(event: unknown): event is { target: Node } { return !!(event as { target?: Node }).target; } + +// Attributes we are interested in: +const ATTRIBUTES_TO_INDEX = ['id', 'class', 'aria-label', 'role', 'name']; + +function getAttributesToIndex(attributes: Record) { + const obj = {}; + for (const key of ATTRIBUTES_TO_INDEX) { + if (attributes.hasOwnProperty(key)) { + obj[key] = attributes[key]; + } + } + + return obj; +} From ba522318b1fa7859565f26c028df0fa9f93d0ccb Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Mon, 13 Mar 2023 16:28:15 -0400 Subject: [PATCH 04/13] extract get Attributes, fix textContent not masked, fix tests --- .../suites/replay/errors/errorMode/test.ts | 11 ++-- .../replay/errors/errorsInSession/test.ts | 14 +++-- .../suites/replay/errors/template.html | 2 +- .../utils/replayEventTemplates.ts | 1 - packages/replay/src/coreHandlers/handleDom.ts | 61 +++++++------------ .../util/getAttributesToRecord.ts | 16 +++++ .../util/getAttributesToRecord.test.ts | 32 ++++++++++ 7 files changed, 85 insertions(+), 52 deletions(-) create mode 100644 packages/replay/src/coreHandlers/util/getAttributesToRecord.ts create mode 100644 packages/replay/test/unit/coreHandlers/util/getAttributesToRecord.test.ts diff --git a/packages/integration-tests/suites/replay/errors/errorMode/test.ts b/packages/integration-tests/suites/replay/errors/errorMode/test.ts index 7bc4419266aa..949050ac15a9 100644 --- a/packages/integration-tests/suites/replay/errors/errorMode/test.ts +++ b/packages/integration-tests/suites/replay/errors/errorMode/test.ts @@ -98,16 +98,18 @@ sentryTest( expect.arrayContaining([ { ...expectedClickBreadcrumb, - message: 'body > button#error', data: { nodeId: expect.any(Number), node: { attributes: { + 'aria-label': '***** *****', + class: 'btn btn-error', id: 'error', + role: 'button', }, id: expect.any(Number), - tagName: 'button', - textContent: 'Throw Error', + tagName: 'div', + textContent: '***** *****', }, }, }, @@ -144,13 +146,12 @@ sentryTest( expect.arrayContaining([ { ...expectedClickBreadcrumb, - message: 'body > button#log', data: { node: { attributes: { id: 'log' }, id: expect.any(Number), tagName: 'button', - textContent: 'Log stuff to the console', + textContent: '*** ***** ** *** *******', }, nodeId: expect.any(Number), }, diff --git a/packages/integration-tests/suites/replay/errors/errorsInSession/test.ts b/packages/integration-tests/suites/replay/errors/errorsInSession/test.ts index 9920bb8dd328..75a0fb94b3aa 100644 --- a/packages/integration-tests/suites/replay/errors/errorsInSession/test.ts +++ b/packages/integration-tests/suites/replay/errors/errorsInSession/test.ts @@ -65,13 +65,17 @@ sentryTest( expect.arrayContaining([ { ...expectedClickBreadcrumb, - message: 'body > button#error', data: { node: { - attributes: { id: 'error' }, + attributes: { + 'aria-label': '***** *****', + class: 'btn btn-error', + id: 'error', + role: 'button', + }, id: expect.any(Number), - tagName: 'button', - textContent: 'Throw Error', + tagName: 'div', + textContent: '***** *****', }, nodeId: expect.any(Number), }, @@ -125,7 +129,6 @@ sentryTest( expect.arrayContaining([ { ...expectedClickBreadcrumb, - message: 'body > button#drop', data: { node: { attributes: { @@ -134,6 +137,7 @@ sentryTest( id: expect.any(Number), tagName: 'button', textContent: 'Throw Error but drop it', + textContent: '***** ***** *** **** **', }, nodeId: expect.any(Number), }, diff --git a/packages/integration-tests/suites/replay/errors/template.html b/packages/integration-tests/suites/replay/errors/template.html index 99dcde1a9a88..50be112df43b 100644 --- a/packages/integration-tests/suites/replay/errors/template.html +++ b/packages/integration-tests/suites/replay/errors/template.html @@ -5,7 +5,7 @@ - +
Throw Error
diff --git a/packages/integration-tests/utils/replayEventTemplates.ts b/packages/integration-tests/utils/replayEventTemplates.ts index c417624f9b9a..33d24ca87d66 100644 --- a/packages/integration-tests/utils/replayEventTemplates.ts +++ b/packages/integration-tests/utils/replayEventTemplates.ts @@ -158,7 +158,6 @@ export const expectedClickBreadcrumb = { timestamp: expect.any(Number), type: 'default', category: 'ui.click', - message: expect.any(String), data: { nodeId: expect.any(Number), node: { diff --git a/packages/replay/src/coreHandlers/handleDom.ts b/packages/replay/src/coreHandlers/handleDom.ts index fc5b9e0dc608..9a1181cde56a 100644 --- a/packages/replay/src/coreHandlers/handleDom.ts +++ b/packages/replay/src/coreHandlers/handleDom.ts @@ -1,11 +1,11 @@ import type { INode } from '@sentry-internal/rrweb-snapshot'; import { NodeType } from '@sentry-internal/rrweb-snapshot'; import type { Breadcrumb } from '@sentry/types'; -import { htmlTreeAsString } from '@sentry/utils'; import type { ReplayContainer } from '../types'; import { createBreadcrumb } from '../util/createBreadcrumb'; import { addBreadcrumbEvent } from './addBreadcrumbEvent'; +import { getAttributesToRecord } from './util/getAttributesToRecord'; interface DomHandlerData { name: string; @@ -32,19 +32,16 @@ export const handleDomListener: (replay: ReplayContainer) => (handlerData: DomHa * An event handler to react to DOM events. */ function handleDom(handlerData: DomHandlerData): Breadcrumb | null { - // Taken from https://github.com/getsentry/sentry-javascript/blob/master/packages/browser/src/integrations/breadcrumbs.ts#L112 - let target; let targetNode: Node | INode | undefined; // Accessing event.target can throw (see getsentry/raven-js#838, #768) try { targetNode = getTargetNode(handlerData); - target = htmlTreeAsString(targetNode); } catch (e) { - target = ''; + // Nothing to do } - if (target.length === 0) { + if (!targetNode) { return null; } @@ -54,26 +51,24 @@ function handleDom(handlerData: DomHandlerData): Breadcrumb | null { return createBreadcrumb({ category: `ui.${handlerData.name}`, - message: target, - data: { - ...(serializedNode - ? { - nodeId: serializedNode.id, - node: { - id: serializedNode.id, - tagName: serializedNode.tagName, - textContent: targetNode - ? Array.from(targetNode.childNodes) - .filter((node: Node | INode) => '__sn' in node && node.__sn.type === NodeType.Text) - .map(node => node.textContent) - .join('') - : '', - // TODO: strict list of attributes - attributes: getAttributesToIndex(serializedNode.attributes), - }, - } - : {}), - }, + data: serializedNode + ? { + nodeId: serializedNode.id, + node: { + id: serializedNode.id, + tagName: serializedNode.tagName, + textContent: targetNode + ? Array.from(targetNode.childNodes) + .map( + (node: Node | INode) => '__sn' in node && node.__sn.type === NodeType.Text && node.__sn.textContent, + ) + .filter(Boolean) // filter out empty values + .join('') + : '', + attributes: getAttributesToRecord(serializedNode.attributes), + }, + } + : {}, }); } @@ -88,17 +83,3 @@ function getTargetNode(handlerData: DomHandlerData): Node { function isEventWithTarget(event: unknown): event is { target: Node } { return !!(event as { target?: Node }).target; } - -// Attributes we are interested in: -const ATTRIBUTES_TO_INDEX = ['id', 'class', 'aria-label', 'role', 'name']; - -function getAttributesToIndex(attributes: Record) { - const obj = {}; - for (const key of ATTRIBUTES_TO_INDEX) { - if (attributes.hasOwnProperty(key)) { - obj[key] = attributes[key]; - } - } - - return obj; -} diff --git a/packages/replay/src/coreHandlers/util/getAttributesToRecord.ts b/packages/replay/src/coreHandlers/util/getAttributesToRecord.ts new file mode 100644 index 000000000000..c7dc996507bb --- /dev/null +++ b/packages/replay/src/coreHandlers/util/getAttributesToRecord.ts @@ -0,0 +1,16 @@ +// Attributes we are interested in: +const ATTRIBUTES_TO_RECORD = new Set(['id', 'class', 'aria-label', 'role', 'name']); + +/** + * Inclusion list of attributes that we want to record from the DOM element + */ +export function getAttributesToRecord(attributes: Record): Record { + const obj: Record = {}; + for (const key in attributes) { + if (ATTRIBUTES_TO_RECORD.has(key)) { + obj[key] = attributes[key]; + } + } + + return obj; +} diff --git a/packages/replay/test/unit/coreHandlers/util/getAttributesToRecord.test.ts b/packages/replay/test/unit/coreHandlers/util/getAttributesToRecord.test.ts new file mode 100644 index 000000000000..569c41944e14 --- /dev/null +++ b/packages/replay/test/unit/coreHandlers/util/getAttributesToRecord.test.ts @@ -0,0 +1,32 @@ +import { getAttributesToRecord } from '../../../../src/coreHandlers/util/getAttributesToRecord'; + +it('records only included attributes', function () { + expect( + getAttributesToRecord({ + id: 'foo', + classList: ['btn', 'btn-primary'], + }), + ).toEqual({ + id: 'foo', + classList: ['btn', 'btn-primary'], + }); + + expect( + getAttributesToRecord({ + id: 'foo', + classList: ['btn', 'btn-primary'], + tabIndex: 2, + ariaDescribedBy: 'tooltip-1', + }), + ).toEqual({ + id: 'foo', + classList: ['btn', 'btn-primary'], + }); + + expect( + getAttributesToRecord({ + tabIndex: 2, + ariaDescribedBy: 'tooltip-1', + }), + ).toEqual({}); +}); From 8aa9ee1bf4db86af65322be7426c0910cae718a1 Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Mon, 13 Mar 2023 16:37:44 -0400 Subject: [PATCH 05/13] dupe textContent --- .../suites/replay/errors/errorsInSession/test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/integration-tests/suites/replay/errors/errorsInSession/test.ts b/packages/integration-tests/suites/replay/errors/errorsInSession/test.ts index 75a0fb94b3aa..5f1d8b6b12ff 100644 --- a/packages/integration-tests/suites/replay/errors/errorsInSession/test.ts +++ b/packages/integration-tests/suites/replay/errors/errorsInSession/test.ts @@ -136,7 +136,6 @@ sentryTest( }, id: expect.any(Number), tagName: 'button', - textContent: 'Throw Error but drop it', textContent: '***** ***** *** **** **', }, nodeId: expect.any(Number), From 75255136bc2ead8f5123097ffec3a3313dcdeb7b Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Tue, 14 Mar 2023 17:15:32 -0400 Subject: [PATCH 06/13] add more attributes, create separate test --- .../suites/replay/clickBreadcrumbs/init.js | 18 +++ .../replay/clickBreadcrumbs/template.html | 17 +++ .../suites/replay/clickBreadcrumbs/test.ts | 108 ++++++++++++++++++ .../suites/replay/errors/errorMode/test.ts | 5 +- .../replay/errors/errorsInSession/test.ts | 5 +- .../suites/replay/errors/template.html | 2 +- packages/replay/src/coreHandlers/handleDom.ts | 1 + .../util/getAttributesToRecord.ts | 20 +++- 8 files changed, 165 insertions(+), 11 deletions(-) create mode 100644 packages/integration-tests/suites/replay/clickBreadcrumbs/init.js create mode 100644 packages/integration-tests/suites/replay/clickBreadcrumbs/template.html create mode 100644 packages/integration-tests/suites/replay/clickBreadcrumbs/test.ts diff --git a/packages/integration-tests/suites/replay/clickBreadcrumbs/init.js b/packages/integration-tests/suites/replay/clickBreadcrumbs/init.js new file mode 100644 index 000000000000..a850366eaebf --- /dev/null +++ b/packages/integration-tests/suites/replay/clickBreadcrumbs/init.js @@ -0,0 +1,18 @@ +import * as Sentry from '@sentry/browser'; + +window.Sentry = Sentry; +window.Replay = new Sentry.Replay({ + flushMinDelay: 500, + flushMaxDelay: 500, + useCompression: false, + blockAllMedia: false, +}); + +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/integration-tests/suites/replay/clickBreadcrumbs/template.html b/packages/integration-tests/suites/replay/clickBreadcrumbs/template.html new file mode 100644 index 000000000000..56a956a95d24 --- /dev/null +++ b/packages/integration-tests/suites/replay/clickBreadcrumbs/template.html @@ -0,0 +1,17 @@ + + + + + + +
An Error
+ + + + diff --git a/packages/integration-tests/suites/replay/clickBreadcrumbs/test.ts b/packages/integration-tests/suites/replay/clickBreadcrumbs/test.ts new file mode 100644 index 000000000000..4febe9d9e29a --- /dev/null +++ b/packages/integration-tests/suites/replay/clickBreadcrumbs/test.ts @@ -0,0 +1,108 @@ +import { expect } from '@playwright/test'; + +import { sentryTest } from '../../../utils/fixtures'; +import { envelopeRequestParser, waitForErrorRequest } from '../../../utils/helpers'; +import { + expectedClickBreadcrumb, +} from '../../../utils/replayEventTemplates'; +import { + getReplayEvent, + getReplayRecordingContent, + shouldSkipReplayTest, + waitForReplayRequest, +} from '../../../utils/replayHelpers'; + +sentryTest('replay should have correct click breadcrumbs', async ({ forceFlushReplay, getLocalTestPath, page }) => { + if (shouldSkipReplayTest()) { + sentryTest.skip(); + } + + const reqPromise0 = waitForReplayRequest(page, 0); + const reqPromise1 = waitForReplayRequest(page, 1); + // const reqPromise2 = waitForReplayRequest(page, 2); + + 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 page.click('#error'); + await page.click('#img'); + await page.click('.sentry-unmask'); + await forceFlushReplay(); + const req1 = await reqPromise1 + // const event0 = getReplayEvent(req0); + const content1 = getReplayRecordingContent(req1); + expect(content1.breadcrumbs).toEqual( + expect.arrayContaining([ + { + ...expectedClickBreadcrumb, + data: { + nodeId: expect.any(Number), + node: { + attributes: { + 'aria-label': '** *****', + class: 'btn btn-error', + id: 'error', + role: 'button', + }, + id: expect.any(Number), + tagName: 'div', + textContent: '** *****', + }, + }, + }, + ]) + ); + + expect(content1.breadcrumbs).toEqual( + expect.arrayContaining([ + { + ...expectedClickBreadcrumb, + data: { + nodeId: expect.any(Number), + node: { + attributes: { + 'alt': 'Alt Text', + id: 'img', + }, + id: expect.any(Number), + tagName: 'img', + textContent: '', + }, + }, + }, + ]) + ); + + expect(content1.breadcrumbs).toEqual( + expect.arrayContaining([ + { + ...expectedClickBreadcrumb, + data: { + nodeId: expect.any(Number), + node: { + attributes: { + // TODO(rrweb): This is a bug in our rrweb fork! + // This attribute should be unmasked. + // 'aria-label': 'Unmasked label', + 'aria-label': '******** *****', + 'class': 'sentry-unmask', + }, + id: expect.any(Number), + tagName: 'button', + textContent: 'Unmasked', + }, + }, + }, + ]) + ); +}); diff --git a/packages/integration-tests/suites/replay/errors/errorMode/test.ts b/packages/integration-tests/suites/replay/errors/errorMode/test.ts index 949050ac15a9..93111d829f7a 100644 --- a/packages/integration-tests/suites/replay/errors/errorMode/test.ts +++ b/packages/integration-tests/suites/replay/errors/errorMode/test.ts @@ -102,13 +102,10 @@ sentryTest( nodeId: expect.any(Number), node: { attributes: { - 'aria-label': '***** *****', - class: 'btn btn-error', id: 'error', - role: 'button', }, id: expect.any(Number), - tagName: 'div', + tagName: 'button', textContent: '***** *****', }, }, diff --git a/packages/integration-tests/suites/replay/errors/errorsInSession/test.ts b/packages/integration-tests/suites/replay/errors/errorsInSession/test.ts index 5f1d8b6b12ff..d33d1df87d3c 100644 --- a/packages/integration-tests/suites/replay/errors/errorsInSession/test.ts +++ b/packages/integration-tests/suites/replay/errors/errorsInSession/test.ts @@ -68,13 +68,10 @@ sentryTest( data: { node: { attributes: { - 'aria-label': '***** *****', - class: 'btn btn-error', id: 'error', - role: 'button', }, id: expect.any(Number), - tagName: 'div', + tagName: 'button', textContent: '***** *****', }, nodeId: expect.any(Number), diff --git a/packages/integration-tests/suites/replay/errors/template.html b/packages/integration-tests/suites/replay/errors/template.html index 50be112df43b..99dcde1a9a88 100644 --- a/packages/integration-tests/suites/replay/errors/template.html +++ b/packages/integration-tests/suites/replay/errors/template.html @@ -5,7 +5,7 @@ -
Throw Error
+ diff --git a/packages/replay/src/coreHandlers/handleDom.ts b/packages/replay/src/coreHandlers/handleDom.ts index 9a1181cde56a..8450c4dcaa04 100644 --- a/packages/replay/src/coreHandlers/handleDom.ts +++ b/packages/replay/src/coreHandlers/handleDom.ts @@ -63,6 +63,7 @@ function handleDom(handlerData: DomHandlerData): Breadcrumb | null { (node: Node | INode) => '__sn' in node && node.__sn.type === NodeType.Text && node.__sn.textContent, ) .filter(Boolean) // filter out empty values + .map(text => (text as string).trim()) .join('') : '', attributes: getAttributesToRecord(serializedNode.attributes), diff --git a/packages/replay/src/coreHandlers/util/getAttributesToRecord.ts b/packages/replay/src/coreHandlers/util/getAttributesToRecord.ts index c7dc996507bb..3a1717c4e31e 100644 --- a/packages/replay/src/coreHandlers/util/getAttributesToRecord.ts +++ b/packages/replay/src/coreHandlers/util/getAttributesToRecord.ts @@ -1,5 +1,15 @@ // Attributes we are interested in: -const ATTRIBUTES_TO_RECORD = new Set(['id', 'class', 'aria-label', 'role', 'name']); +const ATTRIBUTES_TO_RECORD = new Set([ + 'id', + 'class', + 'aria-label', + 'role', + 'name', + 'alt', + 'title', + 'data-test-id', + 'data-testid', +]); /** * Inclusion list of attributes that we want to record from the DOM element @@ -8,7 +18,13 @@ export function getAttributesToRecord(attributes: Record): Reco const obj: Record = {}; for (const key in attributes) { if (ATTRIBUTES_TO_RECORD.has(key)) { - obj[key] = attributes[key]; + let normalizedKey = key; + + if (key === 'data-testid' || key === 'data-test-id') { + normalizedKey = 'testId'; + } + + obj[normalizedKey] = attributes[key]; } } From 5ed6118763d642f28f9f0e9df8e62d68650a1e37 Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Tue, 14 Mar 2023 17:18:13 -0400 Subject: [PATCH 07/13] run 100x for flake --- .../suites/replay/clickBreadcrumbs/test.ts | 169 +++++++++--------- 1 file changed, 82 insertions(+), 87 deletions(-) diff --git a/packages/integration-tests/suites/replay/clickBreadcrumbs/test.ts b/packages/integration-tests/suites/replay/clickBreadcrumbs/test.ts index 4febe9d9e29a..e46df7a273be 100644 --- a/packages/integration-tests/suites/replay/clickBreadcrumbs/test.ts +++ b/packages/integration-tests/suites/replay/clickBreadcrumbs/test.ts @@ -1,108 +1,103 @@ import { expect } from '@playwright/test'; import { sentryTest } from '../../../utils/fixtures'; -import { envelopeRequestParser, waitForErrorRequest } from '../../../utils/helpers'; -import { - expectedClickBreadcrumb, -} from '../../../utils/replayEventTemplates'; -import { - getReplayEvent, - getReplayRecordingContent, - shouldSkipReplayTest, - waitForReplayRequest, -} from '../../../utils/replayHelpers'; +import { expectedClickBreadcrumb } from '../../../utils/replayEventTemplates'; +import { getReplayRecordingContent, shouldSkipReplayTest, waitForReplayRequest } from '../../../utils/replayHelpers'; -sentryTest('replay should have correct click breadcrumbs', async ({ forceFlushReplay, getLocalTestPath, page }) => { - if (shouldSkipReplayTest()) { - sentryTest.skip(); - } +for (let i = 0; i < 100; i++) { + sentryTest( + `replay should have correct click breadcrumbs${i}`, + async ({ forceFlushReplay, getLocalTestPath, page }) => { + if (shouldSkipReplayTest()) { + sentryTest.skip(); + } - const reqPromise0 = waitForReplayRequest(page, 0); - const reqPromise1 = waitForReplayRequest(page, 1); - // const reqPromise2 = waitForReplayRequest(page, 2); + const reqPromise0 = waitForReplayRequest(page, 0); + const reqPromise1 = waitForReplayRequest(page, 1); - await page.route('https://dsn.ingest.sentry.io/**/*', route => { - return route.fulfill({ - status: 200, - contentType: 'application/json', - body: JSON.stringify({ id: 'test-id' }), - }); - }); + 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 }); + const url = await getLocalTestPath({ testDir: __dirname }); - await page.goto(url); - await reqPromise0 + await page.goto(url); + await reqPromise0; - await page.click('#error'); - await page.click('#img'); - await page.click('.sentry-unmask'); - await forceFlushReplay(); - const req1 = await reqPromise1 - // const event0 = getReplayEvent(req0); - const content1 = getReplayRecordingContent(req1); - expect(content1.breadcrumbs).toEqual( - expect.arrayContaining([ - { - ...expectedClickBreadcrumb, - data: { - nodeId: expect.any(Number), - node: { - attributes: { - 'aria-label': '** *****', - class: 'btn btn-error', - id: 'error', - role: 'button', + await page.click('#error'); + await page.click('#img'); + await page.click('.sentry-unmask'); + await forceFlushReplay(); + const req1 = await reqPromise1; + const content1 = getReplayRecordingContent(req1); + expect(content1.breadcrumbs).toEqual( + expect.arrayContaining([ + { + ...expectedClickBreadcrumb, + data: { + nodeId: expect.any(Number), + node: { + attributes: { + 'aria-label': '** *****', + class: 'btn btn-error', + id: 'error', + role: 'button', + }, + id: expect.any(Number), + tagName: 'div', + textContent: '** *****', }, - id: expect.any(Number), - tagName: 'div', - textContent: '** *****', }, }, - }, - ]) - ); + ]), + ); - expect(content1.breadcrumbs).toEqual( - expect.arrayContaining([ - { - ...expectedClickBreadcrumb, - data: { - nodeId: expect.any(Number), - node: { - attributes: { - 'alt': 'Alt Text', - id: 'img', + expect(content1.breadcrumbs).toEqual( + expect.arrayContaining([ + { + ...expectedClickBreadcrumb, + data: { + nodeId: expect.any(Number), + node: { + attributes: { + alt: 'Alt Text', + id: 'img', + }, + id: expect.any(Number), + tagName: 'img', + textContent: '', }, - id: expect.any(Number), - tagName: 'img', - textContent: '', }, }, - }, - ]) - ); + ]), + ); - expect(content1.breadcrumbs).toEqual( - expect.arrayContaining([ - { - ...expectedClickBreadcrumb, - data: { - nodeId: expect.any(Number), - node: { - attributes: { - // TODO(rrweb): This is a bug in our rrweb fork! - // This attribute should be unmasked. - // 'aria-label': 'Unmasked label', - 'aria-label': '******** *****', - 'class': 'sentry-unmask', + expect(content1.breadcrumbs).toEqual( + expect.arrayContaining([ + { + ...expectedClickBreadcrumb, + data: { + nodeId: expect.any(Number), + node: { + attributes: { + // TODO(rrweb): This is a bug in our rrweb fork! + // This attribute should be unmasked. + // 'aria-label': 'Unmasked label', + 'aria-label': '******** *****', + class: 'sentry-unmask', + }, + id: expect.any(Number), + tagName: 'button', + textContent: 'Unmasked', }, - id: expect.any(Number), - tagName: 'button', - textContent: 'Unmasked', }, }, - }, - ]) + ]), + ); + }, ); -}); +} From 8bfeb839e22782d00e86090f93463ba64ceadf67 Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Wed, 15 Mar 2023 10:54:54 -0400 Subject: [PATCH 08/13] moved... --- .../suites/replay/clickBreadcrumbs/init.js | 0 .../suites/replay/clickBreadcrumbs/template.html | 0 .../suites/replay/clickBreadcrumbs/test.ts | 0 3 files changed, 0 insertions(+), 0 deletions(-) rename packages/{integration-tests => browser-integration-tests}/suites/replay/clickBreadcrumbs/init.js (100%) rename packages/{integration-tests => browser-integration-tests}/suites/replay/clickBreadcrumbs/template.html (100%) rename packages/{integration-tests => browser-integration-tests}/suites/replay/clickBreadcrumbs/test.ts (100%) diff --git a/packages/integration-tests/suites/replay/clickBreadcrumbs/init.js b/packages/browser-integration-tests/suites/replay/clickBreadcrumbs/init.js similarity index 100% rename from packages/integration-tests/suites/replay/clickBreadcrumbs/init.js rename to packages/browser-integration-tests/suites/replay/clickBreadcrumbs/init.js diff --git a/packages/integration-tests/suites/replay/clickBreadcrumbs/template.html b/packages/browser-integration-tests/suites/replay/clickBreadcrumbs/template.html similarity index 100% rename from packages/integration-tests/suites/replay/clickBreadcrumbs/template.html rename to packages/browser-integration-tests/suites/replay/clickBreadcrumbs/template.html diff --git a/packages/integration-tests/suites/replay/clickBreadcrumbs/test.ts b/packages/browser-integration-tests/suites/replay/clickBreadcrumbs/test.ts similarity index 100% rename from packages/integration-tests/suites/replay/clickBreadcrumbs/test.ts rename to packages/browser-integration-tests/suites/replay/clickBreadcrumbs/test.ts From 2f7e73ee037bc83130755d20d635cc5da56b7266 Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Wed, 15 Mar 2023 11:09:55 -0400 Subject: [PATCH 09/13] fix tests --- .../replay/src/coreHandlers/util/getAttributesToRecord.ts | 3 ++- .../unit/coreHandlers/util/getAttributesToRecord.test.ts | 8 ++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/replay/src/coreHandlers/util/getAttributesToRecord.ts b/packages/replay/src/coreHandlers/util/getAttributesToRecord.ts index 3a1717c4e31e..7168a3243add 100644 --- a/packages/replay/src/coreHandlers/util/getAttributesToRecord.ts +++ b/packages/replay/src/coreHandlers/util/getAttributesToRecord.ts @@ -1,4 +1,5 @@ -// Attributes we are interested in: +// Note that these are the serialized attributes and not attributes directly on +// the DOM Node. Attributes we are interested in: const ATTRIBUTES_TO_RECORD = new Set([ 'id', 'class', diff --git a/packages/replay/test/unit/coreHandlers/util/getAttributesToRecord.test.ts b/packages/replay/test/unit/coreHandlers/util/getAttributesToRecord.test.ts index 569c41944e14..46211820e2c7 100644 --- a/packages/replay/test/unit/coreHandlers/util/getAttributesToRecord.test.ts +++ b/packages/replay/test/unit/coreHandlers/util/getAttributesToRecord.test.ts @@ -4,23 +4,23 @@ it('records only included attributes', function () { expect( getAttributesToRecord({ id: 'foo', - classList: ['btn', 'btn-primary'], + class: 'btn btn-primary', }), ).toEqual({ id: 'foo', - classList: ['btn', 'btn-primary'], + class: 'btn btn-primary', }); expect( getAttributesToRecord({ id: 'foo', - classList: ['btn', 'btn-primary'], + class: 'btn btn-primary', tabIndex: 2, ariaDescribedBy: 'tooltip-1', }), ).toEqual({ id: 'foo', - classList: ['btn', 'btn-primary'], + class: 'btn btn-primary', }); expect( From e6167272bfb53fcaee2e6cf471d59fe690c183ee Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Wed, 15 Mar 2023 16:44:50 -0400 Subject: [PATCH 10/13] skip firefox/webkit test --- .../suites/replay/clickBreadcrumbs/test.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/browser-integration-tests/suites/replay/clickBreadcrumbs/test.ts b/packages/browser-integration-tests/suites/replay/clickBreadcrumbs/test.ts index e46df7a273be..0884810e3552 100644 --- a/packages/browser-integration-tests/suites/replay/clickBreadcrumbs/test.ts +++ b/packages/browser-integration-tests/suites/replay/clickBreadcrumbs/test.ts @@ -7,8 +7,9 @@ import { getReplayRecordingContent, shouldSkipReplayTest, waitForReplayRequest } for (let i = 0; i < 100; i++) { sentryTest( `replay should have correct click breadcrumbs${i}`, - async ({ forceFlushReplay, getLocalTestPath, page }) => { - if (shouldSkipReplayTest()) { + async ({ forceFlushReplay, getLocalTestPath, page, browserName }) => { + // TODO(replay): This is flakey on firefox and webkit where we do not always get the latest mutation. + if (shouldSkipReplayTest() || ['firefox', 'webkit'].includes(browserName)) { sentryTest.skip(); } From f836c0b7c46a4d682bf701b0facc197a02ad9016 Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Wed, 15 Mar 2023 16:45:28 -0400 Subject: [PATCH 11/13] restore `message` in breadcrumb for now --- .../suites/replay/clickBreadcrumbs/test.ts | 3 +++ .../suites/replay/errors/errorMode/test.ts | 2 ++ .../suites/replay/errors/errorsInSession/test.ts | 2 ++ .../utils/replayEventTemplates.ts | 1 + packages/replay/src/coreHandlers/handleDom.ts | 10 +++++----- .../replay/test/integration/errorSampleRate.test.ts | 2 +- 6 files changed, 14 insertions(+), 6 deletions(-) diff --git a/packages/browser-integration-tests/suites/replay/clickBreadcrumbs/test.ts b/packages/browser-integration-tests/suites/replay/clickBreadcrumbs/test.ts index 0884810e3552..6883d18c025c 100644 --- a/packages/browser-integration-tests/suites/replay/clickBreadcrumbs/test.ts +++ b/packages/browser-integration-tests/suites/replay/clickBreadcrumbs/test.ts @@ -39,6 +39,7 @@ for (let i = 0; i < 100; i++) { expect.arrayContaining([ { ...expectedClickBreadcrumb, + message: 'body > div#error.btn.btn-error[aria-label="An Error"]', data: { nodeId: expect.any(Number), node: { @@ -61,6 +62,7 @@ for (let i = 0; i < 100; i++) { expect.arrayContaining([ { ...expectedClickBreadcrumb, + message: 'body > button > img#img[alt="Alt Text"]', data: { nodeId: expect.any(Number), node: { @@ -81,6 +83,7 @@ for (let i = 0; i < 100; i++) { expect.arrayContaining([ { ...expectedClickBreadcrumb, + message: 'body > button.sentry-unmask[aria-label="Unmasked label"]', data: { nodeId: expect.any(Number), node: { diff --git a/packages/browser-integration-tests/suites/replay/errors/errorMode/test.ts b/packages/browser-integration-tests/suites/replay/errors/errorMode/test.ts index 93111d829f7a..826e04effc32 100644 --- a/packages/browser-integration-tests/suites/replay/errors/errorMode/test.ts +++ b/packages/browser-integration-tests/suites/replay/errors/errorMode/test.ts @@ -98,6 +98,7 @@ sentryTest( expect.arrayContaining([ { ...expectedClickBreadcrumb, + message: 'body > button#error', data: { nodeId: expect.any(Number), node: { @@ -143,6 +144,7 @@ sentryTest( expect.arrayContaining([ { ...expectedClickBreadcrumb, + message: 'body > button#log', data: { node: { attributes: { id: 'log' }, diff --git a/packages/browser-integration-tests/suites/replay/errors/errorsInSession/test.ts b/packages/browser-integration-tests/suites/replay/errors/errorsInSession/test.ts index d33d1df87d3c..4643b72d2706 100644 --- a/packages/browser-integration-tests/suites/replay/errors/errorsInSession/test.ts +++ b/packages/browser-integration-tests/suites/replay/errors/errorsInSession/test.ts @@ -65,6 +65,7 @@ sentryTest( expect.arrayContaining([ { ...expectedClickBreadcrumb, + message: 'body > button#error', data: { node: { attributes: { @@ -126,6 +127,7 @@ sentryTest( expect.arrayContaining([ { ...expectedClickBreadcrumb, + message: 'body > button#drop', data: { node: { attributes: { diff --git a/packages/browser-integration-tests/utils/replayEventTemplates.ts b/packages/browser-integration-tests/utils/replayEventTemplates.ts index 33d24ca87d66..c417624f9b9a 100644 --- a/packages/browser-integration-tests/utils/replayEventTemplates.ts +++ b/packages/browser-integration-tests/utils/replayEventTemplates.ts @@ -158,6 +158,7 @@ export const expectedClickBreadcrumb = { timestamp: expect.any(Number), type: 'default', category: 'ui.click', + message: expect.any(String), data: { nodeId: expect.any(Number), node: { diff --git a/packages/replay/src/coreHandlers/handleDom.ts b/packages/replay/src/coreHandlers/handleDom.ts index 8450c4dcaa04..8d005afa1b46 100644 --- a/packages/replay/src/coreHandlers/handleDom.ts +++ b/packages/replay/src/coreHandlers/handleDom.ts @@ -1,6 +1,7 @@ import type { INode } from '@sentry-internal/rrweb-snapshot'; import { NodeType } from '@sentry-internal/rrweb-snapshot'; import type { Breadcrumb } from '@sentry/types'; +import { htmlTreeAsString } from '@sentry/utils'; import type { ReplayContainer } from '../types'; import { createBreadcrumb } from '../util/createBreadcrumb'; @@ -32,17 +33,15 @@ export const handleDomListener: (replay: ReplayContainer) => (handlerData: DomHa * An event handler to react to DOM events. */ function handleDom(handlerData: DomHandlerData): Breadcrumb | null { + let target; let targetNode: Node | INode | undefined; // Accessing event.target can throw (see getsentry/raven-js#838, #768) try { targetNode = getTargetNode(handlerData); + target = htmlTreeAsString(targetNode); } catch (e) { - // Nothing to do - } - - if (!targetNode) { - return null; + target = ''; } // `__sn` property is the serialized node created by rrweb @@ -51,6 +50,7 @@ function handleDom(handlerData: DomHandlerData): Breadcrumb | null { return createBreadcrumb({ category: `ui.${handlerData.name}`, + message: target, data: serializedNode ? { nodeId: serializedNode.id, diff --git a/packages/replay/test/integration/errorSampleRate.test.ts b/packages/replay/test/integration/errorSampleRate.test.ts index 1c4ef227db6c..d4bfa7279a95 100644 --- a/packages/replay/test/integration/errorSampleRate.test.ts +++ b/packages/replay/test/integration/errorSampleRate.test.ts @@ -54,7 +54,7 @@ describe('Integration | errorSampleRate', () => { expect(mockRecord.takeFullSnapshot).not.toHaveBeenCalled(); expect(replay).not.toHaveLastSentReplay(); - // Does not capture mouse click + // Does not capture on mouse click domHandler({ name: 'click', }); From eb89070aebd23c8299f0ae291a284b31128cb7b1 Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Wed, 15 Mar 2023 18:13:07 -0400 Subject: [PATCH 12/13] remove bench --- .../suites/replay/clickBreadcrumbs/test.ts | 166 +++++++++--------- 1 file changed, 82 insertions(+), 84 deletions(-) diff --git a/packages/browser-integration-tests/suites/replay/clickBreadcrumbs/test.ts b/packages/browser-integration-tests/suites/replay/clickBreadcrumbs/test.ts index 6883d18c025c..6929c26bee12 100644 --- a/packages/browser-integration-tests/suites/replay/clickBreadcrumbs/test.ts +++ b/packages/browser-integration-tests/suites/replay/clickBreadcrumbs/test.ts @@ -4,104 +4,102 @@ import { sentryTest } from '../../../utils/fixtures'; import { expectedClickBreadcrumb } from '../../../utils/replayEventTemplates'; import { getReplayRecordingContent, shouldSkipReplayTest, waitForReplayRequest } from '../../../utils/replayHelpers'; -for (let i = 0; i < 100; i++) { - sentryTest( - `replay should have correct click breadcrumbs${i}`, - async ({ forceFlushReplay, getLocalTestPath, page, browserName }) => { - // TODO(replay): This is flakey on firefox and webkit where we do not always get the latest mutation. - if (shouldSkipReplayTest() || ['firefox', 'webkit'].includes(browserName)) { - sentryTest.skip(); - } +sentryTest( + 'replay should have correct click breadcrumbs', + async ({ forceFlushReplay, getLocalTestPath, page, browserName }) => { + // TODO(replay): This is flakey on firefox and webkit where we do not always get the latest mutation. + if (shouldSkipReplayTest() || ['firefox', 'webkit'].includes(browserName)) { + sentryTest.skip(); + } - const reqPromise0 = waitForReplayRequest(page, 0); - const reqPromise1 = waitForReplayRequest(page, 1); + const reqPromise0 = waitForReplayRequest(page, 0); + const reqPromise1 = waitForReplayRequest(page, 1); - await page.route('https://dsn.ingest.sentry.io/**/*', route => { - return route.fulfill({ - status: 200, - contentType: 'application/json', - body: JSON.stringify({ id: 'test-id' }), - }); + 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 }); + const url = await getLocalTestPath({ testDir: __dirname }); - await page.goto(url); - await reqPromise0; + await page.goto(url); + await reqPromise0; - await page.click('#error'); - await page.click('#img'); - await page.click('.sentry-unmask'); - await forceFlushReplay(); - const req1 = await reqPromise1; - const content1 = getReplayRecordingContent(req1); - expect(content1.breadcrumbs).toEqual( - expect.arrayContaining([ - { - ...expectedClickBreadcrumb, - message: 'body > div#error.btn.btn-error[aria-label="An Error"]', - data: { - nodeId: expect.any(Number), - node: { - attributes: { - 'aria-label': '** *****', - class: 'btn btn-error', - id: 'error', - role: 'button', - }, - id: expect.any(Number), - tagName: 'div', - textContent: '** *****', + await page.click('#error'); + await page.click('#img'); + await page.click('.sentry-unmask'); + await forceFlushReplay(); + const req1 = await reqPromise1; + const content1 = getReplayRecordingContent(req1); + expect(content1.breadcrumbs).toEqual( + expect.arrayContaining([ + { + ...expectedClickBreadcrumb, + message: 'body > div#error.btn.btn-error[aria-label="An Error"]', + data: { + nodeId: expect.any(Number), + node: { + attributes: { + 'aria-label': '** *****', + class: 'btn btn-error', + id: 'error', + role: 'button', }, + id: expect.any(Number), + tagName: 'div', + textContent: '** *****', }, }, - ]), - ); + }, + ]), + ); - expect(content1.breadcrumbs).toEqual( - expect.arrayContaining([ - { - ...expectedClickBreadcrumb, - message: 'body > button > img#img[alt="Alt Text"]', - data: { - nodeId: expect.any(Number), - node: { - attributes: { - alt: 'Alt Text', - id: 'img', - }, - id: expect.any(Number), - tagName: 'img', - textContent: '', + expect(content1.breadcrumbs).toEqual( + expect.arrayContaining([ + { + ...expectedClickBreadcrumb, + message: 'body > button > img#img[alt="Alt Text"]', + data: { + nodeId: expect.any(Number), + node: { + attributes: { + alt: 'Alt Text', + id: 'img', }, + id: expect.any(Number), + tagName: 'img', + textContent: '', }, }, - ]), - ); + }, + ]), + ); - expect(content1.breadcrumbs).toEqual( - expect.arrayContaining([ - { - ...expectedClickBreadcrumb, - message: 'body > button.sentry-unmask[aria-label="Unmasked label"]', - data: { - nodeId: expect.any(Number), - node: { - attributes: { - // TODO(rrweb): This is a bug in our rrweb fork! - // This attribute should be unmasked. - // 'aria-label': 'Unmasked label', - 'aria-label': '******** *****', - class: 'sentry-unmask', - }, - id: expect.any(Number), - tagName: 'button', - textContent: 'Unmasked', + expect(content1.breadcrumbs).toEqual( + expect.arrayContaining([ + { + ...expectedClickBreadcrumb, + message: 'body > button.sentry-unmask[aria-label="Unmasked label"]', + data: { + nodeId: expect.any(Number), + node: { + attributes: { + // TODO(rrweb): This is a bug in our rrweb fork! + // This attribute should be unmasked. + // 'aria-label': 'Unmasked label', + 'aria-label': '******** *****', + class: 'sentry-unmask', }, + id: expect.any(Number), + tagName: 'button', + textContent: 'Unmasked', }, }, - ]), - ); - }, - ); -} + }, + ]), + ); + }, +); From 12691022b51237481e8ec186078516d70863ab9f Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Wed, 15 Mar 2023 18:46:42 -0400 Subject: [PATCH 13/13] remove clickBreadcrumbs, use existing customEvents, skip some flakey tests on firefox --- .../suites/replay/clickBreadcrumbs/init.js | 18 --- .../replay/clickBreadcrumbs/template.html | 17 --- .../suites/replay/clickBreadcrumbs/test.ts | 105 ------------------ .../suites/replay/customEvents/init.js | 1 + .../suites/replay/customEvents/template.html | 10 +- .../suites/replay/customEvents/test.ts | 91 ++++++++++++--- .../replay/errors/errorsInSession/test.ts | 5 +- 7 files changed, 89 insertions(+), 158 deletions(-) delete mode 100644 packages/browser-integration-tests/suites/replay/clickBreadcrumbs/init.js delete mode 100644 packages/browser-integration-tests/suites/replay/clickBreadcrumbs/template.html delete mode 100644 packages/browser-integration-tests/suites/replay/clickBreadcrumbs/test.ts diff --git a/packages/browser-integration-tests/suites/replay/clickBreadcrumbs/init.js b/packages/browser-integration-tests/suites/replay/clickBreadcrumbs/init.js deleted file mode 100644 index a850366eaebf..000000000000 --- a/packages/browser-integration-tests/suites/replay/clickBreadcrumbs/init.js +++ /dev/null @@ -1,18 +0,0 @@ -import * as Sentry from '@sentry/browser'; - -window.Sentry = Sentry; -window.Replay = new Sentry.Replay({ - flushMinDelay: 500, - flushMaxDelay: 500, - useCompression: false, - blockAllMedia: false, -}); - -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/clickBreadcrumbs/template.html b/packages/browser-integration-tests/suites/replay/clickBreadcrumbs/template.html deleted file mode 100644 index 56a956a95d24..000000000000 --- a/packages/browser-integration-tests/suites/replay/clickBreadcrumbs/template.html +++ /dev/null @@ -1,17 +0,0 @@ - - - - - - -
An Error
- - - - diff --git a/packages/browser-integration-tests/suites/replay/clickBreadcrumbs/test.ts b/packages/browser-integration-tests/suites/replay/clickBreadcrumbs/test.ts deleted file mode 100644 index 6929c26bee12..000000000000 --- a/packages/browser-integration-tests/suites/replay/clickBreadcrumbs/test.ts +++ /dev/null @@ -1,105 +0,0 @@ -import { expect } from '@playwright/test'; - -import { sentryTest } from '../../../utils/fixtures'; -import { expectedClickBreadcrumb } from '../../../utils/replayEventTemplates'; -import { getReplayRecordingContent, shouldSkipReplayTest, waitForReplayRequest } from '../../../utils/replayHelpers'; - -sentryTest( - 'replay should have correct click breadcrumbs', - async ({ forceFlushReplay, getLocalTestPath, page, browserName }) => { - // TODO(replay): This is flakey on firefox and webkit where we do not always get the latest mutation. - if (shouldSkipReplayTest() || ['firefox', 'webkit'].includes(browserName)) { - sentryTest.skip(); - } - - const reqPromise0 = waitForReplayRequest(page, 0); - const reqPromise1 = waitForReplayRequest(page, 1); - - 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 page.click('#error'); - await page.click('#img'); - await page.click('.sentry-unmask'); - await forceFlushReplay(); - const req1 = await reqPromise1; - const content1 = getReplayRecordingContent(req1); - expect(content1.breadcrumbs).toEqual( - expect.arrayContaining([ - { - ...expectedClickBreadcrumb, - message: 'body > div#error.btn.btn-error[aria-label="An Error"]', - data: { - nodeId: expect.any(Number), - node: { - attributes: { - 'aria-label': '** *****', - class: 'btn btn-error', - id: 'error', - role: 'button', - }, - id: expect.any(Number), - tagName: 'div', - textContent: '** *****', - }, - }, - }, - ]), - ); - - expect(content1.breadcrumbs).toEqual( - expect.arrayContaining([ - { - ...expectedClickBreadcrumb, - message: 'body > button > img#img[alt="Alt Text"]', - data: { - nodeId: expect.any(Number), - node: { - attributes: { - alt: 'Alt Text', - id: 'img', - }, - id: expect.any(Number), - tagName: 'img', - textContent: '', - }, - }, - }, - ]), - ); - - expect(content1.breadcrumbs).toEqual( - expect.arrayContaining([ - { - ...expectedClickBreadcrumb, - message: 'body > button.sentry-unmask[aria-label="Unmasked label"]', - data: { - nodeId: expect.any(Number), - node: { - attributes: { - // TODO(rrweb): This is a bug in our rrweb fork! - // This attribute should be unmasked. - // 'aria-label': 'Unmasked label', - 'aria-label': '******** *****', - class: 'sentry-unmask', - }, - id: expect.any(Number), - tagName: 'button', - textContent: 'Unmasked', - }, - }, - }, - ]), - ); - }, -); diff --git a/packages/browser-integration-tests/suites/replay/customEvents/init.js b/packages/browser-integration-tests/suites/replay/customEvents/init.js index db6a0aa21821..a850366eaebf 100644 --- a/packages/browser-integration-tests/suites/replay/customEvents/init.js +++ b/packages/browser-integration-tests/suites/replay/customEvents/init.js @@ -5,6 +5,7 @@ window.Replay = new Sentry.Replay({ flushMinDelay: 500, flushMaxDelay: 500, useCompression: false, + blockAllMedia: false, }); Sentry.init({ diff --git a/packages/browser-integration-tests/suites/replay/customEvents/template.html b/packages/browser-integration-tests/suites/replay/customEvents/template.html index 31cfc73ec3c3..56a956a95d24 100644 --- a/packages/browser-integration-tests/suites/replay/customEvents/template.html +++ b/packages/browser-integration-tests/suites/replay/customEvents/template.html @@ -4,6 +4,14 @@ - +
An Error
+ + diff --git a/packages/browser-integration-tests/suites/replay/customEvents/test.ts b/packages/browser-integration-tests/suites/replay/customEvents/test.ts index a0d790a5b62c..e64a76badea6 100644 --- a/packages/browser-integration-tests/suites/replay/customEvents/test.ts +++ b/packages/browser-integration-tests/suites/replay/customEvents/test.ts @@ -14,6 +14,7 @@ import type { PerformanceSpan } from '../../../utils/replayHelpers'; import { getCustomRecordingEvents, getReplayEvent, + getReplayRecordingContent, shouldSkipReplayTest, waitForReplayRequest, } from '../../../utils/replayHelpers'; @@ -81,8 +82,9 @@ sentryTest( sentryTest( 'replay recording should contain a click breadcrumb when a button is clicked', - async ({ getLocalTestPath, page }) => { - if (shouldSkipReplayTest()) { + async ({ forceFlushReplay, getLocalTestPath, page, browserName }) => { + // TODO(replay): This is flakey on firefox and webkit where clicks are flakey + if (shouldSkipReplayTest() || ['firefox', 'webkit'].includes(browserName)) { sentryTest.skip(); } @@ -100,21 +102,80 @@ sentryTest( const url = await getLocalTestPath({ testDir: __dirname }); await page.goto(url); - const replayEvent0 = getReplayEvent(await reqPromise0); - const { breadcrumbs: breadcrumbs0 } = getCustomRecordingEvents(await reqPromise0); - - expect(replayEvent0).toEqual(getExpectedReplayEvent({ segment_id: 0 })); - expect(breadcrumbs0.length).toEqual(0); - - await page.click('button'); - - const replayEvent1 = getReplayEvent(await reqPromise1); - const { breadcrumbs: breadcrumbs1 } = getCustomRecordingEvents(await reqPromise1); + await reqPromise0; + + await page.click('#error'); + await page.click('#img'); + await page.click('.sentry-unmask'); + await forceFlushReplay(); + const req1 = await reqPromise1; + const content1 = getReplayRecordingContent(req1); + expect(content1.breadcrumbs).toEqual( + expect.arrayContaining([ + { + ...expectedClickBreadcrumb, + message: 'body > div#error.btn.btn-error[aria-label="An Error"]', + data: { + nodeId: expect.any(Number), + node: { + attributes: { + 'aria-label': '** *****', + class: 'btn btn-error', + id: 'error', + role: 'button', + }, + id: expect.any(Number), + tagName: 'div', + textContent: '** *****', + }, + }, + }, + ]), + ); - expect(replayEvent1).toEqual( - getExpectedReplayEvent({ segment_id: 1, urls: [], replay_start_timestamp: undefined }), + expect(content1.breadcrumbs).toEqual( + expect.arrayContaining([ + { + ...expectedClickBreadcrumb, + message: 'body > button > img#img[alt="Alt Text"]', + data: { + nodeId: expect.any(Number), + node: { + attributes: { + alt: 'Alt Text', + id: 'img', + }, + id: expect.any(Number), + tagName: 'img', + textContent: '', + }, + }, + }, + ]), ); - expect(breadcrumbs1).toEqual([expectedClickBreadcrumb]); + expect(content1.breadcrumbs).toEqual( + expect.arrayContaining([ + { + ...expectedClickBreadcrumb, + message: 'body > button.sentry-unmask[aria-label="Unmasked label"]', + data: { + nodeId: expect.any(Number), + node: { + attributes: { + // TODO(rrweb): This is a bug in our rrweb fork! + // This attribute should be unmasked. + // 'aria-label': 'Unmasked label', + 'aria-label': '******** *****', + class: 'sentry-unmask', + }, + id: expect.any(Number), + tagName: 'button', + textContent: 'Unmasked', + }, + }, + }, + ]), + ); }, ); diff --git a/packages/browser-integration-tests/suites/replay/errors/errorsInSession/test.ts b/packages/browser-integration-tests/suites/replay/errors/errorsInSession/test.ts index 4643b72d2706..80d36b6688e2 100644 --- a/packages/browser-integration-tests/suites/replay/errors/errorsInSession/test.ts +++ b/packages/browser-integration-tests/suites/replay/errors/errorsInSession/test.ts @@ -12,8 +12,9 @@ import { sentryTest( '[session-mode] replay event should contain an error id of an error that occurred during session recording', - async ({ getLocalTestPath, page }) => { - if (shouldSkipReplayTest()) { + async ({ getLocalTestPath, page, browserName }) => { + // TODO(replay): This is flakey on firefox where clicks are flakey + if (shouldSkipReplayTest() || ['firefox'].includes(browserName)) { sentryTest.skip(); }