Skip to content

Commit 3572788

Browse files
logaretmclaude
andauthored
fix(replay): use live click attributes in breadcrumbs (#20262)
Fixes replay element attributes grabbing a potentially stale version of the attributes, we basically now prefer the live element if available, otherwise we keep the old behavior. closes #20238 --------- Co-authored-by: GPT-5 <noreply@anthropic.com>
1 parent 7284606 commit 3572788

File tree

5 files changed

+216
-4
lines changed

5 files changed

+216
-4
lines changed

.size-limit.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -269,7 +269,7 @@ module.exports = [
269269
path: createCDNPath('bundle.tracing.replay.min.js'),
270270
gzip: false,
271271
brotli: false,
272-
limit: '247 KB',
272+
limit: '248 KB',
273273
},
274274
{
275275
name: 'CDN Bundle (incl. Tracing, Replay, Logs, Metrics) - uncompressed',

dev-packages/browser-integration-tests/suites/replay/slowClick/mutation/test.ts

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,66 @@ sentryTest('mutation after threshold results in slow click', async ({ forceFlush
5656
expect(slowClickBreadcrumbs[0]?.data?.timeAfterClickMs).toBeLessThan(3501);
5757
});
5858

59+
sentryTest(
60+
'uses updated attributes for click breadcrumbs after mutation',
61+
async ({ forceFlushReplay, getLocalTestUrl, page }) => {
62+
if (shouldSkipReplayTest()) {
63+
sentryTest.skip();
64+
}
65+
66+
const url = await getLocalTestUrl({ testDir: __dirname });
67+
68+
const replayRequestPromise = waitForReplayRequest(page, 0);
69+
const segmentReqWithClickBreadcrumbPromise = waitForReplayRequest(page, (_event, res) => {
70+
const { breadcrumbs } = getCustomRecordingEvents(res);
71+
72+
return breadcrumbs.some(breadcrumb => breadcrumb.category === 'ui.click');
73+
});
74+
75+
await page.goto(url);
76+
await replayRequestPromise;
77+
78+
await forceFlushReplay();
79+
80+
await page.evaluate(() => {
81+
const target = document.getElementById('next-question-button');
82+
if (!target) {
83+
throw new Error('Could not find target button');
84+
}
85+
86+
target.id = 'save-note-button';
87+
target.setAttribute('data-testid', 'save-note-button');
88+
});
89+
90+
await page.getByRole('button', { name: 'Next question' }).click();
91+
await forceFlushReplay();
92+
93+
const segmentReqWithClickBreadcrumb = await segmentReqWithClickBreadcrumbPromise;
94+
95+
const { breadcrumbs } = getCustomRecordingEvents(segmentReqWithClickBreadcrumb);
96+
const updatedClickBreadcrumb = breadcrumbs.find(breadcrumb => breadcrumb.category === 'ui.click');
97+
98+
expect(updatedClickBreadcrumb).toEqual({
99+
category: 'ui.click',
100+
data: {
101+
node: {
102+
attributes: {
103+
id: 'save-note-button',
104+
testId: 'save-note-button',
105+
},
106+
id: expect.any(Number),
107+
tagName: 'button',
108+
textContent: '**** ********',
109+
},
110+
nodeId: expect.any(Number),
111+
},
112+
message: 'body > button#save-note-button',
113+
timestamp: expect.any(Number),
114+
type: 'default',
115+
});
116+
},
117+
);
118+
59119
sentryTest('multiple clicks are counted', async ({ getLocalTestUrl, page }) => {
60120
if (shouldSkipReplayTest()) {
61121
sentryTest.skip();

dev-packages/browser-integration-tests/suites/replay/slowClick/template.html

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
<body>
77
<button id="mutationButton">Trigger mutation</button>
88
<div id="mutationDiv">Trigger mutation</div>
9+
<button id="next-question-button" data-testid="next-question-button">Next question</button>
910
<button id="mutationButtonImmediately">Trigger mutation immediately</button>
1011
<button
1112
id="mutationButtonInline"

packages/replay-internal/src/util/handleRecordingEmit.ts

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,17 @@
1-
import { EventType } from '@sentry-internal/rrweb';
1+
import { EventType, IncrementalSource, record } from '@sentry-internal/rrweb';
2+
import { NodeType } from '@sentry-internal/rrweb-snapshot';
23
import { updateClickDetectorForRecordingEvent } from '../coreHandlers/handleClick';
34
import { DEBUG_BUILD } from '../debug-build';
45
import { saveSession } from '../session/saveSession';
56
import type { RecordingEvent, ReplayContainer, ReplayOptionFrameEvent } from '../types';
67
import { addEventSync } from './addEvent';
78
import { debug } from './logger';
89

10+
type MutationAttributeData = {
11+
id: number;
12+
attributes: Record<string, string | number | true | null>;
13+
};
14+
915
type RecordingEmitCallback = (event: RecordingEvent, isCheckout?: boolean) => void;
1016

1117
/**
@@ -29,6 +35,8 @@ export function getHandleRecordingEmit(replay: ReplayContainer): RecordingEmitCa
2935
const isCheckout = _isCheckout || !hadFirstEvent;
3036
hadFirstEvent = true;
3137

38+
syncMirrorAttributesFromMutationEvent(event);
39+
3240
if (replay.clickDetector) {
3341
updateClickDetectorForRecordingEvent(replay.clickDetector, event);
3442
}
@@ -112,6 +120,40 @@ export function getHandleRecordingEmit(replay: ReplayContainer): RecordingEmitCa
112120
};
113121
}
114122

123+
export function syncMirrorAttributesFromMutationEvent(event: RecordingEvent): void {
124+
const data = event.data;
125+
126+
if (
127+
event.type !== EventType.IncrementalSnapshot ||
128+
!data ||
129+
typeof data !== 'object' ||
130+
!('source' in data) ||
131+
data.source !== IncrementalSource.Mutation ||
132+
!('attributes' in data) ||
133+
!Array.isArray(data.attributes)
134+
) {
135+
return;
136+
}
137+
138+
for (const mutation of data.attributes as MutationAttributeData[]) {
139+
const node = record.mirror.getNode(mutation.id);
140+
const meta = node && record.mirror.getMeta(node);
141+
142+
if (meta?.type !== NodeType.Element) {
143+
continue;
144+
}
145+
146+
for (const [attributeName, value] of Object.entries(mutation.attributes)) {
147+
if (value === null) {
148+
// oxlint-disable-next-line typescript/no-dynamic-delete
149+
delete meta.attributes[attributeName];
150+
} else {
151+
meta.attributes[attributeName] = value;
152+
}
153+
}
154+
}
155+
}
156+
115157
/**
116158
* Exported for tests
117159
*/

packages/replay-internal/test/unit/util/handleRecordingEmit.test.ts

Lines changed: 111 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,18 @@
33
*/
44

55
import '../../utils/mock-internal-setTimeout';
6-
import { EventType } from '@sentry-internal/rrweb';
6+
import { EventType, IncrementalSource, record } from '@sentry-internal/rrweb';
7+
import { NodeType, type serializedElementNodeWithId } from '@sentry-internal/rrweb-snapshot';
78
import type { MockInstance } from 'vitest';
89
import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest';
10+
import { handleDom } from '../../../src/coreHandlers/handleDom';
911
import type { ReplayOptionFrameEvent } from '../../../src/types';
1012
import * as SentryAddEvent from '../../../src/util/addEvent';
11-
import { createOptionsEvent, getHandleRecordingEmit } from '../../../src/util/handleRecordingEmit';
13+
import {
14+
createOptionsEvent,
15+
getHandleRecordingEmit,
16+
syncMirrorAttributesFromMutationEvent,
17+
} from '../../../src/util/handleRecordingEmit';
1218
import { BASE_TIMESTAMP } from '../..';
1319
import { setupReplayContainer } from '../../utils/setupReplayContainer';
1420

@@ -30,6 +36,7 @@ describe('Unit | util | handleRecordingEmit', () => {
3036

3137
afterEach(function () {
3238
addEventMock.mockReset();
39+
vi.restoreAllMocks();
3340
});
3441

3542
it('interprets first event as checkout event', async function () {
@@ -95,4 +102,106 @@ describe('Unit | util | handleRecordingEmit', () => {
95102
expect(addEventMock).toHaveBeenNthCalledWith(3, replay, event, true);
96103
expect(addEventMock).toHaveBeenLastCalledWith(replay, { ...optionsEvent, timestamp: BASE_TIMESTAMP }, false);
97104
});
105+
106+
it('syncs mirror attributes from mutation events', function () {
107+
const target = document.createElement('button');
108+
target.textContent = 'Save Note';
109+
110+
const meta = {
111+
id: 42,
112+
type: NodeType.Element,
113+
tagName: 'button',
114+
childNodes: [{ id: 43, type: NodeType.Text, textContent: 'Save Note' }],
115+
attributes: {
116+
id: 'next-question-button',
117+
'data-testid': 'next-question-button',
118+
},
119+
};
120+
121+
vi.spyOn(record.mirror, 'getNode').mockReturnValue(target);
122+
vi.spyOn(record.mirror, 'getMeta').mockReturnValue(meta as serializedElementNodeWithId);
123+
vi.spyOn(record.mirror, 'getId').mockReturnValue(42);
124+
125+
syncMirrorAttributesFromMutationEvent({
126+
type: EventType.IncrementalSnapshot,
127+
timestamp: BASE_TIMESTAMP + 10,
128+
data: {
129+
source: IncrementalSource.Mutation,
130+
texts: [],
131+
attributes: [
132+
{
133+
id: 42,
134+
attributes: {
135+
id: 'save-note-button',
136+
'data-testid': 'save-note-button',
137+
},
138+
},
139+
],
140+
removes: [],
141+
adds: [],
142+
},
143+
});
144+
145+
expect(
146+
handleDom({
147+
name: 'click',
148+
event: { target },
149+
}),
150+
).toEqual({
151+
category: 'ui.click',
152+
data: {
153+
nodeId: 42,
154+
node: {
155+
id: 42,
156+
tagName: 'button',
157+
textContent: 'Save Note',
158+
attributes: {
159+
id: 'save-note-button',
160+
testId: 'save-note-button',
161+
},
162+
},
163+
},
164+
message: 'button',
165+
timestamp: expect.any(Number),
166+
type: 'default',
167+
});
168+
});
169+
170+
it('preserves masked mutation attribute values', function () {
171+
const target = document.createElement('button');
172+
173+
const meta = {
174+
id: 42,
175+
type: NodeType.Element,
176+
tagName: 'button',
177+
childNodes: [],
178+
attributes: {
179+
'aria-label': 'Save Note',
180+
},
181+
};
182+
183+
vi.spyOn(record.mirror, 'getNode').mockReturnValue(target);
184+
vi.spyOn(record.mirror, 'getMeta').mockReturnValue(meta as serializedElementNodeWithId);
185+
186+
syncMirrorAttributesFromMutationEvent({
187+
type: EventType.IncrementalSnapshot,
188+
timestamp: BASE_TIMESTAMP + 10,
189+
data: {
190+
source: IncrementalSource.Mutation,
191+
texts: [],
192+
attributes: [
193+
{
194+
id: 42,
195+
attributes: {
196+
'aria-label': '*********',
197+
},
198+
},
199+
],
200+
removes: [],
201+
adds: [],
202+
},
203+
});
204+
205+
expect(meta.attributes['aria-label']).toBe('*********');
206+
});
98207
});

0 commit comments

Comments
 (0)