Skip to content

Commit

Permalink
ref(utils): Introduce getEnvelopeType helper (#4751)
Browse files Browse the repository at this point in the history 
Simplify new transport send by grabbing the envelope category from the
envelope instead of passing it in explicitly.
  • Loading branch information
@AbhiPrasad
AbhiPrasad authored Mar 23, 2022
1 parent 6d63e58 commit 5f6335d
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 26 deletions.
8 changes: 6 additions & 2 deletions packages/core/src/transports/base.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { Envelope, EventStatus } from '@sentry/types';
import {
disabledUntil,
eventStatusFromHttpCode,
getEnvelopeType,
isRateLimited,
makePromiseBuffer,
PromiseBuffer,
Expand Down Expand Up @@ -68,6 +69,7 @@ export interface BrowserTransportOptions extends BaseTransportOptions {

// TODO: Move into Node transport
export interface NodeTransportOptions extends BaseTransportOptions {
headers?: Record<string, string>;
// Set a HTTP proxy that should be used for outbound requests.
httpProxy?: string;
// Set a HTTPS proxy that should be used for outbound requests.
Expand All @@ -81,7 +83,7 @@ export interface NewTransport {
// TODO(v7): Remove this as we will no longer have split between
// old and new transports.
$: boolean;
send(request: Envelope, category: TransportCategory): PromiseLike<TransportResponse>;
send(request: Envelope): PromiseLike<TransportResponse>;
flush(timeout?: number): PromiseLike<boolean>;
}

Expand All @@ -104,7 +106,9 @@ export function createTransport(

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

function send(envelope: Envelope, category: TransportCategory): PromiseLike<TransportResponse> {
function send(envelope: Envelope): PromiseLike<TransportResponse> {
const envCategory = getEnvelopeType(envelope);
const category = envCategory === 'event' ? 'error' : (envCategory as TransportCategory);
const request: TransportRequest = {
category,
body: serializeEnvelope(envelope),
Expand Down
44 changes: 22 additions & 22 deletions packages/core/test/lib/transports/base.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ describe('createTransport', () => {
expect(req.body).toEqual(serializeEnvelope(ERROR_ENVELOPE));
return resolvedSyncPromise({ statusCode: 200, reason: 'OK' });
});
const res = await transport.send(ERROR_ENVELOPE, ERROR_TRANSPORT_CATEGORY);
const res = await transport.send(ERROR_ENVELOPE);
expect(res.status).toBe('success');
expect(res.reason).toBe('OK');
});
Expand All @@ -59,7 +59,7 @@ describe('createTransport', () => {
return resolvedSyncPromise({ statusCode: 400, reason: 'Bad Request' });
});
try {
await transport.send(ERROR_ENVELOPE, ERROR_TRANSPORT_CATEGORY);
await transport.send(ERROR_ENVELOPE);
} catch (res) {
expect(res.status).toBe('invalid');
expect(res.reason).toBe('Bad Request');
Expand All @@ -73,7 +73,7 @@ describe('createTransport', () => {
return resolvedSyncPromise({ statusCode: 500 });
});
try {
await transport.send(TRANSACTION_ENVELOPE, TRANSACTION_TRANSPORT_CATEGORY);
await transport.send(TRANSACTION_ENVELOPE);
} catch (res) {
expect(res.status).toBe('failed');
expect(res.reason).toBe('Unknown transport error');
Expand Down Expand Up @@ -135,7 +135,7 @@ describe('createTransport', () => {
});

try {
await transport.send(ERROR_ENVELOPE, ERROR_TRANSPORT_CATEGORY);
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()}`);
Expand All @@ -144,13 +144,13 @@ describe('createTransport', () => {
setTransportResponse({ statusCode: 200 });

try {
await transport.send(ERROR_ENVELOPE, ERROR_TRANSPORT_CATEGORY);
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 res = await transport.send(ERROR_ENVELOPE, ERROR_TRANSPORT_CATEGORY);
const res = await transport.send(ERROR_ENVELOPE);
expect(res.status).toBe('success');
});

Expand Down Expand Up @@ -181,7 +181,7 @@ describe('createTransport', () => {
});

try {
await transport.send(ERROR_ENVELOPE, ERROR_TRANSPORT_CATEGORY);
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()}`);
Expand All @@ -190,20 +190,20 @@ describe('createTransport', () => {
setTransportResponse({ statusCode: 200 });

try {
await transport.send(TRANSACTION_ENVELOPE, TRANSACTION_TRANSPORT_CATEGORY);
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()}`);
}

try {
await transport.send(ERROR_ENVELOPE, ERROR_TRANSPORT_CATEGORY);
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 res = await transport.send(TRANSACTION_ENVELOPE, TRANSACTION_TRANSPORT_CATEGORY);
const res = await transport.send(TRANSACTION_ENVELOPE);
expect(res.status).toBe('success');
});

Expand Down Expand Up @@ -234,21 +234,21 @@ describe('createTransport', () => {
});

try {
await transport.send(ERROR_ENVELOPE, ERROR_TRANSPORT_CATEGORY);
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, ERROR_TRANSPORT_CATEGORY);
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, TRANSACTION_TRANSPORT_CATEGORY);
await transport.send(TRANSACTION_ENVELOPE);
} catch (res) {
expect(res.status).toBe('rate_limit');
expect(res.reason).toBe(
Expand All @@ -258,10 +258,10 @@ describe('createTransport', () => {

setTransportResponse({ statusCode: 200 });

const eventRes = await transport.send(ERROR_ENVELOPE, ERROR_TRANSPORT_CATEGORY);
const eventRes = await transport.send(ERROR_ENVELOPE);
expect(eventRes.status).toBe('success');

const transactionRes = await transport.send(TRANSACTION_ENVELOPE, TRANSACTION_TRANSPORT_CATEGORY);
const transactionRes = await transport.send(TRANSACTION_ENVELOPE);
expect(transactionRes.status).toBe('success');
});

Expand Down Expand Up @@ -296,21 +296,21 @@ describe('createTransport', () => {
});

try {
await transport.send(ERROR_ENVELOPE, ERROR_TRANSPORT_CATEGORY);
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, ERROR_TRANSPORT_CATEGORY);
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, TRANSACTION_TRANSPORT_CATEGORY);
await transport.send(TRANSACTION_ENVELOPE);
} catch (res) {
expect(res.status).toBe('rate_limit');
expect(res.reason).toBe(
Expand All @@ -320,10 +320,10 @@ describe('createTransport', () => {

setTransportResponse({ statusCode: 200 });

const eventRes = await transport.send(ERROR_ENVELOPE, ERROR_TRANSPORT_CATEGORY);
const eventRes = await transport.send(ERROR_ENVELOPE);
expect(eventRes.status).toBe('success');

const transactionRes = await transport.send(TRANSACTION_ENVELOPE, TRANSACTION_TRANSPORT_CATEGORY);
const transactionRes = await transport.send(TRANSACTION_ENVELOPE);
expect(transactionRes.status).toBe('success');
});

Expand Down Expand Up @@ -352,14 +352,14 @@ describe('createTransport', () => {
});

try {
await transport.send(ERROR_ENVELOPE, ERROR_TRANSPORT_CATEGORY);
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, TRANSACTION_TRANSPORT_CATEGORY);
await transport.send(TRANSACTION_ENVELOPE);
} catch (res) {
expect(res.status).toBe('rate_limit');
expect(res.reason).toBe(
Expand Down
2 changes: 1 addition & 1 deletion packages/types/src/envelope.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ type BaseEnvelope<EH extends BaseEnvelopeHeaders, I extends BaseEnvelopeItem<Bas

type EventItemHeaders = {
type: 'event' | 'transaction';
sample_rates: [{ id?: TransactionSamplingMethod; rate?: number }];
sample_rates?: [{ id?: TransactionSamplingMethod; rate?: number }];
};
type AttachmentItemHeaders = { type: 'attachment'; filename: string };
type UserFeedbackItemHeaders = { type: 'user_report' };
Expand Down
8 changes: 8 additions & 0 deletions packages/utils/src/envelope.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,14 @@ export function addItemToEnvelope<E extends Envelope>(envelope: E, newItem: E[1]
return [headers, [...items, newItem]] as E;
}

/**
* Get the type of the envelope. Grabs the type from the first envelope item.
*/
export function getEnvelopeType<E extends Envelope>(envelope: E): string {
const [, [[firstItemHeader]]] = envelope;
return firstItemHeader.type;
}

/**
* Serializes an envelope into a string.
*/
Expand Down
12 changes: 11 additions & 1 deletion packages/utils/test/envelope.test.ts
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { EventEnvelope } from '@sentry/types';

import { addItemToEnvelope, createEnvelope, serializeEnvelope } from '../src/envelope';
import { addItemToEnvelope, createEnvelope, getEnvelopeType, serializeEnvelope } from '../src/envelope';
import { parseEnvelope } from './testutils';

describe('envelope', () => {
Expand Down Expand Up @@ -44,4 +44,14 @@ describe('envelope', () => {
expect(parsedNewEnvelope[2]).toEqual({ event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2' });
});
});

describe('getEnvelopeType', () => {
it('returns the type of the envelope', () => {
const env = createEnvelope<EventEnvelope>({ event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2', sent_at: '123' }, [
[{ type: 'event' }, { event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2' }],
[{ type: 'attachment', filename: 'me.txt' }, '123456'],
]);
expect(getEnvelopeType(env)).toEqual('event');
});
});
});

0 comments on commit 5f6335d

Please sign in to comment.