Skip to content

ref(replay): Handle checkouts more explicitly #7321

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
merged 2 commits into from
Mar 7, 2023
+239 −125
Merged

ref(replay): Handle checkouts more explicitly #7321

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3375302
ref: Extract `handleRecordingEmit` into util
mydea Feb 22, 2023
c6cef1b
ref(replay): Improve handling of checkout events
mydea Mar 7, 2023
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
69 changes: 38 additions & 31 deletions packages/integration-tests/suites/replay/multiple-pages/test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,21 +45,31 @@ sentryTest(

const reqPromise0 = waitForReplayRequest(page, 0);
const reqPromise1 = waitForReplayRequest(page, 1);
const reqPromise2 = waitForReplayRequest(page, 2);
Copy link
Member

Choose a reason for hiding this comment

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

(no action required)
Good change, let's just always keep these at the top of tests for consistency going forward

const reqPromise3 = waitForReplayRequest(page, 3);
const reqPromise4 = waitForReplayRequest(page, 4);
const reqPromise5 = waitForReplayRequest(page, 5);
const reqPromise6 = waitForReplayRequest(page, 6);
const reqPromise7 = waitForReplayRequest(page, 7);
const reqPromise8 = waitForReplayRequest(page, 8);
const reqPromise9 = waitForReplayRequest(page, 9);

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

await page.goto(url);
const replayEvent0 = getReplayEvent(await reqPromise0);
const recording0 = getReplayRecordingContent(await reqPromise0);
const req0 = await reqPromise0;
const replayEvent0 = getReplayEvent(req0);
const recording0 = getReplayRecordingContent(req0);

expect(replayEvent0).toEqual(getExpectedReplayEvent({ segment_id: 0 }));
expect(normalize(recording0.fullSnapshots)).toMatchSnapshot('seg-0-snap-full');
expect(recording0.incrementalSnapshots.length).toEqual(0);

await page.click('#go-background');

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

expect(replayEvent1).toEqual(
getExpectedReplayEvent({ segment_id: 1, urls: [], replay_start_timestamp: undefined }),
Expand Down Expand Up @@ -91,20 +101,19 @@ sentryTest(

await page.reload();

const reqPromise2 = waitForReplayRequest(page, 2);
const reqPromise3 = waitForReplayRequest(page, 3);

const replayEvent2 = getReplayEvent(await reqPromise2);
const recording2 = getReplayRecordingContent(await reqPromise2);
const req2 = await reqPromise2;
const replayEvent2 = getReplayEvent(req2);
const recording2 = getReplayRecordingContent(req2);

expect(replayEvent2).toEqual(getExpectedReplayEvent({ segment_id: 2, replay_start_timestamp: undefined }));
expect(normalize(recording2.fullSnapshots)).toMatchSnapshot('seg-2-snap-full');
expect(recording2.incrementalSnapshots.length).toEqual(0);

await page.click('#go-background');

const replayEvent3 = getReplayEvent(await reqPromise3);
const recording3 = getReplayRecordingContent(await reqPromise3);
const req3 = await reqPromise3;
const replayEvent3 = getReplayEvent(req3);
const recording3 = getReplayRecordingContent(req3);

expect(replayEvent3).toEqual(
getExpectedReplayEvent({ segment_id: 3, urls: [], replay_start_timestamp: undefined }),
Expand Down Expand Up @@ -134,11 +143,9 @@ sentryTest(

await page.click('a');

const reqPromise4 = waitForReplayRequest(page, 4);
const reqPromise5 = waitForReplayRequest(page, 5);

const replayEvent4 = getReplayEvent(await reqPromise4);
const recording4 = getReplayRecordingContent(await reqPromise4);
const req4 = await reqPromise4;
const replayEvent4 = getReplayEvent(req4);
const recording4 = getReplayRecordingContent(req4);

expect(replayEvent4).toEqual(
getExpectedReplayEvent({
Expand All @@ -161,8 +168,9 @@ sentryTest(

await page.click('#go-background');

const replayEvent5 = getReplayEvent(await reqPromise5);
const recording5 = getReplayRecordingContent(await reqPromise5);
const req5 = await reqPromise5;
const replayEvent5 = getReplayEvent(req5);
const recording5 = getReplayRecordingContent(req5);

expect(replayEvent5).toEqual(
getExpectedReplayEvent({
Expand Down Expand Up @@ -207,9 +215,9 @@ sentryTest(

await page.click('#spa-navigation');

const reqPromise6 = waitForReplayRequest(page, 6);
const replayEvent6 = getReplayEvent(await reqPromise6);
const recording6 = getReplayRecordingContent(await reqPromise6);
const req6 = await reqPromise6;
const replayEvent6 = getReplayEvent(req6);
const recording6 = getReplayRecordingContent(req6);

expect(replayEvent6).toEqual(
getExpectedReplayEvent({
Expand All @@ -231,9 +239,9 @@ sentryTest(

await page.click('#go-background');

const reqPromise7 = waitForReplayRequest(page, 7);
const replayEvent7 = getReplayEvent(await reqPromise7);
const recording7 = getReplayRecordingContent(await reqPromise7);
const req7 = await reqPromise7;
const replayEvent7 = getReplayEvent(req7);
const recording7 = getReplayRecordingContent(req7);

expect(replayEvent7).toEqual(
getExpectedReplayEvent({
Expand Down Expand Up @@ -279,11 +287,9 @@ sentryTest(

await page.click('a');

const reqPromise8 = waitForReplayRequest(page, 8);
const reqPromise9 = waitForReplayRequest(page, 9);

const replayEvent8 = getReplayEvent(await reqPromise8);
const recording8 = getReplayRecordingContent(await reqPromise8);
const req8 = await reqPromise8;
const replayEvent8 = getReplayEvent(req8);
const recording8 = getReplayRecordingContent(req8);

expect(replayEvent8).toEqual(
getExpectedReplayEvent({
Expand All @@ -296,8 +302,9 @@ sentryTest(

await page.click('#go-background');

const replayEvent9 = getReplayEvent(await reqPromise9);
const recording9 = getReplayRecordingContent(await reqPromise9);
const req9 = await reqPromise9;
const replayEvent9 = getReplayEvent(req9);
const recording9 = getReplayRecordingContent(req9);

expect(replayEvent9).toEqual(
getExpectedReplayEvent({
Expand Down
112 changes: 23 additions & 89 deletions packages/replay/src/replay.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import type {
EventBuffer,
InternalEventContext,
PopEventContext,
RecordingEvent,
RecordingOptions,
ReplayContainer as ReplayContainerInterface,
ReplayPluginOptions,
Expand All @@ -30,6 +29,7 @@ import { createBreadcrumb } from './util/createBreadcrumb';
import { createPerformanceEntries } from './util/createPerformanceEntries';
import { createPerformanceSpans } from './util/createPerformanceSpans';
import { debounce } from './util/debounce';
import { getHandleRecordingEmit } from './util/handleRecordingEmit';
import { isExpired } from './util/isExpired';
import { isSessionExpired } from './util/isSessionExpired';
import { overwriteRecordDroppedEvent, restoreRecordDroppedEvent } from './util/monkeyPatchRecordDroppedEvent';
Expand Down Expand Up @@ -155,7 +155,7 @@ export class ReplayContainer implements ReplayContainerInterface {
* _performanceObserver, Recording, Sentry SDK, etc)
*/
public start(): void {
this._setInitialState();
this.setInitialState();

if (!this._loadAndCheckSession()) {
return;
Expand Down Expand Up @@ -207,7 +207,7 @@ export class ReplayContainer implements ReplayContainerInterface {
// Without this, it would record forever, until an error happens, which we don't want
// instead, we'll always keep the last 60 seconds of replay before an error happened
...(this.recordingMode === 'error' && { checkoutEveryNms: ERROR_CHECKOUT_TIME }),
emit: this._handleRecordingEmit,
emit: getHandleRecordingEmit(this),
onMutation: (mutations: unknown[]) => {
if (this._options._experiments.captureMutationSize) {
const count = mutations.length;
Expand Down Expand Up @@ -420,6 +420,25 @@ export class ReplayContainer implements ReplayContainerInterface {
return false;
}

/**
* Capture some initial state that can change throughout the lifespan of the
* replay. This is required because otherwise they would be captured at the
* first flush.
*/
public setInitialState(): void {
const urlPath = `${WINDOW.location.pathname}${WINDOW.location.hash}${WINDOW.location.search}`;
const url = `${WINDOW.location.origin}${urlPath}`;

this.performanceEvents = [];

// Reset _context as well
this._clearContext();

this._context.initialUrl = url;
this._context.initialTimestamp = new Date().getTime();
this._context.urls.push(url);
}

/** A wrapper to conditionally capture exceptions. */
private _handleException(error: unknown): void {
__DEBUG_BUILD__ && logger.error('[Replay]', error);
Expand All @@ -445,7 +464,7 @@ export class ReplayContainer implements ReplayContainerInterface {
// If session was newly created (i.e. was not loaded from storage), then
// enable flag to create the root replay
if (type === 'new') {
this._setInitialState();
this.setInitialState();
}

const currentSessionId = this.getSessionId();
Expand All @@ -463,25 +482,6 @@ export class ReplayContainer implements ReplayContainerInterface {
return true;
}

/**
* Capture some initial state that can change throughout the lifespan of the
* replay. This is required because otherwise they would be captured at the
* first flush.
*/
private _setInitialState(): void {
const urlPath = `${WINDOW.location.pathname}${WINDOW.location.hash}${WINDOW.location.search}`;
const url = `${WINDOW.location.origin}${urlPath}`;

this.performanceEvents = [];

// Reset _context as well
this._clearContext();

this._context.initialUrl = url;
this._context.initialTimestamp = new Date().getTime();
this._context.urls.push(url);
}

/**
* Adds listeners to record events for the replay
*/
Expand Down Expand Up @@ -533,72 +533,6 @@ export class ReplayContainer implements ReplayContainerInterface {
}
}

/**
* Handler for recording events.
*
* Adds to event buffer, and has varying flushing behaviors if the event was a checkout.
*/
private _handleRecordingEmit: (event: RecordingEvent, isCheckout?: boolean) => void = (
event: RecordingEvent,
isCheckout?: boolean,
) => {
// If this is false, it means session is expired, create and a new session and wait for checkout
if (!this.checkAndHandleExpiredSession()) {
__DEBUG_BUILD__ && logger.warn('[Replay] Received replay event after session expired.');

return;
}

this.addUpdate(() => {
// The session is always started immediately on pageload/init, but for
// error-only replays, it should reflect the most recent checkout
// when an error occurs. Clear any state that happens before this current
// checkout. This needs to happen before `addEvent()` which updates state
// dependent on this reset.
if (this.recordingMode === 'error' && event.type === 2) {
this._setInitialState();
}

// We need to clear existing events on a checkout, otherwise they are
// incremental event updates and should be appended
void addEvent(this, event, isCheckout);

// Different behavior for full snapshots (type=2), ignore other event types
// See https://github.com/rrweb-io/rrweb/blob/d8f9290ca496712aa1e7d472549480c4e7876594/packages/rrweb/src/types.ts#L16
if (event.type !== 2) {
return false;
}

// If there is a previousSessionId after a full snapshot occurs, then
// the replay session was started due to session expiration. The new session
// is started before triggering a new checkout and contains the id
// of the previous session. Do not immediately flush in this case
// to avoid capturing only the checkout and instead the replay will
// be captured if they perform any follow-up actions.
if (this.session && this.session.previousSessionId) {
return true;
}

// See note above re: session start needs to reflect the most recent
// checkout.
if (this.recordingMode === 'error' && this.session && this._context.earliestEvent) {
this.session.started = this._context.earliestEvent;
this._maybeSaveSession();
}

// Flush immediately so that we do not miss the first segment, otherwise
// it can prevent loading on the UI. This will cause an increase in short
// replays (e.g. opening and closing a tab quickly), but these can be
// filtered on the UI.
if (this.recordingMode === 'session') {
// We want to ensure the worker is ready, as otherwise we'd always send the first event uncompressed
void this.flushImmediate();
}

return true;
});
};

/**
* Handle when visibility of the page content changes. Opening a new tab will
* cause the state to change to hidden because of content of current page will
Expand Down
2 changes: 2 additions & 0 deletions packages/replay/src/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,7 @@ export interface EventBuffer {

/**
* Add an event to the event buffer.
* `isCheckout` is true if this is either the very first event, or an event triggered by `checkoutEveryNms`.
*
* Returns a promise that resolves if the event was successfully added, else rejects.
*/
Expand Down Expand Up @@ -314,6 +315,7 @@ export interface ReplayContainer {
getOptions(): ReplayPluginOptions;
getSessionId(): string | undefined;
checkAndHandleExpiredSession(): boolean | void;
setInitialState(): void;
}

export interface ReplayPerformanceEntry {
Expand Down
3 changes: 2 additions & 1 deletion packages/replay/src/util/addEvent.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ import { logger } from '@sentry/utils';
import type { AddEventResult, RecordingEvent, ReplayContainer } from '../types';

/**
* Add an event to the event buffer
* Add an event to the event buffer.
* `isCheckout` is true if this is either the very first event, or an event triggered by `checkoutEveryNms`.
*/
export async function addEvent(
replay: ReplayContainer,
Expand Down
Loading