Skip to content

Commit 0c6d134

Browse files
mydeamydea
authored
Merge pull request #7318 from getsentry/fn/fix-master
[Gitflow] Merge develop into master
2 parents b40082b + 648ec0a commit 0c6d134

File tree

15 files changed

+73
-62
lines changed

15 files changed

+73
-62
lines changed

‎packages/integration-tests/suites/replay/errors/errorMode/test.ts

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { expect } from '@playwright/test';
22

33
import { sentryTest } from '../../../../utils/fixtures';
4-
import { envelopeRequestParser } from '../../../../utils/helpers';
4+
import { envelopeRequestParser, waitForErrorRequest } from '../../../../utils/helpers';
55
import {
66
expectedClickBreadcrumb,
77
expectedConsoleBreadcrumb,
@@ -10,6 +10,7 @@ import {
1010
import {
1111
getReplayEvent,
1212
getReplayRecordingContent,
13+
isReplayEvent,
1314
shouldSkipReplayTest,
1415
waitForReplayRequest,
1516
} from '../../../../utils/replayHelpers';
@@ -26,14 +27,18 @@ sentryTest(
2627
const reqPromise0 = waitForReplayRequest(page, 0);
2728
const reqPromise1 = waitForReplayRequest(page, 1);
2829
const reqPromise2 = waitForReplayRequest(page, 2);
30+
const reqErrorPromise = waitForErrorRequest(page);
2931

3032
await page.route('https://dsn.ingest.sentry.io/**/*', route => {
3133
const event = envelopeRequestParser(route.request());
3234
// error events have no type field
3335
if (event && !event.type && event.event_id) {
3436
errorEventId = event.event_id;
3537
}
36-
callsToSentry++;
38+
// We only want to count errors & replays here
39+
if (event && (!event.type || isReplayEvent(event))) {
40+
callsToSentry++;
41+
}
3742

3843
return route.fulfill({
3944
status: 200,
@@ -46,13 +51,16 @@ sentryTest(
4651

4752
await page.goto(url);
4853
await page.click('#go-background');
54+
await new Promise(resolve => setTimeout(resolve, 1000));
55+
4956
expect(callsToSentry).toEqual(0);
5057

5158
await page.click('#error');
5259
const req0 = await reqPromise0;
5360

5461
await page.click('#go-background');
5562
const req1 = await reqPromise1;
63+
await reqErrorPromise;
5664

5765
expect(callsToSentry).toEqual(3); // 1 error, 2 replay events
5866

@@ -69,11 +77,12 @@ sentryTest(
6977
const event2 = getReplayEvent(req2);
7078
const content2 = getReplayRecordingContent(req2);
7179

80+
expect(callsToSentry).toBe(4); // 1 error, 3 replay events
81+
7282
expect(event0).toEqual(
7383
getExpectedReplayEvent({
7484
contexts: { replay: { error_sample_rate: 1, session_sample_rate: 0 } },
75-
// @ts-ignore this is fine
76-
error_ids: [errorEventId],
85+
error_ids: [errorEventId!],
7786
replay_type: 'error',
7887
}),
7988
);
@@ -97,7 +106,6 @@ sentryTest(
97106
expect(event1).toEqual(
98107
getExpectedReplayEvent({
99108
contexts: { replay: { error_sample_rate: 1, session_sample_rate: 0 } },
100-
// @ts-ignore this is fine
101109
replay_type: 'error', // although we're in session mode, we still send 'error' as replay_type
102110
replay_start_timestamp: undefined,
103111
segment_id: 1,
@@ -108,14 +116,12 @@ sentryTest(
108116
// Also the second snapshot should have a full snapshot, as we switched from error to session
109117
// mode which triggers another checkout
110118
expect(content1.fullSnapshots).toHaveLength(1);
111-
expect(content1.incrementalSnapshots).toHaveLength(0);
112119

113120
// The next event should just be a normal replay event as we're now in session mode and
114121
// we continue recording everything
115122
expect(event2).toEqual(
116123
getExpectedReplayEvent({
117124
contexts: { replay: { error_sample_rate: 1, session_sample_rate: 0 } },
118-
// @ts-ignore this is fine
119125
replay_type: 'error',
120126
replay_start_timestamp: undefined,
121127
segment_id: 2,

‎packages/integration-tests/suites/replay/errors/errorsInSession/test.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@ sentryTest(
3939
const url = await getLocalTestPath({ testDir: __dirname });
4040

4141
await page.goto(url);
42-
await page.click('#go-background');
4342
const req0 = await reqPromise0;
4443

4544
await page.click('#error');
@@ -57,7 +56,6 @@ sentryTest(
5756
getExpectedReplayEvent({
5857
replay_start_timestamp: undefined,
5958
segment_id: 1,
60-
// @ts-ignore this is fine
6159
error_ids: [errorEventId],
6260
urls: [],
6361
}),

‎packages/integration-tests/suites/replay/errors/init.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@ import * as Sentry from '@sentry/browser';
22

33
window.Sentry = Sentry;
44
window.Replay = new Sentry.Replay({
5-
flushMinDelay: 500,
6-
flushMaxDelay: 500,
5+
flushMinDelay: 1000,
6+
flushMaxDelay: 1000,
77
});
88

99
Sentry.init({

‎packages/integration-tests/utils/helpers.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,23 @@ async function getSentryEvents(page: Page, url?: string): Promise<Array<Event>>
108108
return eventsHandle.jsonValue();
109109
}
110110

111+
export function waitForErrorRequest(page: Page): Promise<Request> {
112+
return page.waitForRequest(req => {
113+
const postData = req.postData();
114+
if (!postData) {
115+
return false;
116+
}
117+
118+
try {
119+
const event = envelopeRequestParser(req);
120+
121+
return !event.type;
122+
} catch {
123+
return false;
124+
}
125+
});
126+
}
127+
111128
/**
112129
* Waits until a number of requests matching urlRgx at the given URL arrive.
113130
* If the timout option is configured, this function will abort waiting, even if it hasn't reveived the configured

‎packages/integration-tests/utils/replayHelpers.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ export function waitForReplayRequest(page: Page, segmentId?: number): Promise<Re
6464
});
6565
}
6666

67-
function isReplayEvent(event: Event): event is ReplayEvent {
67+
export function isReplayEvent(event: Event): event is ReplayEvent {
6868
return event.type === 'replay_event';
6969
}
7070

‎packages/replay/src/constants.ts

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,6 @@ export const UNABLE_TO_SEND_REPLAY = 'Unable to send Replay';
1414
// The idle limit for a session
1515
export const SESSION_IDLE_DURATION = 300_000; // 5 minutes in ms
1616

17-
// Grace period to keep a session when a user changes tabs or hides window
18-
export const VISIBILITY_CHANGE_TIMEOUT = SESSION_IDLE_DURATION;
19-
2017
// The maximum length of a session
2118
export const MAX_SESSION_LIFE = 3_600_000; // 60 minutes
2219

‎packages/replay/src/replay.ts

Lines changed: 8 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,7 @@ import { captureException, getCurrentHub } from '@sentry/core';
44
import type { Breadcrumb, ReplayRecordingMode } from '@sentry/types';
55
import { logger } from '@sentry/utils';
66

7-
import {
8-
ERROR_CHECKOUT_TIME,
9-
MAX_SESSION_LIFE,
10-
SESSION_IDLE_DURATION,
11-
VISIBILITY_CHANGE_TIMEOUT,
12-
WINDOW,
13-
} from './constants';
7+
import { ERROR_CHECKOUT_TIME, MAX_SESSION_LIFE, SESSION_IDLE_DURATION, WINDOW } from './constants';
148
import { setupPerformanceObserver } from './coreHandlers/performanceObserver';
159
import { createEventBuffer } from './eventBuffer';
1610
import { getSession } from './session/getSession';
@@ -367,7 +361,7 @@ export class ReplayContainer implements ReplayContainerInterface {
367361
* Returns true if session is not expired, false otherwise.
368362
* @hidden
369363
*/
370-
public checkAndHandleExpiredSession(expiry?: number): boolean | void {
364+
public checkAndHandleExpiredSession(): boolean | void {
371365
const oldSessionId = this.getSessionId();
372366

373367
// Prevent starting a new session if the last user activity is older than
@@ -382,7 +376,7 @@ export class ReplayContainer implements ReplayContainerInterface {
382376

383377
// --- There is recent user activity --- //
384378
// This will create a new session if expired, based on expiry length
385-
if (!this._loadAndCheckSession(expiry)) {
379+
if (!this._loadAndCheckSession()) {
386380
return;
387381
}
388382

@@ -412,9 +406,9 @@ export class ReplayContainer implements ReplayContainerInterface {
412406
* Loads (or refreshes) the current session.
413407
* Returns false if session is not recorded.
414408
*/
415-
private _loadAndCheckSession(expiry = SESSION_IDLE_DURATION): boolean {
409+
private _loadAndCheckSession(): boolean {
416410
const { type, session } = getSession({
417-
expiry,
411+
expiry: SESSION_IDLE_DURATION,
418412
stickySession: Boolean(this._options.stickySession),
419413
currentSession: this.session,
420414
sessionSampleRate: this._options.sessionSampleRate,
@@ -626,7 +620,7 @@ export class ReplayContainer implements ReplayContainerInterface {
626620
return;
627621
}
628622

629-
const expired = isSessionExpired(this.session, VISIBILITY_CHANGE_TIMEOUT);
623+
const expired = isSessionExpired(this.session, SESSION_IDLE_DURATION);
630624

631625
if (breadcrumb && !expired) {
632626
this._createCustomBreadcrumb(breadcrumb);
@@ -646,10 +640,10 @@ export class ReplayContainer implements ReplayContainerInterface {
646640
return;
647641
}
648642

649-
const isSessionActive = this.checkAndHandleExpiredSession(VISIBILITY_CHANGE_TIMEOUT);
643+
const isSessionActive = this.checkAndHandleExpiredSession();
650644

651645
if (!isSessionActive) {
652-
// If the user has come back to the page within VISIBILITY_CHANGE_TIMEOUT
646+
// If the user has come back to the page within SESSION_IDLE_DURATION
653647
// ms, we will re-use the existing session, otherwise create a new
654648
// session
655649
__DEBUG_BUILD__ && logger.log('[Replay] Document has become active, but session has expired');

‎packages/replay/test/integration/errorSampleRate.test.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import {
55
ERROR_CHECKOUT_TIME,
66
MAX_SESSION_LIFE,
77
REPLAY_SESSION_KEY,
8-
VISIBILITY_CHANGE_TIMEOUT,
8+
SESSION_IDLE_DURATION,
99
WINDOW,
1010
} from '../../src/constants';
1111
import type { ReplayContainer } from '../../src/replay';
@@ -154,15 +154,15 @@ describe('Integration | errorSampleRate', () => {
154154
});
155155
});
156156

157-
it('does not send a replay when triggering a full dom snapshot when document becomes visible after [VISIBILITY_CHANGE_TIMEOUT]ms', async () => {
157+
it('does not send a replay when triggering a full dom snapshot when document becomes visible after [SESSION_IDLE_DURATION]ms', async () => {
158158
Object.defineProperty(document, 'visibilityState', {
159159
configurable: true,
160160
get: function () {
161161
return 'visible';
162162
},
163163
});
164164

165-
jest.advanceTimersByTime(VISIBILITY_CHANGE_TIMEOUT + 1);
165+
jest.advanceTimersByTime(SESSION_IDLE_DURATION + 1);
166166

167167
document.dispatchEvent(new Event('visibilitychange'));
168168

@@ -186,8 +186,8 @@ describe('Integration | errorSampleRate', () => {
186186

187187
expect(replay).not.toHaveLastSentReplay();
188188

189-
// User comes back before `VISIBILITY_CHANGE_TIMEOUT` elapses
190-
jest.advanceTimersByTime(VISIBILITY_CHANGE_TIMEOUT - 100);
189+
// User comes back before `SESSION_IDLE_DURATION` elapses
190+
jest.advanceTimersByTime(SESSION_IDLE_DURATION - 100);
191191
Object.defineProperty(document, 'visibilityState', {
192192
configurable: true,
193193
get: function () {

‎packages/replay/test/integration/events.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ describe('Integration | events', () => {
4040
// Create a new session and clear mocks because a segment (from initial
4141
// checkout) will have already been uploaded by the time the tests run
4242
clearSession(replay);
43-
replay['_loadAndCheckSession'](0);
43+
replay['_loadAndCheckSession']();
4444
mockTransportSend.mockClear();
4545
});
4646

@@ -93,7 +93,7 @@ describe('Integration | events', () => {
9393

9494
it('has correct timestamps when there are events earlier than initial timestamp', async function () {
9595
clearSession(replay);
96-
replay['_loadAndCheckSession'](0);
96+
replay['_loadAndCheckSession']();
9797
mockTransportSend.mockClear();
9898
Object.defineProperty(document, 'visibilityState', {
9999
configurable: true,

‎packages/replay/test/integration/flush.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import * as SentryUtils from '@sentry/utils';
22

3-
import { DEFAULT_FLUSH_MIN_DELAY, SESSION_IDLE_DURATION, WINDOW } from '../../src/constants';
3+
import { DEFAULT_FLUSH_MIN_DELAY, WINDOW } from '../../src/constants';
44
import type { ReplayContainer } from '../../src/replay';
55
import type { EventBuffer } from '../../src/types';
66
import * as AddMemoryEntry from '../../src/util/addMemoryEntry';
@@ -95,7 +95,7 @@ describe('Integration | flush', () => {
9595
jest.setSystemTime(new Date(BASE_TIMESTAMP));
9696
sessionStorage.clear();
9797
clearSession(replay);
98-
replay['_loadAndCheckSession'](SESSION_IDLE_DURATION);
98+
replay['_loadAndCheckSession']();
9999
mockRecord.takeFullSnapshot.mockClear();
100100
Object.defineProperty(WINDOW, 'location', {
101101
value: prevLocation,

‎packages/replay/test/integration/rateLimiting.test.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { getCurrentHub } from '@sentry/core';
22
import type { Transport } from '@sentry/types';
33

4-
import { DEFAULT_FLUSH_MIN_DELAY, SESSION_IDLE_DURATION } from '../../src/constants';
4+
import { DEFAULT_FLUSH_MIN_DELAY } from '../../src/constants';
55
import type { ReplayContainer } from '../../src/replay';
66
import * as SendReplayRequest from '../../src/util/sendReplayRequest';
77
import { BASE_TIMESTAMP, mockSdk } from '../index';
@@ -46,7 +46,7 @@ describe('Integration | rate-limiting behaviour', () => {
4646
// Create a new session and clear mocks because a segment (from initial
4747
// checkout) will have already been uploaded by the time the tests run
4848
clearSession(replay);
49-
replay['_loadAndCheckSession'](0);
49+
replay['_loadAndCheckSession']();
5050

5151
mockSendReplayRequest.mockClear();
5252
});
@@ -57,7 +57,7 @@ describe('Integration | rate-limiting behaviour', () => {
5757
jest.setSystemTime(new Date(BASE_TIMESTAMP));
5858
clearSession(replay);
5959
jest.clearAllMocks();
60-
replay['_loadAndCheckSession'](SESSION_IDLE_DURATION);
60+
replay['_loadAndCheckSession']();
6161
});
6262

6363
afterAll(() => {

‎packages/replay/test/integration/sendReplayEvent.test.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import * as SentryCore from '@sentry/core';
22
import type { Transport } from '@sentry/types';
33
import * as SentryUtils from '@sentry/utils';
44

5-
import { DEFAULT_FLUSH_MIN_DELAY, SESSION_IDLE_DURATION, WINDOW } from '../../src/constants';
5+
import { DEFAULT_FLUSH_MIN_DELAY, WINDOW } from '../../src/constants';
66
import type { ReplayContainer } from '../../src/replay';
77
import { addEvent } from '../../src/util/addEvent';
88
import * as SendReplayRequest from '../../src/util/sendReplayRequest';
@@ -59,7 +59,7 @@ describe('Integration | sendReplayEvent', () => {
5959
// Create a new session and clear mocks because a segment (from initial
6060
// checkout) will have already been uploaded by the time the tests run
6161
clearSession(replay);
62-
replay['_loadAndCheckSession'](0);
62+
replay['_loadAndCheckSession']();
6363

6464
mockSendReplayRequest.mockClear();
6565
});
@@ -69,7 +69,7 @@ describe('Integration | sendReplayEvent', () => {
6969
await new Promise(process.nextTick);
7070
jest.setSystemTime(new Date(BASE_TIMESTAMP));
7171
clearSession(replay);
72-
replay['_loadAndCheckSession'](SESSION_IDLE_DURATION);
72+
replay['_loadAndCheckSession']();
7373
});
7474

7575
afterAll(() => {

0 commit comments

Comments
 (0)