From 7a86c7f16d96c07eb5328aef335c472d1b1e6838 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Fri, 4 Mar 2022 10:52:16 -0500 Subject: [PATCH 01/12] feat(utils): Introduce rate limit helpers --- packages/utils/src/ratelimit.ts | 89 +++++++++++++++ packages/utils/test/ratelimit.test.ts | 158 ++++++++++++++++++++++++++ 2 files changed, 247 insertions(+) create mode 100644 packages/utils/src/ratelimit.ts create mode 100644 packages/utils/test/ratelimit.test.ts diff --git a/packages/utils/src/ratelimit.ts b/packages/utils/src/ratelimit.ts new file mode 100644 index 000000000000..868a8eec3cd0 --- /dev/null +++ b/packages/utils/src/ratelimit.ts @@ -0,0 +1,89 @@ +export type RateLimits = Record; + +export const DEFAULT_RETRY_AFTER = 60 * 1000; // 60 seconds + +/** + * Extracts Retry-After value from the request header or returns default value + * @param header string representation of 'Retry-After' header + * @param now current unix timestamp + * + * TODO: Export this function after we remove `parseRetryAfterHeader` from + * utils/src/misc.ts + */ +export function parseRetryAfterHeader(header: string, now: number = Date.now()): number { + const headerDelay = parseInt(`${header}`, 10); + if (!isNaN(headerDelay)) { + return headerDelay * 1000; + } + + const headerDate = Date.parse(`${header}`); + if (!isNaN(headerDate)) { + return headerDate - now; + } + + return DEFAULT_RETRY_AFTER; +} + +/** + * Gets the time that given category is disabled until for rate limiting + */ +export function disabledUntil(limits: RateLimits, category: string): number { + return limits[category] || limits.all || 0; +} + +/** + * Checks if a category is rate limited + */ +export function isRateLimited(limits: RateLimits, category: string, now: number = Date.now()): boolean { + return disabledUntil(limits, category) > now; +} + +/** + * Update ratelimits from incoming headers. + * Returns true if headers contains a non-empty rate limiting header. + */ +export function updateRateLimits( + limits: RateLimits, + headers: Record, + now: number = Date.now(), +): RateLimits { + const updatedRateLimits: RateLimits = { + ...limits, + }; + + // "The name is case-insensitive." + // https://developer.mozilla.org/en-US/docs/Web/API/Headers/get + const rlHeader = headers['x-sentry-rate-limits']; + const raHeader = headers['retry-after']; + + if (rlHeader) { + /** + * rate limit headers are of the form + *
,
,.. + * where each
is of the form + * : : : + * where + * is a delay in seconds + * is the event type(s) (error, transaction, etc) being rate limited and is of the form + * ;;... + * is what's being limited (org, project, or key) - ignored by SDK + * is an arbitrary string like "org_quota" - ignored by SDK + */ + for (const limit of rlHeader.trim().split(',')) { + const parameters = limit.split(':', 2); + const headerDelay = parseInt(parameters[0], 10); + const delay = (!isNaN(headerDelay) ? headerDelay : 60) * 1000; // 60sec default + if (!parameters[1]) { + updatedRateLimits.all = now + delay; + } else { + for (const category of parameters[1].split(';')) { + updatedRateLimits[category] = now + delay; + } + } + } + } else if (raHeader) { + updatedRateLimits.all = now + parseRetryAfterHeader(raHeader, now); + } + + return updatedRateLimits; +} diff --git a/packages/utils/test/ratelimit.test.ts b/packages/utils/test/ratelimit.test.ts new file mode 100644 index 000000000000..ba592426453e --- /dev/null +++ b/packages/utils/test/ratelimit.test.ts @@ -0,0 +1,158 @@ +import { + parseRetryAfterHeader, + DEFAULT_RETRY_AFTER, + disabledUntil, + isRateLimited, + RateLimits, + updateRateLimits, +} from '../src/ratelimit'; + +describe('parseRetryAfterHeader()', () => { + test('should fallback to 60s when incorrect header provided', () => { + expect(parseRetryAfterHeader('x')).toEqual(DEFAULT_RETRY_AFTER); + }); + + test('should correctly parse delay-based header', () => { + expect(parseRetryAfterHeader('1337')).toEqual(1337 * 1000); + }); + + test('should correctly parse date-based header', () => { + expect( + parseRetryAfterHeader('Wed, 21 Oct 2015 07:28:13 GMT', new Date('Wed, 21 Oct 2015 07:28:00 GMT').getTime()), + ).toEqual(13 * 1000); + }); +}); + +describe('disabledUntil()', () => { + test('should return 0 when no match', () => { + expect(disabledUntil({}, 'error')).toEqual(0); + }); + + test('should return matched value', () => { + expect(disabledUntil({ error: 42 }, 'error')).toEqual(42); + }); + + test('should fallback to `all` category', () => { + expect(disabledUntil({ all: 42 }, 'error')).toEqual(42); + }); +}); + +describe('isRateLimited()', () => { + test('should return false when no match', () => { + expect(isRateLimited({}, 'error')).toEqual(false); + }); + + test('should return false when matched value is in the past', () => { + expect(isRateLimited({ error: 10 }, 'error', 42)).toEqual(false); + }); + + test('should return true when matched value is in the future', () => { + expect(isRateLimited({ error: 50 }, 'error', 42)).toEqual(true); + }); + + test('should fallback to the `all` category when given one is not matched', () => { + expect(isRateLimited({ all: 10 }, 'error', 42)).toEqual(false); + expect(isRateLimited({ all: 50 }, 'error', 42)).toEqual(true); + }); +}); + +describe('updateRateLimits()', () => { + test('should return same limits when no headers provided', () => { + const rateLimits: RateLimits = { + error: 42, + transaction: 1337, + }; + const headers = {}; + const updatedRateLimits = updateRateLimits(rateLimits, headers); + expect(rateLimits).toStrictEqual(updatedRateLimits); + }); + + test('should update the `all` category based on `retry-after` header ', () => { + const rateLimits: RateLimits = {}; + const headers = { + 'retry-after': '42', + }; + const updatedRateLimits = updateRateLimits(rateLimits, headers, 0); + expect(updatedRateLimits.all).toEqual(42 * 1000); + }); + + test('should update a single category based on `x-sentry-rate-limits` header', () => { + const rateLimits: RateLimits = {}; + const headers = { + 'x-sentry-rate-limits': '13:error', + }; + const updatedRateLimits = updateRateLimits(rateLimits, headers, 0); + expect(updatedRateLimits.error).toEqual(13 * 1000); + }); + + test('should update a multiple categories based on `x-sentry-rate-limits` header', () => { + const rateLimits: RateLimits = {}; + const headers = { + 'x-sentry-rate-limits': '13:error;transaction', + }; + const updatedRateLimits = updateRateLimits(rateLimits, headers, 0); + expect(updatedRateLimits.error).toEqual(13 * 1000); + expect(updatedRateLimits.transaction).toEqual(13 * 1000); + }); + + test('should update limits based on multi `x-sentry-rate-limits` header', () => { + const rateLimits: RateLimits = {}; + const headers = { + 'x-sentry-rate-limits': '13:error,15:transaction', + }; + const updatedRateLimits = updateRateLimits(rateLimits, headers, 0); + expect(updatedRateLimits.error).toEqual(13 * 1000); + expect(updatedRateLimits.transaction).toEqual(15 * 1000); + }); + + test('should use last entry from multi `x-sentry-rate-limits` header for a given category', () => { + const rateLimits: RateLimits = {}; + const headers = { + 'x-sentry-rate-limits': '13:error,15:transaction;error', + }; + const updatedRateLimits = updateRateLimits(rateLimits, headers, 0); + expect(updatedRateLimits.error).toEqual(15 * 1000); + expect(updatedRateLimits.transaction).toEqual(15 * 1000); + }); + + test('should fallback to `all` if `x-sentry-rate-limits` header is missing a category', () => { + const rateLimits: RateLimits = {}; + const headers = { + 'x-sentry-rate-limits': '13', + }; + const updatedRateLimits = updateRateLimits(rateLimits, headers, 0); + expect(updatedRateLimits.all).toEqual(13 * 1000); + }); + + test('should use 60s default if delay in `x-sentry-rate-limits` header is malformed', () => { + const rateLimits: RateLimits = {}; + const headers = { + 'x-sentry-rate-limits': 'x', + }; + const updatedRateLimits = updateRateLimits(rateLimits, headers, 0); + expect(updatedRateLimits.all).toEqual(60 * 1000); + }); + + test('should preserve previous limits', () => { + const rateLimits: RateLimits = { + error: 1337, + }; + const headers = { + 'x-sentry-rate-limits': '13:transaction', + }; + const updatedRateLimits = updateRateLimits(rateLimits, headers, 0); + expect(updatedRateLimits.error).toEqual(1337); + expect(updatedRateLimits.transaction).toEqual(13 * 1000); + }); + + test('should give priority to `x-sentry-rate-limits` over `retry-after` header if both provided', () => { + const rateLimits: RateLimits = {}; + const headers = { + 'retry-after': '42', + 'x-sentry-rate-limits': '13:error', + }; + const updatedRateLimits = updateRateLimits(rateLimits, headers, 0); + expect(updatedRateLimits.error).toEqual(13 * 1000); + expect(updatedRateLimits.all).toBeUndefined(); + }); +}); From 379067f11e0b0dae9b599a9b179b2c630daa7d59 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Fri, 4 Mar 2022 10:54:12 -0500 Subject: [PATCH 02/12] feat(utils): Delete from utils --- packages/utils/src/misc.ts | 25 ------------------------- packages/utils/test/misc.test.ts | 21 --------------------- 2 files changed, 46 deletions(-) diff --git a/packages/utils/src/misc.ts b/packages/utils/src/misc.ts index 11f823115215..c49f0946bc44 100644 --- a/packages/utils/src/misc.ts +++ b/packages/utils/src/misc.ts @@ -188,31 +188,6 @@ export function parseSemver(input: string): SemVer { }; } -const defaultRetryAfter = 60 * 1000; // 60 seconds - -/** - * Extracts Retry-After value from the request header or returns default value - * @param now current unix timestamp - * @param header string representation of 'Retry-After' header - */ -export function parseRetryAfterHeader(now: number, header?: string | number | null): number { - if (!header) { - return defaultRetryAfter; - } - - const headerDelay = parseInt(`${header}`, 10); - if (!isNaN(headerDelay)) { - return headerDelay * 1000; - } - - const headerDate = Date.parse(`${header}`); - if (!isNaN(headerDate)) { - return headerDate - now; - } - - return defaultRetryAfter; -} - /** * This function adds context (pre/post/line) lines to the provided frame * diff --git a/packages/utils/test/misc.test.ts b/packages/utils/test/misc.test.ts index 7f013122bcc5..6820c4e9e186 100644 --- a/packages/utils/test/misc.test.ts +++ b/packages/utils/test/misc.test.ts @@ -5,7 +5,6 @@ import { addExceptionMechanism, checkOrSetAlreadyCaught, getEventDescription, - parseRetryAfterHeader, stripUrlQueryAndFragment, } from '../src/misc'; @@ -118,26 +117,6 @@ describe('getEventDescription()', () => { }); }); -describe('parseRetryAfterHeader', () => { - test('no header', () => { - expect(parseRetryAfterHeader(Date.now())).toEqual(60 * 1000); - }); - - test('incorrect header', () => { - expect(parseRetryAfterHeader(Date.now(), 'x')).toEqual(60 * 1000); - }); - - test('delay header', () => { - expect(parseRetryAfterHeader(Date.now(), '1337')).toEqual(1337 * 1000); - }); - - test('date header', () => { - expect( - parseRetryAfterHeader(new Date('Wed, 21 Oct 2015 07:28:00 GMT').getTime(), 'Wed, 21 Oct 2015 07:28:13 GMT'), - ).toEqual(13 * 1000); - }); -}); - describe('addContextToFrame', () => { const lines = [ '1: a', From 890d28477b54107ef6c5f1e1bb35205f9ad7e5c6 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Fri, 4 Mar 2022 10:55:13 -0500 Subject: [PATCH 03/12] add ratelimit export --- packages/utils/src/index.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/utils/src/index.ts b/packages/utils/src/index.ts index 6ffe7222569e..0bd885273f12 100644 --- a/packages/utils/src/index.ts +++ b/packages/utils/src/index.ts @@ -24,3 +24,4 @@ export * from './tracing'; export * from './env'; export * from './envelope'; export * from './clientreport'; +export * from './ratelimit'; From 0b0cf29216f3f1bb6bd3fb5233fa6252c8235f91 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Fri, 4 Mar 2022 10:55:26 -0500 Subject: [PATCH 04/12] remove TODO --- packages/utils/src/ratelimit.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/utils/src/ratelimit.ts b/packages/utils/src/ratelimit.ts index 868a8eec3cd0..7356b7c51fa9 100644 --- a/packages/utils/src/ratelimit.ts +++ b/packages/utils/src/ratelimit.ts @@ -7,8 +7,6 @@ export const DEFAULT_RETRY_AFTER = 60 * 1000; // 60 seconds * @param header string representation of 'Retry-After' header * @param now current unix timestamp * - * TODO: Export this function after we remove `parseRetryAfterHeader` from - * utils/src/misc.ts */ export function parseRetryAfterHeader(header: string, now: number = Date.now()): number { const headerDelay = parseInt(`${header}`, 10); From c946cf69ba3f6531cae4bf2c628fa36e6c2504b4 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Fri, 4 Mar 2022 11:00:25 -0500 Subject: [PATCH 05/12] comment --- packages/utils/src/ratelimit.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/utils/src/ratelimit.ts b/packages/utils/src/ratelimit.ts index 7356b7c51fa9..a9d824ff00fd 100644 --- a/packages/utils/src/ratelimit.ts +++ b/packages/utils/src/ratelimit.ts @@ -1,3 +1,4 @@ +// Keeping the key broad until we add the new transports export type RateLimits = Record; export const DEFAULT_RETRY_AFTER = 60 * 1000; // 60 seconds From 769b4fe3786c0c533d703fb73d7e179507625e33 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Fri, 4 Mar 2022 11:26:37 -0500 Subject: [PATCH 06/12] fix types --- packages/browser/src/transports/base.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/browser/src/transports/base.ts b/packages/browser/src/transports/base.ts index e519dd56dc88..60dffda0c246 100644 --- a/packages/browser/src/transports/base.ts +++ b/packages/browser/src/transports/base.ts @@ -226,7 +226,7 @@ export abstract class BaseTransport implements Transport { } return true; } else if (raHeader) { - this._rateLimits.all = new Date(now + parseRetryAfterHeader(now, raHeader)); + this._rateLimits.all = new Date(now + parseRetryAfterHeader(raHeader, now)); return true; } return false; From fabcf575534c4fc19ae445bd6ac8452391b84583 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Fri, 4 Mar 2022 11:32:45 -0500 Subject: [PATCH 07/12] fix node types --- packages/node/src/transports/base/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/node/src/transports/base/index.ts b/packages/node/src/transports/base/index.ts index 6225bed07c14..71ffe5783ad2 100644 --- a/packages/node/src/transports/base/index.ts +++ b/packages/node/src/transports/base/index.ts @@ -181,7 +181,7 @@ export abstract class BaseTransport implements Transport { } return true; } else if (raHeader) { - this._rateLimits.all = new Date(now + parseRetryAfterHeader(now, raHeader)); + this._rateLimits.all = new Date(now + parseRetryAfterHeader(raHeader, now)); return true; } return false; From 0b811af2f470ba8268d827766de3e6722220476f Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Fri, 4 Mar 2022 12:50:51 -0500 Subject: [PATCH 08/12] import --- packages/utils/test/ratelimit.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/utils/test/ratelimit.test.ts b/packages/utils/test/ratelimit.test.ts index ba592426453e..e69a9ff9d0c4 100644 --- a/packages/utils/test/ratelimit.test.ts +++ b/packages/utils/test/ratelimit.test.ts @@ -1,8 +1,8 @@ import { - parseRetryAfterHeader, DEFAULT_RETRY_AFTER, disabledUntil, isRateLimited, + parseRetryAfterHeader, RateLimits, updateRateLimits, } from '../src/ratelimit'; From 51d138b26e6b9b2370ed4a8ca04290c9e1f2fd53 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Fri, 4 Mar 2022 11:12:05 -0500 Subject: [PATCH 09/12] ref(browser): Use ratelimit utils in base transport --- packages/browser/src/transports/base.ts | 65 +++++++------------------ 1 file changed, 18 insertions(+), 47 deletions(-) diff --git a/packages/browser/src/transports/base.ts b/packages/browser/src/transports/base.ts index 60dffda0c246..cfd64517e8a1 100644 --- a/packages/browser/src/transports/base.ts +++ b/packages/browser/src/transports/base.ts @@ -19,15 +19,18 @@ import { } from '@sentry/types'; import { createClientReportEnvelope, + disabledUntil, dsnToString, eventStatusFromHttpCode, getGlobalObject, isDebugBuild, + isRateLimited, logger, makePromiseBuffer, - parseRetryAfterHeader, PromiseBuffer, + RateLimits, serializeEnvelope, + updateRateLimits, } from '@sentry/utils'; import { sendReport } from './utils'; @@ -53,7 +56,7 @@ export abstract class BaseTransport implements Transport { protected readonly _buffer: PromiseBuffer = makePromiseBuffer(30); /** Locks transport after receiving rate limits in a response */ - protected readonly _rateLimits: Record = {}; + protected _rateLimits: RateLimits = {}; protected _outcomes: { [key: string]: number } = {}; @@ -165,13 +168,12 @@ export abstract class BaseTransport implements Transport { reject: (reason?: unknown) => void; }): void { const status = eventStatusFromHttpCode(response.status); - /** - * "The name is case-insensitive." - * https://developer.mozilla.org/en-US/docs/Web/API/Headers/get - */ - const limited = this._handleRateLimit(headers); - if (limited && isDebugBuild()) { - logger.warn(`Too many ${requestType} requests, backing off until: ${this._disabledUntil(requestType)}`); + + this._rateLimits = updateRateLimits(this._rateLimits, headers); + if (isRateLimited(this._rateLimits, requestType) && isDebugBuild()) { + logger.warn( + `Too many ${requestType} requests, backing off until: ${disabledUntil(this._rateLimits, requestType)}`, + ); } if (status === 'success') { @@ -184,52 +186,21 @@ export abstract class BaseTransport implements Transport { /** * Gets the time that given category is disabled until for rate limiting + * + * @deprecated Please use `disabledUntil` from @sentry/utils */ - protected _disabledUntil(requestType: SentryRequestType): Date { + protected _disabledUntil(requestType: SentryRequestType): number { const category = requestTypeToCategory(requestType); - return this._rateLimits[category] || this._rateLimits.all; + return disabledUntil(this._rateLimits, category); } /** * Checks if a category is rate limited + * + * @deprecated Please use `isRateLimited` from @sentry/utils */ protected _isRateLimited(requestType: SentryRequestType): boolean { - return this._disabledUntil(requestType) > new Date(Date.now()); - } - - /** - * Sets internal _rateLimits from incoming headers. Returns true if headers contains a non-empty rate limiting header. - */ - protected _handleRateLimit(headers: Record): boolean { - const now = Date.now(); - const rlHeader = headers['x-sentry-rate-limits']; - const raHeader = headers['retry-after']; - - if (rlHeader) { - // rate limit headers are of the form - //
,
,.. - // where each
is of the form - // : : : - // where - // is a delay in ms - // is the event type(s) (error, transaction, etc) being rate limited and is of the form - // ;;... - // is what's being limited (org, project, or key) - ignored by SDK - // is an arbitrary string like "org_quota" - ignored by SDK - for (const limit of rlHeader.trim().split(',')) { - const parameters = limit.split(':', 2); - const headerDelay = parseInt(parameters[0], 10); - const delay = (!isNaN(headerDelay) ? headerDelay : 60) * 1000; // 60sec default - for (const category of parameters[1].split(';')) { - this._rateLimits[category || 'all'] = new Date(now + delay); - } - } - return true; - } else if (raHeader) { - this._rateLimits.all = new Date(now + parseRetryAfterHeader(raHeader, now)); - return true; - } - return false; + return isRateLimited(this._rateLimits, requestType); } protected abstract _sendRequest( From 55ecfe10bc72eed8674422a9b26b1fe851c4a517 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Mon, 7 Mar 2022 08:41:19 -0500 Subject: [PATCH 10/12] lint fixes --- packages/browser/src/transports/base.ts | 10 ++++++---- packages/browser/src/transports/fetch.ts | 2 ++ packages/browser/src/transports/xhr.ts | 2 ++ packages/browser/test/unit/transports/fetch.test.ts | 3 ++- 4 files changed, 12 insertions(+), 5 deletions(-) diff --git a/packages/browser/src/transports/base.ts b/packages/browser/src/transports/base.ts index cfd64517e8a1..bdb4678d02d8 100644 --- a/packages/browser/src/transports/base.ts +++ b/packages/browser/src/transports/base.ts @@ -170,7 +170,8 @@ export abstract class BaseTransport implements Transport { const status = eventStatusFromHttpCode(response.status); this._rateLimits = updateRateLimits(this._rateLimits, headers); - if (isRateLimited(this._rateLimits, requestType) && isDebugBuild()) { + const category = requestTypeToCategory(requestType); + if (isRateLimited(this._rateLimits, category) && isDebugBuild()) { logger.warn( `Too many ${requestType} requests, backing off until: ${disabledUntil(this._rateLimits, requestType)}`, ); @@ -189,9 +190,9 @@ export abstract class BaseTransport implements Transport { * * @deprecated Please use `disabledUntil` from @sentry/utils */ - protected _disabledUntil(requestType: SentryRequestType): number { + protected _disabledUntil(requestType: SentryRequestType): Date { const category = requestTypeToCategory(requestType); - return disabledUntil(this._rateLimits, category); + return new Date(disabledUntil(this._rateLimits, category)); } /** @@ -200,7 +201,8 @@ export abstract class BaseTransport implements Transport { * @deprecated Please use `isRateLimited` from @sentry/utils */ protected _isRateLimited(requestType: SentryRequestType): boolean { - return isRateLimited(this._rateLimits, requestType); + const category = requestTypeToCategory(requestType); + return isRateLimited(this._rateLimits, category); } protected abstract _sendRequest( diff --git a/packages/browser/src/transports/fetch.ts b/packages/browser/src/transports/fetch.ts index 23b6b380e455..cddf1b53c1ae 100644 --- a/packages/browser/src/transports/fetch.ts +++ b/packages/browser/src/transports/fetch.ts @@ -21,12 +21,14 @@ export class FetchTransport extends BaseTransport { * @param originalPayload Original payload used to create SentryRequest */ protected _sendRequest(sentryRequest: SentryRequest, originalPayload: Event | Session): PromiseLike { + // eslint-disable-next-line deprecation/deprecation if (this._isRateLimited(sentryRequest.type)) { this.recordLostEvent('ratelimit_backoff', sentryRequest.type); return Promise.reject({ event: originalPayload, type: sentryRequest.type, + // eslint-disable-next-line deprecation/deprecation reason: `Transport for ${sentryRequest.type} requests locked till ${this._disabledUntil( sentryRequest.type, )} due to too many requests.`, diff --git a/packages/browser/src/transports/xhr.ts b/packages/browser/src/transports/xhr.ts index e73a355995db..5da7de258bf6 100644 --- a/packages/browser/src/transports/xhr.ts +++ b/packages/browser/src/transports/xhr.ts @@ -10,12 +10,14 @@ export class XHRTransport extends BaseTransport { * @param originalPayload Original payload used to create SentryRequest */ protected _sendRequest(sentryRequest: SentryRequest, originalPayload: Event | Session): PromiseLike { + // eslint-disable-next-line deprecation/deprecation if (this._isRateLimited(sentryRequest.type)) { this.recordLostEvent('ratelimit_backoff', sentryRequest.type); return Promise.reject({ event: originalPayload, type: sentryRequest.type, + // eslint-disable-next-line deprecation/deprecation reason: `Transport for ${sentryRequest.type} requests locked till ${this._disabledUntil( sentryRequest.type, )} due to too many requests.`, diff --git a/packages/browser/test/unit/transports/fetch.test.ts b/packages/browser/test/unit/transports/fetch.test.ts index afb427b4026e..fbafefd85543 100644 --- a/packages/browser/test/unit/transports/fetch.test.ts +++ b/packages/browser/test/unit/transports/fetch.test.ts @@ -221,6 +221,7 @@ describe('FetchTransport', () => { await transport.sendEvent(eventPayload); throw new Error('unreachable!'); } catch (res) { + console.log(res); expect((res as Response).status).toBe(429); expect((res as Response).reason).toBe( `Transport for event requests locked till ${new Date(afterLimit)} due to too many requests.`, @@ -293,7 +294,7 @@ describe('FetchTransport', () => { expect(fetch).toHaveBeenCalledTimes(3); }); - it('back-off using X-Sentry-Rate-Limits with multiple categories', async () => { + it.only('back-off using X-Sentry-Rate-Limits with multiple categories', async () => { const retryAfterSeconds = 10; const beforeLimit = Date.now(); const withinLimit = beforeLimit + (retryAfterSeconds / 2) * 1000; From 861c02ce41fb064447618b631e5fffa6f15193ce Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Mon, 14 Mar 2022 10:51:05 +0100 Subject: [PATCH 11/12] lint fix --- packages/browser/test/unit/transports/fetch.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/browser/test/unit/transports/fetch.test.ts b/packages/browser/test/unit/transports/fetch.test.ts index fbafefd85543..033c89742be1 100644 --- a/packages/browser/test/unit/transports/fetch.test.ts +++ b/packages/browser/test/unit/transports/fetch.test.ts @@ -221,7 +221,6 @@ describe('FetchTransport', () => { await transport.sendEvent(eventPayload); throw new Error('unreachable!'); } catch (res) { - console.log(res); expect((res as Response).status).toBe(429); expect((res as Response).reason).toBe( `Transport for event requests locked till ${new Date(afterLimit)} due to too many requests.`, From 439eb156915d98d256d84237fd744814e2775706 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Mon, 14 Mar 2022 14:28:43 +0100 Subject: [PATCH 12/12] fix tests --- packages/browser/src/transports/base.ts | 9 ++++----- .../browser/test/unit/transports/fetch.test.ts | 18 +++++++++++++++--- .../browser/test/unit/transports/xhr.test.ts | 12 ++++++++++++ 3 files changed, 31 insertions(+), 8 deletions(-) diff --git a/packages/browser/src/transports/base.ts b/packages/browser/src/transports/base.ts index ffe98bfbad37..4e9b23fcfba9 100644 --- a/packages/browser/src/transports/base.ts +++ b/packages/browser/src/transports/base.ts @@ -170,12 +170,11 @@ export abstract class BaseTransport implements Transport { const status = eventStatusFromHttpCode(response.status); this._rateLimits = updateRateLimits(this._rateLimits, headers); - const category = requestTypeToCategory(requestType); - if (isRateLimited(this._rateLimits, category) && isDebugBuild()) { + // eslint-disable-next-line deprecation/deprecation + if (this._isRateLimited(requestType) && isDebugBuild()) { isDebugBuild() && - logger.warn( - `Too many ${requestType} requests, backing off until: ${disabledUntil(this._rateLimits, requestType)}`, - ); + // eslint-disable-next-line deprecation/deprecation + logger.warn(`Too many ${requestType} requests, backing off until: ${this._disabledUntil(requestType)}`); } if (status === 'success') { diff --git a/packages/browser/test/unit/transports/fetch.test.ts b/packages/browser/test/unit/transports/fetch.test.ts index 033c89742be1..f7af4e38349c 100644 --- a/packages/browser/test/unit/transports/fetch.test.ts +++ b/packages/browser/test/unit/transports/fetch.test.ts @@ -193,7 +193,9 @@ describe('FetchTransport', () => { jest .spyOn(Date, 'now') - // 1st event - _isRateLimited - false + // 1st event - updateRateLimits - false + .mockImplementationOnce(() => beforeLimit) + // 1st event - _handleRateLimit .mockImplementationOnce(() => beforeLimit) // 1st event - _handleRateLimit .mockImplementationOnce(() => beforeLimit) @@ -247,6 +249,10 @@ describe('FetchTransport', () => { .mockImplementationOnce(() => beforeLimit) // 1st event - _handleRateLimit .mockImplementationOnce(() => beforeLimit) + // 1st event - _isRateLimited + .mockImplementationOnce(() => beforeLimit) + // 1st event - _handleRateLimit + .mockImplementationOnce(() => beforeLimit) // 2nd event - _isRateLimited - false (different category) .mockImplementationOnce(() => withinLimit) // 2nd event - _handleRateLimit @@ -293,7 +299,7 @@ describe('FetchTransport', () => { expect(fetch).toHaveBeenCalledTimes(3); }); - it.only('back-off using X-Sentry-Rate-Limits with multiple categories', async () => { + it('back-off using X-Sentry-Rate-Limits with multiple categories', async () => { const retryAfterSeconds = 10; const beforeLimit = Date.now(); const withinLimit = beforeLimit + (retryAfterSeconds / 2) * 1000; @@ -303,7 +309,9 @@ describe('FetchTransport', () => { .spyOn(Date, 'now') // 1st event - _isRateLimited - false .mockImplementationOnce(() => beforeLimit) - // 1st event - _handleRateLimit + // 1st event - updateRateLimits + .mockImplementationOnce(() => beforeLimit) + // 1st event - _isRateLimited .mockImplementationOnce(() => beforeLimit) // 2nd event - _isRateLimited - true (event category) .mockImplementationOnce(() => withinLimit) @@ -376,6 +384,8 @@ describe('FetchTransport', () => { .mockImplementationOnce(() => beforeLimit) // 1st event - _handleRateLimit .mockImplementationOnce(() => beforeLimit) + // 1st event - _isRateLimited + .mockImplementationOnce(() => beforeLimit) // 2nd event - _isRateLimited - true (event category) .mockImplementationOnce(() => withinLimit) // 3rd event - _isRateLimited - true (transaction category) @@ -447,6 +457,8 @@ describe('FetchTransport', () => { .mockImplementationOnce(() => beforeLimit) // 1st event - _handleRateLimit .mockImplementationOnce(() => beforeLimit) + // 1st event - _isRateLimited + .mockImplementationOnce(() => beforeLimit) // 2nd event - _isRateLimited - true .mockImplementationOnce(() => withinLimit) // 3rd event - _isRateLimited - false diff --git a/packages/browser/test/unit/transports/xhr.test.ts b/packages/browser/test/unit/transports/xhr.test.ts index fcf7c26211da..2a0f43d89815 100644 --- a/packages/browser/test/unit/transports/xhr.test.ts +++ b/packages/browser/test/unit/transports/xhr.test.ts @@ -126,6 +126,8 @@ describe('XHRTransport', () => { .mockImplementationOnce(() => beforeLimit) // 1st event - _handleRateLimit .mockImplementationOnce(() => beforeLimit) + // 1st event - _handleRateLimit + .mockImplementationOnce(() => beforeLimit) // 2nd event - _isRateLimited - true .mockImplementationOnce(() => withinLimit) // 3rd event - _isRateLimited - false @@ -175,6 +177,10 @@ describe('XHRTransport', () => { .mockImplementationOnce(() => beforeLimit) // 1st event - _handleRateLimit .mockImplementationOnce(() => beforeLimit) + // 1st event - _isRateLimited + .mockImplementationOnce(() => beforeLimit) + // 1st event - _handleRateLimit + .mockImplementationOnce(() => beforeLimit) // 2nd event - _isRateLimited - false (different category) .mockImplementationOnce(() => withinLimit) // 2nd event - _handleRateLimit @@ -236,6 +242,8 @@ describe('XHRTransport', () => { .mockImplementationOnce(() => beforeLimit) // 1st event - _handleRateLimit .mockImplementationOnce(() => beforeLimit) + // 1st event - _isRateLimited + .mockImplementationOnce(() => beforeLimit) // 2nd event - _isRateLimited - true (event category) .mockImplementationOnce(() => withinLimit) // 3rd event - _isRateLimited - true (transaction category) @@ -307,6 +315,8 @@ describe('XHRTransport', () => { .mockImplementationOnce(() => beforeLimit) // 1st event - _handleRateLimit .mockImplementationOnce(() => beforeLimit) + // 1st event - _isRateLimited + .mockImplementationOnce(() => beforeLimit) // 2nd event - _isRateLimited - true (event category) .mockImplementationOnce(() => withinLimit) // 3rd event - _isRateLimited - true (transaction category) @@ -377,6 +387,8 @@ describe('XHRTransport', () => { .mockImplementationOnce(() => beforeLimit) // 1st event - _handleRateLimit .mockImplementationOnce(() => beforeLimit) + // 1st event - _handleRateLimit + .mockImplementationOnce(() => beforeLimit) // 2nd event - _isRateLimited - true .mockImplementationOnce(() => withinLimit) // 3rd event - _isRateLimited - false