Skip to content

Commit 067ad93

Browse files
authored
fix(replay): Ensure replay_id is removed from frozen DSC when stopped (#13893)
When a replay is stopped (for whatever reason), we want to ensure to remove the `replay_id` from the DSC. This can be in two places: 1. Frozen onto the current spans DSC 2. On the current scope propagation context DSC We just reset it on both. Closes #13872
1 parent bd96876 commit 067ad93

File tree

6 files changed

+119
-4
lines changed

6 files changed

+119
-4
lines changed

.size-limit.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ module.exports = [
180180
name: 'CDN Bundle (incl. Tracing, Replay)',
181181
path: createCDNPath('bundle.tracing.replay.min.js'),
182182
gzip: true,
183-
limit: '73 KB',
183+
limit: '74 KB',
184184
},
185185
{
186186
name: 'CDN Bundle (incl. Tracing, Replay, Feedback)',
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
import * as Sentry from '@sentry/browser';
2+
3+
window._triggerError = function (errorCount) {
4+
Sentry.captureException(new Error(`This is error #${errorCount}`));
5+
};

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

+80-1
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,12 @@ import type * as Sentry from '@sentry/browser';
33
import type { EventEnvelopeHeaders } from '@sentry/types';
44

55
import { sentryTest } from '../../../utils/fixtures';
6-
import { envelopeRequestParser, shouldSkipTracingTest, waitForTransactionRequest } from '../../../utils/helpers';
6+
import {
7+
envelopeRequestParser,
8+
shouldSkipTracingTest,
9+
waitForErrorRequest,
10+
waitForTransactionRequest,
11+
} from '../../../utils/helpers';
712
import { getReplaySnapshot, shouldSkipReplayTest, waitForReplayRunning } from '../../../utils/replayHelpers';
813

914
type TestWindow = Window & {
@@ -216,3 +221,77 @@ sentryTest(
216221
});
217222
},
218223
);
224+
225+
sentryTest('should add replay_id to error DSC while replay is active', async ({ getLocalTestPath, page }) => {
226+
if (shouldSkipReplayTest()) {
227+
sentryTest.skip();
228+
}
229+
230+
const hasTracing = !shouldSkipTracingTest();
231+
232+
await page.route('https://dsn.ingest.sentry.io/**/*', route => {
233+
return route.fulfill({
234+
status: 200,
235+
contentType: 'application/json',
236+
body: JSON.stringify({ id: 'test-id' }),
237+
});
238+
});
239+
240+
const url = await getLocalTestPath({ testDir: __dirname });
241+
await page.goto(url);
242+
243+
const error1Req = waitForErrorRequest(page, event => event.exception?.values?.[0].value === 'This is error #1');
244+
const error2Req = waitForErrorRequest(page, event => event.exception?.values?.[0].value === 'This is error #2');
245+
246+
// We want to wait for the transaction to be done, to ensure we have a consistent test
247+
const transactionReq = hasTracing ? waitForTransactionRequest(page) : Promise.resolve();
248+
249+
// Wait for this to be available
250+
await page.waitForFunction('!!window.Replay');
251+
252+
// We have to start replay before we finish the transaction, otherwise the DSC will not be frozen with the Replay ID
253+
await page.evaluate('window.Replay.start();');
254+
await waitForReplayRunning(page);
255+
await transactionReq;
256+
257+
await page.evaluate('window._triggerError(1)');
258+
259+
const error1Header = envelopeRequestParser(await error1Req, 0) as EventEnvelopeHeaders;
260+
const replay = await getReplaySnapshot(page);
261+
262+
expect(replay.session?.id).toBeDefined();
263+
264+
expect(error1Header.trace).toBeDefined();
265+
expect(error1Header.trace).toEqual({
266+
environment: 'production',
267+
trace_id: expect.any(String),
268+
public_key: 'public',
269+
replay_id: replay.session?.id,
270+
...(hasTracing
271+
? {
272+
sample_rate: '1',
273+
sampled: 'true',
274+
}
275+
: {}),
276+
});
277+
278+
// Now end replay and trigger another error, it should not have a replay_id in DSC anymore
279+
await page.evaluate('window.Replay.stop();');
280+
await page.waitForFunction('!window.Replay.getReplayId();');
281+
await page.evaluate('window._triggerError(2)');
282+
283+
const error2Header = envelopeRequestParser(await error2Req, 0) as EventEnvelopeHeaders;
284+
285+
expect(error2Header.trace).toBeDefined();
286+
expect(error2Header.trace).toEqual({
287+
environment: 'production',
288+
trace_id: expect.any(String),
289+
public_key: 'public',
290+
...(hasTracing
291+
? {
292+
sample_rate: '1',
293+
sampled: 'true',
294+
}
295+
: {}),
296+
});
297+
});

dev-packages/browser-integration-tests/utils/helpers.ts

+10-2
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,7 @@ export async function waitForTransactionRequestOnUrl(page: Page, url: string): P
199199
return req;
200200
}
201201

202-
export function waitForErrorRequest(page: Page): Promise<Request> {
202+
export function waitForErrorRequest(page: Page, callback?: (event: Event) => boolean): Promise<Request> {
203203
return page.waitForRequest(req => {
204204
const postData = req.postData();
205205
if (!postData) {
@@ -209,7 +209,15 @@ export function waitForErrorRequest(page: Page): Promise<Request> {
209209
try {
210210
const event = envelopeRequestParser(req);
211211

212-
return !event.type;
212+
if (event.type) {
213+
return false;
214+
}
215+
216+
if (callback) {
217+
return callback(event);
218+
}
219+
220+
return true;
213221
} catch {
214222
return false;
215223
}

packages/replay-internal/src/replay.ts

+3
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ import { debounce } from './util/debounce';
5353
import { getHandleRecordingEmit } from './util/handleRecordingEmit';
5454
import { isExpired } from './util/isExpired';
5555
import { isSessionExpired } from './util/isSessionExpired';
56+
import { resetReplayIdOnDynamicSamplingContext } from './util/resetReplayIdOnDynamicSamplingContext';
5657
import { sendReplay } from './util/sendReplay';
5758
import { RateLimitError } from './util/sendReplayRequest';
5859
import type { SKIPPED } from './util/throttle';
@@ -446,6 +447,8 @@ export class ReplayContainer implements ReplayContainerInterface {
446447
try {
447448
DEBUG_BUILD && logger.info(`Stopping Replay${reason ? ` triggered by ${reason}` : ''}`);
448449

450+
resetReplayIdOnDynamicSamplingContext();
451+
449452
this._removeListeners();
450453
this.stopRecording();
451454

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
import { getActiveSpan, getCurrentScope, getDynamicSamplingContextFromSpan } from '@sentry/core';
2+
import type { DynamicSamplingContext } from '@sentry/types';
3+
4+
/**
5+
* Reset the `replay_id` field on the DSC.
6+
*/
7+
export function resetReplayIdOnDynamicSamplingContext(): void {
8+
// Reset DSC on the current scope, if there is one
9+
const dsc = getCurrentScope().getPropagationContext().dsc;
10+
if (dsc) {
11+
delete dsc.replay_id;
12+
}
13+
14+
// Clear it from frozen DSC on the active span
15+
const activeSpan = getActiveSpan();
16+
if (activeSpan) {
17+
const dsc = getDynamicSamplingContextFromSpan(activeSpan);
18+
delete (dsc as Partial<DynamicSamplingContext>).replay_id;
19+
}
20+
}

0 commit comments

Comments
 (0)