Skip to content

ref(replay): Extract flush min and max delay default values to constants #6612

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.

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
Jan 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions packages/replay/src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,8 @@ export const DEFAULT_ERROR_SAMPLE_RATE = 1.0;

/** The select to use for the `maskAllText` option */
export const MASK_ALL_TEXT_SELECTOR = 'body *:not(style), body *:not(script)';

/** Default flush delays */
export const DEFAULT_FLUSH_MIN_DELAY = 5_000;
export const DEFAULT_FLUSH_MAX_DELAY = 15_000;
export const INITIAL_FLUSH_DELAY = 5_000;
15 changes: 11 additions & 4 deletions packages/replay/src/integration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,14 @@ import { getCurrentHub } from '@sentry/core';
import type { BrowserClientReplayOptions } from '@sentry/types';
import { Integration } from '@sentry/types';

import { DEFAULT_ERROR_SAMPLE_RATE, DEFAULT_SESSION_SAMPLE_RATE, MASK_ALL_TEXT_SELECTOR } from './constants';
import {
DEFAULT_ERROR_SAMPLE_RATE,
DEFAULT_FLUSH_MAX_DELAY,
DEFAULT_FLUSH_MIN_DELAY,
DEFAULT_SESSION_SAMPLE_RATE,
INITIAL_FLUSH_DELAY,
MASK_ALL_TEXT_SELECTOR,
} from './constants';
import { ReplayContainer } from './replay';
import type { RecordingOptions, ReplayConfiguration, ReplayPluginOptions } from './types';
import { isBrowser } from './util/isBrowser';
Expand Down Expand Up @@ -40,9 +47,9 @@ export class Replay implements Integration {
private _replay?: ReplayContainer;

constructor({
flushMinDelay = 5000,
flushMaxDelay = 15000,
initialFlushDelay = 5000,
flushMinDelay = DEFAULT_FLUSH_MIN_DELAY,
flushMaxDelay = DEFAULT_FLUSH_MAX_DELAY,
initialFlushDelay = INITIAL_FLUSH_DELAY,
stickySession = true,
useCompression = true,
sessionSampleRate,
Expand Down
6 changes: 3 additions & 3 deletions packages/replay/test/unit/flush.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import * as SentryUtils from '@sentry/utils';

import { SESSION_IDLE_DURATION, WINDOW } from '../../src/constants';
import { DEFAULT_FLUSH_MIN_DELAY, SESSION_IDLE_DURATION, WINDOW } from '../../src/constants';
import * as AddMemoryEntry from '../../src/util/addMemoryEntry';
import { createPerformanceSpans } from '../../src/util/createPerformanceSpans';
import { createPerformanceEntries } from './../../src/createPerformanceEntry';
Expand Down Expand Up @@ -145,7 +145,7 @@ it('long first flush enqueues following events', async () => {
domHandler({
name: 'click',
});
await advanceTimers(5000);
await advanceTimers(DEFAULT_FLUSH_MIN_DELAY);
// flush #2 @ t=5s - due to click
expect(replay.flush).toHaveBeenCalledTimes(2);

Expand Down Expand Up @@ -212,7 +212,7 @@ it('long first flush enqueues following events', async () => {
});
// flush #5 @ t=25s - debounced flush calls `flush`
// 20s + `flushMinDelay` which is 5 seconds
await advanceTimers(5000);
await advanceTimers(DEFAULT_FLUSH_MIN_DELAY);
expect(replay.flush).toHaveBeenCalledTimes(5);
expect(replay.runFlush).toHaveBeenCalledTimes(2);
expect(mockSendReplay).toHaveBeenLastCalledWith({
Expand Down
22 changes: 11 additions & 11 deletions packages/replay/test/unit/index-errorSampleRate.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { captureException } from '@sentry/core';

import { REPLAY_SESSION_KEY, VISIBILITY_CHANGE_TIMEOUT, WINDOW } from '../../src/constants';
import { DEFAULT_FLUSH_MIN_DELAY, REPLAY_SESSION_KEY, VISIBILITY_CHANGE_TIMEOUT, WINDOW } from '../../src/constants';
import { addEvent } from '../../src/util/addEvent';
import { ReplayContainer } from './../../src/replay';
import { PerformanceEntryResource } from './../fixtures/performanceEntry/resource';
Expand Down Expand Up @@ -54,7 +54,7 @@ describe('Replay (errorSampleRate)', () => {
expect(replay).not.toHaveLastSentReplay();

captureException(new Error('testing'));
jest.advanceTimersByTime(5000);
jest.advanceTimersByTime(DEFAULT_FLUSH_MIN_DELAY);
await new Promise(process.nextTick);

expect(replay).toHaveSentReplay({
Expand Down Expand Up @@ -99,15 +99,15 @@ describe('Replay (errorSampleRate)', () => {
events: JSON.stringify([{ data: { isCheckout: true }, timestamp: BASE_TIMESTAMP + 5020, type: 2 }]),
});

jest.advanceTimersByTime(5000);
jest.advanceTimersByTime(DEFAULT_FLUSH_MIN_DELAY);

// New checkout when we call `startRecording` again after uploading segment
// after an error occurs
expect(replay).toHaveLastSentReplay({
events: JSON.stringify([
{
data: { isCheckout: true },
timestamp: BASE_TIMESTAMP + 5000 + 20,
timestamp: BASE_TIMESTAMP + DEFAULT_FLUSH_MIN_DELAY + 20,
type: 2,
},
]),
Expand All @@ -118,7 +118,7 @@ describe('Replay (errorSampleRate)', () => {
name: 'click',
});

jest.advanceTimersByTime(5000);
jest.advanceTimersByTime(DEFAULT_FLUSH_MIN_DELAY);
await new Promise(process.nextTick);

expect(replay).toHaveLastSentReplay({
Expand Down Expand Up @@ -245,12 +245,12 @@ describe('Replay (errorSampleRate)', () => {
expect(replay).not.toHaveLastSentReplay();

// There should also not be another attempt at an upload 5 seconds after the last replay event
await advanceTimers(5000);
await advanceTimers(DEFAULT_FLUSH_MIN_DELAY);
expect(replay).not.toHaveLastSentReplay();

// Let's make sure it continues to work
mockRecord._emitter(TEST_EVENT);
await advanceTimers(5000);
await advanceTimers(DEFAULT_FLUSH_MIN_DELAY);
jest.runAllTimers();
await new Promise(process.nextTick);
expect(replay).not.toHaveLastSentReplay();
Expand Down Expand Up @@ -294,11 +294,11 @@ describe('Replay (errorSampleRate)', () => {
jest.runAllTimers();
await new Promise(process.nextTick);

jest.advanceTimersByTime(5000);
jest.advanceTimersByTime(DEFAULT_FLUSH_MIN_DELAY);

captureException(new Error('testing'));

jest.advanceTimersByTime(5000);
jest.advanceTimersByTime(DEFAULT_FLUSH_MIN_DELAY);
await new Promise(process.nextTick);

expect(replay).toHaveSentReplay({
Expand All @@ -309,7 +309,7 @@ describe('Replay (errorSampleRate)', () => {
// (advance timers + waiting for flush after the checkout) and
// extra time is likely due to async of `addMemoryEntry()`

timestamp: (BASE_TIMESTAMP + 5000 + 5000 + 20) / 1000,
timestamp: (BASE_TIMESTAMP + DEFAULT_FLUSH_MIN_DELAY + DEFAULT_FLUSH_MIN_DELAY + 20) / 1000,
error_ids: [expect.any(String)],
trace_ids: [],
urls: ['http://localhost/'],
Expand Down Expand Up @@ -400,7 +400,7 @@ it('sends a replay after loading the session multiple times', async () => {

captureException(new Error('testing'));

jest.advanceTimersByTime(5000);
jest.advanceTimersByTime(DEFAULT_FLUSH_MIN_DELAY);
await new Promise(process.nextTick);

expect(replay).toHaveSentReplay({
Expand Down
8 changes: 4 additions & 4 deletions packages/replay/test/unit/index-noSticky.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { getCurrentHub } from '@sentry/core';
import { Transport } from '@sentry/types';
import * as SentryUtils from '@sentry/utils';

import { SESSION_IDLE_DURATION, VISIBILITY_CHANGE_TIMEOUT } from '../../src/constants';
import { DEFAULT_FLUSH_MIN_DELAY, SESSION_IDLE_DURATION, VISIBILITY_CHANGE_TIMEOUT } from '../../src/constants';
import { addEvent } from '../../src/util/addEvent';
import { ReplayContainer } from './../../src/replay';
import { BASE_TIMESTAMP, mockRrweb, mockSdk } from './../index';
Expand Down Expand Up @@ -197,7 +197,7 @@ describe('Replay (no sticky)', () => {

// There should also not be another attempt at an upload 5 seconds after the last replay event
mockTransport.mockClear();
await advanceTimers(5000);
await advanceTimers(DEFAULT_FLUSH_MIN_DELAY);
expect(replay).not.toHaveLastSentReplay();

expect(replay.session?.lastActivity).toBe(BASE_TIMESTAMP);
Expand All @@ -208,7 +208,7 @@ describe('Replay (no sticky)', () => {
// Let's make sure it continues to work
mockTransport.mockClear();
mockRecord._emitter(TEST_EVENT);
await advanceTimers(5000);
await advanceTimers(DEFAULT_FLUSH_MIN_DELAY);
expect(replay).toHaveLastSentReplay({ events: JSON.stringify([TEST_EVENT]) });
});

Expand Down Expand Up @@ -252,7 +252,7 @@ describe('Replay (no sticky)', () => {
name: 'click',
});

await advanceTimers(5000);
await advanceTimers(DEFAULT_FLUSH_MIN_DELAY);

const newTimestamp = BASE_TIMESTAMP + FIFTEEN_MINUTES;
const breadcrumbTimestamp = newTimestamp + 20; // I don't know where this 20ms comes from
Expand Down
29 changes: 15 additions & 14 deletions packages/replay/test/unit/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { Event, Scope } from '@sentry/types';
import { EventType } from 'rrweb';

import {
DEFAULT_FLUSH_MIN_DELAY,
MASK_ALL_TEXT_SELECTOR,
MAX_SESSION_LIFE,
REPLAY_SESSION_KEY,
Expand Down Expand Up @@ -336,7 +337,7 @@ describe('Replay', () => {

// There should also not be another attempt at an upload 5 seconds after the last replay event
mockTransportSend.mockClear();
await advanceTimers(5000);
await advanceTimers(DEFAULT_FLUSH_MIN_DELAY);

expect(replay).not.toHaveLastSentReplay();

Expand All @@ -348,7 +349,7 @@ describe('Replay', () => {
// Let's make sure it continues to work
mockTransportSend.mockClear();
mockRecord._emitter(TEST_EVENT);
await advanceTimers(5000);
await advanceTimers(DEFAULT_FLUSH_MIN_DELAY);
expect(replay).toHaveLastSentReplay({ events: JSON.stringify([TEST_EVENT]) });
});

Expand Down Expand Up @@ -402,7 +403,7 @@ describe('Replay', () => {
name: 'click',
});

await advanceTimers(5000);
await advanceTimers(DEFAULT_FLUSH_MIN_DELAY);

const newTimestamp = BASE_TIMESTAMP + FIFTEEN_MINUTES;
const breadcrumbTimestamp = newTimestamp + 20; // I don't know where this 20ms comes from
Expand Down Expand Up @@ -479,7 +480,7 @@ describe('Replay', () => {
});

WINDOW.dispatchEvent(new Event('blur'));
await advanceTimers(5000);
await advanceTimers(DEFAULT_FLUSH_MIN_DELAY);

expect(mockRecord.takeFullSnapshot).not.toHaveBeenCalled();
expect(replay).not.toHaveLastSentReplay();
Expand All @@ -498,7 +499,7 @@ describe('Replay', () => {

const NEW_TEST_EVENT = {
data: { name: 'test' },
timestamp: BASE_TIMESTAMP + MAX_SESSION_LIFE + 5000 + 20,
timestamp: BASE_TIMESTAMP + MAX_SESSION_LIFE + DEFAULT_FLUSH_MIN_DELAY + 20,
type: 3,
};

Expand All @@ -509,9 +510,9 @@ describe('Replay', () => {
await new Promise(process.nextTick);

expect(replay).not.toHaveSameSession(initialSession);
await advanceTimers(5000);
await advanceTimers(DEFAULT_FLUSH_MIN_DELAY);

const newTimestamp = BASE_TIMESTAMP + MAX_SESSION_LIFE + 5000 + 20; // I don't know where this 20ms comes from
const newTimestamp = BASE_TIMESTAMP + MAX_SESSION_LIFE + DEFAULT_FLUSH_MIN_DELAY + 20; // I don't know where this 20ms comes from
const breadcrumbTimestamp = newTimestamp;

jest.runAllTimers();
Expand Down Expand Up @@ -591,13 +592,13 @@ describe('Replay', () => {
throw new Error('Something bad happened');
});
mockRecord._emitter(TEST_EVENT);
await advanceTimers(5000);
await advanceTimers(DEFAULT_FLUSH_MIN_DELAY);

expect(mockRecord.takeFullSnapshot).not.toHaveBeenCalled();
mockTransportSend.mockImplementationOnce(() => {
throw new Error('Something bad happened');
});
await advanceTimers(5000);
await advanceTimers(DEFAULT_FLUSH_MIN_DELAY);

// next tick should retry and succeed
mockConsole.mockRestore();
Expand Down Expand Up @@ -625,7 +626,7 @@ describe('Replay', () => {
expect(replay.session?.segmentId).toBe(1);

// next tick should do nothing
await advanceTimers(5000);
await advanceTimers(DEFAULT_FLUSH_MIN_DELAY);
expect(replay).not.toHaveLastSentReplay();
});

Expand All @@ -648,12 +649,12 @@ describe('Replay', () => {
});
mockRecord._emitter(TEST_EVENT);

await advanceTimers(5000);
await advanceTimers(DEFAULT_FLUSH_MIN_DELAY);

expect(mockRecord.takeFullSnapshot).not.toHaveBeenCalled();
expect(replay.sendReplayRequest).toHaveBeenCalledTimes(1);

await advanceTimers(5000);
await advanceTimers(DEFAULT_FLUSH_MIN_DELAY);
expect(replay.sendReplayRequest).toHaveBeenCalledTimes(2);

await advanceTimers(10000);
Expand Down Expand Up @@ -865,11 +866,11 @@ describe('Replay', () => {
const TEST_EVENT = { data: {}, timestamp: BASE_TIMESTAMP, type: 2 };
mockRecord._emitter(TEST_EVENT);

await advanceTimers(5000);
await advanceTimers(DEFAULT_FLUSH_MIN_DELAY);
expect(replay.flush).toHaveBeenCalledTimes(1);

// Make sure there's nothing queued up after
await advanceTimers(5000);
await advanceTimers(DEFAULT_FLUSH_MIN_DELAY);
expect(replay.flush).toHaveBeenCalledTimes(1);
});
});
Expand Down