diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index c9899662ccf0..1f2bc293caf2 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -315,7 +315,9 @@ export abstract class BaseClient implements Client { /** * @inheritDoc */ - public recordDroppedEvent(reason: EventDropReason, category: DataCategory): void { + public recordDroppedEvent(reason: EventDropReason, category: DataCategory, _event?: Event): void { + // Note: we use `event` in replay, where we overwrite this hook. + if (this._options.sendClientReports) { // We want to track each category (error, transaction, session) separately // but still keep the distinction between different type of outcomes. @@ -632,7 +634,7 @@ export abstract class BaseClient implements Client { // 0.0 === 0% events are sent // Sampling for transaction happens somewhere else if (!isTransaction && typeof sampleRate === 'number' && Math.random() > sampleRate) { - this.recordDroppedEvent('sample_rate', 'error'); + this.recordDroppedEvent('sample_rate', 'error', event); return rejectedSyncPromise( new SentryError( `Discarding event because it's not included in the random sample (sampling rate = ${sampleRate})`, @@ -644,7 +646,7 @@ export abstract class BaseClient implements Client { return this._prepareEvent(event, hint, scope) .then(prepared => { if (prepared === null) { - this.recordDroppedEvent('event_processor', event.type || 'error'); + this.recordDroppedEvent('event_processor', event.type || 'error', event); throw new SentryError('An event processor returned `null`, will not send event.', 'log'); } @@ -658,7 +660,7 @@ export abstract class BaseClient implements Client { }) .then(processedEvent => { if (processedEvent === null) { - this.recordDroppedEvent('before_send', event.type || 'error'); + this.recordDroppedEvent('before_send', event.type || 'error', event); throw new SentryError(`\`${beforeSendProcessorName}\` returned \`null\`, will not send event.`, 'log'); } diff --git a/packages/core/src/transports/base.ts b/packages/core/src/transports/base.ts index d9c58b789706..23a7d00d70df 100644 --- a/packages/core/src/transports/base.ts +++ b/packages/core/src/transports/base.ts @@ -1,7 +1,10 @@ import { Envelope, EnvelopeItem, + EnvelopeItemType, + Event, EventDropReason, + EventItem, InternalBaseTransportOptions, Transport, TransportRequestExecutor, @@ -45,7 +48,8 @@ export function createTransport( forEachEnvelopeItem(envelope, (item, type) => { const envelopeItemDataCategory = envelopeItemTypeToDataCategory(type); if (isRateLimited(rateLimits, envelopeItemDataCategory)) { - options.recordDroppedEvent('ratelimit_backoff', envelopeItemDataCategory); + const event: Event | undefined = getEventForEnvelopeItem(item, type); + options.recordDroppedEvent('ratelimit_backoff', envelopeItemDataCategory, event); } else { filteredEnvelopeItems.push(item); } @@ -61,8 +65,9 @@ export function createTransport( // Creates client report for each item in an envelope const recordEnvelopeLoss = (reason: EventDropReason): void => { - forEachEnvelopeItem(filteredEnvelope, (_, type) => { - options.recordDroppedEvent(reason, envelopeItemTypeToDataCategory(type)); + forEachEnvelopeItem(filteredEnvelope, (item, type) => { + const event: Event | undefined = getEventForEnvelopeItem(item, type); + options.recordDroppedEvent(reason, envelopeItemTypeToDataCategory(type), event); }); }; @@ -101,3 +106,11 @@ export function createTransport( flush, }; } + +function getEventForEnvelopeItem(item: Envelope[1][number], type: EnvelopeItemType): Event | undefined { + if (type !== 'event' && type !== 'transaction') { + return undefined; + } + + return Array.isArray(item) ? (item as EventItem)[1] : undefined; +} diff --git a/packages/core/test/lib/base.test.ts b/packages/core/test/lib/base.test.ts index e41dce9e2702..99d6e324f9ca 100644 --- a/packages/core/test/lib/base.test.ts +++ b/packages/core/test/lib/base.test.ts @@ -1237,7 +1237,9 @@ describe('BaseClient', () => { client.captureEvent({ message: 'hello' }, {}); expect(beforeSend).toHaveBeenCalled(); - expect(recordLostEventSpy).toHaveBeenCalledWith('before_send', 'error'); + expect(recordLostEventSpy).toHaveBeenCalledWith('before_send', 'error', { + message: 'hello', + }); }); test('`beforeSendTransaction` records dropped events', () => { @@ -1257,7 +1259,10 @@ describe('BaseClient', () => { client.captureEvent({ transaction: '/dogs/are/great', type: 'transaction' }); expect(beforeSendTransaction).toHaveBeenCalled(); - expect(recordLostEventSpy).toHaveBeenCalledWith('before_send', 'transaction'); + expect(recordLostEventSpy).toHaveBeenCalledWith('before_send', 'transaction', { + transaction: '/dogs/are/great', + type: 'transaction', + }); }); test('event processor drops error event when it returns `null`', () => { @@ -1309,7 +1314,9 @@ describe('BaseClient', () => { client.captureEvent({ message: 'hello' }, {}, scope); - expect(recordLostEventSpy).toHaveBeenCalledWith('event_processor', 'error'); + expect(recordLostEventSpy).toHaveBeenCalledWith('event_processor', 'error', { + message: 'hello', + }); }); test('event processor records dropped transaction events', () => { @@ -1325,7 +1332,10 @@ describe('BaseClient', () => { client.captureEvent({ transaction: '/dogs/are/great', type: 'transaction' }, {}, scope); - expect(recordLostEventSpy).toHaveBeenCalledWith('event_processor', 'transaction'); + expect(recordLostEventSpy).toHaveBeenCalledWith('event_processor', 'transaction', { + transaction: '/dogs/are/great', + type: 'transaction', + }); }); test('mutating transaction name with event processors sets transaction-name-change metadata', () => { @@ -1434,7 +1444,9 @@ describe('BaseClient', () => { const recordLostEventSpy = jest.spyOn(client, 'recordDroppedEvent'); client.captureEvent({ message: 'hello' }, {}); - expect(recordLostEventSpy).toHaveBeenCalledWith('sample_rate', 'error'); + expect(recordLostEventSpy).toHaveBeenCalledWith('sample_rate', 'error', { + message: 'hello', + }); }); }); diff --git a/packages/core/test/lib/transports/base.test.ts b/packages/core/test/lib/transports/base.test.ts index 94d83751a21d..27f21b9391b1 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, TransportMakeRequestResponse } from '@sentry/types'; +import { AttachmentItem, EventEnvelope, EventItem, TransportMakeRequestResponse } from '@sentry/types'; import { createEnvelope, PromiseBuffer, resolvedSyncPromise, serializeEnvelope } from '@sentry/utils'; import { TextEncoder } from 'util'; @@ -13,6 +13,22 @@ const TRANSACTION_ENVELOPE = createEnvelope( [[{ type: 'transaction' }, { event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2' }] as EventItem], ); +const ATTACHMENT_ENVELOPE = createEnvelope( + { event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2', sent_at: '123' }, + [ + [ + { + type: 'attachment', + length: 20, + filename: 'test-file.txt', + content_type: 'text/plain', + attachment_type: 'text', + }, + 'attachment content', + ] as AttachmentItem, + ], +); + const transportOptions = { recordDroppedEvent: () => undefined, // noop textEncoder: new TextEncoder(), @@ -122,7 +138,9 @@ describe('createTransport', () => { await transport.send(ERROR_ENVELOPE); expect(requestExecutor).not.toHaveBeenCalled(); - expect(recordDroppedEventCallback).toHaveBeenCalledWith('ratelimit_backoff', 'error'); + expect(recordDroppedEventCallback).toHaveBeenCalledWith('ratelimit_backoff', 'error', { + event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2', + }); requestExecutor.mockClear(); recordDroppedEventCallback.mockClear(); @@ -164,7 +182,9 @@ describe('createTransport', () => { 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'); + expect(recordDroppedEventCallback).toHaveBeenCalledWith('ratelimit_backoff', 'error', { + event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2', + }); requestExecutor.mockClear(); recordDroppedEventCallback.mockClear(); @@ -186,7 +206,7 @@ describe('createTransport', () => { const { retryAfterSeconds, beforeLimit, withinLimit, afterLimit } = setRateLimitTimes(); const [transport, setTransportResponse, requestExecutor, recordDroppedEventCallback] = createTestTransport({ headers: { - 'x-sentry-rate-limits': `${retryAfterSeconds}:error;transaction:scope`, + 'x-sentry-rate-limits': `${retryAfterSeconds}:error;transaction;attachment:scope`, 'retry-after': null, }, }); @@ -206,13 +226,23 @@ describe('createTransport', () => { 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'); + expect(recordDroppedEventCallback).toHaveBeenCalledWith('ratelimit_backoff', 'transaction', { + event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2', + }); 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'); + expect(recordDroppedEventCallback).toHaveBeenCalledWith('ratelimit_backoff', 'error', { + event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2', + }); + requestExecutor.mockClear(); + recordDroppedEventCallback.mockClear(); + + await transport.send(ATTACHMENT_ENVELOPE); // Attachment envelope should not be sent because of pending rate limit + expect(requestExecutor).not.toHaveBeenCalled(); + expect(recordDroppedEventCallback).toHaveBeenCalledWith('ratelimit_backoff', 'attachment', undefined); requestExecutor.mockClear(); recordDroppedEventCallback.mockClear(); @@ -228,6 +258,12 @@ describe('createTransport', () => { await transport.send(ERROR_ENVELOPE); expect(requestExecutor).toHaveBeenCalledTimes(1); expect(recordDroppedEventCallback).not.toHaveBeenCalled(); + requestExecutor.mockClear(); + recordDroppedEventCallback.mockClear(); + + await transport.send(ATTACHMENT_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 () => { @@ -254,13 +290,17 @@ describe('createTransport', () => { 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'); + expect(recordDroppedEventCallback).toHaveBeenCalledWith('ratelimit_backoff', 'transaction', { + event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2', + }); 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'); + expect(recordDroppedEventCallback).toHaveBeenCalledWith('ratelimit_backoff', 'error', { + event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2', + }); requestExecutor.mockClear(); recordDroppedEventCallback.mockClear(); diff --git a/packages/types/src/client.ts b/packages/types/src/client.ts index 5e4fe6d2da43..ce474e237fe6 100644 --- a/packages/types/src/client.ts +++ b/packages/types/src/client.ts @@ -126,6 +126,7 @@ export interface Client { * * @param reason The reason why the event got dropped. * @param category The data category of the dropped event. + * @param event The dropped event. */ - recordDroppedEvent(reason: EventDropReason, category: DataCategory): void; + recordDroppedEvent(reason: EventDropReason, dataCategory: DataCategory, event?: Event): void; } diff --git a/packages/types/src/transport.ts b/packages/types/src/transport.ts index dae3ddd5f920..b9beb2148f20 100644 --- a/packages/types/src/transport.ts +++ b/packages/types/src/transport.ts @@ -1,5 +1,4 @@ -import { EventDropReason } from './clientreport'; -import { DataCategory } from './datacategory'; +import { Client } from './client'; import { Envelope } from './envelope'; import { TextEncoderInternal } from './textencoder'; @@ -18,7 +17,7 @@ export type TransportMakeRequestResponse = { export interface InternalBaseTransportOptions { bufferSize?: number; - recordDroppedEvent: (reason: EventDropReason, dataCategory: DataCategory) => void; + recordDroppedEvent: Client['recordDroppedEvent']; textEncoder?: TextEncoderInternal; }