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 1c8aa83cba7e059d3e7ea449ad8f3c09e8163264 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Mon, 7 Mar 2022 07:53:41 -0500 Subject: [PATCH 09/12] Update packages/utils/test/ratelimit.test.ts Co-authored-by: Katie Byers --- 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 e69a9ff9d0c4..e3be9866d68a 100644 --- a/packages/utils/test/ratelimit.test.ts +++ b/packages/utils/test/ratelimit.test.ts @@ -85,7 +85,7 @@ describe('updateRateLimits()', () => { expect(updatedRateLimits.error).toEqual(13 * 1000); }); - test('should update a multiple categories based on `x-sentry-rate-limits` header', () => { + test('should update multiple categories based on `x-sentry-rate-limits` header', () => { const rateLimits: RateLimits = {}; const headers = { 'x-sentry-rate-limits': '13:error;transaction', From ba74718dd2fdc2df04ec3f55cc086b5ff613dee4 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Mon, 7 Mar 2022 07:53:52 -0500 Subject: [PATCH 10/12] Update packages/utils/test/ratelimit.test.ts Co-authored-by: Katie Byers --- 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 e3be9866d68a..b057e174f45d 100644 --- a/packages/utils/test/ratelimit.test.ts +++ b/packages/utils/test/ratelimit.test.ts @@ -133,7 +133,7 @@ describe('updateRateLimits()', () => { expect(updatedRateLimits.all).toEqual(60 * 1000); }); - test('should preserve previous limits', () => { + test('should preserve limits for categories not in header', () => { const rateLimits: RateLimits = { error: 1337, }; From 18f49b0ef2d27315f4d81b02848a6dd42f149e08 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Mon, 7 Mar 2022 07:53:58 -0500 Subject: [PATCH 11/12] Update packages/utils/test/ratelimit.test.ts Co-authored-by: Katie Byers --- 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 b057e174f45d..e835154ded30 100644 --- a/packages/utils/test/ratelimit.test.ts +++ b/packages/utils/test/ratelimit.test.ts @@ -95,7 +95,7 @@ describe('updateRateLimits()', () => { expect(updatedRateLimits.transaction).toEqual(13 * 1000); }); - test('should update limits based on multi `x-sentry-rate-limits` header', () => { + test('should update multiple categories with different values based on multi `x-sentry-rate-limits` header', () => { const rateLimits: RateLimits = {}; const headers = { 'x-sentry-rate-limits': '13:error,15:transaction', From ad823635e916b483e08ddc274abf193c05bf0249 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Mon, 7 Mar 2022 07:58:03 -0500 Subject: [PATCH 12/12] PR review --- packages/utils/src/ratelimit.ts | 12 ++++++------ packages/utils/test/ratelimit.test.ts | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/utils/src/ratelimit.ts b/packages/utils/src/ratelimit.ts index a9d824ff00fd..59906c0abdaf 100644 --- a/packages/utils/src/ratelimit.ts +++ b/packages/utils/src/ratelimit.ts @@ -52,10 +52,10 @@ export function updateRateLimits( // "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']; + const rateLimitHeader = headers['x-sentry-rate-limits']; + const retryAfterHeader = headers['retry-after']; - if (rlHeader) { + if (rateLimitHeader) { /** * rate limit headers are of the form *
,
,.. @@ -68,7 +68,7 @@ export function updateRateLimits( * 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(',')) { + for (const limit of rateLimitHeader.trim().split(',')) { const parameters = limit.split(':', 2); const headerDelay = parseInt(parameters[0], 10); const delay = (!isNaN(headerDelay) ? headerDelay : 60) * 1000; // 60sec default @@ -80,8 +80,8 @@ export function updateRateLimits( } } } - } else if (raHeader) { - updatedRateLimits.all = now + parseRetryAfterHeader(raHeader, now); + } else if (retryAfterHeader) { + updatedRateLimits.all = now + parseRetryAfterHeader(retryAfterHeader, now); } return updatedRateLimits; diff --git a/packages/utils/test/ratelimit.test.ts b/packages/utils/test/ratelimit.test.ts index e835154ded30..fec1b3413ae7 100644 --- a/packages/utils/test/ratelimit.test.ts +++ b/packages/utils/test/ratelimit.test.ts @@ -64,7 +64,7 @@ describe('updateRateLimits()', () => { }; const headers = {}; const updatedRateLimits = updateRateLimits(rateLimits, headers); - expect(rateLimits).toStrictEqual(updatedRateLimits); + expect(updatedRateLimits).toEqual(rateLimits); }); test('should update the `all` category based on `retry-after` header ', () => {