Skip to content

Commit caec2f7

Browse files
mydeamydea
committed
ref: Bring back rate limiting behavior
1 parent dff1a44 commit caec2f7

File tree

4 files changed

+60
-41
lines changed

4 files changed

+60
-41
lines changed

‎packages/replay/src/replay.ts

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
/* eslint-disable max-lines */ // TODO: We might want to split this file up
22
import { addGlobalEventProcessor, captureException, getCurrentHub } from '@sentry/core';
33
import type { Breadcrumb, ReplayRecordingMode } from '@sentry/types';
4-
import { addInstrumentationHandler, logger } from '@sentry/utils';
4+
import type { RateLimits } from '@sentry/utils';
5+
import { addInstrumentationHandler, disabledUntil, logger } from '@sentry/utils';
56
import { EventType, record } from 'rrweb';
67

78
import { MAX_SESSION_LIFE, SESSION_IDLE_DURATION, VISIBILITY_CHANGE_TIMEOUT, WINDOW } from './constants';
@@ -38,6 +39,7 @@ import { isExpired } from './util/isExpired';
3839
import { isSessionExpired } from './util/isSessionExpired';
3940
import { overwriteRecordDroppedEvent, restoreRecordDroppedEvent } from './util/monkeyPatchRecordDroppedEvent';
4041
import { sendReplay } from './util/sendReplay';
42+
import { RateLimitError } from './util/sendReplayRequest';
4143

4244
/**
4345
* The main replay container class, which holds all the state and methods for recording and sending replays.
@@ -108,11 +110,6 @@ export class ReplayContainer implements ReplayContainerInterface {
108110
initialUrl: '',
109111
};
110112

111-
/**
112-
* A RateLimits object holding the rate-limit durations in case a sent replay event was rate-limited.
113-
*/
114-
private _rateLimits: RateLimits = {};
115-
116113
public constructor({
117114
options,
118115
recordingOptions,
@@ -827,6 +824,9 @@ export class ReplayContainer implements ReplayContainerInterface {
827824
timestamp: new Date().getTime(),
828825
});
829826
} catch (err) {
827+
if (err instanceof RateLimitError) {
828+
this._handleRateLimit(err.rateLimits);
829+
}
830830
this._handleException(err);
831831
}
832832
}
@@ -889,14 +889,14 @@ export class ReplayContainer implements ReplayContainerInterface {
889889
/**
890890
* Pauses the replay and resumes it after the rate-limit duration is over.
891891
*/
892-
private _handleRateLimit(): void {
892+
private _handleRateLimit(rateLimits: RateLimits): void {
893893
// in case recording is already paused, we don't need to do anything, as we might have already paused because of a
894894
// rate limit
895895
if (this.isPaused()) {
896896
return;
897897
}
898898

899-
const rateLimitEnd = disabledUntil(this._rateLimits, 'replay');
899+
const rateLimitEnd = disabledUntil(rateLimits, 'replay');
900900
const rateLimitDuration = rateLimitEnd - Date.now();
901901

902902
if (rateLimitDuration > 0) {

‎packages/replay/src/util/sendReplay.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { captureException, setContext } from '@sentry/core';
22

33
import { RETRY_BASE_INTERVAL, RETRY_MAX_COUNT, UNABLE_TO_SEND_REPLAY } from '../constants';
44
import type { SendReplayData } from '../types';
5-
import { sendReplayRequest } from './sendReplayRequest';
5+
import { RateLimitError, sendReplayRequest } from './sendReplayRequest';
66

77
/**
88
* Finalize and send the current replay event to Sentry
@@ -25,6 +25,10 @@ export async function sendReplay(
2525
await sendReplayRequest(replayData);
2626
return true;
2727
} catch (err) {
28+
if (err instanceof RateLimitError) {
29+
throw err;
30+
}
31+
2832
// Capture error for every failed replay
2933
setContext('Replays', {
3034
_retryCount: retryConfig.count,

‎packages/replay/src/util/sendReplayRequest.ts

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { getCurrentHub } from '@sentry/core';
22
import type { ReplayEvent, TransportMakeRequestResponse } from '@sentry/types';
3-
import { logger } from '@sentry/utils';
3+
import type { RateLimits } from '@sentry/utils';
4+
import { isRateLimited, logger, updateRateLimits } from '@sentry/utils';
45

56
import { REPLAY_EVENT_NAME, UNABLE_TO_SEND_REPLAY } from '../constants';
67
import type { SendReplayData } from '../types';
@@ -106,9 +107,32 @@ export async function sendReplayRequest({
106107

107108
const envelope = createReplayEnvelope(replayEvent, preparedRecordingData, dsn, client.getOptions().tunnel);
108109

110+
let response: void | TransportMakeRequestResponse;
111+
109112
try {
110-
return await transport.send(envelope);
113+
response = await transport.send(envelope);
111114
} catch {
112115
throw new Error(UNABLE_TO_SEND_REPLAY);
113116
}
117+
118+
// TODO (v8): we can remove this guard once transport.send's type signature doesn't include void anymore
119+
if (response) {
120+
const rateLimits = updateRateLimits({}, response);
121+
if (isRateLimited(rateLimits, 'replay')) {
122+
throw new RateLimitError(rateLimits);
123+
}
124+
}
125+
return response;
126+
}
127+
128+
/**
129+
* This error indicates that we hit a rate limit API error.
130+
*/
131+
export class RateLimitError extends Error {
132+
public rateLimits: RateLimits;
133+
134+
public constructor(rateLimits: RateLimits) {
135+
super('Rate limit hit');
136+
this.rateLimits = rateLimits;
137+
}
114138
}

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

Lines changed: 21 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import type { Transport, TransportMakeRequestResponse } from '@sentry/types';
33

44
import { DEFAULT_FLUSH_MIN_DELAY, SESSION_IDLE_DURATION } from '../../src/constants';
55
import type { ReplayContainer } from '../../src/replay';
6+
import * as SendReplayRequest from '../../src/util/sendReplayRequest';
67
import { BASE_TIMESTAMP, mockSdk } from '../index';
78
import { mockRrweb } from '../mocks/mockRrweb';
89
import { clearSession } from '../utils/clearSession';
@@ -16,12 +17,11 @@ async function advanceTimers(time: number) {
1617
}
1718

1819
type MockTransportSend = jest.MockedFunction<Transport['send']>;
19-
type MockSendReplayRequest = jest.MockedFunction<ReplayContainer['_sendReplayRequest']>;
2020

2121
describe('Integration | rate-limiting behaviour', () => {
2222
let replay: ReplayContainer;
2323
let mockTransportSend: MockTransportSend;
24-
let mockSendReplayRequest: MockSendReplayRequest;
24+
let mockSendReplayRequest: jest.MockedFunction<any>;
2525
const { record: mockRecord } = mockRrweb();
2626

2727
beforeAll(async () => {
@@ -33,12 +33,9 @@ describe('Integration | rate-limiting behaviour', () => {
3333
},
3434
}));
3535

36-
// @ts-ignore private API
37-
jest.spyOn(replay, '_sendReplayRequest');
38-
3936
jest.runAllTimers();
4037
mockTransportSend = getCurrentHub()?.getClient()?.getTransport()?.send as MockTransportSend;
41-
mockSendReplayRequest = replay['_sendReplayRequest'] as MockSendReplayRequest;
38+
mockSendReplayRequest = jest.spyOn(SendReplayRequest, 'sendReplayRequest');
4239
});
4340

4441
beforeEach(() => {
@@ -52,8 +49,6 @@ describe('Integration | rate-limiting behaviour', () => {
5249
replay['_loadSession']({ expiry: 0 });
5350

5451
mockSendReplayRequest.mockClear();
55-
56-
replay['_rateLimits'] = {};
5752
});
5853

5954
afterEach(async () => {
@@ -92,15 +87,13 @@ describe('Integration | rate-limiting behaviour', () => {
9287
},
9388
},
9489
] as TransportMakeRequestResponse[])(
95-
'pauses recording and flushing a rate limit is hit and resumes both after the rate limit duration is over',
90+
'pauses recording and flushing a rate limit is hit and resumes both after the rate limit duration is over %j',
9691
async rateLimitResponse => {
9792
expect(replay.session?.segmentId).toBe(0);
9893
jest.spyOn(replay, 'pause');
9994
jest.spyOn(replay, 'resume');
10095
// @ts-ignore private API
10196
jest.spyOn(replay, '_handleRateLimit');
102-
// @ts-ignore private API
103-
jest.spyOn(replay, '_sendReplay');
10497

10598
const TEST_EVENT = { data: {}, timestamp: BASE_TIMESTAMP, type: 2 };
10699

@@ -115,7 +108,7 @@ describe('Integration | rate-limiting behaviour', () => {
115108

116109
expect(mockRecord.takeFullSnapshot).not.toHaveBeenCalled();
117110
expect(mockTransportSend).toHaveBeenCalledTimes(1);
118-
expect(replay).toHaveLastSentReplay({ events: JSON.stringify([TEST_EVENT]) });
111+
expect(replay).toHaveLastSentReplay({ recordingData: JSON.stringify([TEST_EVENT]) });
119112

120113
expect(replay['_handleRateLimit']).toHaveBeenCalledTimes(1);
121114
// resume() was called once before we even started
@@ -136,7 +129,7 @@ describe('Integration | rate-limiting behaviour', () => {
136129
mockRecord._emitter(ev);
137130
await advanceTimers(DEFAULT_FLUSH_MIN_DELAY);
138131
expect(replay.isPaused()).toBe(true);
139-
expect(replay['_sendReplay']).toHaveBeenCalledTimes(1);
132+
expect(mockSendReplayRequest).toHaveBeenCalledTimes(1);
140133
expect(mockTransportSend).toHaveBeenCalledTimes(1);
141134
}
142135

@@ -148,9 +141,9 @@ describe('Integration | rate-limiting behaviour', () => {
148141
expect(replay.resume).toHaveBeenCalledTimes(1);
149142
expect(replay.isPaused()).toBe(false);
150143

151-
expect(replay['_sendReplay']).toHaveBeenCalledTimes(2);
144+
expect(mockSendReplayRequest).toHaveBeenCalledTimes(2);
152145
expect(replay).toHaveLastSentReplay({
153-
events: JSON.stringify([
146+
recordingData: JSON.stringify([
154147
{ data: { isCheckout: true }, timestamp: BASE_TIMESTAMP + DEFAULT_FLUSH_MIN_DELAY * 7, type: 2 },
155148
]),
156149
});
@@ -165,14 +158,14 @@ describe('Integration | rate-limiting behaviour', () => {
165158

166159
// T = base + 40
167160
await advanceTimers(DEFAULT_FLUSH_MIN_DELAY);
168-
expect(replay['_sendReplay']).toHaveBeenCalledTimes(3);
169-
expect(replay).toHaveLastSentReplay({ events: JSON.stringify([TEST_EVENT3]) });
161+
expect(mockSendReplayRequest).toHaveBeenCalledTimes(3);
162+
expect(replay).toHaveLastSentReplay({ recordingData: JSON.stringify([TEST_EVENT3]) });
170163

171164
// nothing should happen afterwards
172165
// T = base + 60
173166
await advanceTimers(20_000);
174-
expect(replay['_sendReplay']).toHaveBeenCalledTimes(3);
175-
expect(replay).toHaveLastSentReplay({ events: JSON.stringify([TEST_EVENT3]) });
167+
expect(mockSendReplayRequest).toHaveBeenCalledTimes(3);
168+
expect(replay).toHaveLastSentReplay({ recordingData: JSON.stringify([TEST_EVENT3]) });
176169

177170
// events array should be empty
178171
expect(replay.eventBuffer?.pendingLength).toBe(0);
@@ -185,8 +178,6 @@ describe('Integration | rate-limiting behaviour', () => {
185178
jest.spyOn(replay, 'resume');
186179
// @ts-ignore private API
187180
jest.spyOn(replay, '_handleRateLimit');
188-
// @ts-ignore private API
189-
jest.spyOn(replay, '_sendReplay');
190181

191182
const TEST_EVENT = { data: {}, timestamp: BASE_TIMESTAMP, type: 2 };
192183

@@ -201,7 +192,7 @@ describe('Integration | rate-limiting behaviour', () => {
201192

202193
expect(mockRecord.takeFullSnapshot).not.toHaveBeenCalled();
203194
expect(mockTransportSend).toHaveBeenCalledTimes(1);
204-
expect(replay).toHaveLastSentReplay({ events: JSON.stringify([TEST_EVENT]) });
195+
expect(replay).toHaveLastSentReplay({ recordingData: JSON.stringify([TEST_EVENT]) });
205196

206197
expect(replay['_handleRateLimit']).toHaveBeenCalledTimes(1);
207198
// resume() was called once before we even started
@@ -223,7 +214,7 @@ describe('Integration | rate-limiting behaviour', () => {
223214
mockRecord._emitter(ev);
224215
await advanceTimers(DEFAULT_FLUSH_MIN_DELAY);
225216
expect(replay.isPaused()).toBe(true);
226-
expect(replay['_sendReplay']).toHaveBeenCalledTimes(1);
217+
expect(mockSendReplayRequest).toHaveBeenCalledTimes(1);
227218
expect(mockTransportSend).toHaveBeenCalledTimes(1);
228219
}
229220

@@ -235,9 +226,9 @@ describe('Integration | rate-limiting behaviour', () => {
235226
expect(replay.resume).toHaveBeenCalledTimes(1);
236227
expect(replay.isPaused()).toBe(false);
237228

238-
expect(replay['_sendReplay']).toHaveBeenCalledTimes(2);
229+
expect(mockSendReplayRequest).toHaveBeenCalledTimes(2);
239230
expect(replay).toHaveLastSentReplay({
240-
events: JSON.stringify([
231+
recordingData: JSON.stringify([
241232
{ data: { isCheckout: true }, timestamp: BASE_TIMESTAMP + DEFAULT_FLUSH_MIN_DELAY * 13, type: 2 },
242233
]),
243234
});
@@ -252,14 +243,14 @@ describe('Integration | rate-limiting behaviour', () => {
252243

253244
// T = base + 65
254245
await advanceTimers(DEFAULT_FLUSH_MIN_DELAY);
255-
expect(replay['_sendReplay']).toHaveBeenCalledTimes(3);
256-
expect(replay).toHaveLastSentReplay({ events: JSON.stringify([TEST_EVENT3]) });
246+
expect(mockSendReplayRequest).toHaveBeenCalledTimes(3);
247+
expect(replay).toHaveLastSentReplay({ recordingData: JSON.stringify([TEST_EVENT3]) });
257248

258249
// nothing should happen afterwards
259250
// T = base + 85
260251
await advanceTimers(20_000);
261-
expect(replay['_sendReplay']).toHaveBeenCalledTimes(3);
262-
expect(replay).toHaveLastSentReplay({ events: JSON.stringify([TEST_EVENT3]) });
252+
expect(mockSendReplayRequest).toHaveBeenCalledTimes(3);
253+
expect(replay).toHaveLastSentReplay({ recordingData: JSON.stringify([TEST_EVENT3]) });
263254

264255
// events array should be empty
265256
expect(replay.eventBuffer?.pendingLength).toBe(0);
@@ -291,7 +282,7 @@ describe('Integration | rate-limiting behaviour', () => {
291282
expect(mockRecord.takeFullSnapshot).not.toHaveBeenCalled();
292283
expect(mockTransportSend).toHaveBeenCalledTimes(1);
293284

294-
expect(replay).toHaveLastSentReplay({ events: JSON.stringify([TEST_EVENT]) });
285+
expect(replay).toHaveLastSentReplay({ recordingData: JSON.stringify([TEST_EVENT]) });
295286

296287
expect(replay['_handleRateLimit']).toHaveBeenCalledTimes(1);
297288
expect(replay.resume).not.toHaveBeenCalled();

0 commit comments

Comments
 (0)