Skip to content

Commit 7821ba7

Browse files
mydeamydea
authored
feat(core): Pass event as third argument to recordDroppedEvent (#6289)
This is optional. We need this for replay.
1 parent a00dceb commit 7821ba7

File tree

6 files changed

+91
-24
lines changed

6 files changed

+91
-24
lines changed

‎packages/core/src/baseclient.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -315,7 +315,9 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
315315
/**
316316
* @inheritDoc
317317
*/
318-
public recordDroppedEvent(reason: EventDropReason, category: DataCategory): void {
318+
public recordDroppedEvent(reason: EventDropReason, category: DataCategory, _event?: Event): void {
319+
// Note: we use `event` in replay, where we overwrite this hook.
320+
319321
if (this._options.sendClientReports) {
320322
// We want to track each category (error, transaction, session) separately
321323
// but still keep the distinction between different type of outcomes.
@@ -632,7 +634,7 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
632634
// 0.0 === 0% events are sent
633635
// Sampling for transaction happens somewhere else
634636
if (!isTransaction && typeof sampleRate === 'number' && Math.random() > sampleRate) {
635-
this.recordDroppedEvent('sample_rate', 'error');
637+
this.recordDroppedEvent('sample_rate', 'error', event);
636638
return rejectedSyncPromise(
637639
new SentryError(
638640
`Discarding event because it's not included in the random sample (sampling rate = ${sampleRate})`,
@@ -644,7 +646,7 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
644646
return this._prepareEvent(event, hint, scope)
645647
.then(prepared => {
646648
if (prepared === null) {
647-
this.recordDroppedEvent('event_processor', event.type || 'error');
649+
this.recordDroppedEvent('event_processor', event.type || 'error', event);
648650
throw new SentryError('An event processor returned `null`, will not send event.', 'log');
649651
}
650652

@@ -658,7 +660,7 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
658660
})
659661
.then(processedEvent => {
660662
if (processedEvent === null) {
661-
this.recordDroppedEvent('before_send', event.type || 'error');
663+
this.recordDroppedEvent('before_send', event.type || 'error', event);
662664
throw new SentryError(`\`${beforeSendProcessorName}\` returned \`null\`, will not send event.`, 'log');
663665
}
664666

‎packages/core/src/transports/base.ts

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
import {
22
Envelope,
33
EnvelopeItem,
4+
EnvelopeItemType,
5+
Event,
46
EventDropReason,
7+
EventItem,
58
InternalBaseTransportOptions,
69
Transport,
710
TransportRequestExecutor,
@@ -45,7 +48,8 @@ export function createTransport(
4548
forEachEnvelopeItem(envelope, (item, type) => {
4649
const envelopeItemDataCategory = envelopeItemTypeToDataCategory(type);
4750
if (isRateLimited(rateLimits, envelopeItemDataCategory)) {
48-
options.recordDroppedEvent('ratelimit_backoff', envelopeItemDataCategory);
51+
const event: Event | undefined = getEventForEnvelopeItem(item, type);
52+
options.recordDroppedEvent('ratelimit_backoff', envelopeItemDataCategory, event);
4953
} else {
5054
filteredEnvelopeItems.push(item);
5155
}
@@ -61,8 +65,9 @@ export function createTransport(
6165

6266
// Creates client report for each item in an envelope
6367
const recordEnvelopeLoss = (reason: EventDropReason): void => {
64-
forEachEnvelopeItem(filteredEnvelope, (_, type) => {
65-
options.recordDroppedEvent(reason, envelopeItemTypeToDataCategory(type));
68+
forEachEnvelopeItem(filteredEnvelope, (item, type) => {
69+
const event: Event | undefined = getEventForEnvelopeItem(item, type);
70+
options.recordDroppedEvent(reason, envelopeItemTypeToDataCategory(type), event);
6671
});
6772
};
6873

@@ -101,3 +106,11 @@ export function createTransport(
101106
flush,
102107
};
103108
}
109+
110+
function getEventForEnvelopeItem(item: Envelope[1][number], type: EnvelopeItemType): Event | undefined {
111+
if (type !== 'event' && type !== 'transaction') {
112+
return undefined;
113+
}
114+
115+
return Array.isArray(item) ? (item as EventItem)[1] : undefined;
116+
}

‎packages/core/test/lib/base.test.ts

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1237,7 +1237,9 @@ describe('BaseClient', () => {
12371237
client.captureEvent({ message: 'hello' }, {});
12381238

12391239
expect(beforeSend).toHaveBeenCalled();
1240-
expect(recordLostEventSpy).toHaveBeenCalledWith('before_send', 'error');
1240+
expect(recordLostEventSpy).toHaveBeenCalledWith('before_send', 'error', {
1241+
message: 'hello',
1242+
});
12411243
});
12421244

12431245
test('`beforeSendTransaction` records dropped events', () => {
@@ -1257,7 +1259,10 @@ describe('BaseClient', () => {
12571259
client.captureEvent({ transaction: '/dogs/are/great', type: 'transaction' });
12581260

12591261
expect(beforeSendTransaction).toHaveBeenCalled();
1260-
expect(recordLostEventSpy).toHaveBeenCalledWith('before_send', 'transaction');
1262+
expect(recordLostEventSpy).toHaveBeenCalledWith('before_send', 'transaction', {
1263+
transaction: '/dogs/are/great',
1264+
type: 'transaction',
1265+
});
12611266
});
12621267

12631268
test('event processor drops error event when it returns `null`', () => {
@@ -1309,7 +1314,9 @@ describe('BaseClient', () => {
13091314

13101315
client.captureEvent({ message: 'hello' }, {}, scope);
13111316

1312-
expect(recordLostEventSpy).toHaveBeenCalledWith('event_processor', 'error');
1317+
expect(recordLostEventSpy).toHaveBeenCalledWith('event_processor', 'error', {
1318+
message: 'hello',
1319+
});
13131320
});
13141321

13151322
test('event processor records dropped transaction events', () => {
@@ -1325,7 +1332,10 @@ describe('BaseClient', () => {
13251332

13261333
client.captureEvent({ transaction: '/dogs/are/great', type: 'transaction' }, {}, scope);
13271334

1328-
expect(recordLostEventSpy).toHaveBeenCalledWith('event_processor', 'transaction');
1335+
expect(recordLostEventSpy).toHaveBeenCalledWith('event_processor', 'transaction', {
1336+
transaction: '/dogs/are/great',
1337+
type: 'transaction',
1338+
});
13291339
});
13301340

13311341
test('mutating transaction name with event processors sets transaction-name-change metadata', () => {
@@ -1434,7 +1444,9 @@ describe('BaseClient', () => {
14341444
const recordLostEventSpy = jest.spyOn(client, 'recordDroppedEvent');
14351445

14361446
client.captureEvent({ message: 'hello' }, {});
1437-
expect(recordLostEventSpy).toHaveBeenCalledWith('sample_rate', 'error');
1447+
expect(recordLostEventSpy).toHaveBeenCalledWith('sample_rate', 'error', {
1448+
message: 'hello',
1449+
});
14381450
});
14391451
});
14401452

‎packages/core/test/lib/transports/base.test.ts

Lines changed: 48 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { EventEnvelope, EventItem, TransportMakeRequestResponse } from '@sentry/types';
1+
import { AttachmentItem, EventEnvelope, EventItem, TransportMakeRequestResponse } from '@sentry/types';
22
import { createEnvelope, PromiseBuffer, resolvedSyncPromise, serializeEnvelope } from '@sentry/utils';
33
import { TextEncoder } from 'util';
44

@@ -13,6 +13,22 @@ const TRANSACTION_ENVELOPE = createEnvelope<EventEnvelope>(
1313
[[{ type: 'transaction' }, { event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2' }] as EventItem],
1414
);
1515

16+
const ATTACHMENT_ENVELOPE = createEnvelope<EventEnvelope>(
17+
{ event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2', sent_at: '123' },
18+
[
19+
[
20+
{
21+
type: 'attachment',
22+
length: 20,
23+
filename: 'test-file.txt',
24+
content_type: 'text/plain',
25+
attachment_type: 'text',
26+
},
27+
'attachment content',
28+
] as AttachmentItem,
29+
],
30+
);
31+
1632
const transportOptions = {
1733
recordDroppedEvent: () => undefined, // noop
1834
textEncoder: new TextEncoder(),
@@ -122,7 +138,9 @@ describe('createTransport', () => {
122138

123139
await transport.send(ERROR_ENVELOPE);
124140
expect(requestExecutor).not.toHaveBeenCalled();
125-
expect(recordDroppedEventCallback).toHaveBeenCalledWith('ratelimit_backoff', 'error');
141+
expect(recordDroppedEventCallback).toHaveBeenCalledWith('ratelimit_backoff', 'error', {
142+
event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2',
143+
});
126144
requestExecutor.mockClear();
127145
recordDroppedEventCallback.mockClear();
128146

@@ -164,7 +182,9 @@ describe('createTransport', () => {
164182

165183
await transport.send(ERROR_ENVELOPE); // Error envelope should not be sent because of pending rate limit
166184
expect(requestExecutor).not.toHaveBeenCalled();
167-
expect(recordDroppedEventCallback).toHaveBeenCalledWith('ratelimit_backoff', 'error');
185+
expect(recordDroppedEventCallback).toHaveBeenCalledWith('ratelimit_backoff', 'error', {
186+
event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2',
187+
});
168188
requestExecutor.mockClear();
169189
recordDroppedEventCallback.mockClear();
170190

@@ -186,7 +206,7 @@ describe('createTransport', () => {
186206
const { retryAfterSeconds, beforeLimit, withinLimit, afterLimit } = setRateLimitTimes();
187207
const [transport, setTransportResponse, requestExecutor, recordDroppedEventCallback] = createTestTransport({
188208
headers: {
189-
'x-sentry-rate-limits': `${retryAfterSeconds}:error;transaction:scope`,
209+
'x-sentry-rate-limits': `${retryAfterSeconds}:error;transaction;attachment:scope`,
190210
'retry-after': null,
191211
},
192212
});
@@ -206,13 +226,23 @@ describe('createTransport', () => {
206226

207227
await transport.send(TRANSACTION_ENVELOPE); // Transaction envelope should not be sent because of pending rate limit
208228
expect(requestExecutor).not.toHaveBeenCalled();
209-
expect(recordDroppedEventCallback).toHaveBeenCalledWith('ratelimit_backoff', 'transaction');
229+
expect(recordDroppedEventCallback).toHaveBeenCalledWith('ratelimit_backoff', 'transaction', {
230+
event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2',
231+
});
210232
requestExecutor.mockClear();
211233
recordDroppedEventCallback.mockClear();
212234

213235
await transport.send(ERROR_ENVELOPE); // Error envelope should not be sent because of pending rate limit
214236
expect(requestExecutor).not.toHaveBeenCalled();
215-
expect(recordDroppedEventCallback).toHaveBeenCalledWith('ratelimit_backoff', 'error');
237+
expect(recordDroppedEventCallback).toHaveBeenCalledWith('ratelimit_backoff', 'error', {
238+
event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2',
239+
});
240+
requestExecutor.mockClear();
241+
recordDroppedEventCallback.mockClear();
242+
243+
await transport.send(ATTACHMENT_ENVELOPE); // Attachment envelope should not be sent because of pending rate limit
244+
expect(requestExecutor).not.toHaveBeenCalled();
245+
expect(recordDroppedEventCallback).toHaveBeenCalledWith('ratelimit_backoff', 'attachment', undefined);
216246
requestExecutor.mockClear();
217247
recordDroppedEventCallback.mockClear();
218248

@@ -228,6 +258,12 @@ describe('createTransport', () => {
228258
await transport.send(ERROR_ENVELOPE);
229259
expect(requestExecutor).toHaveBeenCalledTimes(1);
230260
expect(recordDroppedEventCallback).not.toHaveBeenCalled();
261+
requestExecutor.mockClear();
262+
recordDroppedEventCallback.mockClear();
263+
264+
await transport.send(ATTACHMENT_ENVELOPE);
265+
expect(requestExecutor).toHaveBeenCalledTimes(1);
266+
expect(recordDroppedEventCallback).not.toHaveBeenCalled();
231267
});
232268

233269
it('back-off using X-Sentry-Rate-Limits with missing categories should lock them all', async () => {
@@ -254,13 +290,17 @@ describe('createTransport', () => {
254290

255291
await transport.send(TRANSACTION_ENVELOPE); // Transaction envelope should not be sent because of pending rate limit
256292
expect(requestExecutor).not.toHaveBeenCalled();
257-
expect(recordDroppedEventCallback).toHaveBeenCalledWith('ratelimit_backoff', 'transaction');
293+
expect(recordDroppedEventCallback).toHaveBeenCalledWith('ratelimit_backoff', 'transaction', {
294+
event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2',
295+
});
258296
requestExecutor.mockClear();
259297
recordDroppedEventCallback.mockClear();
260298

261299
await transport.send(ERROR_ENVELOPE); // Error envelope should not be sent because of pending rate limit
262300
expect(requestExecutor).not.toHaveBeenCalled();
263-
expect(recordDroppedEventCallback).toHaveBeenCalledWith('ratelimit_backoff', 'error');
301+
expect(recordDroppedEventCallback).toHaveBeenCalledWith('ratelimit_backoff', 'error', {
302+
event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2',
303+
});
264304
requestExecutor.mockClear();
265305
recordDroppedEventCallback.mockClear();
266306

‎packages/types/src/client.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,7 @@ export interface Client<O extends ClientOptions = ClientOptions> {
126126
*
127127
* @param reason The reason why the event got dropped.
128128
* @param category The data category of the dropped event.
129+
* @param event The dropped event.
129130
*/
130-
recordDroppedEvent(reason: EventDropReason, category: DataCategory): void;
131+
recordDroppedEvent(reason: EventDropReason, dataCategory: DataCategory, event?: Event): void;
131132
}

‎packages/types/src/transport.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
1-
import { EventDropReason } from './clientreport';
2-
import { DataCategory } from './datacategory';
1+
import { Client } from './client';
32
import { Envelope } from './envelope';
43
import { TextEncoderInternal } from './textencoder';
54

@@ -18,7 +17,7 @@ export type TransportMakeRequestResponse = {
1817

1918
export interface InternalBaseTransportOptions {
2019
bufferSize?: number;
21-
recordDroppedEvent: (reason: EventDropReason, dataCategory: DataCategory) => void;
20+
recordDroppedEvent: Client['recordDroppedEvent'];
2221
textEncoder?: TextEncoderInternal;
2322
}
2423

0 commit comments

Comments
 (0)