fix(replay): Start replay in afterAllSetup instead of next tick#12709

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.

Sign up for GitHub

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

Merged
mydea merged 3 commits into from
Jul 2, 2024
Merged

fix(replay): Start replay in afterAllSetup instead of next tick #12709

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e435d1e
fix(replay): Start replay immediately, not in next tick
mydea Jul 1, 2024
acef6e5
small fixes
mydea Jul 1, 2024
eeae961
run replay in `afterAllSetup`
mydea Jul 1, 2024
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
1 change: 1 addition & 0 deletions .github/workflows/build.yml
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -761,6 +761,7 @@ jobs:
- loader_debug
- loader_tracing
- loader_replay
- loader_replay_buffer
- loader_tracing_replay

steps:
Expand Down
5 changes: 4 additions & 1 deletion dev-packages/browser-integration-tests/loader-suites/loader/noOnLoad/replay/test.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -3,8 +3,11 @@ import { expect } from '@playwright/test';
import { sentryTest } from '../../../../utils/fixtures';
import { getReplayEvent, shouldSkipReplayTest, waitForReplayRequest } from '../../../../utils/replayHelpers';

const bundle = process.env.PW_BUNDLE || '';

sentryTest('should capture a replay', async ({ getLocalTestUrl, page }) => {
if (shouldSkipReplayTest()) {
// When in buffer mode, there will not be a replay by default
if (shouldSkipReplayTest() || bundle === 'loader_replay_buffer') {
sentryTest.skip();
}

Expand Down
1 change: 1 addition & 0 deletions dev-packages/browser-integration-tests/loader-suites/loader/noOnLoad/replayError/subject.js
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
window.doSomethingWrong();
35 changes: 35 additions & 0 deletions dev-packages/browser-integration-tests/loader-suites/loader/noOnLoad/replayError/test.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
import { expect } from '@playwright/test';

import { sentryTest } from '../../../../utils/fixtures';
import { envelopeRequestParser, waitForErrorRequestOnUrl } from '../../../../utils/helpers';
import { getReplayEvent, shouldSkipReplayTest, waitForReplayRequest } from '../../../../utils/replayHelpers';

sentryTest('should capture a replay & attach an error', async ({ getLocalTestUrl, 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 req = waitForReplayRequest(page);

const url = await getLocalTestUrl({ testDir: __dirname });
const reqError = await waitForErrorRequestOnUrl(page, url);

const errorEventData = envelopeRequestParser(reqError);
expect(errorEventData.exception?.values?.length).toBe(1);
expect(errorEventData.exception?.values?.[0]?.value).toContain('window.doSomethingWrong is not a function');

const eventData = getReplayEvent(await req);

expect(eventData).toBeDefined();
expect(eventData.segment_id).toBe(0);

expect(errorEventData.tags?.replayId).toEqual(eventData.replay_id);
});
2 changes: 2 additions & 0 deletions dev-packages/browser-integration-tests/loader-suites/loader/onLoad/customReplay/init.js
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -6,5 +6,7 @@ Sentry.onLoad(function () {
useCompression: false,
}),
],

replaysSessionSampleRate: 1,
});
});
4 changes: 3 additions & 1 deletion dev-packages/browser-integration-tests/loader-suites/loader/onLoad/replay/init.js
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
Sentry.onLoad(function () {
Sentry.init({});
Sentry.init({
replaysSessionSampleRate: 1,
});
});
1 change: 1 addition & 0 deletions dev-packages/browser-integration-tests/package.json
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -32,6 +32,7 @@
"test:loader:eager": "PW_BUNDLE=loader_eager yarn test:loader",
"test:loader:tracing": "PW_BUNDLE=loader_tracing yarn test:loader",
"test:loader:replay": "PW_BUNDLE=loader_replay yarn test:loader",
"test:loader:replay_buffer": "PW_BUNDLE=loader_replay_buffer yarn test:loader",
"test:loader:full": "PW_BUNDLE=loader_tracing_replay yarn test:loader",
"test:loader:debug": "PW_BUNDLE=loader_debug yarn test:loader",
"test:ci": "yarn test:all --reporter='line'",
Expand Down
0 ...on-tests/suites/replay/bufferMode/init.js → ...ts/suites/replay/bufferModeManual/init.js
View file
Open in desktop
File renamed without changes.
0 ...tests/suites/replay/bufferMode/subject.js → ...suites/replay/bufferModeManual/subject.js
View file
Open in desktop
File renamed without changes.
0 ...ts/suites/replay/bufferMode/template.html → ...tes/replay/bufferModeManual/template.html
View file
Open in desktop
File renamed without changes.
0 ...on-tests/suites/replay/bufferMode/test.ts → ...ts/suites/replay/bufferModeManual/test.ts
View file
Open in desktop
File renamed without changes.
1 change: 1 addition & 0 deletions dev-packages/browser-integration-tests/suites/replay/errors/immediateError/subject.js
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
window.doSomethingWrong();
38 changes: 38 additions & 0 deletions dev-packages/browser-integration-tests/suites/replay/errors/immediateError/test.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
import { expect } from '@playwright/test';

import { sentryTest } from '../../../../utils/fixtures';
import { envelopeRequestParser, waitForErrorRequestOnUrl } from '../../../../utils/helpers';
import { getReplayEvent, shouldSkipReplayTest, waitForReplayRequest } from '../../../../utils/replayHelpers';

sentryTest(
'[error-mode] should capture error that happens immediately after init',
async ({ getLocalTestUrl, 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 req = waitForReplayRequest(page);

const url = await getLocalTestUrl({ testDir: __dirname });
const reqError = await waitForErrorRequestOnUrl(page, url);

const errorEventData = envelopeRequestParser(reqError);
expect(errorEventData.exception?.values?.length).toBe(1);
expect(errorEventData.exception?.values?.[0]?.value).toContain('window.doSomethingWrong is not a function');

const eventData = getReplayEvent(await req);

expect(eventData).toBeDefined();
expect(eventData.segment_id).toBe(0);

expect(errorEventData.tags?.replayId).toEqual(eventData.replay_id);
},
);
5 changes: 5 additions & 0 deletions dev-packages/browser-integration-tests/utils/generatePlugin.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -55,6 +55,7 @@ const BUNDLE_PATHS: Record<string, Record<string, string>> = {
loader_debug: 'build/bundles/bundle.debug.min.js',
loader_tracing: 'build/bundles/bundle.tracing.min.js',
loader_replay: 'build/bundles/bundle.replay.min.js',
loader_replay_buffer: 'build/bundles/bundle.replay.min.js',
loader_tracing_replay: 'build/bundles/bundle.tracing.replay.debug.min.js',
},
integrations: {
Expand DownExpand Up@@ -96,6 +97,10 @@ export const LOADER_CONFIGS: Record<string, { options: Record<string, unknown>;
options: { replaysSessionSampleRate: 1, replaysOnErrorSampleRate: 1 },
lazy: false,
},
loader_replay_buffer: {
options: { replaysSessionSampleRate: 0, replaysOnErrorSampleRate: 1 },
lazy: false,
},
loader_tracing_replay: {
options: { tracesSampleRate: 1, replaysSessionSampleRate: 1, replaysOnErrorSampleRate: 1, debug: true },
lazy: false,
Expand Down
50 changes: 13 additions & 37 deletions packages/replay-internal/src/integration.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
import { getClient, parseSampleRate } from '@sentry/core';
import type { BrowserClientReplayOptions, Integration, IntegrationFn } from '@sentry/types';
import { parseSampleRate } from '@sentry/core';
import type { BrowserClientReplayOptions, Client, Integration, IntegrationFn } from '@sentry/types';
import { consoleSandbox, dropUndefinedKeys, isBrowser } from '@sentry/utils';

import {
Expand DownExpand Up@@ -215,22 +215,13 @@ export class Replay implements Integration {
/**
* Setup and initialize replay container
*/
public setupOnce(): void {
if (!isBrowser()) {
public afterAllSetup(client: Client): void {
if (!isBrowser() || this._replay) {
return;
}

this._setup();

// Once upon a time, we tried to create a transaction in `setupOnce` and it would
// potentially create a transaction before some native SDK integrations have run
// and applied their own global event processor. An example is:
// https://github.com/getsentry/sentry-javascript/blob/b47ceafbdac7f8b99093ce6023726ad4687edc48/packages/browser/src/integrations/useragent.ts
//
// So we call `this._initialize()` in next event loop as a workaround to wait for other
// global event processors to finish. This is no longer needed, but keeping it
// here to avoid any future issues.
setTimeout(() => this._initialize());
this._setup(client);
this._initialize(client);
}

/**
Expand DownExpand Up@@ -301,24 +292,19 @@ export class Replay implements Integration {
/**
* Initializes replay.
*/
protected _initialize(): void {
protected _initialize(client: Client): void {
if (!this._replay) {
return;
}
Comment on lines 296 to 298

@andreiborzaandreiborzaJul 1, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we remove this now?


// We have to run this in _initialize, because this runs in setTimeout
// So when this runs all integrations have been added
// Before this, we cannot access integrations on the client,
// so we need to mutate the options here
this._maybeLoadFromReplayCanvasIntegration();

this._maybeLoadFromReplayCanvasIntegration(client);
this._replay.initializeSampling();
}

/** Setup the integration. */
private _setup(): void {
private _setup(client: Client): void {
// Client is not available in constructor, so we need to wait until setupOnce
const finalOptions = loadReplayOptionsFromClient(this._initialOptions);
const finalOptions = loadReplayOptionsFromClient(this._initialOptions, client);

this._replay = new ReplayContainer({
options: finalOptions,
Expand All@@ -327,12 +313,11 @@ export class Replay implements Integration {
}

/** Get canvas options from ReplayCanvas integration, if it is also added. */
private _maybeLoadFromReplayCanvasIntegration(): void {
private _maybeLoadFromReplayCanvasIntegration(client: Client): void {
// To save bundle size, we skip checking for stuff here
// and instead just try-catch everything - as generally this should all be defined
/* eslint-disable @typescript-eslint/no-non-null-assertion */
try {
const client = getClient()!;
const canvasIntegration = client.getIntegrationByName('ReplayCanvas') as Integration & {
getOptions(): ReplayCanvasIntegrationOptions;
};
Expand All@@ -349,24 +334,15 @@ export class Replay implements Integration {
}

/** Parse Replay-related options from SDK options */
function loadReplayOptionsFromClient(initialOptions: InitialReplayPluginOptions): ReplayPluginOptions {
const client = getClient();
const opt = client && (client.getOptions() as BrowserClientReplayOptions);
function loadReplayOptionsFromClient(initialOptions: InitialReplayPluginOptions, client: Client): ReplayPluginOptions {
const opt = client.getOptions() as BrowserClientReplayOptions;

const finalOptions: ReplayPluginOptions = {
sessionSampleRate: 0,
errorSampleRate: 0,
...dropUndefinedKeys(initialOptions),
};

if (!opt) {
consoleSandbox(() => {
// eslint-disable-next-line no-console
console.warn('SDK client is not available.');
});
return finalOptions;
}

const replaysSessionSampleRate = parseSampleRate(opt.replaysSessionSampleRate);
const replaysOnErrorSampleRate = parseSampleRate(opt.replaysOnErrorSampleRate);

Expand Down
5 changes: 3 additions & 2 deletions packages/replay-internal/test/integration/coreHandlers/handleGlobalEvent.test.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
import { getClient } from '@sentry/core';
import type { Event } from '@sentry/types';

import { REPLAY_EVENT_NAME, SESSION_IDLE_EXPIRE_DURATION } from '../../../src/constants';
Expand DownExpand Up@@ -133,8 +134,8 @@ describe('Integration | coreHandlers | handleGlobalEvent', () => {

it('tags errors and transactions with replay id for session samples', async () => {
const { replay, integration } = await resetSdkMock({});
// @ts-expect-error protected but ok to use for testing
integration._initialize();
integration['_initialize'](getClient()!);

@s1gr1ds1gr1dJul 2, 2024•
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

omg this syntax 😅 Not wrong at all, just JS/TS 🙄 😆

@mydeamydeaJul 2, 2024

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, it is definitely weird, but I prefer this over having to do @ts-expect-error 😅


const transaction = Transaction();
const error = Error();
expect(handleGlobalEventListener(replay)(transaction, {})).toEqual(
Expand Down
6 changes: 3 additions & 3 deletions packages/replay-internal/test/integration/errorSampleRate.test.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -866,7 +866,7 @@ describe('Integration | errorSampleRate', () => {
},
autoStart: false,
});
integration['_initialize']();
integration['_initialize'](getClient()!);

expect(replay.recordingMode).toBe('session');
const sessionId = replay.getSessionId();
Expand DownExpand Up@@ -899,7 +899,7 @@ describe('Integration | errorSampleRate', () => {
},
autoStart: false,
});
integration['_initialize']();
integration['_initialize'](getClient()!);

vi.runAllTimers();

Expand DownExpand Up@@ -943,7 +943,7 @@ describe('Integration | errorSampleRate', () => {
},
autoStart: false,
});
integration['_initialize']();
integration['_initialize'](getClient()!);
const optionsEvent = createOptionsEvent(replay);

const TEST_EVENT = getTestEventIncremental({ timestamp: BASE_TIMESTAMP });
Expand Down
4 changes: 2 additions & 2 deletions packages/replay-internal/test/integration/sampling.test.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
import { getClient } from '@sentry/core';
import { resetSdkMock } from '../mocks/resetSdkMock';
import { useFakeTimers } from '../utils/use-fake-timers';

Expand DownExpand Up@@ -57,8 +58,7 @@ describe('Integration | sampling', () => {
// @ts-expect-error private API
const spyAddListeners = vi.spyOn(replay, '_addListeners');

// @ts-expect-error protected
integration._initialize();
integration['_initialize'](getClient()!);

vi.runAllTimers();

Expand Down
18 changes: 7 additions & 11 deletions packages/replay-internal/test/mocks/mockSdk.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -61,12 +61,8 @@ export async function mockSdk({ replayOptions, sentryOptions, autoStart = true }
_initialized = value;
}

public setupOnce(): void {
// do nothing
}

public initialize(): void {
return super._initialize();
public afterAllSetup(): void {
// do nothing, we need to manually initialize this
}
}

Expand All@@ -76,7 +72,7 @@ export async function mockSdk({ replayOptions, sentryOptions, autoStart = true }
...replayOptions,
});

init({
const client = init({
...getDefaultClientOptions(),
dsn: 'https://dsn@ingest.f00.f00/1',
autoSessionTracking: false,
Expand All@@ -86,14 +82,14 @@ export async function mockSdk({ replayOptions, sentryOptions, autoStart = true }
replaysOnErrorSampleRate: 0.0,
...sentryOptions,
integrations: [replayIntegration],
});
})!;

// Instead of `setupOnce`, which is tricky to test, we call this manually here
replayIntegration['_setup']();
// Instead of `afterAllSetup`, which is tricky to test, we call this manually here
replayIntegration['_setup'](client);

if (autoStart) {
// Only exists in our mock
replayIntegration.initialize();
replayIntegration['_initialize'](client);
}

const replay = replayIntegration['_replay']!;
Expand Down
5 changes: 3 additions & 2 deletions packages/replay-internal/test/utils/TestClient.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
import { BaseClient, createTransport, initAndBind } from '@sentry/core';
import type {
BrowserClientReplayOptions,
Client,
ClientOptions,
Event,
ParameterizedString,
Expand DownExpand Up@@ -33,8 +34,8 @@ export class TestClient extends BaseClient<TestClientOptions> {
}
}

export function init(options: TestClientOptions): void {
initAndBind(TestClient, options);
export function init(options: TestClientOptions): Client {
return initAndBind(TestClient, options);
}

export function getDefaultClientOptions(options: Partial<TestClientOptions> = {}): ClientOptions {
Expand Down
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all 
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '

Skip to content
fix(replay): Start replay in afterAllSetup instead of next tick#12709

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.

 Sign up for GitHub

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

 Merged

mydea
merged 3 commits into

from

Jul 2, 2024
 Merged


fix(replay): Start replay in afterAllSetup instead of next tick
#12709
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


3 commits


Select commit
Hold shift + click to select a range
e435d1e

fix(replay): Start replay immediately, not in next tick


mydea Jul 1, 2024
acef6e5

small fixes


mydea Jul 1, 2024
eeae961

run replay in `afterAllSetup`


mydea Jul 1, 2024

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


1 change: 1 addition & 0 deletions
.github/workflows/build.yml

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -761,6 +761,7 @@ jobs:
 - loader_debug
 - loader_tracing
 - loader_replay
 - loader_replay_buffer
 - loader_tracing_replay

 steps:
Expand Down

5 changes: 4 additions & 1 deletion
dev-packages/browser-integration-tests/loader-suites/loader/noOnLoad/replay/test.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -3,8 +3,11 @@ import { expect } from '@playwright/test';
import { sentryTest } from '../../../../utils/fixtures';
import { getReplayEvent, shouldSkipReplayTest, waitForReplayRequest } from '../../../../utils/replayHelpers';

const bundle = process.env.PW_BUNDLE || '';

sentryTest('should capture a replay', async ({ getLocalTestUrl, page }) => {
 if (shouldSkipReplayTest()) {
 // When in buffer mode, there will not be a replay by default
 if (shouldSkipReplayTest() || bundle === 'loader_replay_buffer') {
 sentryTest.skip();
 }

Expand Down

1 change: 1 addition & 0 deletions
dev-packages/browser-integration-tests/loader-suites/loader/noOnLoad/replayError/subject.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
window.doSomethingWrong();

35 changes: 35 additions & 0 deletions
dev-packages/browser-integration-tests/loader-suites/loader/noOnLoad/replayError/test.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
import { expect } from '@playwright/test';

import { sentryTest } from '../../../../utils/fixtures';
import { envelopeRequestParser, waitForErrorRequestOnUrl } from '../../../../utils/helpers';
import { getReplayEvent, shouldSkipReplayTest, waitForReplayRequest } from '../../../../utils/replayHelpers';

sentryTest('should capture a replay & attach an error', async ({ getLocalTestUrl, 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 req = waitForReplayRequest(page);

 const url = await getLocalTestUrl({ testDir: __dirname });
 const reqError = await waitForErrorRequestOnUrl(page, url);

 const errorEventData = envelopeRequestParser(reqError);
 expect(errorEventData.exception?.values?.length).toBe(1);
 expect(errorEventData.exception?.values?.[0]?.value).toContain('window.doSomethingWrong is not a function');

 const eventData = getReplayEvent(await req);

 expect(eventData).toBeDefined();
 expect(eventData.segment_id).toBe(0);

 expect(errorEventData.tags?.replayId).toEqual(eventData.replay_id);
});

2 changes: 2 additions & 0 deletions
dev-packages/browser-integration-tests/loader-suites/loader/onLoad/customReplay/init.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -6,5 +6,7 @@ Sentry.onLoad(function () {
 useCompression: false,
 }),
 ],

 replaysSessionSampleRate: 1,
 });
});

4 changes: 3 additions & 1 deletion
dev-packages/browser-integration-tests/loader-suites/loader/onLoad/replay/init.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
Sentry.onLoad(function () {
 Sentry.init({});
 Sentry.init({
 replaysSessionSampleRate: 1,
 });
});

1 change: 1 addition & 0 deletions
dev-packages/browser-integration-tests/package.json

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -32,6 +32,7 @@
 "test:loader:eager": "PW_BUNDLE=loader_eager yarn test:loader",
 "test:loader:tracing": "PW_BUNDLE=loader_tracing yarn test:loader",
 "test:loader:replay": "PW_BUNDLE=loader_replay yarn test:loader",
 "test:loader:replay_buffer": "PW_BUNDLE=loader_replay_buffer yarn test:loader",
 "test:loader:full": "PW_BUNDLE=loader_tracing_replay yarn test:loader",
 "test:loader:debug": "PW_BUNDLE=loader_debug yarn test:loader",
 "test:ci": "yarn test:all --reporter='line'",
Expand Down

0
...on-tests/suites/replay/bufferMode/init.js → ...ts/suites/replay/bufferModeManual/init.js

View file


Open in desktop


File renamed without changes.


0
...tests/suites/replay/bufferMode/subject.js → ...suites/replay/bufferModeManual/subject.js

View file


Open in desktop


File renamed without changes.


0
...ts/suites/replay/bufferMode/template.html → ...tes/replay/bufferModeManual/template.html

View file


Open in desktop


File renamed without changes.


0
...on-tests/suites/replay/bufferMode/test.ts → ...ts/suites/replay/bufferModeManual/test.ts

View file


Open in desktop


File renamed without changes.


1 change: 1 addition & 0 deletions
dev-packages/browser-integration-tests/suites/replay/errors/immediateError/subject.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
window.doSomethingWrong();

38 changes: 38 additions & 0 deletions
dev-packages/browser-integration-tests/suites/replay/errors/immediateError/test.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
import { expect } from '@playwright/test';

import { sentryTest } from '../../../../utils/fixtures';
import { envelopeRequestParser, waitForErrorRequestOnUrl } from '../../../../utils/helpers';
import { getReplayEvent, shouldSkipReplayTest, waitForReplayRequest } from '../../../../utils/replayHelpers';

sentryTest(
 '[error-mode] should capture error that happens immediately after init',
 async ({ getLocalTestUrl, 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 req = waitForReplayRequest(page);

 const url = await getLocalTestUrl({ testDir: __dirname });
 const reqError = await waitForErrorRequestOnUrl(page, url);

 const errorEventData = envelopeRequestParser(reqError);
 expect(errorEventData.exception?.values?.length).toBe(1);
 expect(errorEventData.exception?.values?.[0]?.value).toContain('window.doSomethingWrong is not a function');

 const eventData = getReplayEvent(await req);

 expect(eventData).toBeDefined();
 expect(eventData.segment_id).toBe(0);

 expect(errorEventData.tags?.replayId).toEqual(eventData.replay_id);
 },
);

5 changes: 5 additions & 0 deletions
dev-packages/browser-integration-tests/utils/generatePlugin.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -55,6 +55,7 @@ const BUNDLE_PATHS: Record<string, Record<string, string>> = {
 loader_debug: 'build/bundles/bundle.debug.min.js',
 loader_tracing: 'build/bundles/bundle.tracing.min.js',
 loader_replay: 'build/bundles/bundle.replay.min.js',
 loader_replay_buffer: 'build/bundles/bundle.replay.min.js',
 loader_tracing_replay: 'build/bundles/bundle.tracing.replay.debug.min.js',
 },
 integrations: {
Expand DownExpand Up@@ -96,6 +97,10 @@ export const LOADER_CONFIGS: Record<string, { options: Record<string, unknown>;
 options: { replaysSessionSampleRate: 1, replaysOnErrorSampleRate: 1 },
 lazy: false,
 },
 loader_replay_buffer: {
 options: { replaysSessionSampleRate: 0, replaysOnErrorSampleRate: 1 },
 lazy: false,
 },
 loader_tracing_replay: {
 options: { tracesSampleRate: 1, replaysSessionSampleRate: 1, replaysOnErrorSampleRate: 1, debug: true },
 lazy: false,
Expand Down

50 changes: 13 additions & 37 deletions
packages/replay-internal/src/integration.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
import { getClient, parseSampleRate } from '@sentry/core';
import type { BrowserClientReplayOptions, Integration, IntegrationFn } from '@sentry/types';
import { parseSampleRate } from '@sentry/core';
import type { BrowserClientReplayOptions, Client, Integration, IntegrationFn } from '@sentry/types';
import { consoleSandbox, dropUndefinedKeys, isBrowser } from '@sentry/utils';

import {
Expand DownExpand Up@@ -215,22 +215,13 @@ export class Replay implements Integration {
 /**
 * Setup and initialize replay container
 */
 public setupOnce(): void {
 if (!isBrowser()) {
 public afterAllSetup(client: Client): void {
 if (!isBrowser() || this._replay) {
 return;
 }

 this._setup();

 // Once upon a time, we tried to create a transaction in `setupOnce` and it would
 // potentially create a transaction before some native SDK integrations have run
 // and applied their own global event processor. An example is:
 // https://github.com/getsentry/sentry-javascript/blob/b47ceafbdac7f8b99093ce6023726ad4687edc48/packages/browser/src/integrations/useragent.ts
 //
 // So we call `this._initialize()` in next event loop as a workaround to wait for other
 // global event processors to finish. This is no longer needed, but keeping it
 // here to avoid any future issues.
 setTimeout(() => this._initialize());
 this._setup(client);
 this._initialize(client);
 }

 /**
Expand DownExpand Up@@ -301,24 +292,19 @@ export class Replay implements Integration {
 /**
 * Initializes replay.
 */
 protected _initialize(): void {
 protected _initialize(client: Client): void {
 if (!this._replay) {
 return;
 }

Comment on lines

296
 to 
298

@andreiborzaandreiborzaJul 1, 2024

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we remove this now?

 // We have to run this in _initialize, because this runs in setTimeout
 // So when this runs all integrations have been added
 // Before this, we cannot access integrations on the client,
 // so we need to mutate the options here
 this._maybeLoadFromReplayCanvasIntegration();

 this._maybeLoadFromReplayCanvasIntegration(client);
 this._replay.initializeSampling();
 }

 /** Setup the integration. */
 private _setup(): void {
 private _setup(client: Client): void {
 // Client is not available in constructor, so we need to wait until setupOnce
 const finalOptions = loadReplayOptionsFromClient(this._initialOptions);
 const finalOptions = loadReplayOptionsFromClient(this._initialOptions, client);

 this._replay = new ReplayContainer({
 options: finalOptions,
Expand All@@ -327,12 +313,11 @@ export class Replay implements Integration {
 }

 /** Get canvas options from ReplayCanvas integration, if it is also added. */
 private _maybeLoadFromReplayCanvasIntegration(): void {
 private _maybeLoadFromReplayCanvasIntegration(client: Client): void {
 // To save bundle size, we skip checking for stuff here
 // and instead just try-catch everything - as generally this should all be defined
 /* eslint-disable @typescript-eslint/no-non-null-assertion */
 try {
 const client = getClient()!;
 const canvasIntegration = client.getIntegrationByName('ReplayCanvas') as Integration & {
 getOptions(): ReplayCanvasIntegrationOptions;
 };
Expand All@@ -349,24 +334,15 @@ export class Replay implements Integration {
}

/** Parse Replay-related options from SDK options */
function loadReplayOptionsFromClient(initialOptions: InitialReplayPluginOptions): ReplayPluginOptions {
 const client = getClient();
 const opt = client && (client.getOptions() as BrowserClientReplayOptions);
function loadReplayOptionsFromClient(initialOptions: InitialReplayPluginOptions, client: Client): ReplayPluginOptions {
 const opt = client.getOptions() as BrowserClientReplayOptions;

 const finalOptions: ReplayPluginOptions = {
 sessionSampleRate: 0,
 errorSampleRate: 0,
 ...dropUndefinedKeys(initialOptions),
 };

 if (!opt) {
 consoleSandbox(() => {
 // eslint-disable-next-line no-console
 console.warn('SDK client is not available.');
 });
 return finalOptions;
 }

 const replaysSessionSampleRate = parseSampleRate(opt.replaysSessionSampleRate);
 const replaysOnErrorSampleRate = parseSampleRate(opt.replaysOnErrorSampleRate);

Expand Down

5 changes: 3 additions & 2 deletions
packages/replay-internal/test/integration/coreHandlers/handleGlobalEvent.test.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
import { getClient } from '@sentry/core';
import type { Event } from '@sentry/types';

import { REPLAY_EVENT_NAME, SESSION_IDLE_EXPIRE_DURATION } from '../../../src/constants';
Expand DownExpand Up@@ -133,8 +134,8 @@ describe('Integration | coreHandlers | handleGlobalEvent', () => {

 it('tags errors and transactions with replay id for session samples', async () => {
 const { replay, integration } = await resetSdkMock({});
// @ts-expect-error protected but ok to use for testing
 integration._initialize();
integration['_initialize'](getClient()!);
@s1gr1ds1gr1dJul 2, 2024•

edited

Loading

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

omg this syntax 😅 Not wrong at all, just JS/TS 🙄 😆
@mydeamydeaJul 2, 2024

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, it is definitely weird, but I prefer this over having to do @ts-expect-error 😅

 const transaction = Transaction();
 const error = Error();
 expect(handleGlobalEventListener(replay)(transaction, {})).toEqual(
Expand Down

6 changes: 3 additions & 3 deletions
packages/replay-internal/test/integration/errorSampleRate.test.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -866,7 +866,7 @@ describe('Integration | errorSampleRate', () => {
 },
 autoStart: false,
 });
 integration['_initialize']();
 integration['_initialize'](getClient()!);

 expect(replay.recordingMode).toBe('session');
 const sessionId = replay.getSessionId();
Expand DownExpand Up@@ -899,7 +899,7 @@ describe('Integration | errorSampleRate', () => {
 },
 autoStart: false,
 });
 integration['_initialize']();
 integration['_initialize'](getClient()!);

 vi.runAllTimers();

Expand DownExpand Up@@ -943,7 +943,7 @@ describe('Integration | errorSampleRate', () => {
 },
 autoStart: false,
 });
 integration['_initialize']();
 integration['_initialize'](getClient()!);
 const optionsEvent = createOptionsEvent(replay);

 const TEST_EVENT = getTestEventIncremental({ timestamp: BASE_TIMESTAMP });
Expand Down

4 changes: 2 additions & 2 deletions
packages/replay-internal/test/integration/sampling.test.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
import { getClient } from '@sentry/core';
import { resetSdkMock } from '../mocks/resetSdkMock';
import { useFakeTimers } from '../utils/use-fake-timers';

Expand DownExpand Up@@ -57,8 +58,7 @@ describe('Integration | sampling', () => {
 // @ts-expect-error private API
 const spyAddListeners = vi.spyOn(replay, '_addListeners');

 // @ts-expect-error protected
 integration._initialize();
 integration['_initialize'](getClient()!);

 vi.runAllTimers();

Expand Down

18 changes: 7 additions & 11 deletions
packages/replay-internal/test/mocks/mockSdk.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -61,12 +61,8 @@ export async function mockSdk({ replayOptions, sentryOptions, autoStart = true }
 _initialized = value;
 }

 public setupOnce(): void {
 // do nothing
 }

 public initialize(): void {
 return super._initialize();
 public afterAllSetup(): void {
 // do nothing, we need to manually initialize this
 }
 }

Expand All@@ -76,7 +72,7 @@ export async function mockSdk({ replayOptions, sentryOptions, autoStart = true }
 ...replayOptions,
 });

 init({
const client = init({
 ...getDefaultClientOptions(),
 dsn: 'https://dsn@ingest.f00.f00/1',
 autoSessionTracking: false,
Expand All@@ -86,14 +82,14 @@ export async function mockSdk({ replayOptions, sentryOptions, autoStart = true }
 replaysOnErrorSampleRate: 0.0,
 ...sentryOptions,
 integrations: [replayIntegration],
 });
 })!;

 // Instead of `setupOnce`, which is tricky to test, we call this manually here
 replayIntegration['_setup']();
 // Instead of `afterAllSetup`, which is tricky to test, we call this manually here
 replayIntegration['_setup'](client);

 if (autoStart) {
 // Only exists in our mock
 replayIntegration.initialize();
 replayIntegration['_initialize'](client);
 }

 const replay = replayIntegration['_replay']!;
Expand Down

5 changes: 3 additions & 2 deletions
packages/replay-internal/test/utils/TestClient.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
import { BaseClient, createTransport, initAndBind } from '@sentry/core';
import type {
 BrowserClientReplayOptions,
 Client,
 ClientOptions,
 Event,
 ParameterizedString,
Expand DownExpand Up@@ -33,8 +34,8 @@ export class TestClient extends BaseClient<TestClientOptions> {
 }
}

export function init(options: TestClientOptions): void {
 initAndBind(TestClient, options);
export function init(options: TestClientOptions): Client {
return initAndBind(TestClient, options);
}

export function getDefaultClientOptions(options: Partial<TestClientOptions> = {}): ClientOptions {
Expand Down

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix");
}
} catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
fix(replay): Start replay in afterAllSetup instead of next tick#12709

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.

 Sign up for GitHub

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

 Merged

mydea
merged 3 commits into

from

Jul 2, 2024
 Merged


fix(replay): Start replay in afterAllSetup instead of next tick
#12709
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


3 commits


Select commit
Hold shift + click to select a range
e435d1e

fix(replay): Start replay immediately, not in next tick


mydea Jul 1, 2024
acef6e5

small fixes


mydea Jul 1, 2024
eeae961

run replay in `afterAllSetup`


mydea Jul 1, 2024

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


1 change: 1 addition & 0 deletions
.github/workflows/build.yml

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -761,6 +761,7 @@ jobs:
 - loader_debug
 - loader_tracing
 - loader_replay
 - loader_replay_buffer
 - loader_tracing_replay

 steps:
Expand Down

5 changes: 4 additions & 1 deletion
dev-packages/browser-integration-tests/loader-suites/loader/noOnLoad/replay/test.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -3,8 +3,11 @@ import { expect } from '@playwright/test';
import { sentryTest } from '../../../../utils/fixtures';
import { getReplayEvent, shouldSkipReplayTest, waitForReplayRequest } from '../../../../utils/replayHelpers';

const bundle = process.env.PW_BUNDLE || '';

sentryTest('should capture a replay', async ({ getLocalTestUrl, page }) => {
 if (shouldSkipReplayTest()) {
 // When in buffer mode, there will not be a replay by default
 if (shouldSkipReplayTest() || bundle === 'loader_replay_buffer') {
 sentryTest.skip();
 }

Expand Down

1 change: 1 addition & 0 deletions
dev-packages/browser-integration-tests/loader-suites/loader/noOnLoad/replayError/subject.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
window.doSomethingWrong();

35 changes: 35 additions & 0 deletions
dev-packages/browser-integration-tests/loader-suites/loader/noOnLoad/replayError/test.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
import { expect } from '@playwright/test';

import { sentryTest } from '../../../../utils/fixtures';
import { envelopeRequestParser, waitForErrorRequestOnUrl } from '../../../../utils/helpers';
import { getReplayEvent, shouldSkipReplayTest, waitForReplayRequest } from '../../../../utils/replayHelpers';

sentryTest('should capture a replay & attach an error', async ({ getLocalTestUrl, 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 req = waitForReplayRequest(page);

 const url = await getLocalTestUrl({ testDir: __dirname });
 const reqError = await waitForErrorRequestOnUrl(page, url);

 const errorEventData = envelopeRequestParser(reqError);
 expect(errorEventData.exception?.values?.length).toBe(1);
 expect(errorEventData.exception?.values?.[0]?.value).toContain('window.doSomethingWrong is not a function');

 const eventData = getReplayEvent(await req);

 expect(eventData).toBeDefined();
 expect(eventData.segment_id).toBe(0);

 expect(errorEventData.tags?.replayId).toEqual(eventData.replay_id);
});

2 changes: 2 additions & 0 deletions
dev-packages/browser-integration-tests/loader-suites/loader/onLoad/customReplay/init.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -6,5 +6,7 @@ Sentry.onLoad(function () {
 useCompression: false,
 }),
 ],

 replaysSessionSampleRate: 1,
 });
});

4 changes: 3 additions & 1 deletion
dev-packages/browser-integration-tests/loader-suites/loader/onLoad/replay/init.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
Sentry.onLoad(function () {
 Sentry.init({});
 Sentry.init({
 replaysSessionSampleRate: 1,
 });
});

1 change: 1 addition & 0 deletions
dev-packages/browser-integration-tests/package.json

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -32,6 +32,7 @@
 "test:loader:eager": "PW_BUNDLE=loader_eager yarn test:loader",
 "test:loader:tracing": "PW_BUNDLE=loader_tracing yarn test:loader",
 "test:loader:replay": "PW_BUNDLE=loader_replay yarn test:loader",
 "test:loader:replay_buffer": "PW_BUNDLE=loader_replay_buffer yarn test:loader",
 "test:loader:full": "PW_BUNDLE=loader_tracing_replay yarn test:loader",
 "test:loader:debug": "PW_BUNDLE=loader_debug yarn test:loader",
 "test:ci": "yarn test:all --reporter='line'",
Expand Down

0
...on-tests/suites/replay/bufferMode/init.js → ...ts/suites/replay/bufferModeManual/init.js

View file


Open in desktop


File renamed without changes.


0
...tests/suites/replay/bufferMode/subject.js → ...suites/replay/bufferModeManual/subject.js

View file


Open in desktop


File renamed without changes.


0
...ts/suites/replay/bufferMode/template.html → ...tes/replay/bufferModeManual/template.html

View file


Open in desktop


File renamed without changes.


0
...on-tests/suites/replay/bufferMode/test.ts → ...ts/suites/replay/bufferModeManual/test.ts

View file


Open in desktop


File renamed without changes.


1 change: 1 addition & 0 deletions
dev-packages/browser-integration-tests/suites/replay/errors/immediateError/subject.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
window.doSomethingWrong();

38 changes: 38 additions & 0 deletions
dev-packages/browser-integration-tests/suites/replay/errors/immediateError/test.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
import { expect } from '@playwright/test';

import { sentryTest } from '../../../../utils/fixtures';
import { envelopeRequestParser, waitForErrorRequestOnUrl } from '../../../../utils/helpers';
import { getReplayEvent, shouldSkipReplayTest, waitForReplayRequest } from '../../../../utils/replayHelpers';

sentryTest(
 '[error-mode] should capture error that happens immediately after init',
 async ({ getLocalTestUrl, 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 req = waitForReplayRequest(page);

 const url = await getLocalTestUrl({ testDir: __dirname });
 const reqError = await waitForErrorRequestOnUrl(page, url);

 const errorEventData = envelopeRequestParser(reqError);
 expect(errorEventData.exception?.values?.length).toBe(1);
 expect(errorEventData.exception?.values?.[0]?.value).toContain('window.doSomethingWrong is not a function');

 const eventData = getReplayEvent(await req);

 expect(eventData).toBeDefined();
 expect(eventData.segment_id).toBe(0);

 expect(errorEventData.tags?.replayId).toEqual(eventData.replay_id);
 },
);

5 changes: 5 additions & 0 deletions
dev-packages/browser-integration-tests/utils/generatePlugin.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -55,6 +55,7 @@ const BUNDLE_PATHS: Record<string, Record<string, string>> = {
 loader_debug: 'build/bundles/bundle.debug.min.js',
 loader_tracing: 'build/bundles/bundle.tracing.min.js',
 loader_replay: 'build/bundles/bundle.replay.min.js',
 loader_replay_buffer: 'build/bundles/bundle.replay.min.js',
 loader_tracing_replay: 'build/bundles/bundle.tracing.replay.debug.min.js',
 },
 integrations: {
Expand DownExpand Up@@ -96,6 +97,10 @@ export const LOADER_CONFIGS: Record<string, { options: Record<string, unknown>;
 options: { replaysSessionSampleRate: 1, replaysOnErrorSampleRate: 1 },
 lazy: false,
 },
 loader_replay_buffer: {
 options: { replaysSessionSampleRate: 0, replaysOnErrorSampleRate: 1 },
 lazy: false,
 },
 loader_tracing_replay: {
 options: { tracesSampleRate: 1, replaysSessionSampleRate: 1, replaysOnErrorSampleRate: 1, debug: true },
 lazy: false,
Expand Down

50 changes: 13 additions & 37 deletions
packages/replay-internal/src/integration.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
import { getClient, parseSampleRate } from '@sentry/core';
import type { BrowserClientReplayOptions, Integration, IntegrationFn } from '@sentry/types';
import { parseSampleRate } from '@sentry/core';
import type { BrowserClientReplayOptions, Client, Integration, IntegrationFn } from '@sentry/types';
import { consoleSandbox, dropUndefinedKeys, isBrowser } from '@sentry/utils';

import {
Expand DownExpand Up@@ -215,22 +215,13 @@ export class Replay implements Integration {
 /**
 * Setup and initialize replay container
 */
 public setupOnce(): void {
 if (!isBrowser()) {
 public afterAllSetup(client: Client): void {
 if (!isBrowser() || this._replay) {
 return;
 }

 this._setup();

 // Once upon a time, we tried to create a transaction in `setupOnce` and it would
 // potentially create a transaction before some native SDK integrations have run
 // and applied their own global event processor. An example is:
 // https://github.com/getsentry/sentry-javascript/blob/b47ceafbdac7f8b99093ce6023726ad4687edc48/packages/browser/src/integrations/useragent.ts
 //
 // So we call `this._initialize()` in next event loop as a workaround to wait for other
 // global event processors to finish. This is no longer needed, but keeping it
 // here to avoid any future issues.
 setTimeout(() => this._initialize());
 this._setup(client);
 this._initialize(client);
 }

 /**
Expand DownExpand Up@@ -301,24 +292,19 @@ export class Replay implements Integration {
 /**
 * Initializes replay.
 */
 protected _initialize(): void {
 protected _initialize(client: Client): void {
 if (!this._replay) {
 return;
 }

Comment on lines

296
 to 
298

@andreiborzaandreiborzaJul 1, 2024

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we remove this now?

 // We have to run this in _initialize, because this runs in setTimeout
 // So when this runs all integrations have been added
 // Before this, we cannot access integrations on the client,
 // so we need to mutate the options here
 this._maybeLoadFromReplayCanvasIntegration();

 this._maybeLoadFromReplayCanvasIntegration(client);
 this._replay.initializeSampling();
 }

 /** Setup the integration. */
 private _setup(): void {
 private _setup(client: Client): void {
 // Client is not available in constructor, so we need to wait until setupOnce
 const finalOptions = loadReplayOptionsFromClient(this._initialOptions);
 const finalOptions = loadReplayOptionsFromClient(this._initialOptions, client);

 this._replay = new ReplayContainer({
 options: finalOptions,
Expand All@@ -327,12 +313,11 @@ export class Replay implements Integration {
 }

 /** Get canvas options from ReplayCanvas integration, if it is also added. */
 private _maybeLoadFromReplayCanvasIntegration(): void {
 private _maybeLoadFromReplayCanvasIntegration(client: Client): void {
 // To save bundle size, we skip checking for stuff here
 // and instead just try-catch everything - as generally this should all be defined
 /* eslint-disable @typescript-eslint/no-non-null-assertion */
 try {
 const client = getClient()!;
 const canvasIntegration = client.getIntegrationByName('ReplayCanvas') as Integration & {
 getOptions(): ReplayCanvasIntegrationOptions;
 };
Expand All@@ -349,24 +334,15 @@ export class Replay implements Integration {
}

/** Parse Replay-related options from SDK options */
function loadReplayOptionsFromClient(initialOptions: InitialReplayPluginOptions): ReplayPluginOptions {
 const client = getClient();
 const opt = client && (client.getOptions() as BrowserClientReplayOptions);
function loadReplayOptionsFromClient(initialOptions: InitialReplayPluginOptions, client: Client): ReplayPluginOptions {
 const opt = client.getOptions() as BrowserClientReplayOptions;

 const finalOptions: ReplayPluginOptions = {
 sessionSampleRate: 0,
 errorSampleRate: 0,
 ...dropUndefinedKeys(initialOptions),
 };

 if (!opt) {
 consoleSandbox(() => {
 // eslint-disable-next-line no-console
 console.warn('SDK client is not available.');
 });
 return finalOptions;
 }

 const replaysSessionSampleRate = parseSampleRate(opt.replaysSessionSampleRate);
 const replaysOnErrorSampleRate = parseSampleRate(opt.replaysOnErrorSampleRate);

Expand Down

5 changes: 3 additions & 2 deletions
packages/replay-internal/test/integration/coreHandlers/handleGlobalEvent.test.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
import { getClient } from '@sentry/core';
import type { Event } from '@sentry/types';

import { REPLAY_EVENT_NAME, SESSION_IDLE_EXPIRE_DURATION } from '../../../src/constants';
Expand DownExpand Up@@ -133,8 +134,8 @@ describe('Integration | coreHandlers | handleGlobalEvent', () => {

 it('tags errors and transactions with replay id for session samples', async () => {
 const { replay, integration } = await resetSdkMock({});
// @ts-expect-error protected but ok to use for testing
 integration._initialize();
integration['_initialize'](getClient()!);
@s1gr1ds1gr1dJul 2, 2024•

edited

Loading

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

omg this syntax 😅 Not wrong at all, just JS/TS 🙄 😆
@mydeamydeaJul 2, 2024

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, it is definitely weird, but I prefer this over having to do @ts-expect-error 😅

 const transaction = Transaction();
 const error = Error();
 expect(handleGlobalEventListener(replay)(transaction, {})).toEqual(
Expand Down

6 changes: 3 additions & 3 deletions
packages/replay-internal/test/integration/errorSampleRate.test.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -866,7 +866,7 @@ describe('Integration | errorSampleRate', () => {
 },
 autoStart: false,
 });
 integration['_initialize']();
 integration['_initialize'](getClient()!);

 expect(replay.recordingMode).toBe('session');
 const sessionId = replay.getSessionId();
Expand DownExpand Up@@ -899,7 +899,7 @@ describe('Integration | errorSampleRate', () => {
 },
 autoStart: false,
 });
 integration['_initialize']();
 integration['_initialize'](getClient()!);

 vi.runAllTimers();

Expand DownExpand Up@@ -943,7 +943,7 @@ describe('Integration | errorSampleRate', () => {
 },
 autoStart: false,
 });
 integration['_initialize']();
 integration['_initialize'](getClient()!);
 const optionsEvent = createOptionsEvent(replay);

 const TEST_EVENT = getTestEventIncremental({ timestamp: BASE_TIMESTAMP });
Expand Down

4 changes: 2 additions & 2 deletions
packages/replay-internal/test/integration/sampling.test.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
import { getClient } from '@sentry/core';
import { resetSdkMock } from '../mocks/resetSdkMock';
import { useFakeTimers } from '../utils/use-fake-timers';

Expand DownExpand Up@@ -57,8 +58,7 @@ describe('Integration | sampling', () => {
 // @ts-expect-error private API
 const spyAddListeners = vi.spyOn(replay, '_addListeners');

 // @ts-expect-error protected
 integration._initialize();
 integration['_initialize'](getClient()!);

 vi.runAllTimers();

Expand Down

18 changes: 7 additions & 11 deletions
packages/replay-internal/test/mocks/mockSdk.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -61,12 +61,8 @@ export async function mockSdk({ replayOptions, sentryOptions, autoStart = true }
 _initialized = value;
 }

 public setupOnce(): void {
 // do nothing
 }

 public initialize(): void {
 return super._initialize();
 public afterAllSetup(): void {
 // do nothing, we need to manually initialize this
 }
 }

Expand All@@ -76,7 +72,7 @@ export async function mockSdk({ replayOptions, sentryOptions, autoStart = true }
 ...replayOptions,
 });

 init({
const client = init({
 ...getDefaultClientOptions(),
 dsn: 'https://dsn@ingest.f00.f00/1',
 autoSessionTracking: false,
Expand All@@ -86,14 +82,14 @@ export async function mockSdk({ replayOptions, sentryOptions, autoStart = true }
 replaysOnErrorSampleRate: 0.0,
 ...sentryOptions,
 integrations: [replayIntegration],
 });
 })!;

 // Instead of `setupOnce`, which is tricky to test, we call this manually here
 replayIntegration['_setup']();
 // Instead of `afterAllSetup`, which is tricky to test, we call this manually here
 replayIntegration['_setup'](client);

 if (autoStart) {
 // Only exists in our mock
 replayIntegration.initialize();
 replayIntegration['_initialize'](client);
 }

 const replay = replayIntegration['_replay']!;
Expand Down

5 changes: 3 additions & 2 deletions
packages/replay-internal/test/utils/TestClient.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
import { BaseClient, createTransport, initAndBind } from '@sentry/core';
import type {
 BrowserClientReplayOptions,
 Client,
 ClientOptions,
 Event,
 ParameterizedString,
Expand DownExpand Up@@ -33,8 +34,8 @@ export class TestClient extends BaseClient<TestClientOptions> {
 }
}

export function init(options: TestClientOptions): void {
 initAndBind(TestClient, options);
export function init(options: TestClientOptions): Client {
return initAndBind(TestClient, options);
}

export function getDefaultClientOptions(options: Partial<TestClientOptions> = {}): ClientOptions {
Expand Down

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms");
}
} catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
fix(replay): Start replay in afterAllSetup instead of next tick#12709

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.

 Sign up for GitHub

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

 Merged

mydea
merged 3 commits into

from

Jul 2, 2024
 Merged


fix(replay): Start replay in afterAllSetup instead of next tick
#12709
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


3 commits


Select commit
Hold shift + click to select a range
e435d1e

fix(replay): Start replay immediately, not in next tick


mydea Jul 1, 2024
acef6e5

small fixes


mydea Jul 1, 2024
eeae961

run replay in `afterAllSetup`


mydea Jul 1, 2024

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


1 change: 1 addition & 0 deletions
.github/workflows/build.yml

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -761,6 +761,7 @@ jobs:
 - loader_debug
 - loader_tracing
 - loader_replay
 - loader_replay_buffer
 - loader_tracing_replay

 steps:
Expand Down

5 changes: 4 additions & 1 deletion
dev-packages/browser-integration-tests/loader-suites/loader/noOnLoad/replay/test.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -3,8 +3,11 @@ import { expect } from '@playwright/test';
import { sentryTest } from '../../../../utils/fixtures';
import { getReplayEvent, shouldSkipReplayTest, waitForReplayRequest } from '../../../../utils/replayHelpers';

const bundle = process.env.PW_BUNDLE || '';

sentryTest('should capture a replay', async ({ getLocalTestUrl, page }) => {
 if (shouldSkipReplayTest()) {
 // When in buffer mode, there will not be a replay by default
 if (shouldSkipReplayTest() || bundle === 'loader_replay_buffer') {
 sentryTest.skip();
 }

Expand Down

1 change: 1 addition & 0 deletions
dev-packages/browser-integration-tests/loader-suites/loader/noOnLoad/replayError/subject.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
window.doSomethingWrong();

35 changes: 35 additions & 0 deletions
dev-packages/browser-integration-tests/loader-suites/loader/noOnLoad/replayError/test.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
import { expect } from '@playwright/test';

import { sentryTest } from '../../../../utils/fixtures';
import { envelopeRequestParser, waitForErrorRequestOnUrl } from '../../../../utils/helpers';
import { getReplayEvent, shouldSkipReplayTest, waitForReplayRequest } from '../../../../utils/replayHelpers';

sentryTest('should capture a replay & attach an error', async ({ getLocalTestUrl, 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 req = waitForReplayRequest(page);

 const url = await getLocalTestUrl({ testDir: __dirname });
 const reqError = await waitForErrorRequestOnUrl(page, url);

 const errorEventData = envelopeRequestParser(reqError);
 expect(errorEventData.exception?.values?.length).toBe(1);
 expect(errorEventData.exception?.values?.[0]?.value).toContain('window.doSomethingWrong is not a function');

 const eventData = getReplayEvent(await req);

 expect(eventData).toBeDefined();
 expect(eventData.segment_id).toBe(0);

 expect(errorEventData.tags?.replayId).toEqual(eventData.replay_id);
});

2 changes: 2 additions & 0 deletions
dev-packages/browser-integration-tests/loader-suites/loader/onLoad/customReplay/init.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -6,5 +6,7 @@ Sentry.onLoad(function () {
 useCompression: false,
 }),
 ],

 replaysSessionSampleRate: 1,
 });
});

4 changes: 3 additions & 1 deletion
dev-packages/browser-integration-tests/loader-suites/loader/onLoad/replay/init.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
Sentry.onLoad(function () {
 Sentry.init({});
 Sentry.init({
 replaysSessionSampleRate: 1,
 });
});

1 change: 1 addition & 0 deletions
dev-packages/browser-integration-tests/package.json

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -32,6 +32,7 @@
 "test:loader:eager": "PW_BUNDLE=loader_eager yarn test:loader",
 "test:loader:tracing": "PW_BUNDLE=loader_tracing yarn test:loader",
 "test:loader:replay": "PW_BUNDLE=loader_replay yarn test:loader",
 "test:loader:replay_buffer": "PW_BUNDLE=loader_replay_buffer yarn test:loader",
 "test:loader:full": "PW_BUNDLE=loader_tracing_replay yarn test:loader",
 "test:loader:debug": "PW_BUNDLE=loader_debug yarn test:loader",
 "test:ci": "yarn test:all --reporter='line'",
Expand Down

0
...on-tests/suites/replay/bufferMode/init.js → ...ts/suites/replay/bufferModeManual/init.js

View file


Open in desktop


File renamed without changes.


0
...tests/suites/replay/bufferMode/subject.js → ...suites/replay/bufferModeManual/subject.js

View file


Open in desktop


File renamed without changes.


0
...ts/suites/replay/bufferMode/template.html → ...tes/replay/bufferModeManual/template.html

View file


Open in desktop


File renamed without changes.


0
...on-tests/suites/replay/bufferMode/test.ts → ...ts/suites/replay/bufferModeManual/test.ts

View file


Open in desktop


File renamed without changes.


1 change: 1 addition & 0 deletions
dev-packages/browser-integration-tests/suites/replay/errors/immediateError/subject.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
window.doSomethingWrong();

38 changes: 38 additions & 0 deletions
dev-packages/browser-integration-tests/suites/replay/errors/immediateError/test.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
import { expect } from '@playwright/test';

import { sentryTest } from '../../../../utils/fixtures';
import { envelopeRequestParser, waitForErrorRequestOnUrl } from '../../../../utils/helpers';
import { getReplayEvent, shouldSkipReplayTest, waitForReplayRequest } from '../../../../utils/replayHelpers';

sentryTest(
 '[error-mode] should capture error that happens immediately after init',
 async ({ getLocalTestUrl, 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 req = waitForReplayRequest(page);

 const url = await getLocalTestUrl({ testDir: __dirname });
 const reqError = await waitForErrorRequestOnUrl(page, url);

 const errorEventData = envelopeRequestParser(reqError);
 expect(errorEventData.exception?.values?.length).toBe(1);
 expect(errorEventData.exception?.values?.[0]?.value).toContain('window.doSomethingWrong is not a function');

 const eventData = getReplayEvent(await req);

 expect(eventData).toBeDefined();
 expect(eventData.segment_id).toBe(0);

 expect(errorEventData.tags?.replayId).toEqual(eventData.replay_id);
 },
);

5 changes: 5 additions & 0 deletions
dev-packages/browser-integration-tests/utils/generatePlugin.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -55,6 +55,7 @@ const BUNDLE_PATHS: Record<string, Record<string, string>> = {
 loader_debug: 'build/bundles/bundle.debug.min.js',
 loader_tracing: 'build/bundles/bundle.tracing.min.js',
 loader_replay: 'build/bundles/bundle.replay.min.js',
 loader_replay_buffer: 'build/bundles/bundle.replay.min.js',
 loader_tracing_replay: 'build/bundles/bundle.tracing.replay.debug.min.js',
 },
 integrations: {
Expand DownExpand Up@@ -96,6 +97,10 @@ export const LOADER_CONFIGS: Record<string, { options: Record<string, unknown>;
 options: { replaysSessionSampleRate: 1, replaysOnErrorSampleRate: 1 },
 lazy: false,
 },
 loader_replay_buffer: {
 options: { replaysSessionSampleRate: 0, replaysOnErrorSampleRate: 1 },
 lazy: false,
 },
 loader_tracing_replay: {
 options: { tracesSampleRate: 1, replaysSessionSampleRate: 1, replaysOnErrorSampleRate: 1, debug: true },
 lazy: false,
Expand Down

50 changes: 13 additions & 37 deletions
packages/replay-internal/src/integration.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
import { getClient, parseSampleRate } from '@sentry/core';
import type { BrowserClientReplayOptions, Integration, IntegrationFn } from '@sentry/types';
import { parseSampleRate } from '@sentry/core';
import type { BrowserClientReplayOptions, Client, Integration, IntegrationFn } from '@sentry/types';
import { consoleSandbox, dropUndefinedKeys, isBrowser } from '@sentry/utils';

import {
Expand DownExpand Up@@ -215,22 +215,13 @@ export class Replay implements Integration {
 /**
 * Setup and initialize replay container
 */
 public setupOnce(): void {
 if (!isBrowser()) {
 public afterAllSetup(client: Client): void {
 if (!isBrowser() || this._replay) {
 return;
 }

 this._setup();

 // Once upon a time, we tried to create a transaction in `setupOnce` and it would
 // potentially create a transaction before some native SDK integrations have run
 // and applied their own global event processor. An example is:
 // https://github.com/getsentry/sentry-javascript/blob/b47ceafbdac7f8b99093ce6023726ad4687edc48/packages/browser/src/integrations/useragent.ts
 //
 // So we call `this._initialize()` in next event loop as a workaround to wait for other
 // global event processors to finish. This is no longer needed, but keeping it
 // here to avoid any future issues.
 setTimeout(() => this._initialize());
 this._setup(client);
 this._initialize(client);
 }

 /**
Expand DownExpand Up@@ -301,24 +292,19 @@ export class Replay implements Integration {
 /**
 * Initializes replay.
 */
 protected _initialize(): void {
 protected _initialize(client: Client): void {
 if (!this._replay) {
 return;
 }

Comment on lines

296
 to 
298

@andreiborzaandreiborzaJul 1, 2024

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we remove this now?

 // We have to run this in _initialize, because this runs in setTimeout
 // So when this runs all integrations have been added
 // Before this, we cannot access integrations on the client,
 // so we need to mutate the options here
 this._maybeLoadFromReplayCanvasIntegration();

 this._maybeLoadFromReplayCanvasIntegration(client);
 this._replay.initializeSampling();
 }

 /** Setup the integration. */
 private _setup(): void {
 private _setup(client: Client): void {
 // Client is not available in constructor, so we need to wait until setupOnce
 const finalOptions = loadReplayOptionsFromClient(this._initialOptions);
 const finalOptions = loadReplayOptionsFromClient(this._initialOptions, client);

 this._replay = new ReplayContainer({
 options: finalOptions,
Expand All@@ -327,12 +313,11 @@ export class Replay implements Integration {
 }

 /** Get canvas options from ReplayCanvas integration, if it is also added. */
 private _maybeLoadFromReplayCanvasIntegration(): void {
 private _maybeLoadFromReplayCanvasIntegration(client: Client): void {
 // To save bundle size, we skip checking for stuff here
 // and instead just try-catch everything - as generally this should all be defined
 /* eslint-disable @typescript-eslint/no-non-null-assertion */
 try {
 const client = getClient()!;
 const canvasIntegration = client.getIntegrationByName('ReplayCanvas') as Integration & {
 getOptions(): ReplayCanvasIntegrationOptions;
 };
Expand All@@ -349,24 +334,15 @@ export class Replay implements Integration {
}

/** Parse Replay-related options from SDK options */
function loadReplayOptionsFromClient(initialOptions: InitialReplayPluginOptions): ReplayPluginOptions {
 const client = getClient();
 const opt = client && (client.getOptions() as BrowserClientReplayOptions);
function loadReplayOptionsFromClient(initialOptions: InitialReplayPluginOptions, client: Client): ReplayPluginOptions {
 const opt = client.getOptions() as BrowserClientReplayOptions;

 const finalOptions: ReplayPluginOptions = {
 sessionSampleRate: 0,
 errorSampleRate: 0,
 ...dropUndefinedKeys(initialOptions),
 };

 if (!opt) {
 consoleSandbox(() => {
 // eslint-disable-next-line no-console
 console.warn('SDK client is not available.');
 });
 return finalOptions;
 }

 const replaysSessionSampleRate = parseSampleRate(opt.replaysSessionSampleRate);
 const replaysOnErrorSampleRate = parseSampleRate(opt.replaysOnErrorSampleRate);

Expand Down

5 changes: 3 additions & 2 deletions
packages/replay-internal/test/integration/coreHandlers/handleGlobalEvent.test.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
import { getClient } from '@sentry/core';
import type { Event } from '@sentry/types';

import { REPLAY_EVENT_NAME, SESSION_IDLE_EXPIRE_DURATION } from '../../../src/constants';
Expand DownExpand Up@@ -133,8 +134,8 @@ describe('Integration | coreHandlers | handleGlobalEvent', () => {

 it('tags errors and transactions with replay id for session samples', async () => {
 const { replay, integration } = await resetSdkMock({});
// @ts-expect-error protected but ok to use for testing
 integration._initialize();
integration['_initialize'](getClient()!);
@s1gr1ds1gr1dJul 2, 2024•

edited

Loading

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

omg this syntax 😅 Not wrong at all, just JS/TS 🙄 😆
@mydeamydeaJul 2, 2024

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, it is definitely weird, but I prefer this over having to do @ts-expect-error 😅

 const transaction = Transaction();
 const error = Error();
 expect(handleGlobalEventListener(replay)(transaction, {})).toEqual(
Expand Down

6 changes: 3 additions & 3 deletions
packages/replay-internal/test/integration/errorSampleRate.test.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -866,7 +866,7 @@ describe('Integration | errorSampleRate', () => {
 },
 autoStart: false,
 });
 integration['_initialize']();
 integration['_initialize'](getClient()!);

 expect(replay.recordingMode).toBe('session');
 const sessionId = replay.getSessionId();
Expand DownExpand Up@@ -899,7 +899,7 @@ describe('Integration | errorSampleRate', () => {
 },
 autoStart: false,
 });
 integration['_initialize']();
 integration['_initialize'](getClient()!);

 vi.runAllTimers();

Expand DownExpand Up@@ -943,7 +943,7 @@ describe('Integration | errorSampleRate', () => {
 },
 autoStart: false,
 });
 integration['_initialize']();
 integration['_initialize'](getClient()!);
 const optionsEvent = createOptionsEvent(replay);

 const TEST_EVENT = getTestEventIncremental({ timestamp: BASE_TIMESTAMP });
Expand Down

4 changes: 2 additions & 2 deletions
packages/replay-internal/test/integration/sampling.test.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
import { getClient } from '@sentry/core';
import { resetSdkMock } from '../mocks/resetSdkMock';
import { useFakeTimers } from '../utils/use-fake-timers';

Expand DownExpand Up@@ -57,8 +58,7 @@ describe('Integration | sampling', () => {
 // @ts-expect-error private API
 const spyAddListeners = vi.spyOn(replay, '_addListeners');

 // @ts-expect-error protected
 integration._initialize();
 integration['_initialize'](getClient()!);

 vi.runAllTimers();

Expand Down

18 changes: 7 additions & 11 deletions
packages/replay-internal/test/mocks/mockSdk.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -61,12 +61,8 @@ export async function mockSdk({ replayOptions, sentryOptions, autoStart = true }
 _initialized = value;
 }

 public setupOnce(): void {
 // do nothing
 }

 public initialize(): void {
 return super._initialize();
 public afterAllSetup(): void {
 // do nothing, we need to manually initialize this
 }
 }

Expand All@@ -76,7 +72,7 @@ export async function mockSdk({ replayOptions, sentryOptions, autoStart = true }
 ...replayOptions,
 });

 init({
const client = init({
 ...getDefaultClientOptions(),
 dsn: 'https://dsn@ingest.f00.f00/1',
 autoSessionTracking: false,
Expand All@@ -86,14 +82,14 @@ export async function mockSdk({ replayOptions, sentryOptions, autoStart = true }
 replaysOnErrorSampleRate: 0.0,
 ...sentryOptions,
 integrations: [replayIntegration],
 });
 })!;

 // Instead of `setupOnce`, which is tricky to test, we call this manually here
 replayIntegration['_setup']();
 // Instead of `afterAllSetup`, which is tricky to test, we call this manually here
 replayIntegration['_setup'](client);

 if (autoStart) {
 // Only exists in our mock
 replayIntegration.initialize();
 replayIntegration['_initialize'](client);
 }

 const replay = replayIntegration['_replay']!;
Expand Down

5 changes: 3 additions & 2 deletions
packages/replay-internal/test/utils/TestClient.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
import { BaseClient, createTransport, initAndBind } from '@sentry/core';
import type {
 BrowserClientReplayOptions,
 Client,
 ClientOptions,
 Event,
 ParameterizedString,
Expand DownExpand Up@@ -33,8 +34,8 @@ export class TestClient extends BaseClient<TestClientOptions> {
 }
}

export function init(options: TestClientOptions): void {
 initAndBind(TestClient, options);
export function init(options: TestClientOptions): Client {
return initAndBind(TestClient, options);
}

export function getDefaultClientOptions(options: Partial<TestClientOptions> = {}): ClientOptions {
Expand Down

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links");
}
} catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); }
})();
(function(){
try {
var __m = "youtube.com";
var __re = new RegExp('^' + "youtube\\.com" + '

Skip to content
fix(replay): Start replay in afterAllSetup instead of next tick#12709

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.

 Sign up for GitHub

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

 Merged

mydea
merged 3 commits into

from

Jul 2, 2024
 Merged


fix(replay): Start replay in afterAllSetup instead of next tick
#12709
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


3 commits


Select commit
Hold shift + click to select a range
e435d1e

fix(replay): Start replay immediately, not in next tick


mydea Jul 1, 2024
acef6e5

small fixes


mydea Jul 1, 2024
eeae961

run replay in `afterAllSetup`


mydea Jul 1, 2024

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


1 change: 1 addition & 0 deletions
.github/workflows/build.yml

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -761,6 +761,7 @@ jobs:
 - loader_debug
 - loader_tracing
 - loader_replay
 - loader_replay_buffer
 - loader_tracing_replay

 steps:
Expand Down

5 changes: 4 additions & 1 deletion
dev-packages/browser-integration-tests/loader-suites/loader/noOnLoad/replay/test.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -3,8 +3,11 @@ import { expect } from '@playwright/test';
import { sentryTest } from '../../../../utils/fixtures';
import { getReplayEvent, shouldSkipReplayTest, waitForReplayRequest } from '../../../../utils/replayHelpers';

const bundle = process.env.PW_BUNDLE || '';

sentryTest('should capture a replay', async ({ getLocalTestUrl, page }) => {
 if (shouldSkipReplayTest()) {
 // When in buffer mode, there will not be a replay by default
 if (shouldSkipReplayTest() || bundle === 'loader_replay_buffer') {
 sentryTest.skip();
 }

Expand Down

1 change: 1 addition & 0 deletions
dev-packages/browser-integration-tests/loader-suites/loader/noOnLoad/replayError/subject.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
window.doSomethingWrong();

35 changes: 35 additions & 0 deletions
dev-packages/browser-integration-tests/loader-suites/loader/noOnLoad/replayError/test.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
import { expect } from '@playwright/test';

import { sentryTest } from '../../../../utils/fixtures';
import { envelopeRequestParser, waitForErrorRequestOnUrl } from '../../../../utils/helpers';
import { getReplayEvent, shouldSkipReplayTest, waitForReplayRequest } from '../../../../utils/replayHelpers';

sentryTest('should capture a replay & attach an error', async ({ getLocalTestUrl, 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 req = waitForReplayRequest(page);

 const url = await getLocalTestUrl({ testDir: __dirname });
 const reqError = await waitForErrorRequestOnUrl(page, url);

 const errorEventData = envelopeRequestParser(reqError);
 expect(errorEventData.exception?.values?.length).toBe(1);
 expect(errorEventData.exception?.values?.[0]?.value).toContain('window.doSomethingWrong is not a function');

 const eventData = getReplayEvent(await req);

 expect(eventData).toBeDefined();
 expect(eventData.segment_id).toBe(0);

 expect(errorEventData.tags?.replayId).toEqual(eventData.replay_id);
});

2 changes: 2 additions & 0 deletions
dev-packages/browser-integration-tests/loader-suites/loader/onLoad/customReplay/init.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -6,5 +6,7 @@ Sentry.onLoad(function () {
 useCompression: false,
 }),
 ],

 replaysSessionSampleRate: 1,
 });
});

4 changes: 3 additions & 1 deletion
dev-packages/browser-integration-tests/loader-suites/loader/onLoad/replay/init.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
Sentry.onLoad(function () {
 Sentry.init({});
 Sentry.init({
 replaysSessionSampleRate: 1,
 });
});

1 change: 1 addition & 0 deletions
dev-packages/browser-integration-tests/package.json

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -32,6 +32,7 @@
 "test:loader:eager": "PW_BUNDLE=loader_eager yarn test:loader",
 "test:loader:tracing": "PW_BUNDLE=loader_tracing yarn test:loader",
 "test:loader:replay": "PW_BUNDLE=loader_replay yarn test:loader",
 "test:loader:replay_buffer": "PW_BUNDLE=loader_replay_buffer yarn test:loader",
 "test:loader:full": "PW_BUNDLE=loader_tracing_replay yarn test:loader",
 "test:loader:debug": "PW_BUNDLE=loader_debug yarn test:loader",
 "test:ci": "yarn test:all --reporter='line'",
Expand Down

0
...on-tests/suites/replay/bufferMode/init.js → ...ts/suites/replay/bufferModeManual/init.js

View file


Open in desktop


File renamed without changes.


0
...tests/suites/replay/bufferMode/subject.js → ...suites/replay/bufferModeManual/subject.js

View file


Open in desktop


File renamed without changes.


0
...ts/suites/replay/bufferMode/template.html → ...tes/replay/bufferModeManual/template.html

View file


Open in desktop


File renamed without changes.


0
...on-tests/suites/replay/bufferMode/test.ts → ...ts/suites/replay/bufferModeManual/test.ts

View file


Open in desktop


File renamed without changes.


1 change: 1 addition & 0 deletions
dev-packages/browser-integration-tests/suites/replay/errors/immediateError/subject.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
window.doSomethingWrong();

38 changes: 38 additions & 0 deletions
dev-packages/browser-integration-tests/suites/replay/errors/immediateError/test.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
import { expect } from '@playwright/test';

import { sentryTest } from '../../../../utils/fixtures';
import { envelopeRequestParser, waitForErrorRequestOnUrl } from '../../../../utils/helpers';
import { getReplayEvent, shouldSkipReplayTest, waitForReplayRequest } from '../../../../utils/replayHelpers';

sentryTest(
 '[error-mode] should capture error that happens immediately after init',
 async ({ getLocalTestUrl, 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 req = waitForReplayRequest(page);

 const url = await getLocalTestUrl({ testDir: __dirname });
 const reqError = await waitForErrorRequestOnUrl(page, url);

 const errorEventData = envelopeRequestParser(reqError);
 expect(errorEventData.exception?.values?.length).toBe(1);
 expect(errorEventData.exception?.values?.[0]?.value).toContain('window.doSomethingWrong is not a function');

 const eventData = getReplayEvent(await req);

 expect(eventData).toBeDefined();
 expect(eventData.segment_id).toBe(0);

 expect(errorEventData.tags?.replayId).toEqual(eventData.replay_id);
 },
);

5 changes: 5 additions & 0 deletions
dev-packages/browser-integration-tests/utils/generatePlugin.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -55,6 +55,7 @@ const BUNDLE_PATHS: Record<string, Record<string, string>> = {
 loader_debug: 'build/bundles/bundle.debug.min.js',
 loader_tracing: 'build/bundles/bundle.tracing.min.js',
 loader_replay: 'build/bundles/bundle.replay.min.js',
 loader_replay_buffer: 'build/bundles/bundle.replay.min.js',
 loader_tracing_replay: 'build/bundles/bundle.tracing.replay.debug.min.js',
 },
 integrations: {
Expand DownExpand Up@@ -96,6 +97,10 @@ export const LOADER_CONFIGS: Record<string, { options: Record<string, unknown>;
 options: { replaysSessionSampleRate: 1, replaysOnErrorSampleRate: 1 },
 lazy: false,
 },
 loader_replay_buffer: {
 options: { replaysSessionSampleRate: 0, replaysOnErrorSampleRate: 1 },
 lazy: false,
 },
 loader_tracing_replay: {
 options: { tracesSampleRate: 1, replaysSessionSampleRate: 1, replaysOnErrorSampleRate: 1, debug: true },
 lazy: false,
Expand Down

50 changes: 13 additions & 37 deletions
packages/replay-internal/src/integration.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
import { getClient, parseSampleRate } from '@sentry/core';
import type { BrowserClientReplayOptions, Integration, IntegrationFn } from '@sentry/types';
import { parseSampleRate } from '@sentry/core';
import type { BrowserClientReplayOptions, Client, Integration, IntegrationFn } from '@sentry/types';
import { consoleSandbox, dropUndefinedKeys, isBrowser } from '@sentry/utils';

import {
Expand DownExpand Up@@ -215,22 +215,13 @@ export class Replay implements Integration {
 /**
 * Setup and initialize replay container
 */
 public setupOnce(): void {
 if (!isBrowser()) {
 public afterAllSetup(client: Client): void {
 if (!isBrowser() || this._replay) {
 return;
 }

 this._setup();

 // Once upon a time, we tried to create a transaction in `setupOnce` and it would
 // potentially create a transaction before some native SDK integrations have run
 // and applied their own global event processor. An example is:
 // https://github.com/getsentry/sentry-javascript/blob/b47ceafbdac7f8b99093ce6023726ad4687edc48/packages/browser/src/integrations/useragent.ts
 //
 // So we call `this._initialize()` in next event loop as a workaround to wait for other
 // global event processors to finish. This is no longer needed, but keeping it
 // here to avoid any future issues.
 setTimeout(() => this._initialize());
 this._setup(client);
 this._initialize(client);
 }

 /**
Expand DownExpand Up@@ -301,24 +292,19 @@ export class Replay implements Integration {
 /**
 * Initializes replay.
 */
 protected _initialize(): void {
 protected _initialize(client: Client): void {
 if (!this._replay) {
 return;
 }

Comment on lines

296
 to 
298

@andreiborzaandreiborzaJul 1, 2024

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we remove this now?

 // We have to run this in _initialize, because this runs in setTimeout
 // So when this runs all integrations have been added
 // Before this, we cannot access integrations on the client,
 // so we need to mutate the options here
 this._maybeLoadFromReplayCanvasIntegration();

 this._maybeLoadFromReplayCanvasIntegration(client);
 this._replay.initializeSampling();
 }

 /** Setup the integration. */
 private _setup(): void {
 private _setup(client: Client): void {
 // Client is not available in constructor, so we need to wait until setupOnce
 const finalOptions = loadReplayOptionsFromClient(this._initialOptions);
 const finalOptions = loadReplayOptionsFromClient(this._initialOptions, client);

 this._replay = new ReplayContainer({
 options: finalOptions,
Expand All@@ -327,12 +313,11 @@ export class Replay implements Integration {
 }

 /** Get canvas options from ReplayCanvas integration, if it is also added. */
 private _maybeLoadFromReplayCanvasIntegration(): void {
 private _maybeLoadFromReplayCanvasIntegration(client: Client): void {
 // To save bundle size, we skip checking for stuff here
 // and instead just try-catch everything - as generally this should all be defined
 /* eslint-disable @typescript-eslint/no-non-null-assertion */
 try {
 const client = getClient()!;
 const canvasIntegration = client.getIntegrationByName('ReplayCanvas') as Integration & {
 getOptions(): ReplayCanvasIntegrationOptions;
 };
Expand All@@ -349,24 +334,15 @@ export class Replay implements Integration {
}

/** Parse Replay-related options from SDK options */
function loadReplayOptionsFromClient(initialOptions: InitialReplayPluginOptions): ReplayPluginOptions {
 const client = getClient();
 const opt = client && (client.getOptions() as BrowserClientReplayOptions);
function loadReplayOptionsFromClient(initialOptions: InitialReplayPluginOptions, client: Client): ReplayPluginOptions {
 const opt = client.getOptions() as BrowserClientReplayOptions;

 const finalOptions: ReplayPluginOptions = {
 sessionSampleRate: 0,
 errorSampleRate: 0,
 ...dropUndefinedKeys(initialOptions),
 };

 if (!opt) {
 consoleSandbox(() => {
 // eslint-disable-next-line no-console
 console.warn('SDK client is not available.');
 });
 return finalOptions;
 }

 const replaysSessionSampleRate = parseSampleRate(opt.replaysSessionSampleRate);
 const replaysOnErrorSampleRate = parseSampleRate(opt.replaysOnErrorSampleRate);

Expand Down

5 changes: 3 additions & 2 deletions
packages/replay-internal/test/integration/coreHandlers/handleGlobalEvent.test.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
import { getClient } from '@sentry/core';
import type { Event } from '@sentry/types';

import { REPLAY_EVENT_NAME, SESSION_IDLE_EXPIRE_DURATION } from '../../../src/constants';
Expand DownExpand Up@@ -133,8 +134,8 @@ describe('Integration | coreHandlers | handleGlobalEvent', () => {

 it('tags errors and transactions with replay id for session samples', async () => {
 const { replay, integration } = await resetSdkMock({});
// @ts-expect-error protected but ok to use for testing
 integration._initialize();
integration['_initialize'](getClient()!);
@s1gr1ds1gr1dJul 2, 2024•

edited

Loading

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

omg this syntax 😅 Not wrong at all, just JS/TS 🙄 😆
@mydeamydeaJul 2, 2024

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, it is definitely weird, but I prefer this over having to do @ts-expect-error 😅

 const transaction = Transaction();
 const error = Error();
 expect(handleGlobalEventListener(replay)(transaction, {})).toEqual(
Expand Down

6 changes: 3 additions & 3 deletions
packages/replay-internal/test/integration/errorSampleRate.test.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -866,7 +866,7 @@ describe('Integration | errorSampleRate', () => {
 },
 autoStart: false,
 });
 integration['_initialize']();
 integration['_initialize'](getClient()!);

 expect(replay.recordingMode).toBe('session');
 const sessionId = replay.getSessionId();
Expand DownExpand Up@@ -899,7 +899,7 @@ describe('Integration | errorSampleRate', () => {
 },
 autoStart: false,
 });
 integration['_initialize']();
 integration['_initialize'](getClient()!);

 vi.runAllTimers();

Expand DownExpand Up@@ -943,7 +943,7 @@ describe('Integration | errorSampleRate', () => {
 },
 autoStart: false,
 });
 integration['_initialize']();
 integration['_initialize'](getClient()!);
 const optionsEvent = createOptionsEvent(replay);

 const TEST_EVENT = getTestEventIncremental({ timestamp: BASE_TIMESTAMP });
Expand Down

4 changes: 2 additions & 2 deletions
packages/replay-internal/test/integration/sampling.test.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
import { getClient } from '@sentry/core';
import { resetSdkMock } from '../mocks/resetSdkMock';
import { useFakeTimers } from '../utils/use-fake-timers';

Expand DownExpand Up@@ -57,8 +58,7 @@ describe('Integration | sampling', () => {
 // @ts-expect-error private API
 const spyAddListeners = vi.spyOn(replay, '_addListeners');

 // @ts-expect-error protected
 integration._initialize();
 integration['_initialize'](getClient()!);

 vi.runAllTimers();

Expand Down

18 changes: 7 additions & 11 deletions
packages/replay-internal/test/mocks/mockSdk.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -61,12 +61,8 @@ export async function mockSdk({ replayOptions, sentryOptions, autoStart = true }
 _initialized = value;
 }

 public setupOnce(): void {
 // do nothing
 }

 public initialize(): void {
 return super._initialize();
 public afterAllSetup(): void {
 // do nothing, we need to manually initialize this
 }
 }

Expand All@@ -76,7 +72,7 @@ export async function mockSdk({ replayOptions, sentryOptions, autoStart = true }
 ...replayOptions,
 });

 init({
const client = init({
 ...getDefaultClientOptions(),
 dsn: 'https://dsn@ingest.f00.f00/1',
 autoSessionTracking: false,
Expand All@@ -86,14 +82,14 @@ export async function mockSdk({ replayOptions, sentryOptions, autoStart = true }
 replaysOnErrorSampleRate: 0.0,
 ...sentryOptions,
 integrations: [replayIntegration],
 });
 })!;

 // Instead of `setupOnce`, which is tricky to test, we call this manually here
 replayIntegration['_setup']();
 // Instead of `afterAllSetup`, which is tricky to test, we call this manually here
 replayIntegration['_setup'](client);

 if (autoStart) {
 // Only exists in our mock
 replayIntegration.initialize();
 replayIntegration['_initialize'](client);
 }

 const replay = replayIntegration['_replay']!;
Expand Down

5 changes: 3 additions & 2 deletions
packages/replay-internal/test/utils/TestClient.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
import { BaseClient, createTransport, initAndBind } from '@sentry/core';
import type {
 BrowserClientReplayOptions,
 Client,
 ClientOptions,
 Event,
 ParameterizedString,
Expand DownExpand Up@@ -33,8 +34,8 @@ export class TestClient extends BaseClient<TestClientOptions> {
 }
}

export function init(options: TestClientOptions): void {
 initAndBind(TestClient, options);
export function init(options: TestClientOptions): Client {
return initAndBind(TestClient, options);
}

export function getDefaultClientOptions(options: Partial<TestClientOptions> = {}): ClientOptions {
Expand Down

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default");
}
} catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
fix(replay): Start replay in afterAllSetup instead of next tick#12709

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.

 Sign up for GitHub

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

 Merged

mydea
merged 3 commits into

from

Jul 2, 2024
 Merged


fix(replay): Start replay in afterAllSetup instead of next tick
#12709
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


3 commits


Select commit
Hold shift + click to select a range
e435d1e

fix(replay): Start replay immediately, not in next tick


mydea Jul 1, 2024
acef6e5

small fixes


mydea Jul 1, 2024
eeae961

run replay in `afterAllSetup`


mydea Jul 1, 2024

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


1 change: 1 addition & 0 deletions
.github/workflows/build.yml

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -761,6 +761,7 @@ jobs:
 - loader_debug
 - loader_tracing
 - loader_replay
 - loader_replay_buffer
 - loader_tracing_replay

 steps:
Expand Down

5 changes: 4 additions & 1 deletion
dev-packages/browser-integration-tests/loader-suites/loader/noOnLoad/replay/test.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -3,8 +3,11 @@ import { expect } from '@playwright/test';
import { sentryTest } from '../../../../utils/fixtures';
import { getReplayEvent, shouldSkipReplayTest, waitForReplayRequest } from '../../../../utils/replayHelpers';

const bundle = process.env.PW_BUNDLE || '';

sentryTest('should capture a replay', async ({ getLocalTestUrl, page }) => {
 if (shouldSkipReplayTest()) {
 // When in buffer mode, there will not be a replay by default
 if (shouldSkipReplayTest() || bundle === 'loader_replay_buffer') {
 sentryTest.skip();
 }

Expand Down

1 change: 1 addition & 0 deletions
dev-packages/browser-integration-tests/loader-suites/loader/noOnLoad/replayError/subject.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
window.doSomethingWrong();

35 changes: 35 additions & 0 deletions
dev-packages/browser-integration-tests/loader-suites/loader/noOnLoad/replayError/test.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
import { expect } from '@playwright/test';

import { sentryTest } from '../../../../utils/fixtures';
import { envelopeRequestParser, waitForErrorRequestOnUrl } from '../../../../utils/helpers';
import { getReplayEvent, shouldSkipReplayTest, waitForReplayRequest } from '../../../../utils/replayHelpers';

sentryTest('should capture a replay & attach an error', async ({ getLocalTestUrl, 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 req = waitForReplayRequest(page);

 const url = await getLocalTestUrl({ testDir: __dirname });
 const reqError = await waitForErrorRequestOnUrl(page, url);

 const errorEventData = envelopeRequestParser(reqError);
 expect(errorEventData.exception?.values?.length).toBe(1);
 expect(errorEventData.exception?.values?.[0]?.value).toContain('window.doSomethingWrong is not a function');

 const eventData = getReplayEvent(await req);

 expect(eventData).toBeDefined();
 expect(eventData.segment_id).toBe(0);

 expect(errorEventData.tags?.replayId).toEqual(eventData.replay_id);
});

2 changes: 2 additions & 0 deletions
dev-packages/browser-integration-tests/loader-suites/loader/onLoad/customReplay/init.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -6,5 +6,7 @@ Sentry.onLoad(function () {
 useCompression: false,
 }),
 ],

 replaysSessionSampleRate: 1,
 });
});

4 changes: 3 additions & 1 deletion
dev-packages/browser-integration-tests/loader-suites/loader/onLoad/replay/init.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
Sentry.onLoad(function () {
 Sentry.init({});
 Sentry.init({
 replaysSessionSampleRate: 1,
 });
});

1 change: 1 addition & 0 deletions
dev-packages/browser-integration-tests/package.json

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -32,6 +32,7 @@
 "test:loader:eager": "PW_BUNDLE=loader_eager yarn test:loader",
 "test:loader:tracing": "PW_BUNDLE=loader_tracing yarn test:loader",
 "test:loader:replay": "PW_BUNDLE=loader_replay yarn test:loader",
 "test:loader:replay_buffer": "PW_BUNDLE=loader_replay_buffer yarn test:loader",
 "test:loader:full": "PW_BUNDLE=loader_tracing_replay yarn test:loader",
 "test:loader:debug": "PW_BUNDLE=loader_debug yarn test:loader",
 "test:ci": "yarn test:all --reporter='line'",
Expand Down

0
...on-tests/suites/replay/bufferMode/init.js → ...ts/suites/replay/bufferModeManual/init.js

View file


Open in desktop


File renamed without changes.


0
...tests/suites/replay/bufferMode/subject.js → ...suites/replay/bufferModeManual/subject.js

View file


Open in desktop


File renamed without changes.


0
...ts/suites/replay/bufferMode/template.html → ...tes/replay/bufferModeManual/template.html

View file


Open in desktop


File renamed without changes.


0
...on-tests/suites/replay/bufferMode/test.ts → ...ts/suites/replay/bufferModeManual/test.ts

View file


Open in desktop


File renamed without changes.


1 change: 1 addition & 0 deletions
dev-packages/browser-integration-tests/suites/replay/errors/immediateError/subject.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
window.doSomethingWrong();

38 changes: 38 additions & 0 deletions
dev-packages/browser-integration-tests/suites/replay/errors/immediateError/test.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
import { expect } from '@playwright/test';

import { sentryTest } from '../../../../utils/fixtures';
import { envelopeRequestParser, waitForErrorRequestOnUrl } from '../../../../utils/helpers';
import { getReplayEvent, shouldSkipReplayTest, waitForReplayRequest } from '../../../../utils/replayHelpers';

sentryTest(
 '[error-mode] should capture error that happens immediately after init',
 async ({ getLocalTestUrl, 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 req = waitForReplayRequest(page);

 const url = await getLocalTestUrl({ testDir: __dirname });
 const reqError = await waitForErrorRequestOnUrl(page, url);

 const errorEventData = envelopeRequestParser(reqError);
 expect(errorEventData.exception?.values?.length).toBe(1);
 expect(errorEventData.exception?.values?.[0]?.value).toContain('window.doSomethingWrong is not a function');

 const eventData = getReplayEvent(await req);

 expect(eventData).toBeDefined();
 expect(eventData.segment_id).toBe(0);

 expect(errorEventData.tags?.replayId).toEqual(eventData.replay_id);
 },
);

5 changes: 5 additions & 0 deletions
dev-packages/browser-integration-tests/utils/generatePlugin.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -55,6 +55,7 @@ const BUNDLE_PATHS: Record<string, Record<string, string>> = {
 loader_debug: 'build/bundles/bundle.debug.min.js',
 loader_tracing: 'build/bundles/bundle.tracing.min.js',
 loader_replay: 'build/bundles/bundle.replay.min.js',
 loader_replay_buffer: 'build/bundles/bundle.replay.min.js',
 loader_tracing_replay: 'build/bundles/bundle.tracing.replay.debug.min.js',
 },
 integrations: {
Expand DownExpand Up@@ -96,6 +97,10 @@ export const LOADER_CONFIGS: Record<string, { options: Record<string, unknown>;
 options: { replaysSessionSampleRate: 1, replaysOnErrorSampleRate: 1 },
 lazy: false,
 },
 loader_replay_buffer: {
 options: { replaysSessionSampleRate: 0, replaysOnErrorSampleRate: 1 },
 lazy: false,
 },
 loader_tracing_replay: {
 options: { tracesSampleRate: 1, replaysSessionSampleRate: 1, replaysOnErrorSampleRate: 1, debug: true },
 lazy: false,
Expand Down

50 changes: 13 additions & 37 deletions
packages/replay-internal/src/integration.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
import { getClient, parseSampleRate } from '@sentry/core';
import type { BrowserClientReplayOptions, Integration, IntegrationFn } from '@sentry/types';
import { parseSampleRate } from '@sentry/core';
import type { BrowserClientReplayOptions, Client, Integration, IntegrationFn } from '@sentry/types';
import { consoleSandbox, dropUndefinedKeys, isBrowser } from '@sentry/utils';

import {
Expand DownExpand Up@@ -215,22 +215,13 @@ export class Replay implements Integration {
 /**
 * Setup and initialize replay container
 */
 public setupOnce(): void {
 if (!isBrowser()) {
 public afterAllSetup(client: Client): void {
 if (!isBrowser() || this._replay) {
 return;
 }

 this._setup();

 // Once upon a time, we tried to create a transaction in `setupOnce` and it would
 // potentially create a transaction before some native SDK integrations have run
 // and applied their own global event processor. An example is:
 // https://github.com/getsentry/sentry-javascript/blob/b47ceafbdac7f8b99093ce6023726ad4687edc48/packages/browser/src/integrations/useragent.ts
 //
 // So we call `this._initialize()` in next event loop as a workaround to wait for other
 // global event processors to finish. This is no longer needed, but keeping it
 // here to avoid any future issues.
 setTimeout(() => this._initialize());
 this._setup(client);
 this._initialize(client);
 }

 /**
Expand DownExpand Up@@ -301,24 +292,19 @@ export class Replay implements Integration {
 /**
 * Initializes replay.
 */
 protected _initialize(): void {
 protected _initialize(client: Client): void {
 if (!this._replay) {
 return;
 }

Comment on lines

296
 to 
298

@andreiborzaandreiborzaJul 1, 2024

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we remove this now?

 // We have to run this in _initialize, because this runs in setTimeout
 // So when this runs all integrations have been added
 // Before this, we cannot access integrations on the client,
 // so we need to mutate the options here
 this._maybeLoadFromReplayCanvasIntegration();

 this._maybeLoadFromReplayCanvasIntegration(client);
 this._replay.initializeSampling();
 }

 /** Setup the integration. */
 private _setup(): void {
 private _setup(client: Client): void {
 // Client is not available in constructor, so we need to wait until setupOnce
 const finalOptions = loadReplayOptionsFromClient(this._initialOptions);
 const finalOptions = loadReplayOptionsFromClient(this._initialOptions, client);

 this._replay = new ReplayContainer({
 options: finalOptions,
Expand All@@ -327,12 +313,11 @@ export class Replay implements Integration {
 }

 /** Get canvas options from ReplayCanvas integration, if it is also added. */
 private _maybeLoadFromReplayCanvasIntegration(): void {
 private _maybeLoadFromReplayCanvasIntegration(client: Client): void {
 // To save bundle size, we skip checking for stuff here
 // and instead just try-catch everything - as generally this should all be defined
 /* eslint-disable @typescript-eslint/no-non-null-assertion */
 try {
 const client = getClient()!;
 const canvasIntegration = client.getIntegrationByName('ReplayCanvas') as Integration & {
 getOptions(): ReplayCanvasIntegrationOptions;
 };
Expand All@@ -349,24 +334,15 @@ export class Replay implements Integration {
}

/** Parse Replay-related options from SDK options */
function loadReplayOptionsFromClient(initialOptions: InitialReplayPluginOptions): ReplayPluginOptions {
 const client = getClient();
 const opt = client && (client.getOptions() as BrowserClientReplayOptions);
function loadReplayOptionsFromClient(initialOptions: InitialReplayPluginOptions, client: Client): ReplayPluginOptions {
 const opt = client.getOptions() as BrowserClientReplayOptions;

 const finalOptions: ReplayPluginOptions = {
 sessionSampleRate: 0,
 errorSampleRate: 0,
 ...dropUndefinedKeys(initialOptions),
 };

 if (!opt) {
 consoleSandbox(() => {
 // eslint-disable-next-line no-console
 console.warn('SDK client is not available.');
 });
 return finalOptions;
 }

 const replaysSessionSampleRate = parseSampleRate(opt.replaysSessionSampleRate);
 const replaysOnErrorSampleRate = parseSampleRate(opt.replaysOnErrorSampleRate);

Expand Down

5 changes: 3 additions & 2 deletions
packages/replay-internal/test/integration/coreHandlers/handleGlobalEvent.test.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
import { getClient } from '@sentry/core';
import type { Event } from '@sentry/types';

import { REPLAY_EVENT_NAME, SESSION_IDLE_EXPIRE_DURATION } from '../../../src/constants';
Expand DownExpand Up@@ -133,8 +134,8 @@ describe('Integration | coreHandlers | handleGlobalEvent', () => {

 it('tags errors and transactions with replay id for session samples', async () => {
 const { replay, integration } = await resetSdkMock({});
// @ts-expect-error protected but ok to use for testing
 integration._initialize();
integration['_initialize'](getClient()!);
@s1gr1ds1gr1dJul 2, 2024•

edited

Loading

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

omg this syntax 😅 Not wrong at all, just JS/TS 🙄 😆
@mydeamydeaJul 2, 2024

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, it is definitely weird, but I prefer this over having to do @ts-expect-error 😅

 const transaction = Transaction();
 const error = Error();
 expect(handleGlobalEventListener(replay)(transaction, {})).toEqual(
Expand Down

6 changes: 3 additions & 3 deletions
packages/replay-internal/test/integration/errorSampleRate.test.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -866,7 +866,7 @@ describe('Integration | errorSampleRate', () => {
 },
 autoStart: false,
 });
 integration['_initialize']();
 integration['_initialize'](getClient()!);

 expect(replay.recordingMode).toBe('session');
 const sessionId = replay.getSessionId();
Expand DownExpand Up@@ -899,7 +899,7 @@ describe('Integration | errorSampleRate', () => {
 },
 autoStart: false,
 });
 integration['_initialize']();
 integration['_initialize'](getClient()!);

 vi.runAllTimers();

Expand DownExpand Up@@ -943,7 +943,7 @@ describe('Integration | errorSampleRate', () => {
 },
 autoStart: false,
 });
 integration['_initialize']();
 integration['_initialize'](getClient()!);
 const optionsEvent = createOptionsEvent(replay);

 const TEST_EVENT = getTestEventIncremental({ timestamp: BASE_TIMESTAMP });
Expand Down

4 changes: 2 additions & 2 deletions
packages/replay-internal/test/integration/sampling.test.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
import { getClient } from '@sentry/core';
import { resetSdkMock } from '../mocks/resetSdkMock';
import { useFakeTimers } from '../utils/use-fake-timers';

Expand DownExpand Up@@ -57,8 +58,7 @@ describe('Integration | sampling', () => {
 // @ts-expect-error private API
 const spyAddListeners = vi.spyOn(replay, '_addListeners');

 // @ts-expect-error protected
 integration._initialize();
 integration['_initialize'](getClient()!);

 vi.runAllTimers();

Expand Down

18 changes: 7 additions & 11 deletions
packages/replay-internal/test/mocks/mockSdk.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -61,12 +61,8 @@ export async function mockSdk({ replayOptions, sentryOptions, autoStart = true }
 _initialized = value;
 }

 public setupOnce(): void {
 // do nothing
 }

 public initialize(): void {
 return super._initialize();
 public afterAllSetup(): void {
 // do nothing, we need to manually initialize this
 }
 }

Expand All@@ -76,7 +72,7 @@ export async function mockSdk({ replayOptions, sentryOptions, autoStart = true }
 ...replayOptions,
 });

 init({
const client = init({
 ...getDefaultClientOptions(),
 dsn: 'https://dsn@ingest.f00.f00/1',
 autoSessionTracking: false,
Expand All@@ -86,14 +82,14 @@ export async function mockSdk({ replayOptions, sentryOptions, autoStart = true }
 replaysOnErrorSampleRate: 0.0,
 ...sentryOptions,
 integrations: [replayIntegration],
 });
 })!;

 // Instead of `setupOnce`, which is tricky to test, we call this manually here
 replayIntegration['_setup']();
 // Instead of `afterAllSetup`, which is tricky to test, we call this manually here
 replayIntegration['_setup'](client);

 if (autoStart) {
 // Only exists in our mock
 replayIntegration.initialize();
 replayIntegration['_initialize'](client);
 }

 const replay = replayIntegration['_replay']!;
Expand Down

5 changes: 3 additions & 2 deletions
packages/replay-internal/test/utils/TestClient.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
import { BaseClient, createTransport, initAndBind } from '@sentry/core';
import type {
 BrowserClientReplayOptions,
 Client,
 ClientOptions,
 Event,
 ParameterizedString,
Expand DownExpand Up@@ -33,8 +34,8 @@ export class TestClient extends BaseClient<TestClientOptions> {
 }
}

export function init(options: TestClientOptions): void {
 initAndBind(TestClient, options);
export function init(options: TestClientOptions): Client {
return initAndBind(TestClient, options);
}

export function getDefaultClientOptions(options: Partial<TestClientOptions> = {}): ClientOptions {
Expand Down

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers");
}
} catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
fix(replay): Start replay in afterAllSetup instead of next tick#12709

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.

 Sign up for GitHub

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

 Merged

mydea
merged 3 commits into

from

Jul 2, 2024
 Merged


fix(replay): Start replay in afterAllSetup instead of next tick
#12709
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


3 commits


Select commit
Hold shift + click to select a range
e435d1e

fix(replay): Start replay immediately, not in next tick


mydea Jul 1, 2024
acef6e5

small fixes


mydea Jul 1, 2024
eeae961

run replay in `afterAllSetup`


mydea Jul 1, 2024

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


1 change: 1 addition & 0 deletions
.github/workflows/build.yml

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -761,6 +761,7 @@ jobs:
 - loader_debug
 - loader_tracing
 - loader_replay
 - loader_replay_buffer
 - loader_tracing_replay

 steps:
Expand Down

5 changes: 4 additions & 1 deletion
dev-packages/browser-integration-tests/loader-suites/loader/noOnLoad/replay/test.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -3,8 +3,11 @@ import { expect } from '@playwright/test';
import { sentryTest } from '../../../../utils/fixtures';
import { getReplayEvent, shouldSkipReplayTest, waitForReplayRequest } from '../../../../utils/replayHelpers';

const bundle = process.env.PW_BUNDLE || '';

sentryTest('should capture a replay', async ({ getLocalTestUrl, page }) => {
 if (shouldSkipReplayTest()) {
 // When in buffer mode, there will not be a replay by default
 if (shouldSkipReplayTest() || bundle === 'loader_replay_buffer') {
 sentryTest.skip();
 }

Expand Down

1 change: 1 addition & 0 deletions
dev-packages/browser-integration-tests/loader-suites/loader/noOnLoad/replayError/subject.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
window.doSomethingWrong();

35 changes: 35 additions & 0 deletions
dev-packages/browser-integration-tests/loader-suites/loader/noOnLoad/replayError/test.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
import { expect } from '@playwright/test';

import { sentryTest } from '../../../../utils/fixtures';
import { envelopeRequestParser, waitForErrorRequestOnUrl } from '../../../../utils/helpers';
import { getReplayEvent, shouldSkipReplayTest, waitForReplayRequest } from '../../../../utils/replayHelpers';

sentryTest('should capture a replay & attach an error', async ({ getLocalTestUrl, 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 req = waitForReplayRequest(page);

 const url = await getLocalTestUrl({ testDir: __dirname });
 const reqError = await waitForErrorRequestOnUrl(page, url);

 const errorEventData = envelopeRequestParser(reqError);
 expect(errorEventData.exception?.values?.length).toBe(1);
 expect(errorEventData.exception?.values?.[0]?.value).toContain('window.doSomethingWrong is not a function');

 const eventData = getReplayEvent(await req);

 expect(eventData).toBeDefined();
 expect(eventData.segment_id).toBe(0);

 expect(errorEventData.tags?.replayId).toEqual(eventData.replay_id);
});

2 changes: 2 additions & 0 deletions
dev-packages/browser-integration-tests/loader-suites/loader/onLoad/customReplay/init.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -6,5 +6,7 @@ Sentry.onLoad(function () {
 useCompression: false,
 }),
 ],

 replaysSessionSampleRate: 1,
 });
});

4 changes: 3 additions & 1 deletion
dev-packages/browser-integration-tests/loader-suites/loader/onLoad/replay/init.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
Sentry.onLoad(function () {
 Sentry.init({});
 Sentry.init({
 replaysSessionSampleRate: 1,
 });
});

1 change: 1 addition & 0 deletions
dev-packages/browser-integration-tests/package.json

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -32,6 +32,7 @@
 "test:loader:eager": "PW_BUNDLE=loader_eager yarn test:loader",
 "test:loader:tracing": "PW_BUNDLE=loader_tracing yarn test:loader",
 "test:loader:replay": "PW_BUNDLE=loader_replay yarn test:loader",
 "test:loader:replay_buffer": "PW_BUNDLE=loader_replay_buffer yarn test:loader",
 "test:loader:full": "PW_BUNDLE=loader_tracing_replay yarn test:loader",
 "test:loader:debug": "PW_BUNDLE=loader_debug yarn test:loader",
 "test:ci": "yarn test:all --reporter='line'",
Expand Down

0
...on-tests/suites/replay/bufferMode/init.js → ...ts/suites/replay/bufferModeManual/init.js

View file


Open in desktop


File renamed without changes.


0
...tests/suites/replay/bufferMode/subject.js → ...suites/replay/bufferModeManual/subject.js

View file


Open in desktop


File renamed without changes.


0
...ts/suites/replay/bufferMode/template.html → ...tes/replay/bufferModeManual/template.html

View file


Open in desktop


File renamed without changes.


0
...on-tests/suites/replay/bufferMode/test.ts → ...ts/suites/replay/bufferModeManual/test.ts

View file


Open in desktop


File renamed without changes.


1 change: 1 addition & 0 deletions
dev-packages/browser-integration-tests/suites/replay/errors/immediateError/subject.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
window.doSomethingWrong();

38 changes: 38 additions & 0 deletions
dev-packages/browser-integration-tests/suites/replay/errors/immediateError/test.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
import { expect } from '@playwright/test';

import { sentryTest } from '../../../../utils/fixtures';
import { envelopeRequestParser, waitForErrorRequestOnUrl } from '../../../../utils/helpers';
import { getReplayEvent, shouldSkipReplayTest, waitForReplayRequest } from '../../../../utils/replayHelpers';

sentryTest(
 '[error-mode] should capture error that happens immediately after init',
 async ({ getLocalTestUrl, 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 req = waitForReplayRequest(page);

 const url = await getLocalTestUrl({ testDir: __dirname });
 const reqError = await waitForErrorRequestOnUrl(page, url);

 const errorEventData = envelopeRequestParser(reqError);
 expect(errorEventData.exception?.values?.length).toBe(1);
 expect(errorEventData.exception?.values?.[0]?.value).toContain('window.doSomethingWrong is not a function');

 const eventData = getReplayEvent(await req);

 expect(eventData).toBeDefined();
 expect(eventData.segment_id).toBe(0);

 expect(errorEventData.tags?.replayId).toEqual(eventData.replay_id);
 },
);

5 changes: 5 additions & 0 deletions
dev-packages/browser-integration-tests/utils/generatePlugin.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -55,6 +55,7 @@ const BUNDLE_PATHS: Record<string, Record<string, string>> = {
 loader_debug: 'build/bundles/bundle.debug.min.js',
 loader_tracing: 'build/bundles/bundle.tracing.min.js',
 loader_replay: 'build/bundles/bundle.replay.min.js',
 loader_replay_buffer: 'build/bundles/bundle.replay.min.js',
 loader_tracing_replay: 'build/bundles/bundle.tracing.replay.debug.min.js',
 },
 integrations: {
Expand DownExpand Up@@ -96,6 +97,10 @@ export const LOADER_CONFIGS: Record<string, { options: Record<string, unknown>;
 options: { replaysSessionSampleRate: 1, replaysOnErrorSampleRate: 1 },
 lazy: false,
 },
 loader_replay_buffer: {
 options: { replaysSessionSampleRate: 0, replaysOnErrorSampleRate: 1 },
 lazy: false,
 },
 loader_tracing_replay: {
 options: { tracesSampleRate: 1, replaysSessionSampleRate: 1, replaysOnErrorSampleRate: 1, debug: true },
 lazy: false,
Expand Down

50 changes: 13 additions & 37 deletions
packages/replay-internal/src/integration.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
import { getClient, parseSampleRate } from '@sentry/core';
import type { BrowserClientReplayOptions, Integration, IntegrationFn } from '@sentry/types';
import { parseSampleRate } from '@sentry/core';
import type { BrowserClientReplayOptions, Client, Integration, IntegrationFn } from '@sentry/types';
import { consoleSandbox, dropUndefinedKeys, isBrowser } from '@sentry/utils';

import {
Expand DownExpand Up@@ -215,22 +215,13 @@ export class Replay implements Integration {
 /**
 * Setup and initialize replay container
 */
 public setupOnce(): void {
 if (!isBrowser()) {
 public afterAllSetup(client: Client): void {
 if (!isBrowser() || this._replay) {
 return;
 }

 this._setup();

 // Once upon a time, we tried to create a transaction in `setupOnce` and it would
 // potentially create a transaction before some native SDK integrations have run
 // and applied their own global event processor. An example is:
 // https://github.com/getsentry/sentry-javascript/blob/b47ceafbdac7f8b99093ce6023726ad4687edc48/packages/browser/src/integrations/useragent.ts
 //
 // So we call `this._initialize()` in next event loop as a workaround to wait for other
 // global event processors to finish. This is no longer needed, but keeping it
 // here to avoid any future issues.
 setTimeout(() => this._initialize());
 this._setup(client);
 this._initialize(client);
 }

 /**
Expand DownExpand Up@@ -301,24 +292,19 @@ export class Replay implements Integration {
 /**
 * Initializes replay.
 */
 protected _initialize(): void {
 protected _initialize(client: Client): void {
 if (!this._replay) {
 return;
 }

Comment on lines

296
 to 
298

@andreiborzaandreiborzaJul 1, 2024

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we remove this now?

 // We have to run this in _initialize, because this runs in setTimeout
 // So when this runs all integrations have been added
 // Before this, we cannot access integrations on the client,
 // so we need to mutate the options here
 this._maybeLoadFromReplayCanvasIntegration();

 this._maybeLoadFromReplayCanvasIntegration(client);
 this._replay.initializeSampling();
 }

 /** Setup the integration. */
 private _setup(): void {
 private _setup(client: Client): void {
 // Client is not available in constructor, so we need to wait until setupOnce
 const finalOptions = loadReplayOptionsFromClient(this._initialOptions);
 const finalOptions = loadReplayOptionsFromClient(this._initialOptions, client);

 this._replay = new ReplayContainer({
 options: finalOptions,
Expand All@@ -327,12 +313,11 @@ export class Replay implements Integration {
 }

 /** Get canvas options from ReplayCanvas integration, if it is also added. */
 private _maybeLoadFromReplayCanvasIntegration(): void {
 private _maybeLoadFromReplayCanvasIntegration(client: Client): void {
 // To save bundle size, we skip checking for stuff here
 // and instead just try-catch everything - as generally this should all be defined
 /* eslint-disable @typescript-eslint/no-non-null-assertion */
 try {
 const client = getClient()!;
 const canvasIntegration = client.getIntegrationByName('ReplayCanvas') as Integration & {
 getOptions(): ReplayCanvasIntegrationOptions;
 };
Expand All@@ -349,24 +334,15 @@ export class Replay implements Integration {
}

/** Parse Replay-related options from SDK options */
function loadReplayOptionsFromClient(initialOptions: InitialReplayPluginOptions): ReplayPluginOptions {
 const client = getClient();
 const opt = client && (client.getOptions() as BrowserClientReplayOptions);
function loadReplayOptionsFromClient(initialOptions: InitialReplayPluginOptions, client: Client): ReplayPluginOptions {
 const opt = client.getOptions() as BrowserClientReplayOptions;

 const finalOptions: ReplayPluginOptions = {
 sessionSampleRate: 0,
 errorSampleRate: 0,
 ...dropUndefinedKeys(initialOptions),
 };

 if (!opt) {
 consoleSandbox(() => {
 // eslint-disable-next-line no-console
 console.warn('SDK client is not available.');
 });
 return finalOptions;
 }

 const replaysSessionSampleRate = parseSampleRate(opt.replaysSessionSampleRate);
 const replaysOnErrorSampleRate = parseSampleRate(opt.replaysOnErrorSampleRate);

Expand Down

5 changes: 3 additions & 2 deletions
packages/replay-internal/test/integration/coreHandlers/handleGlobalEvent.test.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
import { getClient } from '@sentry/core';
import type { Event } from '@sentry/types';

import { REPLAY_EVENT_NAME, SESSION_IDLE_EXPIRE_DURATION } from '../../../src/constants';
Expand DownExpand Up@@ -133,8 +134,8 @@ describe('Integration | coreHandlers | handleGlobalEvent', () => {

 it('tags errors and transactions with replay id for session samples', async () => {
 const { replay, integration } = await resetSdkMock({});
// @ts-expect-error protected but ok to use for testing
 integration._initialize();
integration['_initialize'](getClient()!);
@s1gr1ds1gr1dJul 2, 2024•

edited

Loading

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

omg this syntax 😅 Not wrong at all, just JS/TS 🙄 😆
@mydeamydeaJul 2, 2024

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, it is definitely weird, but I prefer this over having to do @ts-expect-error 😅

 const transaction = Transaction();
 const error = Error();
 expect(handleGlobalEventListener(replay)(transaction, {})).toEqual(
Expand Down

6 changes: 3 additions & 3 deletions
packages/replay-internal/test/integration/errorSampleRate.test.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -866,7 +866,7 @@ describe('Integration | errorSampleRate', () => {
 },
 autoStart: false,
 });
 integration['_initialize']();
 integration['_initialize'](getClient()!);

 expect(replay.recordingMode).toBe('session');
 const sessionId = replay.getSessionId();
Expand DownExpand Up@@ -899,7 +899,7 @@ describe('Integration | errorSampleRate', () => {
 },
 autoStart: false,
 });
 integration['_initialize']();
 integration['_initialize'](getClient()!);

 vi.runAllTimers();

Expand DownExpand Up@@ -943,7 +943,7 @@ describe('Integration | errorSampleRate', () => {
 },
 autoStart: false,
 });
 integration['_initialize']();
 integration['_initialize'](getClient()!);
 const optionsEvent = createOptionsEvent(replay);

 const TEST_EVENT = getTestEventIncremental({ timestamp: BASE_TIMESTAMP });
Expand Down

4 changes: 2 additions & 2 deletions
packages/replay-internal/test/integration/sampling.test.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
import { getClient } from '@sentry/core';
import { resetSdkMock } from '../mocks/resetSdkMock';
import { useFakeTimers } from '../utils/use-fake-timers';

Expand DownExpand Up@@ -57,8 +58,7 @@ describe('Integration | sampling', () => {
 // @ts-expect-error private API
 const spyAddListeners = vi.spyOn(replay, '_addListeners');

 // @ts-expect-error protected
 integration._initialize();
 integration['_initialize'](getClient()!);

 vi.runAllTimers();

Expand Down

18 changes: 7 additions & 11 deletions
packages/replay-internal/test/mocks/mockSdk.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -61,12 +61,8 @@ export async function mockSdk({ replayOptions, sentryOptions, autoStart = true }
 _initialized = value;
 }

 public setupOnce(): void {
 // do nothing
 }

 public initialize(): void {
 return super._initialize();
 public afterAllSetup(): void {
 // do nothing, we need to manually initialize this
 }
 }

Expand All@@ -76,7 +72,7 @@ export async function mockSdk({ replayOptions, sentryOptions, autoStart = true }
 ...replayOptions,
 });

 init({
const client = init({
 ...getDefaultClientOptions(),
 dsn: 'https://dsn@ingest.f00.f00/1',
 autoSessionTracking: false,
Expand All@@ -86,14 +82,14 @@ export async function mockSdk({ replayOptions, sentryOptions, autoStart = true }
 replaysOnErrorSampleRate: 0.0,
 ...sentryOptions,
 integrations: [replayIntegration],
 });
 })!;

 // Instead of `setupOnce`, which is tricky to test, we call this manually here
 replayIntegration['_setup']();
 // Instead of `afterAllSetup`, which is tricky to test, we call this manually here
 replayIntegration['_setup'](client);

 if (autoStart) {
 // Only exists in our mock
 replayIntegration.initialize();
 replayIntegration['_initialize'](client);
 }

 const replay = replayIntegration['_replay']!;
Expand Down

5 changes: 3 additions & 2 deletions
packages/replay-internal/test/utils/TestClient.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
import { BaseClient, createTransport, initAndBind } from '@sentry/core';
import type {
 BrowserClientReplayOptions,
 Client,
 ClientOptions,
 Event,
 ParameterizedString,
Expand DownExpand Up@@ -33,8 +34,8 @@ export class TestClient extends BaseClient<TestClientOptions> {
 }
}

export function init(options: TestClientOptions): void {
 initAndBind(TestClient, options);
export function init(options: TestClientOptions): Client {
return initAndBind(TestClient, options);
}

export function getDefaultClientOptions(options: Partial<TestClientOptions> = {}): ClientOptions {
Expand Down

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode");
}
} catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); }
})();
})();

Skip to content
fix(replay): Start replay in afterAllSetup instead of next tick#12709

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.

 Sign up for GitHub

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

 Merged

mydea
merged 3 commits into

from

Jul 2, 2024
 Merged


fix(replay): Start replay in afterAllSetup instead of next tick
#12709
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


3 commits


Select commit
Hold shift + click to select a range
e435d1e

fix(replay): Start replay immediately, not in next tick


mydea Jul 1, 2024
acef6e5

small fixes


mydea Jul 1, 2024
eeae961

run replay in `afterAllSetup`


mydea Jul 1, 2024

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


1 change: 1 addition & 0 deletions
.github/workflows/build.yml

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -761,6 +761,7 @@ jobs:
 - loader_debug
 - loader_tracing
 - loader_replay
 - loader_replay_buffer
 - loader_tracing_replay

 steps:
Expand Down

5 changes: 4 additions & 1 deletion
dev-packages/browser-integration-tests/loader-suites/loader/noOnLoad/replay/test.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -3,8 +3,11 @@ import { expect } from '@playwright/test';
import { sentryTest } from '../../../../utils/fixtures';
import { getReplayEvent, shouldSkipReplayTest, waitForReplayRequest } from '../../../../utils/replayHelpers';

const bundle = process.env.PW_BUNDLE || '';

sentryTest('should capture a replay', async ({ getLocalTestUrl, page }) => {
 if (shouldSkipReplayTest()) {
 // When in buffer mode, there will not be a replay by default
 if (shouldSkipReplayTest() || bundle === 'loader_replay_buffer') {
 sentryTest.skip();
 }

Expand Down

1 change: 1 addition & 0 deletions
dev-packages/browser-integration-tests/loader-suites/loader/noOnLoad/replayError/subject.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
window.doSomethingWrong();

35 changes: 35 additions & 0 deletions
dev-packages/browser-integration-tests/loader-suites/loader/noOnLoad/replayError/test.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
import { expect } from '@playwright/test';

import { sentryTest } from '../../../../utils/fixtures';
import { envelopeRequestParser, waitForErrorRequestOnUrl } from '../../../../utils/helpers';
import { getReplayEvent, shouldSkipReplayTest, waitForReplayRequest } from '../../../../utils/replayHelpers';

sentryTest('should capture a replay & attach an error', async ({ getLocalTestUrl, 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 req = waitForReplayRequest(page);

 const url = await getLocalTestUrl({ testDir: __dirname });
 const reqError = await waitForErrorRequestOnUrl(page, url);

 const errorEventData = envelopeRequestParser(reqError);
 expect(errorEventData.exception?.values?.length).toBe(1);
 expect(errorEventData.exception?.values?.[0]?.value).toContain('window.doSomethingWrong is not a function');

 const eventData = getReplayEvent(await req);

 expect(eventData).toBeDefined();
 expect(eventData.segment_id).toBe(0);

 expect(errorEventData.tags?.replayId).toEqual(eventData.replay_id);
});

2 changes: 2 additions & 0 deletions
dev-packages/browser-integration-tests/loader-suites/loader/onLoad/customReplay/init.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -6,5 +6,7 @@ Sentry.onLoad(function () {
 useCompression: false,
 }),
 ],

 replaysSessionSampleRate: 1,
 });
});

4 changes: 3 additions & 1 deletion
dev-packages/browser-integration-tests/loader-suites/loader/onLoad/replay/init.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
Sentry.onLoad(function () {
 Sentry.init({});
 Sentry.init({
 replaysSessionSampleRate: 1,
 });
});

1 change: 1 addition & 0 deletions
dev-packages/browser-integration-tests/package.json

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -32,6 +32,7 @@
 "test:loader:eager": "PW_BUNDLE=loader_eager yarn test:loader",
 "test:loader:tracing": "PW_BUNDLE=loader_tracing yarn test:loader",
 "test:loader:replay": "PW_BUNDLE=loader_replay yarn test:loader",
 "test:loader:replay_buffer": "PW_BUNDLE=loader_replay_buffer yarn test:loader",
 "test:loader:full": "PW_BUNDLE=loader_tracing_replay yarn test:loader",
 "test:loader:debug": "PW_BUNDLE=loader_debug yarn test:loader",
 "test:ci": "yarn test:all --reporter='line'",
Expand Down

0
...on-tests/suites/replay/bufferMode/init.js → ...ts/suites/replay/bufferModeManual/init.js

View file


Open in desktop


File renamed without changes.


0
...tests/suites/replay/bufferMode/subject.js → ...suites/replay/bufferModeManual/subject.js

View file


Open in desktop


File renamed without changes.


0
...ts/suites/replay/bufferMode/template.html → ...tes/replay/bufferModeManual/template.html

View file


Open in desktop


File renamed without changes.


0
...on-tests/suites/replay/bufferMode/test.ts → ...ts/suites/replay/bufferModeManual/test.ts

View file


Open in desktop


File renamed without changes.


1 change: 1 addition & 0 deletions
dev-packages/browser-integration-tests/suites/replay/errors/immediateError/subject.js

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
window.doSomethingWrong();

38 changes: 38 additions & 0 deletions
dev-packages/browser-integration-tests/suites/replay/errors/immediateError/test.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
import { expect } from '@playwright/test';

import { sentryTest } from '../../../../utils/fixtures';
import { envelopeRequestParser, waitForErrorRequestOnUrl } from '../../../../utils/helpers';
import { getReplayEvent, shouldSkipReplayTest, waitForReplayRequest } from '../../../../utils/replayHelpers';

sentryTest(
 '[error-mode] should capture error that happens immediately after init',
 async ({ getLocalTestUrl, 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 req = waitForReplayRequest(page);

 const url = await getLocalTestUrl({ testDir: __dirname });
 const reqError = await waitForErrorRequestOnUrl(page, url);

 const errorEventData = envelopeRequestParser(reqError);
 expect(errorEventData.exception?.values?.length).toBe(1);
 expect(errorEventData.exception?.values?.[0]?.value).toContain('window.doSomethingWrong is not a function');

 const eventData = getReplayEvent(await req);

 expect(eventData).toBeDefined();
 expect(eventData.segment_id).toBe(0);

 expect(errorEventData.tags?.replayId).toEqual(eventData.replay_id);
 },
);

5 changes: 5 additions & 0 deletions
dev-packages/browser-integration-tests/utils/generatePlugin.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -55,6 +55,7 @@ const BUNDLE_PATHS: Record<string, Record<string, string>> = {
 loader_debug: 'build/bundles/bundle.debug.min.js',
 loader_tracing: 'build/bundles/bundle.tracing.min.js',
 loader_replay: 'build/bundles/bundle.replay.min.js',
 loader_replay_buffer: 'build/bundles/bundle.replay.min.js',
 loader_tracing_replay: 'build/bundles/bundle.tracing.replay.debug.min.js',
 },
 integrations: {
Expand DownExpand Up@@ -96,6 +97,10 @@ export const LOADER_CONFIGS: Record<string, { options: Record<string, unknown>;
 options: { replaysSessionSampleRate: 1, replaysOnErrorSampleRate: 1 },
 lazy: false,
 },
 loader_replay_buffer: {
 options: { replaysSessionSampleRate: 0, replaysOnErrorSampleRate: 1 },
 lazy: false,
 },
 loader_tracing_replay: {
 options: { tracesSampleRate: 1, replaysSessionSampleRate: 1, replaysOnErrorSampleRate: 1, debug: true },
 lazy: false,
Expand Down

50 changes: 13 additions & 37 deletions
packages/replay-internal/src/integration.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
import { getClient, parseSampleRate } from '@sentry/core';
import type { BrowserClientReplayOptions, Integration, IntegrationFn } from '@sentry/types';
import { parseSampleRate } from '@sentry/core';
import type { BrowserClientReplayOptions, Client, Integration, IntegrationFn } from '@sentry/types';
import { consoleSandbox, dropUndefinedKeys, isBrowser } from '@sentry/utils';

import {
Expand DownExpand Up@@ -215,22 +215,13 @@ export class Replay implements Integration {
 /**
 * Setup and initialize replay container
 */
 public setupOnce(): void {
 if (!isBrowser()) {
 public afterAllSetup(client: Client): void {
 if (!isBrowser() || this._replay) {
 return;
 }

 this._setup();

 // Once upon a time, we tried to create a transaction in `setupOnce` and it would
 // potentially create a transaction before some native SDK integrations have run
 // and applied their own global event processor. An example is:
 // https://github.com/getsentry/sentry-javascript/blob/b47ceafbdac7f8b99093ce6023726ad4687edc48/packages/browser/src/integrations/useragent.ts
 //
 // So we call `this._initialize()` in next event loop as a workaround to wait for other
 // global event processors to finish. This is no longer needed, but keeping it
 // here to avoid any future issues.
 setTimeout(() => this._initialize());
 this._setup(client);
 this._initialize(client);
 }

 /**
Expand DownExpand Up@@ -301,24 +292,19 @@ export class Replay implements Integration {
 /**
 * Initializes replay.
 */
 protected _initialize(): void {
 protected _initialize(client: Client): void {
 if (!this._replay) {
 return;
 }

Comment on lines

296
 to 
298

@andreiborzaandreiborzaJul 1, 2024

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we remove this now?

 // We have to run this in _initialize, because this runs in setTimeout
 // So when this runs all integrations have been added
 // Before this, we cannot access integrations on the client,
 // so we need to mutate the options here
 this._maybeLoadFromReplayCanvasIntegration();

 this._maybeLoadFromReplayCanvasIntegration(client);
 this._replay.initializeSampling();
 }

 /** Setup the integration. */
 private _setup(): void {
 private _setup(client: Client): void {
 // Client is not available in constructor, so we need to wait until setupOnce
 const finalOptions = loadReplayOptionsFromClient(this._initialOptions);
 const finalOptions = loadReplayOptionsFromClient(this._initialOptions, client);

 this._replay = new ReplayContainer({
 options: finalOptions,
Expand All@@ -327,12 +313,11 @@ export class Replay implements Integration {
 }

 /** Get canvas options from ReplayCanvas integration, if it is also added. */
 private _maybeLoadFromReplayCanvasIntegration(): void {
 private _maybeLoadFromReplayCanvasIntegration(client: Client): void {
 // To save bundle size, we skip checking for stuff here
 // and instead just try-catch everything - as generally this should all be defined
 /* eslint-disable @typescript-eslint/no-non-null-assertion */
 try {
 const client = getClient()!;
 const canvasIntegration = client.getIntegrationByName('ReplayCanvas') as Integration & {
 getOptions(): ReplayCanvasIntegrationOptions;
 };
Expand All@@ -349,24 +334,15 @@ export class Replay implements Integration {
}

/** Parse Replay-related options from SDK options */
function loadReplayOptionsFromClient(initialOptions: InitialReplayPluginOptions): ReplayPluginOptions {
 const client = getClient();
 const opt = client && (client.getOptions() as BrowserClientReplayOptions);
function loadReplayOptionsFromClient(initialOptions: InitialReplayPluginOptions, client: Client): ReplayPluginOptions {
 const opt = client.getOptions() as BrowserClientReplayOptions;

 const finalOptions: ReplayPluginOptions = {
 sessionSampleRate: 0,
 errorSampleRate: 0,
 ...dropUndefinedKeys(initialOptions),
 };

 if (!opt) {
 consoleSandbox(() => {
 // eslint-disable-next-line no-console
 console.warn('SDK client is not available.');
 });
 return finalOptions;
 }

 const replaysSessionSampleRate = parseSampleRate(opt.replaysSessionSampleRate);
 const replaysOnErrorSampleRate = parseSampleRate(opt.replaysOnErrorSampleRate);

Expand Down

5 changes: 3 additions & 2 deletions
packages/replay-internal/test/integration/coreHandlers/handleGlobalEvent.test.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
import { getClient } from '@sentry/core';
import type { Event } from '@sentry/types';

import { REPLAY_EVENT_NAME, SESSION_IDLE_EXPIRE_DURATION } from '../../../src/constants';
Expand DownExpand Up@@ -133,8 +134,8 @@ describe('Integration | coreHandlers | handleGlobalEvent', () => {

 it('tags errors and transactions with replay id for session samples', async () => {
 const { replay, integration } = await resetSdkMock({});
// @ts-expect-error protected but ok to use for testing
 integration._initialize();
integration['_initialize'](getClient()!);
@s1gr1ds1gr1dJul 2, 2024•

edited

Loading

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

omg this syntax 😅 Not wrong at all, just JS/TS 🙄 😆
@mydeamydeaJul 2, 2024

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, it is definitely weird, but I prefer this over having to do @ts-expect-error 😅

 const transaction = Transaction();
 const error = Error();
 expect(handleGlobalEventListener(replay)(transaction, {})).toEqual(
Expand Down

6 changes: 3 additions & 3 deletions
packages/replay-internal/test/integration/errorSampleRate.test.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -866,7 +866,7 @@ describe('Integration | errorSampleRate', () => {
 },
 autoStart: false,
 });
 integration['_initialize']();
 integration['_initialize'](getClient()!);

 expect(replay.recordingMode).toBe('session');
 const sessionId = replay.getSessionId();
Expand DownExpand Up@@ -899,7 +899,7 @@ describe('Integration | errorSampleRate', () => {
 },
 autoStart: false,
 });
 integration['_initialize']();
 integration['_initialize'](getClient()!);

 vi.runAllTimers();

Expand DownExpand Up@@ -943,7 +943,7 @@ describe('Integration | errorSampleRate', () => {
 },
 autoStart: false,
 });
 integration['_initialize']();
 integration['_initialize'](getClient()!);
 const optionsEvent = createOptionsEvent(replay);

 const TEST_EVENT = getTestEventIncremental({ timestamp: BASE_TIMESTAMP });
Expand Down

4 changes: 2 additions & 2 deletions
packages/replay-internal/test/integration/sampling.test.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
import { getClient } from '@sentry/core';
import { resetSdkMock } from '../mocks/resetSdkMock';
import { useFakeTimers } from '../utils/use-fake-timers';

Expand DownExpand Up@@ -57,8 +58,7 @@ describe('Integration | sampling', () => {
 // @ts-expect-error private API
 const spyAddListeners = vi.spyOn(replay, '_addListeners');

 // @ts-expect-error protected
 integration._initialize();
 integration['_initialize'](getClient()!);

 vi.runAllTimers();

Expand Down

18 changes: 7 additions & 11 deletions
packages/replay-internal/test/mocks/mockSdk.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -61,12 +61,8 @@ export async function mockSdk({ replayOptions, sentryOptions, autoStart = true }
 _initialized = value;
 }

 public setupOnce(): void {
 // do nothing
 }

 public initialize(): void {
 return super._initialize();
 public afterAllSetup(): void {
 // do nothing, we need to manually initialize this
 }
 }

Expand All@@ -76,7 +72,7 @@ export async function mockSdk({ replayOptions, sentryOptions, autoStart = true }
 ...replayOptions,
 });

 init({
const client = init({
 ...getDefaultClientOptions(),
 dsn: 'https://dsn@ingest.f00.f00/1',
 autoSessionTracking: false,
Expand All@@ -86,14 +82,14 @@ export async function mockSdk({ replayOptions, sentryOptions, autoStart = true }
 replaysOnErrorSampleRate: 0.0,
 ...sentryOptions,
 integrations: [replayIntegration],
 });
 })!;

 // Instead of `setupOnce`, which is tricky to test, we call this manually here
 replayIntegration['_setup']();
 // Instead of `afterAllSetup`, which is tricky to test, we call this manually here
 replayIntegration['_setup'](client);

 if (autoStart) {
 // Only exists in our mock
 replayIntegration.initialize();
 replayIntegration['_initialize'](client);
 }

 const replay = replayIntegration['_replay']!;
Expand Down

5 changes: 3 additions & 2 deletions
packages/replay-internal/test/utils/TestClient.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
import { BaseClient, createTransport, initAndBind } from '@sentry/core';
import type {
 BrowserClientReplayOptions,
 Client,
 ClientOptions,
 Event,
 ParameterizedString,
Expand DownExpand Up@@ -33,8 +34,8 @@ export class TestClient extends BaseClient<TestClientOptions> {
 }
}

export function init(options: TestClientOptions): void {
 initAndBind(TestClient, options);
export function init(options: TestClientOptions): Client {
return initAndBind(TestClient, options);
}

export function getDefaultClientOptions(options: Partial<TestClientOptions> = {}): ClientOptions {
Expand Down