Skip to content

Commit

Permalink
ref(replay): Extract flush min and max delay default values to consta…
Browse files Browse the repository at this point in the history
…nts (#6612)

Extract the default values of flush min and max delays into constants
  • Loading branch information
Lms24 authored Jan 4, 2023
1 parent b674c26 commit b03a32b
Show file tree
Hide file tree
Showing 6 changed files with 49 additions and 36 deletions.
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

0 comments on commit b03a32b

Please sign in to comment.