Skip to content

Commit

Permalink
fix(replay): Handle edge cases & more logging
Browse files Browse the repository at this point in the history
  • Loading branch information
mydea committed Jul 18, 2023
1 parent c59d333 commit fead140
Show file tree
Hide file tree
Showing 3 changed files with 126 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@ Sentry.init({
sampleRate: 0,
replaysSessionSampleRate: 1.0,
replaysOnErrorSampleRate: 0.0,
debug: true,

integrations: [window.Replay],
});

Expand Down
103 changes: 101 additions & 2 deletions packages/browser-integration-tests/suites/replay/sessionMaxAge/test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ import {

const SESSION_MAX_AGE = 2000;

sentryTest('handles session that exceeds max age', async ({ getLocalTestPath, page }) => {
if (shouldSkipReplayTest()) {
sentryTest('handles session that exceeds max age', async ({ getLocalTestPath, page, browserName }) => {
if (shouldSkipReplayTest() || browserName === 'firefox') {
sentryTest.skip();
}

Expand Down Expand Up @@ -82,3 +82,102 @@ sentryTest('handles session that exceeds max age', async ({ getLocalTestPath, pa
const stringifiedSnapshot2 = normalize(fullSnapshots2[0]);
expect(stringifiedSnapshot2).toMatchSnapshot('snapshot-2.json');
});

sentryTest(
'handles many consequitive events in session that exceeds max age',
async ({ getLocalTestPath, page, browserName }) => {
if (shouldSkipReplayTest() || browserName === 'firefox') {
sentryTest.skip();
}

const reqPromise0 = waitForReplayRequest(page, 0);
const reqPromise1 = waitForReplayRequest(page, 1);

await page.route('https://dsn.ingest.sentry.io/**/*', route => {
return route.fulfill({
status: 200,
contentType: 'application/json',
body: JSON.stringify({ id: 'test-id' }),
});
});

const url = await getLocalTestPath({ testDir: __dirname });

await page.goto(url);

const replay0 = await getReplaySnapshot(page);
// We use the `initialTimestamp` of the replay to do any time based calculations
const startTimestamp = replay0._context.initialTimestamp;

const req0 = await reqPromise0;

const replayEvent0 = getReplayEvent(req0);
expect(replayEvent0).toEqual(getExpectedReplayEvent({}));

const fullSnapshots0 = getFullRecordingSnapshots(req0);
expect(fullSnapshots0.length).toEqual(1);
const stringifiedSnapshot = normalize(fullSnapshots0[0]);
expect(stringifiedSnapshot).toMatchSnapshot('snapshot-0.json');

// Wait again for a new segment 0 (=new session)
const reqPromise2 = waitForReplayRequest(page, 0);

// Wait for an incremental snapshot
// Wait half of the session max age (after initial flush), but account for potentially slow runners
const timePassed1 = Date.now() - startTimestamp;
await new Promise(resolve => setTimeout(resolve, Math.max(SESSION_MAX_AGE / 2 - timePassed1, 0)));

await page.click('#button1');

const req1 = await reqPromise1;
const replayEvent1 = getReplayEvent(req1);

expect(replayEvent1).toEqual(getExpectedReplayEvent({ segment_id: 1, urls: [] }));

const replay1 = await getReplaySnapshot(page);
const oldSessionId = replay1.session?.id;

// Wait for session to expire
const timePassed2 = Date.now() - startTimestamp;
await new Promise(resolve => setTimeout(resolve, Math.max(SESSION_MAX_AGE - timePassed2, 0)));

await page.evaluate(`
Object.defineProperty(document, 'visibilityState', {
configurable: true,
get: function () {
return 'hidden';
},
});
document.dispatchEvent(new Event('visibilitychange'));
`);

// Many things going on at the same time...
void page.evaluate(`
Object.defineProperty(document, 'visibilityState', {
configurable: true,
get: function () {
return 'visible';
},
});
document.dispatchEvent(new Event('visibilitychange'));
`);
void page.click('#button1');
void page.click('#button2');
await new Promise(resolve => setTimeout(resolve, 1));
void page.click('#button1');
void page.click('#button2');

const req2 = await reqPromise2;
const replay2 = await getReplaySnapshot(page);

expect(replay2.session?.id).not.toEqual(oldSessionId);

const replayEvent2 = getReplayEvent(req2);
expect(replayEvent2).toEqual(getExpectedReplayEvent({}));

const fullSnapshots2 = getFullRecordingSnapshots(req2);
expect(fullSnapshots2.length).toEqual(1);
const stringifiedSnapshot2 = normalize(fullSnapshots2[0]);
expect(stringifiedSnapshot2).toMatchSnapshot('snapshot-2.json');
},
);
26 changes: 25 additions & 1 deletion packages/replay/src/replay.ts
Original file line number Diff line number Diff line change
Expand Up @@ -389,6 +389,12 @@ export class ReplayContainer implements ReplayContainerInterface {

// Re-start recording, but in "session" recording mode

// When `traceInternals` is enabled, we want to log this to the console
// Else, use the regular debug output
// eslint-disable-next-line
const log = this.getOptions()._experiments.traceInternals ? console.warn : logger.log;
log('[Replay] An error was detected in buffer mode, starting to send replay...');

// Reset all "capture on error" configuration before
// starting a new recording
this.recordingMode = 'session';
Expand Down Expand Up @@ -639,6 +645,11 @@ export class ReplayContainer implements ReplayContainerInterface {
}

void this.stop('session expired with refreshing').then(() => {
// Just to double-check we haven't started anywhere else
if (this.isEnabled()) {
return;
}

if (sampled === 'session') {
return this.start();
}
Expand Down Expand Up @@ -947,6 +958,17 @@ export class ReplayContainer implements ReplayContainerInterface {
return;
}

// Unless force is true (which is the case when stop() is called),
// _flush should never be called when not in session mode
// Apart from `stop()`, only debounced flush triggers `_flush()`, which shouldn't happen
if (this.recordingMode !== 'session' && !force) {
// When `traceInternals` is enabled, we want to log this to the console
// Else, use the regular debug output
// eslint-disable-next-line
const log = this.getOptions()._experiments.traceInternals ? console.warn : logger.warn;
log('[Replay] Flushing replay while not in session mode.');
}

return new Promise(resolve => {
if (!this.session) {
resolve();
Expand All @@ -959,7 +981,9 @@ export class ReplayContainer implements ReplayContainerInterface {
},
ensureResumed: () => this.resume(),
onEnd: () => {
__DEBUG_BUILD__ && logger.error('[Replay] Attempting to finish replay event after session expired.');
if (!force) {
__DEBUG_BUILD__ && logger.warn('[Replay] Attempting to finish replay event after session expired.');
}
this._refreshSession();
resolve();
},
Expand Down

0 comments on commit fead140

Please sign in to comment.