From de2822b2c5747efe6b3b94695c8a656cfe4b204b Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Thu, 28 Apr 2022 16:03:22 +0200 Subject: [PATCH] feat(transport): Add client report hook to `makeTransport` (#5008) --- packages/browser/src/transports/fetch.ts | 2 +- packages/browser/src/transports/xhr.ts | 11 +- .../unit/helper/browser-client-options.ts | 2 +- .../test/unit/mocks/simpletransport.ts | 2 +- packages/browser/test/unit/sdk.test.ts | 2 +- .../test/unit/transports/fetch.test.ts | 1 + .../browser/test/unit/transports/xhr.test.ts | 1 + packages/core/src/baseclient.ts | 6 +- packages/core/src/transports/base.ts | 67 +++- .../core/test/lib/transports/base.test.ts | 332 ++++++++---------- packages/core/test/mocks/client.ts | 6 +- packages/core/test/mocks/transport.ts | 2 +- packages/node/src/transports/http.ts | 2 +- .../node/test/helper/node-client-options.ts | 2 +- .../manual/express-scope-separation/start.js | 4 +- .../aggregates-disable-single-session.js | 4 +- .../caught-exception-errored-session.js | 2 +- .../errors-in-session-capped-to-one.js | 2 +- .../single-session/healthy-session.js | 12 +- .../terminal-state-sessions-sent-once.js | 2 +- .../uncaught-exception-crashed-session.js | 4 +- .../unhandled-rejection-crashed-session.js | 2 +- .../node/test/manual/webpack-domain/index.js | 4 +- packages/node/test/transports/http.test.ts | 29 +- packages/node/test/transports/https.test.ts | 33 +- packages/tracing/test/testutils.ts | 2 +- packages/types/src/envelope.ts | 12 +- packages/types/src/index.ts | 2 + packages/types/src/transport.ts | 4 + packages/utils/src/envelope.ts | 34 +- packages/utils/src/ratelimit.ts | 2 +- packages/utils/test/envelope.test.ts | 28 +- 32 files changed, 348 insertions(+), 272 deletions(-) diff --git a/packages/browser/src/transports/fetch.ts b/packages/browser/src/transports/fetch.ts index f500f33b22d6..d6f44090abb9 100644 --- a/packages/browser/src/transports/fetch.ts +++ b/packages/browser/src/transports/fetch.ts @@ -30,5 +30,5 @@ export function makeFetchTransport( })); } - return createTransport({ bufferSize: options.bufferSize }, makeRequest); + return createTransport(options, makeRequest); } diff --git a/packages/browser/src/transports/xhr.ts b/packages/browser/src/transports/xhr.ts index a08d51cf5a9a..4c4982225b85 100644 --- a/packages/browser/src/transports/xhr.ts +++ b/packages/browser/src/transports/xhr.ts @@ -21,17 +21,20 @@ export interface XHRTransportOptions extends BaseTransportOptions { */ export function makeXHRTransport(options: XHRTransportOptions): Transport { function makeRequest(request: TransportRequest): PromiseLike { - return new SyncPromise((resolve, _reject) => { + return new SyncPromise((resolve, reject) => { const xhr = new XMLHttpRequest(); + xhr.onerror = reject; + xhr.onreadystatechange = (): void => { if (xhr.readyState === XHR_READYSTATE_DONE) { - resolve({ + const response = { headers: { 'x-sentry-rate-limits': xhr.getResponseHeader('X-Sentry-Rate-Limits'), 'retry-after': xhr.getResponseHeader('Retry-After'), }, - }); + }; + resolve(response); } }; @@ -47,5 +50,5 @@ export function makeXHRTransport(options: XHRTransportOptions): Transport { }); } - return createTransport({ bufferSize: options.bufferSize }, makeRequest); + return createTransport(options, makeRequest); } diff --git a/packages/browser/test/unit/helper/browser-client-options.ts b/packages/browser/test/unit/helper/browser-client-options.ts index eb4d78768b94..9bdaf2518d40 100644 --- a/packages/browser/test/unit/helper/browser-client-options.ts +++ b/packages/browser/test/unit/helper/browser-client-options.ts @@ -6,7 +6,7 @@ import { BrowserClientOptions } from '../../../src/client'; export function getDefaultBrowserClientOptions(options: Partial = {}): BrowserClientOptions { return { integrations: [], - transport: () => createTransport({}, _ => resolvedSyncPromise({})), + transport: () => createTransport({ recordDroppedEvent: () => undefined }, _ => resolvedSyncPromise({})), stackParser: () => [], ...options, }; diff --git a/packages/browser/test/unit/mocks/simpletransport.ts b/packages/browser/test/unit/mocks/simpletransport.ts index a9c2d5264ae6..cbad94fb310d 100644 --- a/packages/browser/test/unit/mocks/simpletransport.ts +++ b/packages/browser/test/unit/mocks/simpletransport.ts @@ -2,5 +2,5 @@ import { createTransport } from '@sentry/core'; import { resolvedSyncPromise } from '@sentry/utils'; export function makeSimpleTransport() { - return createTransport({}, () => resolvedSyncPromise({})); + return createTransport({ recordDroppedEvent: () => undefined }, () => resolvedSyncPromise({})); } diff --git a/packages/browser/test/unit/sdk.test.ts b/packages/browser/test/unit/sdk.test.ts index b59d380e8e76..a2df869fe90d 100644 --- a/packages/browser/test/unit/sdk.test.ts +++ b/packages/browser/test/unit/sdk.test.ts @@ -15,7 +15,7 @@ const PUBLIC_DSN = 'https://username@domain/123'; function getDefaultBrowserOptions(options: Partial = {}): BrowserOptions { return { integrations: [], - transport: () => createTransport({}, _ => resolvedSyncPromise({})), + transport: () => createTransport({ recordDroppedEvent: () => undefined }, _ => resolvedSyncPromise({})), stackParser: () => [], ...options, }; diff --git a/packages/browser/test/unit/transports/fetch.test.ts b/packages/browser/test/unit/transports/fetch.test.ts index 29b7b241c8dd..63fe62814628 100644 --- a/packages/browser/test/unit/transports/fetch.test.ts +++ b/packages/browser/test/unit/transports/fetch.test.ts @@ -6,6 +6,7 @@ import { FetchImpl } from '../../../src/transports/utils'; const DEFAULT_FETCH_TRANSPORT_OPTIONS: FetchTransportOptions = { url: 'https://sentry.io/api/42/store/?sentry_key=123&sentry_version=7', + recordDroppedEvent: () => undefined, }; const ERROR_ENVELOPE = createEnvelope({ event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2', sent_at: '123' }, [ diff --git a/packages/browser/test/unit/transports/xhr.test.ts b/packages/browser/test/unit/transports/xhr.test.ts index ae144480de08..35ca842db3d1 100644 --- a/packages/browser/test/unit/transports/xhr.test.ts +++ b/packages/browser/test/unit/transports/xhr.test.ts @@ -5,6 +5,7 @@ import { makeXHRTransport, XHRTransportOptions } from '../../../src/transports/x const DEFAULT_XHR_TRANSPORT_OPTIONS: XHRTransportOptions = { url: 'https://sentry.io/api/42/store/?sentry_key=123&sentry_version=7', + recordDroppedEvent: () => undefined, }; const ERROR_ENVELOPE = createEnvelope({ event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2', sent_at: '123' }, [ diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index 1969f2d8ef0a..c50403ab3903 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -97,7 +97,11 @@ export abstract class BaseClient implements Client { if (options.dsn) { this._dsn = makeDsn(options.dsn); const url = getEnvelopeEndpointWithUrlEncodedAuth(this._dsn, options.tunnel); - this._transport = options.transport({ ...options.transportOptions, url }); + this._transport = options.transport({ + recordDroppedEvent: () => undefined, // TODO(v7): Provide a proper function instead of noop + ...options.transportOptions, + url, + }); } else { IS_DEBUG_BUILD && logger.warn('No DSN provided, client will not do anything.'); } diff --git a/packages/core/src/transports/base.ts b/packages/core/src/transports/base.ts index 0491d7422f6b..4e6c04e8c516 100644 --- a/packages/core/src/transports/base.ts +++ b/packages/core/src/transports/base.ts @@ -1,21 +1,28 @@ import { - DataCategory, Envelope, + EnvelopeItem, + EventDropReason, InternalBaseTransportOptions, Transport, TransportRequestExecutor, } from '@sentry/types'; import { - getEnvelopeType, + createEnvelope, + envelopeItemTypeToDataCategory, + forEachEnvelopeItem, isRateLimited, + logger, makePromiseBuffer, PromiseBuffer, RateLimits, resolvedSyncPromise, + SentryError, serializeEnvelope, updateRateLimits, } from '@sentry/utils'; +import { IS_DEBUG_BUILD } from '../flags'; + export const DEFAULT_TRANSPORT_BUFFER_SIZE = 30; /** @@ -34,22 +41,58 @@ export function createTransport( const flush = (timeout?: number): PromiseLike => buffer.drain(timeout); function send(envelope: Envelope): PromiseLike { - const envCategory = getEnvelopeType(envelope); - const category = envCategory === 'event' ? 'error' : (envCategory as DataCategory); + const filteredEnvelopeItems: EnvelopeItem[] = []; + + // Drop rate limited items from envelope + forEachEnvelopeItem(envelope, (item, type) => { + const envelopeItemDataCategory = envelopeItemTypeToDataCategory(type); + if (isRateLimited(rateLimits, envelopeItemDataCategory)) { + options.recordDroppedEvent('ratelimit_backoff', envelopeItemDataCategory); + } else { + filteredEnvelopeItems.push(item); + } + }); - // Don't add to buffer if transport is already rate-limited - if (isRateLimited(rateLimits, category)) { + // Skip sending if envelope is empty after filtering out rate limited events + if (filteredEnvelopeItems.length === 0) { return resolvedSyncPromise(); } - const requestTask = (): PromiseLike => - makeRequest({ body: serializeEnvelope(envelope) }).then(({ headers }): void => { - if (headers) { - rateLimits = updateRateLimits(rateLimits, headers); - } + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const filteredEnvelope: Envelope = createEnvelope(envelope[0], filteredEnvelopeItems as any); + + // Creates client report for each item in an envelope + const recordEnvelopeLoss = (reason: EventDropReason): void => { + forEachEnvelopeItem(filteredEnvelope, (_, type) => { + options.recordDroppedEvent(reason, envelopeItemTypeToDataCategory(type)); }); + }; + + const requestTask = (): PromiseLike => + makeRequest({ body: serializeEnvelope(filteredEnvelope) }).then( + ({ headers }): void => { + if (headers) { + rateLimits = updateRateLimits(rateLimits, headers); + } + }, + error => { + IS_DEBUG_BUILD && logger.error('Failed while recording event:', error); + recordEnvelopeLoss('network_error'); + }, + ); - return buffer.add(requestTask); + return buffer.add(requestTask).then( + result => result, + error => { + if (error instanceof SentryError) { + IS_DEBUG_BUILD && logger.error('Skipped sending event due to full buffer'); + recordEnvelopeLoss('queue_overflow'); + return resolvedSyncPromise(); + } else { + throw error; + } + }, + ); } return { diff --git a/packages/core/test/lib/transports/base.test.ts b/packages/core/test/lib/transports/base.test.ts index 39166a1a06c8..b57cc36f6e6f 100644 --- a/packages/core/test/lib/transports/base.test.ts +++ b/packages/core/test/lib/transports/base.test.ts @@ -1,4 +1,4 @@ -import { EventEnvelope, EventItem, Transport, TransportMakeRequestResponse } from '@sentry/types'; +import { EventEnvelope, EventItem, TransportMakeRequestResponse } from '@sentry/types'; import { createEnvelope, PromiseBuffer, resolvedSyncPromise, serializeEnvelope } from '@sentry/utils'; import { createTransport } from '../../../src/transports/base'; @@ -12,6 +12,10 @@ const TRANSACTION_ENVELOPE = createEnvelope( [[{ type: 'transaction' }, { event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2' }] as EventItem], ); +const transportOptions = { + recordDroppedEvent: () => undefined, // noop +}; + describe('createTransport', () => { it('flushes the buffer', async () => { const mockBuffer: PromiseBuffer = { @@ -19,7 +23,7 @@ describe('createTransport', () => { add: jest.fn(), drain: jest.fn(), }; - const transport = createTransport({}, _ => resolvedSyncPromise({}), mockBuffer); + const transport = createTransport(transportOptions, _ => resolvedSyncPromise({}), mockBuffer); /* eslint-disable @typescript-eslint/unbound-method */ expect(mockBuffer.drain).toHaveBeenCalledTimes(0); await transport.flush(1000); @@ -31,7 +35,7 @@ describe('createTransport', () => { describe('send', () => { it('constructs a request to send to Sentry', async () => { expect.assertions(1); - const transport = createTransport({}, req => { + const transport = createTransport(transportOptions, req => { expect(req.body).toEqual(serializeEnvelope(ERROR_ENVELOPE)); return resolvedSyncPromise({}); }); @@ -41,7 +45,7 @@ describe('createTransport', () => { it('does throw if request fails', async () => { expect.assertions(2); - const transport = createTransport({}, req => { + const transport = createTransport(transportOptions, req => { expect(req.body).toEqual(serializeEnvelope(ERROR_ENVELOPE)); throw new Error(); }); @@ -51,8 +55,7 @@ describe('createTransport', () => { }).toThrow(); }); - // TODO(v7): Add tests back in and test by using client report logic - describe.skip('Rate-limiting', () => { + describe('Rate-limiting', () => { function setRateLimitTimes(): { retryAfterSeconds: number; beforeLimit: number; @@ -66,265 +69,208 @@ describe('createTransport', () => { return { retryAfterSeconds, beforeLimit, withinLimit, afterLimit }; } - function createTestTransport( - initialTransportResponse: TransportMakeRequestResponse, - ): [Transport, (res: TransportMakeRequestResponse) => void] { + function createTestTransport(initialTransportResponse: TransportMakeRequestResponse) { let transportResponse: TransportMakeRequestResponse = initialTransportResponse; function setTransportResponse(res: TransportMakeRequestResponse) { transportResponse = res; } - const transport = createTransport({}, _ => { + const mockRequestExecutor = jest.fn(_ => { return resolvedSyncPromise(transportResponse); }); - return [transport, setTransportResponse]; + const mockRecordDroppedEventCallback = jest.fn(); + + const transport = createTransport({ recordDroppedEvent: mockRecordDroppedEventCallback }, mockRequestExecutor); + + return [transport, setTransportResponse, mockRequestExecutor, mockRecordDroppedEventCallback] as const; } - it('back-off using Retry-After header', async () => { + it('back-off _after_ Retry-After header was received', async () => { const { retryAfterSeconds, beforeLimit, withinLimit, afterLimit } = setRateLimitTimes(); + const [transport, setTransportResponse, requestExecutor, recordDroppedEventCallback] = createTestTransport({}); + + const dateNowSpy = jest.spyOn(Date, 'now').mockImplementation(() => beforeLimit); - jest - .spyOn(Date, 'now') - // 1st event - isRateLimited - FALSE - .mockImplementationOnce(() => beforeLimit) - // 1st event - updateRateLimits - .mockImplementationOnce(() => beforeLimit) - // 2nd event - isRateLimited - TRUE - .mockImplementationOnce(() => withinLimit) - // 3rd event - isRateLimited - FALSE - .mockImplementationOnce(() => afterLimit) - // 3rd event - updateRateLimits - .mockImplementationOnce(() => afterLimit); - - const [transport, setTransportResponse] = createTestTransport({ + await transport.send(ERROR_ENVELOPE); + expect(requestExecutor).toHaveBeenCalledTimes(1); + expect(recordDroppedEventCallback).not.toHaveBeenCalled(); + requestExecutor.mockClear(); + recordDroppedEventCallback.mockClear(); + + setTransportResponse({ headers: { 'x-sentry-rate-limits': null, 'retry-after': `${retryAfterSeconds}`, }, }); - try { - await transport.send(ERROR_ENVELOPE); - } catch (res) { - expect(res.status).toBe('rate_limit'); - expect(res.reason).toBe(`Too many error requests, backing off until: ${new Date(afterLimit).toISOString()}`); - } + await transport.send(ERROR_ENVELOPE); + expect(requestExecutor).toHaveBeenCalledTimes(1); + expect(recordDroppedEventCallback).not.toHaveBeenCalled(); + requestExecutor.mockClear(); + recordDroppedEventCallback.mockClear(); - setTransportResponse({}); + // act like were in the rate limited period + dateNowSpy.mockImplementation(() => withinLimit); - try { - await transport.send(ERROR_ENVELOPE); - } catch (res) { - expect(res.status).toBe('rate_limit'); - expect(res.reason).toBe(`Too many error requests, backing off until: ${new Date(afterLimit).toISOString()}`); - } + await transport.send(ERROR_ENVELOPE); + expect(requestExecutor).not.toHaveBeenCalled(); + expect(recordDroppedEventCallback).toHaveBeenCalledWith('ratelimit_backoff', 'error'); + requestExecutor.mockClear(); + recordDroppedEventCallback.mockClear(); + + // act like it's after the rate limited period + dateNowSpy.mockImplementation(() => afterLimit); await transport.send(ERROR_ENVELOPE); + expect(requestExecutor).toHaveBeenCalledTimes(1); + expect(recordDroppedEventCallback).not.toHaveBeenCalled(); }); it('back-off using X-Sentry-Rate-Limits with single category', async () => { const { retryAfterSeconds, beforeLimit, withinLimit, afterLimit } = setRateLimitTimes(); - - jest - .spyOn(Date, 'now') - // 1st event - isRateLimited - FALSE - .mockImplementationOnce(() => beforeLimit) - // 1st event - updateRateLimits - .mockImplementationOnce(() => beforeLimit) - // 2nd event - isRateLimited - FALSE (different category) - .mockImplementationOnce(() => withinLimit) - // 3rd event - isRateLimited - TRUE - .mockImplementationOnce(() => withinLimit) - // 4th event - isRateLimited - FALSE - .mockImplementationOnce(() => afterLimit) - // 4th event - updateRateLimits - .mockImplementationOnce(() => afterLimit); - - const [transport, setTransportResponse] = createTestTransport({ + const [transport, setTransportResponse, requestExecutor, recordDroppedEventCallback] = createTestTransport({ headers: { 'x-sentry-rate-limits': `${retryAfterSeconds}:error:scope`, 'retry-after': null, }, }); - try { - await transport.send(ERROR_ENVELOPE); - } catch (res) { - expect(res.status).toBe('rate_limit'); - expect(res.reason).toBe(`Too many error requests, backing off until: ${new Date(afterLimit).toISOString()}`); - } + const dateNowSpy = jest.spyOn(Date, 'now').mockImplementation(() => beforeLimit); + + await transport.send(ERROR_ENVELOPE); + expect(requestExecutor).toHaveBeenCalledTimes(1); + expect(recordDroppedEventCallback).not.toHaveBeenCalled(); + requestExecutor.mockClear(); + recordDroppedEventCallback.mockClear(); setTransportResponse({}); - try { - await transport.send(TRANSACTION_ENVELOPE); - } catch (res) { - expect(res.status).toBe('rate_limit'); - expect(res.reason).toBe(`Too many error requests, backing off until: ${new Date(afterLimit).toISOString()}`); - } + // act like were in the rate limited period + dateNowSpy.mockImplementation(() => withinLimit); - try { - await transport.send(ERROR_ENVELOPE); - } catch (res) { - expect(res.status).toBe('rate_limit'); - expect(res.reason).toBe(`Too many error requests, backing off until: ${new Date(afterLimit).toISOString()}`); - } + await transport.send(TRANSACTION_ENVELOPE); // Transaction envelope should be sent + expect(requestExecutor).toHaveBeenCalledTimes(1); + expect(recordDroppedEventCallback).not.toHaveBeenCalled(); + requestExecutor.mockClear(); + recordDroppedEventCallback.mockClear(); + + await transport.send(ERROR_ENVELOPE); // Error envelope should not be sent because of pending rate limit + expect(requestExecutor).not.toHaveBeenCalled(); + expect(recordDroppedEventCallback).toHaveBeenCalledWith('ratelimit_backoff', 'error'); + requestExecutor.mockClear(); + recordDroppedEventCallback.mockClear(); + + // act like it's after the rate limited period + dateNowSpy.mockImplementation(() => afterLimit); await transport.send(TRANSACTION_ENVELOPE); + expect(requestExecutor).toHaveBeenCalledTimes(1); + expect(recordDroppedEventCallback).not.toHaveBeenCalled(); + requestExecutor.mockClear(); + recordDroppedEventCallback.mockClear(); + + await transport.send(ERROR_ENVELOPE); + expect(requestExecutor).toHaveBeenCalledTimes(1); + expect(recordDroppedEventCallback).not.toHaveBeenCalled(); }); it('back-off using X-Sentry-Rate-Limits with multiple categories', async () => { const { retryAfterSeconds, beforeLimit, withinLimit, afterLimit } = setRateLimitTimes(); - - jest - .spyOn(Date, 'now') - // 1st event - isRateLimited - FALSE - .mockImplementationOnce(() => beforeLimit) - // 1st event - updateRateLimits - .mockImplementationOnce(() => beforeLimit) - // 2nd event - isRateLimited - TRUE (different category) - .mockImplementationOnce(() => withinLimit) - // 3rd event - isRateLimited - TRUE - .mockImplementationOnce(() => withinLimit) - // 4th event - isRateLimited - FALSE - .mockImplementationOnce(() => afterLimit) - // 4th event - updateRateLimits - .mockImplementationOnce(() => afterLimit); - - const [transport, setTransportResponse] = createTestTransport({ + const [transport, setTransportResponse, requestExecutor, recordDroppedEventCallback] = createTestTransport({ headers: { 'x-sentry-rate-limits': `${retryAfterSeconds}:error;transaction:scope`, 'retry-after': null, }, }); - try { - await transport.send(ERROR_ENVELOPE); - } catch (res) { - expect(res.status).toBe('rate_limit'); - expect(res.reason).toBe(`Too many error requests, backing off until: ${new Date(afterLimit).toISOString()}`); - } + const dateNowSpy = jest.spyOn(Date, 'now').mockImplementation(() => beforeLimit); - try { - await transport.send(ERROR_ENVELOPE); - } catch (res) { - expect(res.status).toBe('rate_limit'); - expect(res.reason).toBe(`Too many error requests, backing off until: ${new Date(afterLimit).toISOString()}`); - } - - try { - await transport.send(TRANSACTION_ENVELOPE); - } catch (res) { - expect(res.status).toBe('rate_limit'); - expect(res.reason).toBe( - `Too many transaction requests, backing off until: ${new Date(afterLimit).toISOString()}`, - ); - } + await transport.send(ERROR_ENVELOPE); + expect(requestExecutor).toHaveBeenCalledTimes(1); + expect(recordDroppedEventCallback).not.toHaveBeenCalled(); + requestExecutor.mockClear(); + recordDroppedEventCallback.mockClear(); setTransportResponse({}); - await transport.send(ERROR_ENVELOPE); + // act like were in the rate limited period + dateNowSpy.mockImplementation(() => withinLimit); + + await transport.send(TRANSACTION_ENVELOPE); // Transaction envelope should not be sent because of pending rate limit + expect(requestExecutor).not.toHaveBeenCalled(); + expect(recordDroppedEventCallback).toHaveBeenCalledWith('ratelimit_backoff', 'transaction'); + requestExecutor.mockClear(); + recordDroppedEventCallback.mockClear(); + + await transport.send(ERROR_ENVELOPE); // Error envelope should not be sent because of pending rate limit + expect(requestExecutor).not.toHaveBeenCalled(); + expect(recordDroppedEventCallback).toHaveBeenCalledWith('ratelimit_backoff', 'error'); + requestExecutor.mockClear(); + recordDroppedEventCallback.mockClear(); + + // act like it's after the rate limited period + dateNowSpy.mockImplementation(() => afterLimit); + await transport.send(TRANSACTION_ENVELOPE); + expect(requestExecutor).toHaveBeenCalledTimes(1); + expect(recordDroppedEventCallback).not.toHaveBeenCalled(); + requestExecutor.mockClear(); + recordDroppedEventCallback.mockClear(); + + await transport.send(ERROR_ENVELOPE); + expect(requestExecutor).toHaveBeenCalledTimes(1); + expect(recordDroppedEventCallback).not.toHaveBeenCalled(); }); it('back-off using X-Sentry-Rate-Limits with missing categories should lock them all', async () => { const { retryAfterSeconds, beforeLimit, withinLimit, afterLimit } = setRateLimitTimes(); - - jest - .spyOn(Date, 'now') - // 1st event - isRateLimited - false - .mockImplementationOnce(() => beforeLimit) - // 1st event - updateRateLimits - .mockImplementationOnce(() => beforeLimit) - // 2nd event - isRateLimited - true (event category) - .mockImplementationOnce(() => withinLimit) - // 3rd event - isRateLimited - true (transaction category) - .mockImplementationOnce(() => withinLimit) - // 4th event - isRateLimited - false (event category) - .mockImplementationOnce(() => afterLimit) - // 4th event - updateRateLimits - .mockImplementationOnce(() => afterLimit) - // 5th event - isRateLimited - false (transaction category) - .mockImplementationOnce(() => afterLimit) - // 5th event - updateRateLimits - .mockImplementationOnce(() => afterLimit); - - const [transport, setTransportResponse] = createTestTransport({ + const [transport, setTransportResponse, requestExecutor, recordDroppedEventCallback] = createTestTransport({ headers: { 'x-sentry-rate-limits': `${retryAfterSeconds}::scope`, 'retry-after': null, }, }); - try { - await transport.send(ERROR_ENVELOPE); - } catch (res) { - expect(res.status).toBe('rate_limit'); - expect(res.reason).toBe(`Too many error requests, backing off until: ${new Date(afterLimit).toISOString()}`); - } - - try { - await transport.send(ERROR_ENVELOPE); - } catch (res) { - expect(res.status).toBe('rate_limit'); - expect(res.reason).toBe(`Too many error requests, backing off until: ${new Date(afterLimit).toISOString()}`); - } + const dateNowSpy = jest.spyOn(Date, 'now').mockImplementation(() => beforeLimit); - try { - await transport.send(TRANSACTION_ENVELOPE); - } catch (res) { - expect(res.status).toBe('rate_limit'); - expect(res.reason).toBe( - `Too many transaction requests, backing off until: ${new Date(afterLimit).toISOString()}`, - ); - } + await transport.send(ERROR_ENVELOPE); + expect(requestExecutor).toHaveBeenCalledTimes(1); + expect(recordDroppedEventCallback).not.toHaveBeenCalled(); + requestExecutor.mockClear(); + recordDroppedEventCallback.mockClear(); setTransportResponse({}); - await transport.send(ERROR_ENVELOPE); - await transport.send(TRANSACTION_ENVELOPE); - }); + // act like were in the rate limited period + dateNowSpy.mockImplementation(() => withinLimit); - it('back-off using X-Sentry-Rate-Limits should also trigger for 200 responses', async () => { - const { retryAfterSeconds, beforeLimit, withinLimit, afterLimit } = setRateLimitTimes(); + await transport.send(TRANSACTION_ENVELOPE); // Transaction envelope should not be sent because of pending rate limit + expect(requestExecutor).not.toHaveBeenCalled(); + expect(recordDroppedEventCallback).toHaveBeenCalledWith('ratelimit_backoff', 'transaction'); + requestExecutor.mockClear(); + recordDroppedEventCallback.mockClear(); - jest - .spyOn(Date, 'now') - // 1st event - isRateLimited - FALSE - .mockImplementationOnce(() => beforeLimit) - // 1st event - updateRateLimits - .mockImplementationOnce(() => beforeLimit) - // 2nd event - isRateLimited - TRUE - .mockImplementationOnce(() => withinLimit) - // 3rd event - isRateLimited - FALSE - .mockImplementationOnce(() => afterLimit) - // 3rd event - updateRateLimits - .mockImplementationOnce(() => afterLimit); - - const [transport] = createTestTransport({ - headers: { - 'x-sentry-rate-limits': `${retryAfterSeconds}:error;transaction:scope`, - 'retry-after': null, - }, - }); + await transport.send(ERROR_ENVELOPE); // Error envelope should not be sent because of pending rate limit + expect(requestExecutor).not.toHaveBeenCalled(); + expect(recordDroppedEventCallback).toHaveBeenCalledWith('ratelimit_backoff', 'error'); + requestExecutor.mockClear(); + recordDroppedEventCallback.mockClear(); - try { - await transport.send(ERROR_ENVELOPE); - } catch (res) { - expect(res.status).toBe('rate_limit'); - expect(res.reason).toBe(`Too many error requests, backing off until: ${new Date(afterLimit).toISOString()}`); - } + // act like it's after the rate limited period + dateNowSpy.mockImplementation(() => afterLimit); - try { - await transport.send(TRANSACTION_ENVELOPE); - } catch (res) { - expect(res.status).toBe('rate_limit'); - expect(res.reason).toBe( - `Too many transaction requests, backing off until: ${new Date(afterLimit).toISOString()}`, - ); - } + await transport.send(TRANSACTION_ENVELOPE); + expect(requestExecutor).toHaveBeenCalledTimes(1); + expect(recordDroppedEventCallback).not.toHaveBeenCalled(); + requestExecutor.mockClear(); + recordDroppedEventCallback.mockClear(); + + await transport.send(ERROR_ENVELOPE); + expect(requestExecutor).toHaveBeenCalledTimes(1); + expect(recordDroppedEventCallback).not.toHaveBeenCalled(); }); }); }); diff --git a/packages/core/test/mocks/client.ts b/packages/core/test/mocks/client.ts index 737abb7be476..6878a649bf17 100644 --- a/packages/core/test/mocks/client.ts +++ b/packages/core/test/mocks/client.ts @@ -13,7 +13,11 @@ import { createTransport } from '../../src/transports/base'; export function getDefaultTestClientOptions(options: Partial = {}): TestClientOptions { return { integrations: [], - transport: () => createTransport({}, _ => resolvedSyncPromise({})), + transport: () => + createTransport( + { recordDroppedEvent: () => undefined }, // noop + _ => resolvedSyncPromise({}), + ), stackParser: () => [], ...options, }; diff --git a/packages/core/test/mocks/transport.ts b/packages/core/test/mocks/transport.ts index 64b23e1cfd90..f59e72a516a1 100644 --- a/packages/core/test/mocks/transport.ts +++ b/packages/core/test/mocks/transport.ts @@ -10,7 +10,7 @@ export function makeFakeTransport(delay: number = 2000) { let sendCalled = 0; let sentCount = 0; const makeTransport = () => - createTransport({}, () => { + createTransport({ recordDroppedEvent: () => undefined }, () => { sendCalled += 1; return new SyncPromise(async res => { await sleep(delay); diff --git a/packages/node/src/transports/http.ts b/packages/node/src/transports/http.ts index 1ba591c54379..0916b9f0b60a 100644 --- a/packages/node/src/transports/http.ts +++ b/packages/node/src/transports/http.ts @@ -46,7 +46,7 @@ export function makeNodeTransport(options: NodeTransportOptions): Transport { : new nativeHttpModule.Agent({ keepAlive: false, maxSockets: 30, timeout: 2000 }); const requestExecutor = createRequestExecutor(options, options.httpModule ?? nativeHttpModule, agent); - return createTransport({ bufferSize: options.bufferSize }, requestExecutor); + return createTransport(options, requestExecutor); } /** diff --git a/packages/node/test/helper/node-client-options.ts b/packages/node/test/helper/node-client-options.ts index fc31b11738fd..8831af6a2896 100644 --- a/packages/node/test/helper/node-client-options.ts +++ b/packages/node/test/helper/node-client-options.ts @@ -6,7 +6,7 @@ import { NodeClientOptions } from '../../src/types'; export function getDefaultNodeClientOptions(options: Partial = {}): NodeClientOptions { return { integrations: [], - transport: () => createTransport({}, _ => resolvedSyncPromise({})), + transport: () => createTransport({ recordDroppedEvent: () => undefined }, _ => resolvedSyncPromise({})), stackParser: () => [], ...options, }; diff --git a/packages/node/test/manual/express-scope-separation/start.js b/packages/node/test/manual/express-scope-separation/start.js index 37733cf142a5..faf7b381ecbc 100644 --- a/packages/node/test/manual/express-scope-separation/start.js +++ b/packages/node/test/manual/express-scope-separation/start.js @@ -16,7 +16,7 @@ function assertTags(actual, expected) { let remaining = 3; function makeDummyTransport() { - return Sentry.createTransport({}, req => { + return Sentry.createTransport({ recordDroppedEvent: () => undefined }, req => { --remaining; if (!remaining) { @@ -28,7 +28,7 @@ function makeDummyTransport() { return Promise.resolve({ statusCode: 200, }); - }) + }); } Sentry.init({ diff --git a/packages/node/test/manual/release-health/session-aggregates/aggregates-disable-single-session.js b/packages/node/test/manual/release-health/session-aggregates/aggregates-disable-single-session.js index f02788174ea0..ae6a660607b1 100644 --- a/packages/node/test/manual/release-health/session-aggregates/aggregates-disable-single-session.js +++ b/packages/node/test/manual/release-health/session-aggregates/aggregates-disable-single-session.js @@ -28,7 +28,7 @@ function assertSessionAggregates(session, expected) { } function makeDummyTransport() { - return Sentry.createTransport({}, req => { + return Sentry.createTransport({ recordDroppedEvent: () => undefined }, req => { const sessionEnv = req.body.split('\n').map(e => JSON.parse(e)); assertSessionAggregates(sessionEnv[2], { attrs: { release: '1.1' }, @@ -40,7 +40,7 @@ function makeDummyTransport() { return Promise.resolve({ statusCode: 200, }); - }) + }); } Sentry.init({ diff --git a/packages/node/test/manual/release-health/single-session/caught-exception-errored-session.js b/packages/node/test/manual/release-health/single-session/caught-exception-errored-session.js index 686a3eb264ed..b7a9538a3fa2 100644 --- a/packages/node/test/manual/release-health/single-session/caught-exception-errored-session.js +++ b/packages/node/test/manual/release-health/single-session/caught-exception-errored-session.js @@ -9,7 +9,7 @@ const sessionCounts = { validateSessionCountFunction(sessionCounts); function makeDummyTransport() { - return Sentry.createTransport({}, req => { + return Sentry.createTransport({ recordDroppedEvent: () => undefined }, req => { const payload = req.body.split('\n').map(e => JSON.parse(e)); const isSessionPayload = payload[1].type === 'session'; diff --git a/packages/node/test/manual/release-health/single-session/errors-in-session-capped-to-one.js b/packages/node/test/manual/release-health/single-session/errors-in-session-capped-to-one.js index d550614201b9..dae307182ed1 100644 --- a/packages/node/test/manual/release-health/single-session/errors-in-session-capped-to-one.js +++ b/packages/node/test/manual/release-health/single-session/errors-in-session-capped-to-one.js @@ -9,7 +9,7 @@ const sessionCounts = { validateSessionCountFunction(sessionCounts); function makeDummyTransport() { - return Sentry.createTransport({}, req => { + return Sentry.createTransport({ recordDroppedEvent: () => undefined }, req => { const payload = req.body.split('\n').map(e => JSON.parse(e)); const isSessionPayload = payload[1].type === 'session'; diff --git a/packages/node/test/manual/release-health/single-session/healthy-session.js b/packages/node/test/manual/release-health/single-session/healthy-session.js index cfc3909449ef..0533b8a28728 100644 --- a/packages/node/test/manual/release-health/single-session/healthy-session.js +++ b/packages/node/test/manual/release-health/single-session/healthy-session.js @@ -1,9 +1,5 @@ const Sentry = require('../../../../build/cjs'); -const { - assertSessions, - constructStrippedSessionObject, - validateSessionCountFunction, -} = require('../test-utils'); +const { assertSessions, constructStrippedSessionObject, validateSessionCountFunction } = require('../test-utils'); const sessionCounts = { sessionCounter: 0, @@ -13,7 +9,7 @@ const sessionCounts = { validateSessionCountFunction(sessionCounts); function makeDummyTransport() { - return Sentry.createTransport({}, req => { + return Sentry.createTransport({ recordDroppedEvent: () => undefined }, req => { sessionCounts.sessionCounter++; const sessionEnv = req.body.split('\n').map(e => JSON.parse(e)); @@ -21,13 +17,13 @@ function makeDummyTransport() { init: true, status: 'exited', errors: 0, - release: '1.1' + release: '1.1', }); return Promise.resolve({ statusCode: 200, }); - }) + }); } Sentry.init({ diff --git a/packages/node/test/manual/release-health/single-session/terminal-state-sessions-sent-once.js b/packages/node/test/manual/release-health/single-session/terminal-state-sessions-sent-once.js index 6eccd3e21dba..aa0796782c2f 100644 --- a/packages/node/test/manual/release-health/single-session/terminal-state-sessions-sent-once.js +++ b/packages/node/test/manual/release-health/single-session/terminal-state-sessions-sent-once.js @@ -9,7 +9,7 @@ const sessionCounts = { validateSessionCountFunction(sessionCounts); function makeDummyTransport() { - return Sentry.createTransport({}, req => { + return Sentry.createTransport({ recordDroppedEvent: () => undefined }, req => { const payload = req.body.split('\n').map(e => JSON.parse(e)); const isSessionPayload = payload[1].type === 'session'; diff --git a/packages/node/test/manual/release-health/single-session/uncaught-exception-crashed-session.js b/packages/node/test/manual/release-health/single-session/uncaught-exception-crashed-session.js index 5cee4449c23a..6fa3ac0a6821 100644 --- a/packages/node/test/manual/release-health/single-session/uncaught-exception-crashed-session.js +++ b/packages/node/test/manual/release-health/single-session/uncaught-exception-crashed-session.js @@ -2,7 +2,7 @@ const Sentry = require('../../../../build/cjs'); const { assertSessions, constructStrippedSessionObject } = require('../test-utils'); function makeDummyTransport() { - return Sentry.createTransport({}, req => { + return Sentry.createTransport({ recordDroppedEvent: () => undefined }, req => { if (req.category === 'session') { sessionCounts.sessionCounter++; const sessionEnv = req.body.split('\n').map(e => JSON.parse(e)); @@ -17,7 +17,7 @@ function makeDummyTransport() { // We need to explicitly exit process early here to allow for 0 exit code process.exit(0); - }) + }); } Sentry.init({ diff --git a/packages/node/test/manual/release-health/single-session/unhandled-rejection-crashed-session.js b/packages/node/test/manual/release-health/single-session/unhandled-rejection-crashed-session.js index 0024ee16f798..b550260b62b7 100644 --- a/packages/node/test/manual/release-health/single-session/unhandled-rejection-crashed-session.js +++ b/packages/node/test/manual/release-health/single-session/unhandled-rejection-crashed-session.js @@ -9,7 +9,7 @@ const sessionCounts = { validateSessionCountFunction(sessionCounts); function makeDummyTransport() { - return Sentry.createTransport({}, req => { + return Sentry.createTransport({ recordDroppedEvent: () => undefined }, req => { const payload = req.body.split('\n').map(e => JSON.parse(e)); const isSessionPayload = payload[1].type === 'session'; diff --git a/packages/node/test/manual/webpack-domain/index.js b/packages/node/test/manual/webpack-domain/index.js index b970c95d2a98..09ed18f3166f 100644 --- a/packages/node/test/manual/webpack-domain/index.js +++ b/packages/node/test/manual/webpack-domain/index.js @@ -3,7 +3,7 @@ const Sentry = require('../../../build/cjs'); let remaining = 2; function makeDummyTransport() { - return Sentry.createTransport({}, req => { + return Sentry.createTransport({ recordDroppedEvent: () => undefined }, req => { --remaining; if (!remaining) { @@ -14,7 +14,7 @@ function makeDummyTransport() { return Promise.resolve({ status: 'success', }); - }) + }); } Sentry.init({ diff --git a/packages/node/test/transports/http.test.ts b/packages/node/test/transports/http.test.ts index 909532d5a3b8..013468c3eb5d 100644 --- a/packages/node/test/transports/http.test.ts +++ b/packages/node/test/transports/http.test.ts @@ -68,6 +68,11 @@ const EVENT_ENVELOPE = createEnvelope({ event_id: 'aa3ff046696b4b const SERIALIZED_EVENT_ENVELOPE = serializeEnvelope(EVENT_ENVELOPE); +const defaultOptions = { + url: TEST_SERVER_URL, + recordDroppedEvent: () => undefined, +}; + describe('makeNewHttpTransport()', () => { afterEach(() => { jest.clearAllMocks(); @@ -84,7 +89,7 @@ describe('makeNewHttpTransport()', () => { expect(body).toBe(SERIALIZED_EVENT_ENVELOPE); }); - const transport = makeNodeTransport({ url: TEST_SERVER_URL }); + const transport = makeNodeTransport(defaultOptions); await transport.send(EVENT_ENVELOPE); }); @@ -100,7 +105,7 @@ describe('makeNewHttpTransport()', () => { }); const transport = makeNodeTransport({ - url: TEST_SERVER_URL, + ...defaultOptions, headers: { 'X-Some-Custom-Header-1': 'value1', 'X-Some-Custom-Header-2': 'value2', @@ -115,7 +120,7 @@ describe('makeNewHttpTransport()', () => { async serverStatusCode => { await setupTestServer({ statusCode: serverStatusCode }); - const transport = makeNodeTransport({ url: TEST_SERVER_URL }); + const transport = makeNodeTransport(defaultOptions); await expect(transport.send(EVENT_ENVELOPE)).resolves.toBeUndefined(); }, @@ -130,7 +135,7 @@ describe('makeNewHttpTransport()', () => { }, }); - const transport = makeNodeTransport({ url: TEST_SERVER_URL }); + const transport = makeNodeTransport(defaultOptions); await expect(transport.send(EVENT_ENVELOPE)).resolves.toBeUndefined(); }); @@ -143,7 +148,7 @@ describe('makeNewHttpTransport()', () => { }, }); - const transport = makeNodeTransport({ url: TEST_SERVER_URL }); + const transport = makeNodeTransport(defaultOptions); await transport.send(EVENT_ENVELOPE); }); }); @@ -151,6 +156,7 @@ describe('makeNewHttpTransport()', () => { describe('proxy', () => { it('can be configured through option', () => { makeNodeTransport({ + ...defaultOptions, url: 'http://9e9fd4523d784609a5fc0ebb1080592f@sentry.io:8989/mysubpath/50622', proxy: 'http://example.com', }); @@ -162,6 +168,7 @@ describe('makeNewHttpTransport()', () => { it('can be configured through env variables option', () => { process.env.http_proxy = 'http://example.com'; makeNodeTransport({ + ...defaultOptions, url: 'http://9e9fd4523d784609a5fc0ebb1080592f@sentry.io:8989/mysubpath/50622', }); @@ -173,6 +180,7 @@ describe('makeNewHttpTransport()', () => { it('client options have priority over env variables', () => { process.env.http_proxy = 'http://foo.com'; makeNodeTransport({ + ...defaultOptions, url: 'http://9e9fd4523d784609a5fc0ebb1080592f@sentry.io:8989/mysubpath/50622', proxy: 'http://bar.com', }); @@ -185,6 +193,7 @@ describe('makeNewHttpTransport()', () => { it('no_proxy allows for skipping specific hosts', () => { process.env.no_proxy = 'sentry.io'; makeNodeTransport({ + ...defaultOptions, url: 'http://9e9fd4523d784609a5fc0ebb1080592f@sentry.io:8989/mysubpath/50622', proxy: 'http://example.com', }); @@ -199,6 +208,7 @@ describe('makeNewHttpTransport()', () => { process.env.no_proxy = 'sentry.io:8989'; makeNodeTransport({ + ...defaultOptions, url: 'http://9e9fd4523d784609a5fc0ebb1080592f@sentry.io:8989/mysubpath/50622', }); @@ -213,6 +223,7 @@ describe('makeNewHttpTransport()', () => { process.env.no_proxy = 'example.com,sentry.io,wat.com:1337'; makeNodeTransport({ + ...defaultOptions, url: 'http://9e9fd4523d784609a5fc0ebb1080592f@sentry.io:8989/mysubpath/50622', }); @@ -232,7 +243,7 @@ describe('makeNewHttpTransport()', () => { }, }); - makeNodeTransport({ url: TEST_SERVER_URL }); + makeNodeTransport(defaultOptions); const registeredRequestExecutor = (createTransport as jest.Mock).mock.calls[0][1]; const executorResult = registeredRequestExecutor({ @@ -255,7 +266,7 @@ describe('makeNewHttpTransport()', () => { statusCode: SUCCESS, }); - makeNodeTransport({ url: TEST_SERVER_URL }); + makeNodeTransport(defaultOptions); const registeredRequestExecutor = (createTransport as jest.Mock).mock.calls[0][1]; const executorResult = registeredRequestExecutor({ @@ -282,7 +293,7 @@ describe('makeNewHttpTransport()', () => { }, }); - makeNodeTransport({ url: TEST_SERVER_URL }); + makeNodeTransport(defaultOptions); const registeredRequestExecutor = (createTransport as jest.Mock).mock.calls[0][1]; const executorResult = registeredRequestExecutor({ @@ -309,7 +320,7 @@ describe('makeNewHttpTransport()', () => { }, }); - makeNodeTransport({ url: TEST_SERVER_URL }); + makeNodeTransport(defaultOptions); const registeredRequestExecutor = (createTransport as jest.Mock).mock.calls[0][1]; const executorResult = registeredRequestExecutor({ diff --git a/packages/node/test/transports/https.test.ts b/packages/node/test/transports/https.test.ts index dce56983680f..118a9a70dc88 100644 --- a/packages/node/test/transports/https.test.ts +++ b/packages/node/test/transports/https.test.ts @@ -79,6 +79,12 @@ const unsafeHttpsModule: HTTPModule = { }), }; +const defaultOptions = { + httpModule: unsafeHttpsModule, + url: TEST_SERVER_URL, + recordDroppedEvent: () => undefined, // noop +}; + describe('makeNewHttpsTransport()', () => { afterEach(() => { jest.clearAllMocks(); @@ -95,7 +101,7 @@ describe('makeNewHttpsTransport()', () => { expect(body).toBe(SERIALIZED_EVENT_ENVELOPE); }); - const transport = makeNodeTransport({ httpModule: unsafeHttpsModule, url: TEST_SERVER_URL }); + const transport = makeNodeTransport(defaultOptions); await transport.send(EVENT_ENVELOPE); }); @@ -111,8 +117,7 @@ describe('makeNewHttpsTransport()', () => { }); const transport = makeNodeTransport({ - httpModule: unsafeHttpsModule, - url: TEST_SERVER_URL, + ...defaultOptions, headers: { 'X-Some-Custom-Header-1': 'value1', 'X-Some-Custom-Header-2': 'value2', @@ -127,7 +132,7 @@ describe('makeNewHttpsTransport()', () => { async serverStatusCode => { await setupTestServer({ statusCode: serverStatusCode }); - const transport = makeNodeTransport({ httpModule: unsafeHttpsModule, url: TEST_SERVER_URL }); + const transport = makeNodeTransport(defaultOptions); expect(() => { expect(transport.send(EVENT_ENVELOPE)); }).not.toThrow(); @@ -143,7 +148,7 @@ describe('makeNewHttpsTransport()', () => { }, }); - const transport = makeNodeTransport({ httpModule: unsafeHttpsModule, url: TEST_SERVER_URL }); + const transport = makeNodeTransport(defaultOptions); await expect(transport.send(EVENT_ENVELOPE)).resolves.toBeUndefined(); }); @@ -156,7 +161,7 @@ describe('makeNewHttpsTransport()', () => { }, }); - const transport = makeNodeTransport({ httpModule: unsafeHttpsModule, url: TEST_SERVER_URL }); + const transport = makeNodeTransport(defaultOptions); await expect(transport.send(EVENT_ENVELOPE)).resolves.toBeUndefined(); }); @@ -164,6 +169,7 @@ describe('makeNewHttpsTransport()', () => { await setupTestServer({ statusCode: SUCCESS }); const transport = makeNodeTransport({ + ...defaultOptions, httpModule: unsafeHttpsModule, url: TEST_SERVER_URL, caCerts: 'some cert', @@ -184,6 +190,7 @@ describe('makeNewHttpsTransport()', () => { describe('proxy', () => { it('can be configured through option', () => { makeNodeTransport({ + ...defaultOptions, httpModule: unsafeHttpsModule, url: 'https://9e9fd4523d784609a5fc0ebb1080592f@sentry.io:8989/mysubpath/50622', proxy: 'https://example.com', @@ -196,6 +203,7 @@ describe('makeNewHttpsTransport()', () => { it('can be configured through env variables option (http)', () => { process.env.http_proxy = 'https://example.com'; makeNodeTransport({ + ...defaultOptions, httpModule: unsafeHttpsModule, url: 'https://9e9fd4523d784609a5fc0ebb1080592f@sentry.io:8989/mysubpath/50622', }); @@ -208,6 +216,7 @@ describe('makeNewHttpsTransport()', () => { it('can be configured through env variables option (https)', () => { process.env.https_proxy = 'https://example.com'; makeNodeTransport({ + ...defaultOptions, httpModule: unsafeHttpsModule, url: 'https://9e9fd4523d784609a5fc0ebb1080592f@sentry.io:8989/mysubpath/50622', }); @@ -220,6 +229,7 @@ describe('makeNewHttpsTransport()', () => { it('client options have priority over env variables', () => { process.env.https_proxy = 'https://foo.com'; makeNodeTransport({ + ...defaultOptions, httpModule: unsafeHttpsModule, url: 'https://9e9fd4523d784609a5fc0ebb1080592f@sentry.io:8989/mysubpath/50622', proxy: 'https://bar.com', @@ -233,6 +243,7 @@ describe('makeNewHttpsTransport()', () => { it('no_proxy allows for skipping specific hosts', () => { process.env.no_proxy = 'sentry.io'; makeNodeTransport({ + ...defaultOptions, httpModule: unsafeHttpsModule, url: 'https://9e9fd4523d784609a5fc0ebb1080592f@sentry.io:8989/mysubpath/50622', proxy: 'https://example.com', @@ -248,6 +259,7 @@ describe('makeNewHttpsTransport()', () => { process.env.no_proxy = 'sentry.io:8989'; makeNodeTransport({ + ...defaultOptions, httpModule: unsafeHttpsModule, url: 'https://9e9fd4523d784609a5fc0ebb1080592f@sentry.io:8989/mysubpath/50622', }); @@ -263,6 +275,7 @@ describe('makeNewHttpsTransport()', () => { process.env.no_proxy = 'example.com,sentry.io,wat.com:1337'; makeNodeTransport({ + ...defaultOptions, httpModule: unsafeHttpsModule, url: 'https://9e9fd4523d784609a5fc0ebb1080592f@sentry.io:8989/mysubpath/50622', }); @@ -283,7 +296,7 @@ describe('makeNewHttpsTransport()', () => { }, }); - makeNodeTransport({ httpModule: unsafeHttpsModule, url: TEST_SERVER_URL }); + makeNodeTransport(defaultOptions); const registeredRequestExecutor = (createTransport as jest.Mock).mock.calls[0][1]; const executorResult = registeredRequestExecutor({ @@ -306,7 +319,7 @@ describe('makeNewHttpsTransport()', () => { statusCode: SUCCESS, }); - makeNodeTransport({ httpModule: unsafeHttpsModule, url: TEST_SERVER_URL }); + makeNodeTransport(defaultOptions); const registeredRequestExecutor = (createTransport as jest.Mock).mock.calls[0][1]; const executorResult = registeredRequestExecutor({ @@ -333,7 +346,7 @@ describe('makeNewHttpsTransport()', () => { }, }); - makeNodeTransport({ httpModule: unsafeHttpsModule, url: TEST_SERVER_URL }); + makeNodeTransport(defaultOptions); const registeredRequestExecutor = (createTransport as jest.Mock).mock.calls[0][1]; const executorResult = registeredRequestExecutor({ @@ -360,7 +373,7 @@ describe('makeNewHttpsTransport()', () => { }, }); - makeNodeTransport({ httpModule: unsafeHttpsModule, url: TEST_SERVER_URL }); + makeNodeTransport(defaultOptions); const registeredRequestExecutor = (createTransport as jest.Mock).mock.calls[0][1]; const executorResult = registeredRequestExecutor({ diff --git a/packages/tracing/test/testutils.ts b/packages/tracing/test/testutils.ts index 298d65f94ce3..5b4594c64913 100644 --- a/packages/tracing/test/testutils.ts +++ b/packages/tracing/test/testutils.ts @@ -62,7 +62,7 @@ export const testOnlyIfNodeVersionAtLeast = (minVersion: number): jest.It => { export function getDefaultBrowserClientOptions(options: Partial = {}): ClientOptions { return { integrations: [], - transport: () => createTransport({}, _ => resolvedSyncPromise({})), + transport: () => createTransport({ recordDroppedEvent: () => undefined }, _ => resolvedSyncPromise({})), stackParser: () => [], ...options, }; diff --git a/packages/types/src/envelope.ts b/packages/types/src/envelope.ts index 8e5091dbbfa9..33846f077a79 100644 --- a/packages/types/src/envelope.ts +++ b/packages/types/src/envelope.ts @@ -7,6 +7,15 @@ import { UserFeedback } from './user'; // Based on: https://develop.sentry.dev/sdk/envelopes/ +export type EnvelopeItemType = + | 'client_report' + | 'user_report' + | 'session' + | 'sessions' + | 'transaction' + | 'attachment' + | 'event'; + export type BaseEnvelopeHeaders = { [key: string]: unknown; dsn?: string; @@ -15,7 +24,7 @@ export type BaseEnvelopeHeaders = { export type BaseEnvelopeItemHeaders = { [key: string]: unknown; - type: string; + type: EnvelopeItemType; length?: number; }; @@ -56,3 +65,4 @@ export type SessionEnvelope = BaseEnvelope; export type ClientReportEnvelope = BaseEnvelope; export type Envelope = EventEnvelope | SessionEnvelope | ClientReportEnvelope; +export type EnvelopeItem = Envelope[1][number]; diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index d74b24fb5266..545a894a81e7 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -12,6 +12,8 @@ export type { ClientReportEnvelope, ClientReportItem, Envelope, + EnvelopeItemType, + EnvelopeItem, EventEnvelope, EventItem, SessionEnvelope, diff --git a/packages/types/src/transport.ts b/packages/types/src/transport.ts index e87d664e6388..f757b271970a 100644 --- a/packages/types/src/transport.ts +++ b/packages/types/src/transport.ts @@ -1,3 +1,5 @@ +import { EventDropReason } from './clientreport'; +import { DataCategory } from './datacategory'; import { Envelope } from './envelope'; export type TransportRequest = { @@ -14,7 +16,9 @@ export type TransportMakeRequestResponse = { export interface InternalBaseTransportOptions { bufferSize?: number; + recordDroppedEvent: (reason: EventDropReason, dataCategory: DataCategory) => void; } + export interface BaseTransportOptions extends InternalBaseTransportOptions { // url to send the event // transport does not care about dsn specific - client should take care of diff --git a/packages/utils/src/envelope.ts b/packages/utils/src/envelope.ts index 0000e6007aee..a58c48029066 100644 --- a/packages/utils/src/envelope.ts +++ b/packages/utils/src/envelope.ts @@ -1,4 +1,4 @@ -import { Envelope } from '@sentry/types'; +import { DataCategory, Envelope, EnvelopeItem, EnvelopeItemType } from '@sentry/types'; import { isPrimitive } from './is'; @@ -22,11 +22,18 @@ export function addItemToEnvelope(envelope: E, newItem: E[1] } /** - * Get the type of the envelope. Grabs the type from the first envelope item. + * Convenience function to loop through the items and item types of an envelope. + * (This function was mostly created because working with envelope types is painful at the moment) */ -export function getEnvelopeType(envelope: E): string { - const [, [[firstItemHeader]]] = envelope; - return firstItemHeader.type; +export function forEachEnvelopeItem( + envelope: Envelope, + callback: (envelopeItem: E[1][number], envelopeItemType: E[1][number][0]['type']) => void, +): void { + const envelopeItems = envelope[1]; + envelopeItems.forEach((envelopeItem: EnvelopeItem) => { + const envelopeItemType = envelopeItem[0].type; + callback(envelopeItem, envelopeItemType); + }); } /** @@ -48,3 +55,20 @@ export function serializeEnvelope(envelope: Envelope): string { return `${acc}\n${JSON.stringify(itemHeaders)}\n${serializedPayload}`; }, serializedHeaders); } + +const ITEM_TYPE_TO_DATA_CATEGORY_MAP: Record = { + session: 'session', + sessions: 'session', + attachment: 'attachment', + transaction: 'transaction', + event: 'error', + client_report: 'internal', + user_report: 'default', +}; + +/** + * Maps the type of an envelope item to a data category. + */ +export function envelopeItemTypeToDataCategory(type: EnvelopeItemType): DataCategory { + return ITEM_TYPE_TO_DATA_CATEGORY_MAP[type]; +} diff --git a/packages/utils/src/ratelimit.ts b/packages/utils/src/ratelimit.ts index 59906c0abdaf..66614b7fc6b2 100644 --- a/packages/utils/src/ratelimit.ts +++ b/packages/utils/src/ratelimit.ts @@ -1,4 +1,4 @@ -// Keeping the key broad until we add the new transports +// Intentionally keeping the key broad, as we don't know for sure what rate limit headers get returned from backend export type RateLimits = Record; export const DEFAULT_RETRY_AFTER = 60 * 1000; // 60 seconds diff --git a/packages/utils/test/envelope.test.ts b/packages/utils/test/envelope.test.ts index dab2b92d5f47..df07629a083e 100644 --- a/packages/utils/test/envelope.test.ts +++ b/packages/utils/test/envelope.test.ts @@ -1,6 +1,6 @@ import { EventEnvelope } from '@sentry/types'; -import { addItemToEnvelope, createEnvelope, getEnvelopeType, serializeEnvelope } from '../src/envelope'; +import { addItemToEnvelope, createEnvelope, forEachEnvelopeItem, serializeEnvelope } from '../src/envelope'; import { parseEnvelope } from './testutils'; describe('envelope', () => { @@ -45,13 +45,27 @@ describe('envelope', () => { }); }); - describe('getEnvelopeType', () => { - it('returns the type of the envelope', () => { - const env = createEnvelope({ event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2', sent_at: '123' }, [ + describe('forEachEnvelopeItem', () => { + it('loops through an envelope', () => { + const items: EventEnvelope[1] = [ [{ type: 'event' }, { event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2' }], - [{ type: 'attachment', filename: 'me.txt' }, '123456'], - ]); - expect(getEnvelopeType(env)).toEqual('event'); + [{ type: 'attachment', filename: 'bar.txt' }, '123456'], + [{ type: 'attachment', filename: 'foo.txt' }, '123456'], + ]; + + const env = createEnvelope( + { event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2', sent_at: '123' }, + items, + ); + + expect.assertions(6); + + let iteration = 0; + forEachEnvelopeItem(env, (item, type) => { + expect(item).toBe(items[iteration]); + expect(type).toBe(items[iteration][0].type); + iteration = iteration + 1; + }); }); }); });