Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(replay): Ensure replay_id is removed from frozen DSC when stopped #13893

Merged
merged 3 commits into from
Oct 8, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
fix(replay): Ensure replay_id is removed from frozen DSC
  • Loading branch information
mydea committed Oct 8, 2024
commit ee697be1ec91220afcb8257b62ae85b99cf0cd9a
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import * as Sentry from '@sentry/browser';

window._triggerError = function (errorCount) {
Sentry.captureException(new Error(`This is error #${errorCount}`));
};
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,12 @@ import type * as Sentry from '@sentry/browser';
import type { EventEnvelopeHeaders } from '@sentry/types';

import { sentryTest } from '../../../utils/fixtures';
import { envelopeRequestParser, shouldSkipTracingTest, waitForTransactionRequest } from '../../../utils/helpers';
import {
envelopeRequestParser,
shouldSkipTracingTest,
waitForErrorRequest,
waitForTransactionRequest,
} from '../../../utils/helpers';
import { getReplaySnapshot, shouldSkipReplayTest, waitForReplayRunning } from '../../../utils/replayHelpers';

type TestWindow = Window & {
Expand Down Expand Up @@ -216,3 +221,67 @@ sentryTest(
});
},
);

sentryTest('should add replay_id to error DSC while replay is active', async ({ getLocalTestPath, page }) => {
if (shouldSkipReplayTest()) {
sentryTest.skip();
}

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);

const error1Req = waitForErrorRequest(page, event => event.exception?.values?.[0].value === 'This is error #1');
const error2Req = waitForErrorRequest(page, event => event.exception?.values?.[0].value === 'This is error #2');

// We want to wait for the transaction to be done, to ensure we have a consistent test
const transactionReq = shouldSkipTracingTest() ? Promise.resolve() : waitForTransactionRequest(page);

// Wait for this to be available
await page.waitForFunction('!!window.Replay');

// We have to start replay before we finish the transaction, otherwise the DSC will not be frozen with the Replay ID
await page.evaluate('window.Replay.start();');
await waitForReplayRunning(page);
await transactionReq;

await page.evaluate('window._triggerError(1)');

const error1Header = envelopeRequestParser(await error1Req, 0) as EventEnvelopeHeaders;
const replay = await getReplaySnapshot(page);

expect(replay.session?.id).toBeDefined();

expect(error1Header.trace).toBeDefined();
expect(error1Header.trace).toEqual({
environment: 'production',
sample_rate: '1',
trace_id: expect.any(String),
public_key: 'public',
replay_id: replay.session?.id,
sampled: 'true',
});

// Now end replay and trigger another error, it should not have a replay_id in DSC anymore
await page.evaluate('window.Replay.stop();');
await page.waitForFunction('!window.Replay.getReplayId();');
await page.evaluate('window._triggerError(2)');

const error2Header = envelopeRequestParser(await error2Req, 0) as EventEnvelopeHeaders;

expect(error2Header.trace).toBeDefined();
expect(error2Header.trace).toEqual({
environment: 'production',
sample_rate: '1',
trace_id: expect.any(String),
public_key: 'public',
sampled: 'true',
});
});
12 changes: 10 additions & 2 deletions dev-packages/browser-integration-tests/utils/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ export async function waitForTransactionRequestOnUrl(page: Page, url: string): P
return req;
}

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

return !event.type;
if (event.type) {
return false;
}

if (callback) {
return callback(event);
}

return true;
} catch {
return false;
}
Expand Down
3 changes: 3 additions & 0 deletions packages/replay-internal/src/replay.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ import { debounce } from './util/debounce';
import { getHandleRecordingEmit } from './util/handleRecordingEmit';
import { isExpired } from './util/isExpired';
import { isSessionExpired } from './util/isSessionExpired';
import { resetReplayIdOnDynamicSamplingContext } from './util/resetReplayIdOnDynamicSamplingContext';
import { sendReplay } from './util/sendReplay';
import { RateLimitError } from './util/sendReplayRequest';
import type { SKIPPED } from './util/throttle';
Expand Down Expand Up @@ -446,6 +447,8 @@ export class ReplayContainer implements ReplayContainerInterface {
try {
DEBUG_BUILD && logger.info(`Stopping Replay${reason ? ` triggered by ${reason}` : ''}`);

resetReplayIdOnDynamicSamplingContext();

this._removeListeners();
this.stopRecording();

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import { getActiveSpan, getCurrentScope, getDynamicSamplingContextFromSpan } from '@sentry/core';
import type { DynamicSamplingContext } from '@sentry/types';

/**
* Reset the `replay_id` field on the DSC.
*/
export function resetReplayIdOnDynamicSamplingContext(): void {
// Reset DSC on the current scope, if there is one
const dsc = getCurrentScope().getPropagationContext().dsc;
if (dsc) {
delete dsc.replay_id;
}

// Clear it from frozen DSC on the active span
const activeSpan = getActiveSpan();
if (activeSpan) {
const dsc = getDynamicSamplingContextFromSpan(activeSpan);
delete (dsc as Partial<DynamicSamplingContext>).replay_id;
}
}