Skip to content

feat(replay): Flush immediately on DOM checkouts #6463

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 4 commits into from
Dec 13, 2022
Merged

feat(replay): Flush immediately on DOM checkouts #6463

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e077607
feat(replay): Flush immediately on DOM checkouts
billyvg Dec 7, 2022
eef0c77
fix tests
billyvg Dec 8, 2022
b9f73de
update removed `waitForError`
billyvg Dec 12, 2022
ac77102
fix tests
billyvg Dec 13, 2022
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
27 changes: 10 additions & 17 deletions packages/replay/src/replay.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,9 +195,10 @@ export class ReplayContainer implements ReplayContainerInterface {

this.addListeners();

this.startRecording();

// Need to set as enabled before we start recording, as `record()` can trigger a flush with a new checkout
this._isEnabled = true;

this.startRecording();
}

/**
Expand Down Expand Up @@ -485,21 +486,13 @@ export class ReplayContainer implements ReplayContainerInterface {
this._maybeSaveSession();
}

// If the full snapshot is due to an initial load, we will not have
// a previous session ID. In this case, we want to buffer events
// for a set amount of time before flushing. This can help avoid
// capturing replays of users that immediately close the window.
setTimeout(() => this.conditionalFlush(), this._options.initialFlushDelay);

// Cancel any previously debounced flushes to ensure there are no [near]
// simultaneous flushes happening. The latter request should be
// insignificant in this case, so wait for additional user interaction to
// trigger a new flush.
//
// This can happen because there's no guarantee that a recording event
// happens first. e.g. a mouse click can happen and trigger a debounced
// flush before the checkout.
this._debouncedFlush?.cancel();
// 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') {
void this.flushImmediate();
}

return true;
});
Expand Down
54 changes: 24 additions & 30 deletions packages/replay/test/unit/index-errorSampleRate.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { captureException, getCurrentHub } from '@sentry/core';
import { captureException } from '@sentry/core';

import { REPLAY_SESSION_KEY, VISIBILITY_CHANGE_TIMEOUT, WINDOW } from '../../src/constants';
import { addEvent } from '../../src/util/addEvent';
Expand All @@ -19,7 +19,6 @@ async function advanceTimers(time: number) {
describe('Replay (errorSampleRate)', () => {
let replay: ReplayContainer;
let mockRecord: RecordMock;

let domHandler: DomHandler;

beforeEach(async () => {
Expand Down Expand Up @@ -59,6 +58,7 @@ describe('Replay (errorSampleRate)', () => {
await new Promise(process.nextTick);

expect(replay).toHaveSentReplay({
recordingPayloadHeader: { segment_id: 0 },
replayEventPayload: expect.objectContaining({
tags: expect.objectContaining({
errorSampleRate: 1,
Expand Down Expand Up @@ -86,6 +86,19 @@ describe('Replay (errorSampleRate)', () => {
]),
});

// This is from when we stop recording and start a session recording
expect(replay).toHaveLastSentReplay({
recordingPayloadHeader: { segment_id: 1 },
replayEventPayload: expect.objectContaining({
tags: expect.objectContaining({
errorSampleRate: 1,
replayType: 'error',
sessionSampleRate: 0,
}),
}),
events: JSON.stringify([{ data: { isCheckout: true }, timestamp: BASE_TIMESTAMP + 5020, type: 2 }]),
});

jest.runAllTimers();
await new Promise(process.nextTick);

Expand All @@ -111,11 +124,11 @@ describe('Replay (errorSampleRate)', () => {
events: JSON.stringify([
{
type: 5,
timestamp: BASE_TIMESTAMP + 15000 + 40,
timestamp: BASE_TIMESTAMP + 10000 + 40,
data: {
tag: 'breadcrumb',
payload: {
timestamp: (BASE_TIMESTAMP + 15000 + 40) / 1000,
timestamp: (BASE_TIMESTAMP + 10000 + 40) / 1000,
type: 'default',
category: 'ui.click',
message: '<unknown>',
Expand Down Expand Up @@ -287,7 +300,7 @@ describe('Replay (errorSampleRate)', () => {
jest.runAllTimers();
await new Promise(process.nextTick);

expect(replay).toHaveLastSentReplay({
expect(replay).toHaveSentReplay({
events: JSON.stringify([{ data: { isCheckout: true }, timestamp: BASE_TIMESTAMP, type: 2 }, TEST_EVENT]),
replayEventPayload: expect.objectContaining({
replay_start_timestamp: BASE_TIMESTAMP / 1000,
Expand Down Expand Up @@ -338,7 +351,8 @@ describe('Replay (errorSampleRate)', () => {
expect(replay.session?.started).toBe(BASE_TIMESTAMP + ELAPSED + 20);

// Does not capture mouse click
expect(replay).toHaveLastSentReplay({
expect(replay).toHaveSentReplay({
recordingPayloadHeader: { segment_id: 0 },
replayEventPayload: expect.objectContaining({
// Make sure the old performance event is thrown out
replay_start_timestamp: (BASE_TIMESTAMP + ELAPSED + 20) / 1000,
Expand Down Expand Up @@ -373,12 +387,6 @@ it('sends a replay after loading the session multiple times', async () => {
},
autoStart: false,
});

const fn = getCurrentHub()?.getClient()?.getTransport()?.send;
const mockTransportSend = fn
? (jest.spyOn(getCurrentHub().getClient()!.getTransport()!, 'send') as jest.MockedFunction<any>)
: jest.fn();

replay.start();

jest.runAllTimers();
Expand All @@ -393,27 +401,13 @@ it('sends a replay after loading the session multiple times', async () => {
jest.runAllTimers();
await new Promise(process.nextTick);

expect(replay).toHaveLastSentReplay({
expect(replay).toHaveSentReplay({
events: JSON.stringify([{ data: { isCheckout: true }, timestamp: BASE_TIMESTAMP, type: 2 }, TEST_EVENT]),
});

mockTransportSend.mockClear();
expect(replay).not.toHaveLastSentReplay();

jest.runAllTimers();
await new Promise(process.nextTick);
jest.runAllTimers();
await new Promise(process.nextTick);

// New checkout when we call `startRecording` again after uploading segment
// after an error occurs
// Latest checkout when we call `startRecording` again after uploading segment
// after an error occurs (e.g. when we switch to session replay recording)
expect(replay).toHaveLastSentReplay({
events: JSON.stringify([
{
data: { isCheckout: true },
timestamp: BASE_TIMESTAMP + 10000 + 20,
type: 2,
},
]),
events: JSON.stringify([{ data: { isCheckout: true }, timestamp: BASE_TIMESTAMP + 5020, type: 2 }]),
});
});