Skip to content

Commit

Permalink
fix(replay): Start replay immediately, not in next tick
Browse files Browse the repository at this point in the history
This should hopefully fix some race conditions...
  • Loading branch information
mydea committed Jul 1, 2024
1 parent 24dfc66 commit 857a688
Show file tree
Hide file tree
Showing 15 changed files with 93 additions and 14 deletions.
1 change: 1 addition & 0 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -761,6 +761,7 @@ jobs:
- loader_debug
- loader_tracing
- loader_replay
- loader_replay_buffer
- loader_tracing_replay

steps:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ import { sentryTest } from '../../../../utils/fixtures';
import { getReplayEvent, shouldSkipReplayTest, waitForReplayRequest } from '../../../../utils/replayHelpers';

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

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
window.doSomethingWrong();
Original file line number Diff line number Diff 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).toBe('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);
});
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,7 @@ Sentry.onLoad(function () {
useCompression: false,
}),
],

replaysSessionSampleRate: 1,
});
});
Original file line number Diff line number Diff 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
Original file line number Diff line number Diff 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
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
window.doSomethingWrong();
Original file line number Diff line number Diff 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).toBe('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);
},
);
10 changes: 8 additions & 2 deletions dev-packages/browser-integration-tests/utils/generatePlugin.ts
Original file line number Diff line number Diff 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 Down Expand 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 Expand Up @@ -128,8 +133,9 @@ function generateSentryAlias(): Record<string, string> {

const modulePath = path.resolve(PACKAGES_DIR, packageName);

if (useCompiledModule && bundleKey && BUNDLE_PATHS[packageName]?.[bundleKey]) {
const bundlePath = path.resolve(modulePath, BUNDLE_PATHS[packageName][bundleKey]);
const bundleKeyPath = bundleKey && BUNDLE_PATHS[packageName]?.[bundleKey];
if (useCompiledModule && bundleKeyPath) {
const bundlePath = path.resolve(modulePath, bundleKeyPath);

return [packageJSON['name'], bundlePath];
}
Expand Down
11 changes: 1 addition & 10 deletions packages/replay-internal/src/integration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -221,16 +221,7 @@ export class Replay implements Integration {
}

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._initialize();
}

/**
Expand Down

0 comments on commit 857a688

Please sign in to comment.