diff --git a/packages/node/src/transports/base.ts b/packages/node/src/transports/base.ts index e8522ef0304d..688d0511e6cb 100644 --- a/packages/node/src/transports/base.ts +++ b/packages/node/src/transports/base.ts @@ -1,5 +1,15 @@ import { API, SDK_VERSION } from '@sentry/core'; -import { DsnProtocol, Event, Response, SentryRequest, Status, Transport, TransportOptions } from '@sentry/types'; +import { + DsnProtocol, + Event, + Response, + SentryRequest, + SentryRequestType, + Session, + Status, + Transport, + TransportOptions, +} from '@sentry/types'; import { logger, parseRetryAfterHeader, PromiseBuffer, SentryError } from '@sentry/utils'; import * as fs from 'fs'; import * as http from 'http'; @@ -34,6 +44,14 @@ export interface HTTPModule { // ): http.ClientRequest; } +const CATEGORY_MAPPING: { + [key in SentryRequestType]: string; +} = { + event: 'error', + transaction: 'transaction', + session: 'session', +}; + /** Base Transport class implementation */ export abstract class BaseTransport implements Transport { /** The Agent used for corresponding transport */ @@ -48,8 +66,8 @@ export abstract class BaseTransport implements Transport { /** A simple buffer holding all requests. */ protected readonly _buffer: PromiseBuffer = new PromiseBuffer(30); - /** Locks transport after receiving 429 response */ - private _disabledUntil: Date = new Date(Date.now()); + /** Locks transport after receiving rate limits in a response */ + protected readonly _rateLimits: Record = {}; /** Create instance and set this.dsn */ public constructor(public options: TransportOptions) { @@ -123,13 +141,74 @@ export abstract class BaseTransport implements Transport { }; } + /** + * Gets the time that given category is disabled until for rate limiting + */ + protected _disabledUntil(requestType: SentryRequestType): Date { + const category = CATEGORY_MAPPING[requestType]; + return this._rateLimits[category] || this._rateLimits.all; + } + + /** + * Checks if a category is rate limited + */ + 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] && parameters[1].split(';')) || ['all']) { + // categoriesAllowed is added here to ensure we are only storing rate limits for categories we support in this + // sdk and any categories that are not supported will not be added redundantly to the rateLimits object + const categoriesAllowed = [ + ...(Object.keys(CATEGORY_MAPPING) as [SentryRequestType]).map(k => CATEGORY_MAPPING[k]), + 'all', + ]; + if (categoriesAllowed.includes(category)) this._rateLimits[category] = new Date(now + delay); + } + } + return true; + } else if (raHeader) { + this._rateLimits.all = new Date(now + parseRetryAfterHeader(now, raHeader)); + return true; + } + return false; + } + /** JSDoc */ - protected async _send(sentryReq: SentryRequest): Promise { + protected async _send(sentryReq: SentryRequest, originalPayload?: Event | Session): Promise { if (!this.module) { throw new SentryError('No module available'); } - if (new Date(Date.now()) < this._disabledUntil) { - return Promise.reject(new SentryError(`Transport locked till ${this._disabledUntil} due to too many requests.`)); + if (originalPayload && this._isRateLimited(sentryReq.type)) { + return Promise.reject({ + payload: originalPayload, + type: sentryReq.type, + reason: `Transport locked till ${this._disabledUntil(sentryReq.type)} due to too many requests.`, + status: 429, + }); } if (!this._buffer.isReady()) { @@ -147,26 +226,31 @@ export abstract class BaseTransport implements Transport { res.setEncoding('utf8'); + /** + * "Key-value pairs of header names and values. Header names are lower-cased." + * https://nodejs.org/api/http.html#http_message_headers + */ + let retryAfterHeader = res.headers ? res.headers['retry-after'] : ''; + retryAfterHeader = (Array.isArray(retryAfterHeader) ? retryAfterHeader[0] : retryAfterHeader) as string; + + let rlHeader = res.headers ? res.headers['x-sentry-rate-limits'] : ''; + rlHeader = (Array.isArray(rlHeader) ? rlHeader[0] : rlHeader) as string; + + const headers = { + 'x-sentry-rate-limits': rlHeader, + 'retry-after': retryAfterHeader, + }; + + const limited = this._handleRateLimit(headers); + if (limited) logger.warn(`Too many requests, backing off until: ${this._disabledUntil(sentryReq.type)}`); + if (status === Status.Success) { resolve({ status }); } else { - if (status === Status.RateLimit) { - const now = Date.now(); - /** - * "Key-value pairs of header names and values. Header names are lower-cased." - * https://nodejs.org/api/http.html#http_message_headers - */ - let retryAfterHeader = res.headers ? res.headers['retry-after'] : ''; - retryAfterHeader = (Array.isArray(retryAfterHeader) ? retryAfterHeader[0] : retryAfterHeader) as string; - this._disabledUntil = new Date(now + parseRetryAfterHeader(now, retryAfterHeader)); - logger.warn(`Too many requests, backing off till: ${this._disabledUntil}`); - } - let rejectionMessage = `HTTP Error (${statusCode})`; if (res.headers && res.headers['x-sentry-error']) { rejectionMessage += `: ${res.headers['x-sentry-error']}`; } - reject(new SentryError(rejectionMessage)); } diff --git a/packages/node/src/transports/http.ts b/packages/node/src/transports/http.ts index f7760cdeb827..c4de15073f7a 100644 --- a/packages/node/src/transports/http.ts +++ b/packages/node/src/transports/http.ts @@ -20,13 +20,13 @@ export class HTTPTransport extends BaseTransport { * @inheritDoc */ public sendEvent(event: Event): Promise { - return this._send(eventToSentryRequest(event, this._api)); + return this._send(eventToSentryRequest(event, this._api), event); } /** * @inheritDoc */ public sendSession(session: Session): PromiseLike { - return this._send(sessionToSentryRequest(session, this._api)); + return this._send(sessionToSentryRequest(session, this._api), session); } } diff --git a/packages/node/src/transports/https.ts b/packages/node/src/transports/https.ts index 982bf73d3ae6..96499504a2fa 100644 --- a/packages/node/src/transports/https.ts +++ b/packages/node/src/transports/https.ts @@ -20,13 +20,13 @@ export class HTTPSTransport extends BaseTransport { * @inheritDoc */ public sendEvent(event: Event): Promise { - return this._send(eventToSentryRequest(event, this._api)); + return this._send(eventToSentryRequest(event, this._api), event); } /** * @inheritDoc */ public sendSession(session: Session): PromiseLike { - return this._send(sessionToSentryRequest(session, this._api)); + return this._send(sessionToSentryRequest(session, this._api), session); } } diff --git a/packages/node/test/transports/http.test.ts b/packages/node/test/transports/http.test.ts index 5721d9498048..5ffd077c9790 100644 --- a/packages/node/test/transports/http.test.ts +++ b/packages/node/test/transports/http.test.ts @@ -1,5 +1,5 @@ import { Session } from '@sentry/hub'; -import { TransportOptions } from '@sentry/types'; +import { Event, SessionStatus, Status, TransportOptions } from '@sentry/types'; import { SentryError } from '@sentry/utils'; import * as http from 'http'; import * as HttpsProxyAgent from 'https-proxy-agent'; @@ -10,6 +10,27 @@ const mockSetEncoding = jest.fn(); const dsn = 'http://9e9fd4523d784609a5fc0ebb1080592f@sentry.io:8989/mysubpath/50622'; const storePath = '/mysubpath/api/50622/store/'; const envelopePath = '/mysubpath/api/50622/envelope/'; +const eventPayload: Event = { + event_id: '1337', +}; +const transactionPayload: Event = { + event_id: '42', + type: 'transaction', +}; +const sessionPayload: Session = { + environment: 'test', + release: '1.0', + sid: '353463243253453254', + errors: 0, + started: Date.now(), + timestamp: Date.now(), + init: true, + duration: 0, + status: SessionStatus.Exited, + update: jest.fn(), + close: jest.fn(), + toJSON: jest.fn(), +}; let mockReturnCode = 200; let mockHeaders = {}; @@ -141,9 +162,12 @@ describe('HTTPTransport', () => { try { await transport.sendEvent({ message: 'test' }); } catch (e) { - expect(e).toEqual( - new SentryError(`Transport locked till ${new Date(now + retryAfterSeconds * 1000)} due to too many requests.`), + expect(e.status).toEqual(429); + expect(e.reason).toEqual( + `Transport locked till ${new Date(now + retryAfterSeconds * 1000)} due to too many requests.`, ); + expect(e.payload.message).toEqual('test'); + expect(e.type).toEqual('event'); } try { @@ -155,6 +179,388 @@ describe('HTTPTransport', () => { mock.mockRestore(); }); + test('back-off using x-sentry-rate-limits with bogus headers and missing categories should just lock them all', async () => { + const retryAfterSeconds = 60; + mockReturnCode = 429; + mockHeaders = { + 'x-sentry-rate-limits': `sgthrthewhertht`, + }; + const transport = createTransport({ dsn }); + const now = Date.now(); + const mock = jest + .spyOn(Date, 'now') + // 1st event - _isRateLimited - false + .mockReturnValueOnce(now) + // 1st event - _handleRateLimit + .mockReturnValueOnce(now) + // 2nd event - _isRateLimited - true (event category) + .mockReturnValueOnce(now + (retryAfterSeconds / 2) * 1000) + // 3rd event - _isRateLimited - true (transaction category) + .mockReturnValueOnce(now + (retryAfterSeconds / 2) * 1000) + // 4th event - _isRateLimited - false (event category) + .mockReturnValueOnce(now + retryAfterSeconds * 1000) + // 4th event - _handleRateLimit + .mockReturnValueOnce(now + retryAfterSeconds * 1000) + // 5th event - _isRateLimited - false (transaction category) + .mockReturnValueOnce(now + retryAfterSeconds * 1000) + // 5th event - _handleRateLimit + .mockReturnValueOnce(now + retryAfterSeconds * 1000); + + try { + await transport.sendEvent(eventPayload); + } catch (e) { + expect(e).toEqual(new SentryError(`HTTP Error (${mockReturnCode})`)); + } + + try { + await transport.sendEvent(eventPayload); + } catch (e) { + expect(e.status).toEqual(429); + expect(e.reason).toEqual( + `Transport locked till ${new Date(now + retryAfterSeconds * 1000)} due to too many requests.`, + ); + expect(e.payload).toEqual(eventPayload); + expect(e.type).toEqual('event'); + } + + try { + await transport.sendEvent(transactionPayload); + } catch (e) { + expect(e.status).toEqual(429); + expect(e.reason).toEqual( + `Transport locked till ${new Date(now + retryAfterSeconds * 1000)} due to too many requests.`, + ); + expect(e.payload).toEqual(transactionPayload); + expect(e.type).toEqual('transaction'); + } + + mockHeaders = {}; + mockReturnCode = 200; + + const eventRes = await transport.sendEvent(eventPayload); + expect(eventRes.status).toEqual(Status.Success); + + const transactionRes = await transport.sendEvent(transactionPayload); + expect(transactionRes.status).toEqual(Status.Success); + + mock.mockRestore(); + }); + + test('back-off using x-sentry-rate-limits with single category', async () => { + const retryAfterSeconds = 10; + mockReturnCode = 429; + mockHeaders = { + 'x-sentry-rate-limits': `${retryAfterSeconds}:error:scope`, + }; + const transport = createTransport({ dsn }); + const now = Date.now(); + const mock = jest + .spyOn(Date, 'now') + // 1st event - _isRateLimited - false + .mockReturnValueOnce(now) + // 1st event - _handleRateLimit + .mockReturnValueOnce(now) + // 2nd event - _isRateLimited - false (different category) + .mockReturnValueOnce(now + (retryAfterSeconds / 2) * 1000) + // 2nd event - _handleRateLimit + .mockReturnValueOnce(now + (retryAfterSeconds / 2) * 1000) + // 3rd event - _isRateLimited - false (different category - sessions) + .mockReturnValueOnce(now + (retryAfterSeconds / 2) * 1000) + // 3rd event - _handleRateLimit + .mockReturnValueOnce(now + (retryAfterSeconds / 2) * 1000) + // 4th event - _isRateLimited - true + .mockReturnValueOnce(now + (retryAfterSeconds / 2) * 1000) + // 5th event - _isRateLimited - false + .mockReturnValueOnce(now + retryAfterSeconds * 1000) + // 5th event - _handleRateLimit + .mockReturnValueOnce(now + retryAfterSeconds * 1000); + + try { + await transport.sendEvent(eventPayload); + } catch (e) { + expect(e).toEqual(new SentryError(`HTTP Error (${mockReturnCode})`)); + } + + mockHeaders = {}; + mockReturnCode = 200; + + const transactionRes = await transport.sendEvent(transactionPayload); + expect(transactionRes.status).toEqual(Status.Success); + + const sessionsRes = await transport.sendSession(sessionPayload); + expect(sessionsRes.status).toEqual(Status.Success); + + try { + await transport.sendEvent(eventPayload); + } catch (e) { + expect(e.status).toEqual(429); + expect(e.reason).toEqual( + `Transport locked till ${new Date(now + retryAfterSeconds * 1000)} due to too many requests.`, + ); + expect(e.payload).toEqual(eventPayload); + expect(e.type).toEqual('event'); + } + + const eventRes = await transport.sendEvent(eventPayload); + expect(eventRes.status).toEqual(Status.Success); + + mock.mockRestore(); + }); + + test('back-off using x-sentry-rate-limits with multiple category', async () => { + const retryAfterSeconds = 10; + mockReturnCode = 429; + mockHeaders = { + 'x-sentry-rate-limits': `${retryAfterSeconds}:error;transaction;session:scope`, + }; + const transport = createTransport({ dsn }); + const now = Date.now(); + const mock = jest + .spyOn(Date, 'now') + // 1st event - _isRateLimited - false + .mockReturnValueOnce(now) + // 1st event - _handleRateLimit + .mockReturnValueOnce(now) + // 2nd event - _isRateLimited - true (event category) + .mockReturnValueOnce(now + (retryAfterSeconds / 2) * 1000) + // 3rd event - _isRateLimited - true (sessions category) + .mockReturnValueOnce(now + (retryAfterSeconds / 2) * 1000) + // 4th event - _isRateLimited - true (transactions category) + .mockReturnValueOnce(now + (retryAfterSeconds / 2) * 1000) + // 5th event - _isRateLimited - false (event category) + .mockReturnValueOnce(now + retryAfterSeconds * 1000) + // 5th event - _handleRateLimit + .mockReturnValueOnce(now + retryAfterSeconds * 1000) + // 6th event - _isRateLimited - false (sessions category) + .mockReturnValueOnce(now + retryAfterSeconds * 1000) + // 6th event - handleRateLimit + .mockReturnValueOnce(now + retryAfterSeconds * 1000) + // 7th event - _isRateLimited - false (transaction category) + .mockReturnValueOnce(now + retryAfterSeconds * 1000) + // 7th event - handleRateLimit + .mockReturnValueOnce(now + retryAfterSeconds * 1000); + + try { + await transport.sendEvent(eventPayload); + } catch (e) { + expect(e).toEqual(new SentryError(`HTTP Error (${mockReturnCode})`)); + } + + try { + await transport.sendEvent(eventPayload); + } catch (e) { + expect(e.status).toEqual(429); + expect(e.reason).toEqual( + `Transport locked till ${new Date(now + retryAfterSeconds * 1000)} due to too many requests.`, + ); + expect(e.payload).toEqual(eventPayload); + expect(e.type).toEqual('event'); + } + + try { + await transport.sendSession(sessionPayload); + } catch (e) { + expect(e.status).toEqual(429); + expect(e.reason).toEqual( + `Transport locked till ${new Date(now + retryAfterSeconds * 1000)} due to too many requests.`, + ); + expect(e.payload.environment).toEqual(sessionPayload.environment); + expect(e.payload.release).toEqual(sessionPayload.release); + expect(e.payload.sid).toEqual(sessionPayload.sid); + expect(e.type).toEqual('session'); + } + + try { + await transport.sendEvent(transactionPayload); + } catch (e) { + expect(e.status).toEqual(429); + expect(e.reason).toEqual( + `Transport locked till ${new Date(now + retryAfterSeconds * 1000)} due to too many requests.`, + ); + expect(e.payload).toEqual(transactionPayload); + expect(e.type).toEqual('transaction'); + } + + mockHeaders = {}; + mockReturnCode = 200; + + const eventRes = await transport.sendEvent(eventPayload); + expect(eventRes.status).toEqual(Status.Success); + + const sessionsRes = await transport.sendSession(sessionPayload); + expect(sessionsRes.status).toEqual(Status.Success); + + const transactionRes = await transport.sendEvent(transactionPayload); + expect(transactionRes.status).toEqual(Status.Success); + + mock.mockRestore(); + }); + + test('back-off using x-sentry-rate-limits with missing categories should lock them all', async () => { + const retryAfterSeconds = 10; + mockReturnCode = 429; + mockHeaders = { + 'x-sentry-rate-limits': `${retryAfterSeconds}::scope`, + }; + const transport = createTransport({ dsn }); + const now = Date.now(); + const mock = jest + .spyOn(Date, 'now') + // 1st event - _isRateLimited - false + .mockReturnValueOnce(now) + // 1st event - _handleRateLimit + .mockReturnValueOnce(now) + // 2nd event - _isRateLimited - true (event category) + .mockReturnValueOnce(now + (retryAfterSeconds / 2) * 1000) + // 3rd event - _isRateLimited - true (transaction category) + .mockReturnValueOnce(now + (retryAfterSeconds / 2) * 1000) + // 4th event - _isRateLimited - false (event category) + .mockReturnValueOnce(now + retryAfterSeconds * 1000) + // 4th event - _handleRateLimit + .mockReturnValueOnce(now + retryAfterSeconds * 1000) + // 5th event - _isRateLimited - false (transaction category) + .mockReturnValueOnce(now + retryAfterSeconds * 1000) + // 5th event - _handleRateLimit + .mockReturnValueOnce(now + retryAfterSeconds * 1000); + + try { + await transport.sendEvent(eventPayload); + } catch (e) { + expect(e).toEqual(new SentryError(`HTTP Error (${mockReturnCode})`)); + } + + try { + await transport.sendEvent(eventPayload); + } catch (e) { + expect(e.status).toEqual(429); + expect(e.reason).toEqual( + `Transport locked till ${new Date(now + retryAfterSeconds * 1000)} due to too many requests.`, + ); + expect(e.payload).toEqual(eventPayload); + expect(e.type).toEqual('event'); + } + + try { + await transport.sendEvent(transactionPayload); + } catch (e) { + expect(e.status).toEqual(429); + expect(e.reason).toEqual( + `Transport locked till ${new Date(now + retryAfterSeconds * 1000)} due to too many requests.`, + ); + expect(e.payload).toEqual(transactionPayload); + expect(e.type).toEqual('transaction'); + } + + mockHeaders = {}; + mockReturnCode = 200; + + const eventRes = await transport.sendEvent(eventPayload); + expect(eventRes.status).toEqual(Status.Success); + + const transactionRes = await transport.sendEvent(transactionPayload); + expect(transactionRes.status).toEqual(Status.Success); + + mock.mockRestore(); + }); + + test('back-off using x-sentry-rate-limits with bogus categories should be dropped', async () => { + const retryAfterSeconds = 10; + mockReturnCode = 429; + mockHeaders = { + 'x-sentry-rate-limits': `${retryAfterSeconds}:error;safegreg;eqwerw:scope`, + }; + const transport = createTransport({ dsn }); + const now = Date.now(); + const mock = jest + .spyOn(Date, 'now') + // 1st event - _isRateLimited - false + .mockReturnValueOnce(now) + // 1st event - _handleRateLimit + .mockReturnValueOnce(now) + // 2nd event - _isRateLimited - true (event category) + .mockReturnValueOnce(now + (retryAfterSeconds / 2) * 1000) + // 3rd event - _isRateLimited - false (transaction category) + .mockReturnValueOnce(now + (retryAfterSeconds / 2) * 1000) + // 3rd Event - _handleRateLimit + .mockReturnValueOnce(now + (retryAfterSeconds / 2) * 1000) + // 4th event - _isRateLimited - false (event category) + .mockReturnValueOnce(now + retryAfterSeconds * 1000) + // 4th event - _handleRateLimit + .mockReturnValueOnce(now + retryAfterSeconds * 1000); + + try { + await transport.sendEvent(eventPayload); + } catch (e) { + expect(e).toEqual(new SentryError(`HTTP Error (${mockReturnCode})`)); + } + + try { + await transport.sendEvent(eventPayload); + } catch (e) { + expect(e.status).toEqual(429); + expect(e.reason).toEqual( + `Transport locked till ${new Date(now + retryAfterSeconds * 1000)} due to too many requests.`, + ); + expect(e.payload).toEqual(eventPayload); + expect(e.type).toEqual('event'); + } + + mockHeaders = {}; + mockReturnCode = 200; + + const transactionRes = await transport.sendEvent(transactionPayload); + expect(transactionRes.status).toEqual(Status.Success); + + const eventRes = await transport.sendEvent(eventPayload); + expect(eventRes.status).toEqual(Status.Success); + + mock.mockRestore(); + }); + + test('back-off using x-sentry-rate-limits should also trigger for 200 responses', async () => { + const retryAfterSeconds = 10; + mockReturnCode = 200; + mockHeaders = { + 'x-sentry-rate-limits': `${retryAfterSeconds}:error;transaction:scope`, + }; + const transport = createTransport({ dsn }); + const now = Date.now(); + const mock = jest + .spyOn(Date, 'now') + // 1st event - _isRateLimited - false + .mockReturnValueOnce(now) + // 1st event - _handleRateLimit + .mockReturnValueOnce(now) + // 2nd event - _isRateLimited - true + .mockReturnValueOnce(now + (retryAfterSeconds / 2) * 1000) + // 3rd event - _isRateLimited - false + .mockReturnValueOnce(now + retryAfterSeconds * 1000) + // 3rd event - _handleRateLimit + .mockReturnValueOnce(now + retryAfterSeconds * 1000); + + let eventRes = await transport.sendEvent(eventPayload); + expect(eventRes.status).toEqual(Status.Success); + + try { + await transport.sendEvent(eventPayload); + } catch (e) { + expect(e.status).toEqual(429); + expect(e.reason).toEqual( + `Transport locked till ${new Date(now + retryAfterSeconds * 1000)} due to too many requests.`, + ); + expect(e.payload).toEqual(eventPayload); + expect(e.type).toEqual('event'); + } + + mockReturnCode = 200; + mockHeaders = {}; + + eventRes = await transport.sendEvent(eventPayload); + expect(eventRes.status).toEqual(Status.Success); + + mock.mockRestore(); + }); + test('transport options', async () => { mockReturnCode = 200; const transport = createTransport({ diff --git a/packages/node/test/transports/https.test.ts b/packages/node/test/transports/https.test.ts index bd413ff99536..eb3bd8679ab2 100644 --- a/packages/node/test/transports/https.test.ts +++ b/packages/node/test/transports/https.test.ts @@ -147,9 +147,12 @@ describe('HTTPSTransport', () => { try { await transport.sendEvent({ message: 'test' }); } catch (e) { - expect(e).toEqual( - new SentryError(`Transport locked till ${new Date(now + retryAfterSeconds * 1000)} due to too many requests.`), + expect(e.status).toEqual(429); + expect(e.reason).toEqual( + `Transport locked till ${new Date(now + retryAfterSeconds * 1000)} due to too many requests.`, ); + expect(e.payload.message).toEqual('test'); + expect(e.type).toEqual('event'); } try {