Skip to content

Commit 8291249

Browse files
timfishtimfish
authored
feat(transport): Return result through Transport send (#6626)
1 parent 46a4c3c commit 8291249

File tree

5 files changed

+41
-47
lines changed

5 files changed

+41
-47
lines changed

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

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
EventItem,
88
InternalBaseTransportOptions,
99
Transport,
10+
TransportMakeRequestResponse,
1011
TransportRequestExecutor,
1112
} from '@sentry/types';
1213
import {
@@ -35,13 +36,15 @@ export const DEFAULT_TRANSPORT_BUFFER_SIZE = 30;
3536
export function createTransport(
3637
options: InternalBaseTransportOptions,
3738
makeRequest: TransportRequestExecutor,
38-
buffer: PromiseBuffer<void> = makePromiseBuffer(options.bufferSize || DEFAULT_TRANSPORT_BUFFER_SIZE),
39+
buffer: PromiseBuffer<void | TransportMakeRequestResponse> = makePromiseBuffer(
40+
options.bufferSize || DEFAULT_TRANSPORT_BUFFER_SIZE,
41+
),
3942
): Transport {
4043
let rateLimits: RateLimits = {};
4144

4245
const flush = (timeout?: number): PromiseLike<boolean> => buffer.drain(timeout);
4346

44-
function send(envelope: Envelope): PromiseLike<void> {
47+
function send(envelope: Envelope): PromiseLike<void | TransportMakeRequestResponse> {
4548
const filteredEnvelopeItems: EnvelopeItem[] = [];
4649

4750
// Drop rate limited items from envelope
@@ -71,7 +74,7 @@ export function createTransport(
7174
});
7275
};
7376

74-
const requestTask = (): PromiseLike<void> =>
77+
const requestTask = (): PromiseLike<void | TransportMakeRequestResponse> =>
7578
makeRequest({ body: serializeEnvelope(filteredEnvelope, options.textEncoder) }).then(
7679
response => {
7780
// We don't want to throw on NOK responses, but we want to at least log them
@@ -80,10 +83,11 @@ export function createTransport(
8083
}
8184

8285
rateLimits = updateRateLimits(rateLimits, response);
86+
return response;
8387
},
8488
error => {
85-
__DEBUG_BUILD__ && logger.error('Failed while sending event:', error);
8689
recordEnvelopeLoss('network_error');
90+
throw error;
8791
},
8892
);
8993

‎packages/node/test/transports/http.test.ts

Lines changed: 17 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ import { createGunzip } from 'zlib';
77

88
import { makeNodeTransport } from '../../src/transports';
99

10+
const textEncoder = new TextEncoder();
11+
1012
jest.mock('@sentry/core', () => {
1113
const actualCore = jest.requireActual('@sentry/core');
1214
return {
@@ -70,22 +72,19 @@ const EVENT_ENVELOPE = createEnvelope<EventEnvelope>({ event_id: 'aa3ff046696b4b
7072
[{ type: 'event' }, { event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2' }] as EventItem,
7173
]);
7274

73-
const SERIALIZED_EVENT_ENVELOPE = serializeEnvelope(EVENT_ENVELOPE, new TextEncoder());
75+
const SERIALIZED_EVENT_ENVELOPE = serializeEnvelope(EVENT_ENVELOPE, textEncoder);
7476

7577
const ATTACHMENT_ITEM = createAttachmentEnvelopeItem(
7678
{ filename: 'empty-file.bin', data: new Uint8Array(50_000) },
77-
new TextEncoder(),
79+
textEncoder,
7880
);
7981
const EVENT_ATTACHMENT_ENVELOPE = addItemToEnvelope(EVENT_ENVELOPE, ATTACHMENT_ITEM);
80-
const SERIALIZED_EVENT_ATTACHMENT_ENVELOPE = serializeEnvelope(
81-
EVENT_ATTACHMENT_ENVELOPE,
82-
new TextEncoder(),
83-
) as Uint8Array;
82+
const SERIALIZED_EVENT_ATTACHMENT_ENVELOPE = serializeEnvelope(EVENT_ATTACHMENT_ENVELOPE, textEncoder) as Uint8Array;
8483

8584
const defaultOptions = {
8685
url: TEST_SERVER_URL,
8786
recordDroppedEvent: () => undefined,
88-
textEncoder: new TextEncoder(),
87+
textEncoder,
8988
};
9089

9190
describe('makeNewHttpTransport()', () => {
@@ -151,7 +150,9 @@ describe('makeNewHttpTransport()', () => {
151150

152151
const transport = makeNodeTransport(defaultOptions);
153152

154-
await expect(transport.send(EVENT_ENVELOPE)).resolves.toBeUndefined();
153+
await expect(transport.send(EVENT_ENVELOPE)).resolves.toEqual(
154+
expect.objectContaining({ statusCode: serverStatusCode }),
155+
);
155156
},
156157
);
157158

@@ -165,20 +166,13 @@ describe('makeNewHttpTransport()', () => {
165166
});
166167

167168
const transport = makeNodeTransport(defaultOptions);
168-
await expect(transport.send(EVENT_ENVELOPE)).resolves.toBeUndefined();
169-
});
170-
171-
it('should resolve when server responds with rate limit header and status code 200', async () => {
172-
await setupTestServer({
169+
await expect(transport.send(EVENT_ENVELOPE)).resolves.toEqual({
173170
statusCode: SUCCESS,
174-
responseHeaders: {
175-
'Retry-After': '2700',
176-
'X-Sentry-Rate-Limits': '60::organization, 2700::organization',
171+
headers: {
172+
'retry-after': '2700',
173+
'x-sentry-rate-limits': '60::organization, 2700::organization',
177174
},
178175
});
179-
180-
const transport = makeNodeTransport(defaultOptions);
181-
await transport.send(EVENT_ENVELOPE);
182176
});
183177
});
184178

@@ -308,7 +302,7 @@ describe('makeNewHttpTransport()', () => {
308302
const registeredRequestExecutor = (createTransport as jest.Mock).mock.calls[0][1];
309303

310304
const executorResult = registeredRequestExecutor({
311-
body: serializeEnvelope(EVENT_ENVELOPE, new TextEncoder()),
305+
body: serializeEnvelope(EVENT_ENVELOPE, textEncoder),
312306
category: 'error',
313307
});
314308

@@ -328,7 +322,7 @@ describe('makeNewHttpTransport()', () => {
328322
const registeredRequestExecutor = (createTransport as jest.Mock).mock.calls[0][1];
329323

330324
const executorResult = registeredRequestExecutor({
331-
body: serializeEnvelope(EVENT_ENVELOPE, new TextEncoder()),
325+
body: serializeEnvelope(EVENT_ENVELOPE, textEncoder),
332326
category: 'error',
333327
});
334328

@@ -356,7 +350,7 @@ describe('makeNewHttpTransport()', () => {
356350
const registeredRequestExecutor = (createTransport as jest.Mock).mock.calls[0][1];
357351

358352
const executorResult = registeredRequestExecutor({
359-
body: serializeEnvelope(EVENT_ENVELOPE, new TextEncoder()),
353+
body: serializeEnvelope(EVENT_ENVELOPE, textEncoder),
360354
category: 'error',
361355
});
362356

@@ -384,7 +378,7 @@ describe('makeNewHttpTransport()', () => {
384378
const registeredRequestExecutor = (createTransport as jest.Mock).mock.calls[0][1];
385379

386380
const executorResult = registeredRequestExecutor({
387-
body: serializeEnvelope(EVENT_ENVELOPE, new TextEncoder()),
381+
body: serializeEnvelope(EVENT_ENVELOPE, textEncoder),
388382
category: 'error',
389383
});
390384

‎packages/node/test/transports/https.test.ts

Lines changed: 12 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ import { makeNodeTransport } from '../../src/transports';
99
import { HTTPModule, HTTPModuleRequestIncomingMessage } from '../../src/transports/http-module';
1010
import testServerCerts from './test-server-certs';
1111

12+
const textEncoder = new TextEncoder();
13+
1214
jest.mock('@sentry/core', () => {
1315
const actualCore = jest.requireActual('@sentry/core');
1416
return {
@@ -70,7 +72,7 @@ const EVENT_ENVELOPE = createEnvelope<EventEnvelope>({ event_id: 'aa3ff046696b4b
7072
[{ type: 'event' }, { event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2' }] as EventItem,
7173
]);
7274

73-
const SERIALIZED_EVENT_ENVELOPE = serializeEnvelope(EVENT_ENVELOPE, new TextEncoder());
75+
const SERIALIZED_EVENT_ENVELOPE = serializeEnvelope(EVENT_ENVELOPE, textEncoder);
7476

7577
const unsafeHttpsModule: HTTPModule = {
7678
request: jest
@@ -84,7 +86,7 @@ const defaultOptions = {
8486
httpModule: unsafeHttpsModule,
8587
url: TEST_SERVER_URL,
8688
recordDroppedEvent: () => undefined, // noop
87-
textEncoder: new TextEncoder(),
89+
textEncoder,
8890
};
8991

9092
describe('makeNewHttpsTransport()', () => {
@@ -151,20 +153,13 @@ describe('makeNewHttpsTransport()', () => {
151153
});
152154

153155
const transport = makeNodeTransport(defaultOptions);
154-
await expect(transport.send(EVENT_ENVELOPE)).resolves.toBeUndefined();
155-
});
156-
157-
it('should resolve when server responds with rate limit header and status code 200', async () => {
158-
await setupTestServer({
156+
await expect(transport.send(EVENT_ENVELOPE)).resolves.toEqual({
159157
statusCode: SUCCESS,
160-
responseHeaders: {
161-
'Retry-After': '2700',
162-
'X-Sentry-Rate-Limits': '60::organization, 2700::organization',
158+
headers: {
159+
'retry-after': '2700',
160+
'x-sentry-rate-limits': '60::organization, 2700::organization',
163161
},
164162
});
165-
166-
const transport = makeNodeTransport(defaultOptions);
167-
await expect(transport.send(EVENT_ENVELOPE)).resolves.toBeUndefined();
168163
});
169164

170165
it('should use `caCerts` option', async () => {
@@ -299,7 +294,7 @@ describe('makeNewHttpsTransport()', () => {
299294
const registeredRequestExecutor = (createTransport as jest.Mock).mock.calls[0][1];
300295

301296
const executorResult = registeredRequestExecutor({
302-
body: serializeEnvelope(EVENT_ENVELOPE, new TextEncoder()),
297+
body: serializeEnvelope(EVENT_ENVELOPE, textEncoder),
303298
category: 'error',
304299
});
305300

@@ -319,7 +314,7 @@ describe('makeNewHttpsTransport()', () => {
319314
const registeredRequestExecutor = (createTransport as jest.Mock).mock.calls[0][1];
320315

321316
const executorResult = registeredRequestExecutor({
322-
body: serializeEnvelope(EVENT_ENVELOPE, new TextEncoder()),
317+
body: serializeEnvelope(EVENT_ENVELOPE, textEncoder),
323318
category: 'error',
324319
});
325320

@@ -347,7 +342,7 @@ describe('makeNewHttpsTransport()', () => {
347342
const registeredRequestExecutor = (createTransport as jest.Mock).mock.calls[0][1];
348343

349344
const executorResult = registeredRequestExecutor({
350-
body: serializeEnvelope(EVENT_ENVELOPE, new TextEncoder()),
345+
body: serializeEnvelope(EVENT_ENVELOPE, textEncoder),
351346
category: 'error',
352347
});
353348

@@ -375,7 +370,7 @@ describe('makeNewHttpsTransport()', () => {
375370
const registeredRequestExecutor = (createTransport as jest.Mock).mock.calls[0][1];
376371

377372
const executorResult = registeredRequestExecutor({
378-
body: serializeEnvelope(EVENT_ENVELOPE, new TextEncoder()),
373+
body: serializeEnvelope(EVENT_ENVELOPE, textEncoder),
379374
category: 'error',
380375
});
381376

‎packages/replay/src/replay.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/* eslint-disable max-lines */ // TODO: We might want to split this file up
22
import { addGlobalEventProcessor, captureException, getCurrentHub, setContext } from '@sentry/core';
3-
import { Breadcrumb, ReplayEvent } from '@sentry/types';
3+
import { Breadcrumb, ReplayEvent, TransportMakeRequestResponse } from '@sentry/types';
44
import { addInstrumentationHandler, logger } from '@sentry/utils';
55
import { EventType, record } from 'rrweb';
66

@@ -904,7 +904,7 @@ export class ReplayContainer implements ReplayContainerInterface {
904904
segmentId: segment_id,
905905
includeReplayStartTimestamp,
906906
eventContext,
907-
}: SendReplay): Promise<void | undefined> {
907+
}: SendReplay): Promise<void | TransportMakeRequestResponse> {
908908
const payloadWithSequence = createPayload({
909909
events,
910910
headers: {

‎packages/types/src/transport.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,8 @@ export interface BaseTransportOptions extends InternalBaseTransportOptions {
2929
}
3030

3131
export interface Transport {
32-
send(request: Envelope): PromiseLike<void>;
32+
// TODO (v8) Remove void from return as it was only retained to avoid a breaking change
33+
send(request: Envelope): PromiseLike<void | TransportMakeRequestResponse>;
3334
flush(timeout?: number): PromiseLike<boolean>;
3435
}
3536

0 commit comments

Comments
 (0)