Skip to content

Commit 0e15dae

Browse files
AbhiPrasadAbhiPrasad
authored
ref(browser): Use ratelimit utils in base transport (#4686)
This patch converts the browser based transport to use the ratelimit helpers introduced in #4685
1 parent af7081c commit 0e15dae

File tree

5 files changed

+47
-47
lines changed

5 files changed

+47
-47
lines changed

‎packages/browser/src/transports/base.ts‎

Lines changed: 17 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -19,15 +19,18 @@ import {
1919
} from '@sentry/types';
2020
import {
2121
createClientReportEnvelope,
22+
disabledUntil,
2223
dsnToString,
2324
eventStatusFromHttpCode,
2425
getGlobalObject,
2526
isDebugBuild,
27+
isRateLimited,
2628
logger,
2729
makePromiseBuffer,
28-
parseRetryAfterHeader,
2930
PromiseBuffer,
31+
RateLimits,
3032
serializeEnvelope,
33+
updateRateLimits,
3134
} from '@sentry/utils';
3235

3336
import { sendReport } from './utils';
@@ -53,7 +56,7 @@ export abstract class BaseTransport implements Transport {
5356
protected readonly _buffer: PromiseBuffer<SentryResponse> = makePromiseBuffer(30);
5457

5558
/** Locks transport after receiving rate limits in a response */
56-
protected readonly _rateLimits: Record<string, Date> = {};
59+
protected _rateLimits: RateLimits = {};
5760

5861
protected _outcomes: { [key: string]: number } = {};
5962

@@ -165,13 +168,12 @@ export abstract class BaseTransport implements Transport {
165168
reject: (reason?: unknown) => void;
166169
}): void {
167170
const status = eventStatusFromHttpCode(response.status);
168-
/**
169-
* "The name is case-insensitive."
170-
* https://developer.mozilla.org/en-US/docs/Web/API/Headers/get
171-
*/
172-
const limited = this._handleRateLimit(headers);
173-
if (limited) {
171+
172+
this._rateLimits = updateRateLimits(this._rateLimits, headers);
173+
// eslint-disable-next-line deprecation/deprecation
174+
if (this._isRateLimited(requestType) && isDebugBuild()) {
174175
isDebugBuild() &&
176+
// eslint-disable-next-line deprecation/deprecation
175177
logger.warn(`Too many ${requestType} requests, backing off until: ${this._disabledUntil(requestType)}`);
176178
}
177179

@@ -185,52 +187,22 @@ export abstract class BaseTransport implements Transport {
185187

186188
/**
187189
* Gets the time that given category is disabled until for rate limiting
190+
*
191+
* @deprecated Please use `disabledUntil` from @sentry/utils
188192
*/
189193
protected _disabledUntil(requestType: SentryRequestType): Date {
190194
const category = requestTypeToCategory(requestType);
191-
return this._rateLimits[category] || this._rateLimits.all;
195+
return new Date(disabledUntil(this._rateLimits, category));
192196
}
193197

194198
/**
195199
* Checks if a category is rate limited
200+
*
201+
* @deprecated Please use `isRateLimited` from @sentry/utils
196202
*/
197203
protected _isRateLimited(requestType: SentryRequestType): boolean {
198-
return this._disabledUntil(requestType) > new Date(Date.now());
199-
}
200-
201-
/**
202-
* Sets internal _rateLimits from incoming headers. Returns true if headers contains a non-empty rate limiting header.
203-
*/
204-
protected _handleRateLimit(headers: Record<string, string | null>): boolean {
205-
const now = Date.now();
206-
const rlHeader = headers['x-sentry-rate-limits'];
207-
const raHeader = headers['retry-after'];
208-
209-
if (rlHeader) {
210-
// rate limit headers are of the form
211-
// <header>,<header>,..
212-
// where each <header> is of the form
213-
// <retry_after>: <categories>: <scope>: <reason_code>
214-
// where
215-
// <retry_after> is a delay in ms
216-
// <categories> is the event type(s) (error, transaction, etc) being rate limited and is of the form
217-
// <category>;<category>;...
218-
// <scope> is what's being limited (org, project, or key) - ignored by SDK
219-
// <reason_code> is an arbitrary string like "org_quota" - ignored by SDK
220-
for (const limit of rlHeader.trim().split(',')) {
221-
const parameters = limit.split(':', 2);
222-
const headerDelay = parseInt(parameters[0], 10);
223-
const delay = (!isNaN(headerDelay) ? headerDelay : 60) * 1000; // 60sec default
224-
for (const category of parameters[1].split(';')) {
225-
this._rateLimits[category || 'all'] = new Date(now + delay);
226-
}
227-
}
228-
return true;
229-
} else if (raHeader) {
230-
this._rateLimits.all = new Date(now + parseRetryAfterHeader(raHeader, now));
231-
return true;
232-
}
233-
return false;
204+
const category = requestTypeToCategory(requestType);
205+
return isRateLimited(this._rateLimits, category);
234206
}
235207

236208
protected abstract _sendRequest(

‎packages/browser/src/transports/fetch.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,14 @@ export class FetchTransport extends BaseTransport {
2121
* @param originalPayload Original payload used to create SentryRequest
2222
*/
2323
protected _sendRequest(sentryRequest: SentryRequest, originalPayload: Event | Session): PromiseLike<Response> {
24+
// eslint-disable-next-line deprecation/deprecation
2425
if (this._isRateLimited(sentryRequest.type)) {
2526
this.recordLostEvent('ratelimit_backoff', sentryRequest.type);
2627

2728
return Promise.reject({
2829
event: originalPayload,
2930
type: sentryRequest.type,
31+
// eslint-disable-next-line deprecation/deprecation
3032
reason: `Transport for ${sentryRequest.type} requests locked till ${this._disabledUntil(
3133
sentryRequest.type,
3234
)} due to too many requests.`,

‎packages/browser/src/transports/xhr.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,14 @@ export class XHRTransport extends BaseTransport {
1010
* @param originalPayload Original payload used to create SentryRequest
1111
*/
1212
protected _sendRequest(sentryRequest: SentryRequest, originalPayload: Event | Session): PromiseLike<Response> {
13+
// eslint-disable-next-line deprecation/deprecation
1314
if (this._isRateLimited(sentryRequest.type)) {
1415
this.recordLostEvent('ratelimit_backoff', sentryRequest.type);
1516

1617
return Promise.reject({
1718
event: originalPayload,
1819
type: sentryRequest.type,
20+
// eslint-disable-next-line deprecation/deprecation
1921
reason: `Transport for ${sentryRequest.type} requests locked till ${this._disabledUntil(
2022
sentryRequest.type,
2123
)} due to too many requests.`,

‎packages/browser/test/unit/transports/fetch.test.ts‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,9 @@ describe('FetchTransport', () => {
193193

194194
jest
195195
.spyOn(Date, 'now')
196-
// 1st event - _isRateLimited - false
196+
// 1st event - updateRateLimits - false
197+
.mockImplementationOnce(() => beforeLimit)
198+
// 1st event - _handleRateLimit
197199
.mockImplementationOnce(() => beforeLimit)
198200
// 1st event - _handleRateLimit
199201
.mockImplementationOnce(() => beforeLimit)
@@ -247,6 +249,10 @@ describe('FetchTransport', () => {
247249
.mockImplementationOnce(() => beforeLimit)
248250
// 1st event - _handleRateLimit
249251
.mockImplementationOnce(() => beforeLimit)
252+
// 1st event - _isRateLimited
253+
.mockImplementationOnce(() => beforeLimit)
254+
// 1st event - _handleRateLimit
255+
.mockImplementationOnce(() => beforeLimit)
250256
// 2nd event - _isRateLimited - false (different category)
251257
.mockImplementationOnce(() => withinLimit)
252258
// 2nd event - _handleRateLimit
@@ -303,7 +309,9 @@ describe('FetchTransport', () => {
303309
.spyOn(Date, 'now')
304310
// 1st event - _isRateLimited - false
305311
.mockImplementationOnce(() => beforeLimit)
306-
// 1st event - _handleRateLimit
312+
// 1st event - updateRateLimits
313+
.mockImplementationOnce(() => beforeLimit)
314+
// 1st event - _isRateLimited
307315
.mockImplementationOnce(() => beforeLimit)
308316
// 2nd event - _isRateLimited - true (event category)
309317
.mockImplementationOnce(() => withinLimit)
@@ -376,6 +384,8 @@ describe('FetchTransport', () => {
376384
.mockImplementationOnce(() => beforeLimit)
377385
// 1st event - _handleRateLimit
378386
.mockImplementationOnce(() => beforeLimit)
387+
// 1st event - _isRateLimited
388+
.mockImplementationOnce(() => beforeLimit)
379389
// 2nd event - _isRateLimited - true (event category)
380390
.mockImplementationOnce(() => withinLimit)
381391
// 3rd event - _isRateLimited - true (transaction category)
@@ -447,6 +457,8 @@ describe('FetchTransport', () => {
447457
.mockImplementationOnce(() => beforeLimit)
448458
// 1st event - _handleRateLimit
449459
.mockImplementationOnce(() => beforeLimit)
460+
// 1st event - _isRateLimited
461+
.mockImplementationOnce(() => beforeLimit)
450462
// 2nd event - _isRateLimited - true
451463
.mockImplementationOnce(() => withinLimit)
452464
// 3rd event - _isRateLimited - false

‎packages/browser/test/unit/transports/xhr.test.ts‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,8 @@ describe('XHRTransport', () => {
126126
.mockImplementationOnce(() => beforeLimit)
127127
// 1st event - _handleRateLimit
128128
.mockImplementationOnce(() => beforeLimit)
129+
// 1st event - _handleRateLimit
130+
.mockImplementationOnce(() => beforeLimit)
129131
// 2nd event - _isRateLimited - true
130132
.mockImplementationOnce(() => withinLimit)
131133
// 3rd event - _isRateLimited - false
@@ -175,6 +177,10 @@ describe('XHRTransport', () => {
175177
.mockImplementationOnce(() => beforeLimit)
176178
// 1st event - _handleRateLimit
177179
.mockImplementationOnce(() => beforeLimit)
180+
// 1st event - _isRateLimited
181+
.mockImplementationOnce(() => beforeLimit)
182+
// 1st event - _handleRateLimit
183+
.mockImplementationOnce(() => beforeLimit)
178184
// 2nd event - _isRateLimited - false (different category)
179185
.mockImplementationOnce(() => withinLimit)
180186
// 2nd event - _handleRateLimit
@@ -236,6 +242,8 @@ describe('XHRTransport', () => {
236242
.mockImplementationOnce(() => beforeLimit)
237243
// 1st event - _handleRateLimit
238244
.mockImplementationOnce(() => beforeLimit)
245+
// 1st event - _isRateLimited
246+
.mockImplementationOnce(() => beforeLimit)
239247
// 2nd event - _isRateLimited - true (event category)
240248
.mockImplementationOnce(() => withinLimit)
241249
// 3rd event - _isRateLimited - true (transaction category)
@@ -307,6 +315,8 @@ describe('XHRTransport', () => {
307315
.mockImplementationOnce(() => beforeLimit)
308316
// 1st event - _handleRateLimit
309317
.mockImplementationOnce(() => beforeLimit)
318+
// 1st event - _isRateLimited
319+
.mockImplementationOnce(() => beforeLimit)
310320
// 2nd event - _isRateLimited - true (event category)
311321
.mockImplementationOnce(() => withinLimit)
312322
// 3rd event - _isRateLimited - true (transaction category)
@@ -377,6 +387,8 @@ describe('XHRTransport', () => {
377387
.mockImplementationOnce(() => beforeLimit)
378388
// 1st event - _handleRateLimit
379389
.mockImplementationOnce(() => beforeLimit)
390+
// 1st event - _handleRateLimit
391+
.mockImplementationOnce(() => beforeLimit)
380392
// 2nd event - _isRateLimited - true
381393
.mockImplementationOnce(() => withinLimit)
382394
// 3rd event - _isRateLimited - false

0 commit comments

Comments
 (0)