From c7e59ccf9b987f45600df0804dc8532b835df98e Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Tue, 22 Mar 2022 09:24:53 -0400 Subject: [PATCH 1/6] ref(utils): Introduce getEnvelopeType helper Simplify new transport send by grabbing the envelope category from the envelope instead of passing it in explicitly. --- packages/core/src/transports/base.ts | 7 ++- .../core/test/lib/transports/base.test.ts | 44 +++++++++---------- packages/types/src/envelope.ts | 2 +- packages/utils/src/envelope.ts | 8 ++++ packages/utils/test/envelope.test.ts | 12 ++++- 5 files changed, 47 insertions(+), 26 deletions(-) diff --git a/packages/core/src/transports/base.ts b/packages/core/src/transports/base.ts index 3baee090e42f..ef5ef2f94bdb 100644 --- a/packages/core/src/transports/base.ts +++ b/packages/core/src/transports/base.ts @@ -2,6 +2,7 @@ import { Envelope, EventStatus } from '@sentry/types'; import { disabledUntil, eventStatusFromHttpCode, + getEnvelopeType, isRateLimited, makePromiseBuffer, PromiseBuffer, @@ -68,6 +69,7 @@ export interface BrowserTransportOptions extends BaseTransportOptions { // TODO: Move into Node transport export interface NodeTransportOptions extends BaseTransportOptions { + headers?: Record; // Set a HTTP proxy that should be used for outbound requests. httpProxy?: string; // Set a HTTPS proxy that should be used for outbound requests. @@ -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; + send(request: Envelope): PromiseLike; flush(timeout?: number): PromiseLike; } @@ -104,7 +106,8 @@ export function createTransport( const flush = (timeout?: number): PromiseLike => buffer.drain(timeout); - function send(envelope: Envelope, category: TransportCategory): PromiseLike { + function send(envelope: Envelope): PromiseLike { + const category = getEnvelopeType(envelope); const request: TransportRequest = { category, body: serializeEnvelope(envelope), diff --git a/packages/core/test/lib/transports/base.test.ts b/packages/core/test/lib/transports/base.test.ts index 993730e1b90d..cb32bb44bd73 100644 --- a/packages/core/test/lib/transports/base.test.ts +++ b/packages/core/test/lib/transports/base.test.ts @@ -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'); }); @@ -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'); @@ -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'); @@ -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()}`); @@ -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'); }); @@ -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()}`); @@ -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'); }); @@ -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( @@ -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'); }); @@ -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( @@ -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'); }); @@ -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( diff --git a/packages/types/src/envelope.ts b/packages/types/src/envelope.ts index b03d42eb3ea8..8e5091dbbfa9 100644 --- a/packages/types/src/envelope.ts +++ b/packages/types/src/envelope.ts @@ -28,7 +28,7 @@ type BaseEnvelope(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(envelope: E): string { + const [, [[firstItemHeader]]] = envelope; + return firstItemHeader.type; +} + /** * Serializes an envelope into a string. */ diff --git a/packages/utils/test/envelope.test.ts b/packages/utils/test/envelope.test.ts index cb4f65335f36..2cca6981e7e4 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, serializeEnvelope } from '../src/envelope'; +import { addItemToEnvelope, createEnvelope, serializeEnvelope, getEnvelopeType } from '../src/envelope'; import { parseEnvelope } from './testutils'; describe('envelope', () => { @@ -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({ event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2', sent_at: '123' }, [ + [{ type: 'event' }, { event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2' }], + [{ type: 'attachment', filename: 'me.txt' }, '123456'], + ]); + expect(getEnvelopeType(env)).toEqual('event'); + }); + }); }); From 9343899cfd213a14cf5469783baa54770dece199 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Tue, 22 Mar 2022 10:42:37 -0400 Subject: [PATCH 2/6] add cast --- packages/core/src/transports/base.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/src/transports/base.ts b/packages/core/src/transports/base.ts index ef5ef2f94bdb..653f96748756 100644 --- a/packages/core/src/transports/base.ts +++ b/packages/core/src/transports/base.ts @@ -107,7 +107,7 @@ export function createTransport( const flush = (timeout?: number): PromiseLike => buffer.drain(timeout); function send(envelope: Envelope): PromiseLike { - const category = getEnvelopeType(envelope); + const category = getEnvelopeType(envelope) as TransportCategory; const request: TransportRequest = { category, body: serializeEnvelope(envelope), From 5f0b1d0fb6e4ec7c21e0801d70c3fba1ab0410f3 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Tue, 22 Mar 2022 11:15:34 -0400 Subject: [PATCH 3/6] yarn fix --- packages/utils/test/envelope.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/utils/test/envelope.test.ts b/packages/utils/test/envelope.test.ts index 2cca6981e7e4..dab2b92d5f47 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, serializeEnvelope, getEnvelopeType } from '../src/envelope'; +import { addItemToEnvelope, createEnvelope, getEnvelopeType, serializeEnvelope } from '../src/envelope'; import { parseEnvelope } from './testutils'; describe('envelope', () => { From 9a03821bcbdad98387b5001970cb6e1cfd73b129 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Tue, 22 Mar 2022 12:09:32 -0400 Subject: [PATCH 4/6] fix category --- packages/core/src/transports/base.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/core/src/transports/base.ts b/packages/core/src/transports/base.ts index 653f96748756..d5e7218cd87d 100644 --- a/packages/core/src/transports/base.ts +++ b/packages/core/src/transports/base.ts @@ -107,7 +107,8 @@ export function createTransport( const flush = (timeout?: number): PromiseLike => buffer.drain(timeout); function send(envelope: Envelope): PromiseLike { - const category = getEnvelopeType(envelope) as TransportCategory; + const envCategory = getEnvelopeType(envelope); + const category = envCategory === 'event' ? 'error' : (envCategory as TransportCategory); const request: TransportRequest = { category, body: serializeEnvelope(envelope), From 2326bb99444ec0029280e77e2697471b5a9eaaa5 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Tue, 22 Mar 2022 09:54:56 -0400 Subject: [PATCH 5/6] feat(core): Add new transports to base backend Adds new transports to base backend in core. For now, they are gated behind `options._experiments.newTransport = true`. The next step is to add new transports for fetch, xhr (browser) as well as http, https (node). --- packages/core/src/basebackend.ts | 48 ++++++++++++++++--- packages/core/src/request.ts | 41 +++++++++++++++- packages/core/src/transports/base.ts | 5 -- .../core/test/lib/transports/base.test.ts | 5 -- 4 files changed, 81 insertions(+), 18 deletions(-) diff --git a/packages/core/src/basebackend.ts b/packages/core/src/basebackend.ts index 4ba8cce63bd7..aba7919990b8 100644 --- a/packages/core/src/basebackend.ts +++ b/packages/core/src/basebackend.ts @@ -1,6 +1,9 @@ import { Event, EventHint, Options, Session, Severity, Transport } from '@sentry/types'; import { isDebugBuild, logger, SentryError } from '@sentry/utils'; +import { initAPIDetails } from './api'; +import { createEventEnvelope, createSessionEnvelope } from './request'; +import { NewTransport } from './transports/base'; import { NoopTransport } from './transports/noop'; /** @@ -63,6 +66,9 @@ export abstract class BaseBackend implements Backend { /** Cached transport used internally. */ protected _transport: Transport; + /** New v7 Transport that is initialized alongside the old one */ + protected _newTransport?: NewTransport; + /** Creates a new backend instance. */ public constructor(options: O) { this._options = options; @@ -91,9 +97,23 @@ export abstract class BaseBackend implements Backend { * @inheritDoc */ public sendEvent(event: Event): void { - void this._transport.sendEvent(event).then(null, reason => { - isDebugBuild() && logger.error('Error while sending event:', reason); - }); + // TODO(v7): Remove the if-else + if ( + this._newTransport && + this._options.dsn && + this._options._experiments && + this._options._experiments.newTransport + ) { + const api = initAPIDetails(this._options.dsn, this._options._metadata, this._options.tunnel); + const env = createEventEnvelope(event, api); + void this._newTransport.send(env).then(null, reason => { + isDebugBuild() && logger.error('Error while sending event:', reason); + }); + } else { + void this._transport.sendEvent(event).then(null, reason => { + isDebugBuild() && logger.error('Error while sending event:', reason); + }); + } } /** @@ -105,9 +125,23 @@ export abstract class BaseBackend implements Backend { return; } - void this._transport.sendSession(session).then(null, reason => { - isDebugBuild() && logger.error('Error while sending session:', reason); - }); + // TODO(v7): Remove the if-else + if ( + this._newTransport && + this._options.dsn && + this._options._experiments && + this._options._experiments.newTransport + ) { + const api = initAPIDetails(this._options.dsn, this._options._metadata, this._options.tunnel); + const [env] = createSessionEnvelope(session, api); + void this._newTransport.send(env).then(null, reason => { + isDebugBuild() && logger.error('Error while sending session:', reason); + }); + } else { + void this._transport.sendSession(session).then(null, reason => { + isDebugBuild() && logger.error('Error while sending session:', reason); + }); + } } /** @@ -123,4 +157,6 @@ export abstract class BaseBackend implements Backend { protected _setupTransport(): Transport { return new NoopTransport(); } + + protected abstract _setupNewTransport(): NewTransport; } diff --git a/packages/core/src/request.ts b/packages/core/src/request.ts index 01cdd4a3035f..db53a0a367e6 100644 --- a/packages/core/src/request.ts +++ b/packages/core/src/request.ts @@ -39,8 +39,11 @@ function enhanceEventWithSdkInfo(event: Event, sdkInfo?: SdkInfo): Event { return event; } -/** Creates a SentryRequest from a Session. */ -export function sessionToSentryRequest(session: Session | SessionAggregates, api: APIDetails): SentryRequest { +/** Creates an envelope from a Session */ +export function createSessionEnvelope( + session: Session | SessionAggregates, + api: APIDetails, +): [SessionEnvelope, SentryRequestType] { const sdkInfo = getSdkMetadataForEnvelopeHeader(api); const envelopeHeaders = { sent_at: new Date().toISOString(), @@ -54,6 +57,13 @@ export function sessionToSentryRequest(session: Session | SessionAggregates, api // TODO (v7) Have to cast type because envelope items do not accept a `SentryRequestType` const envelopeItem = [{ type } as { type: 'session' | 'sessions' }, session] as SessionItem; const envelope = createEnvelope(envelopeHeaders, [envelopeItem]); + + return [envelope, type]; +} + +/** Creates a SentryRequest from a Session. */ +export function sessionToSentryRequest(session: Session | SessionAggregates, api: APIDetails): SentryRequest { + const [envelope, type] = createSessionEnvelope(session, api); return { body: serializeEnvelope(envelope), type, @@ -61,6 +71,33 @@ export function sessionToSentryRequest(session: Session | SessionAggregates, api }; } +/** + * Create an Envelope from an event. Note that this is duplicated from below, + * but on purpose as this will be refactored in v7. + */ +export function createEventEnvelope(event: Event, api: APIDetails): EventEnvelope { + const sdkInfo = getSdkMetadataForEnvelopeHeader(api); + const eventType = event.type || 'event'; + + const { transactionSampling } = event.sdkProcessingMetadata || {}; + const { method: samplingMethod, rate: sampleRate } = transactionSampling || {}; + + const envelopeHeaders = { + event_id: event.event_id as string, + sent_at: new Date().toISOString(), + ...(sdkInfo && { sdk: sdkInfo }), + ...(!!api.tunnel && { dsn: dsnToString(api.dsn) }), + }; + const eventItem: EventItem = [ + { + type: eventType, + sample_rates: [{ id: samplingMethod, rate: sampleRate }], + }, + event, + ]; + return createEnvelope(envelopeHeaders, [eventItem]); +} + /** Creates a SentryRequest from an event. */ export function eventToSentryRequest(event: Event, api: APIDetails): SentryRequest { const sdkInfo = getSdkMetadataForEnvelopeHeader(api); diff --git a/packages/core/src/transports/base.ts b/packages/core/src/transports/base.ts index d5e7218cd87d..6290fbfec36e 100644 --- a/packages/core/src/transports/base.ts +++ b/packages/core/src/transports/base.ts @@ -79,10 +79,6 @@ export interface NodeTransportOptions extends BaseTransportOptions { } export interface NewTransport { - // If `$` is set, we know that this is a new transport. - // TODO(v7): Remove this as we will no longer have split between - // old and new transports. - $: boolean; send(request: Envelope): PromiseLike; flush(timeout?: number): PromiseLike; } @@ -144,7 +140,6 @@ export function createTransport( } return { - $: true, send, flush, }; diff --git a/packages/core/test/lib/transports/base.test.ts b/packages/core/test/lib/transports/base.test.ts index cb32bb44bd73..2257a67165b1 100644 --- a/packages/core/test/lib/transports/base.test.ts +++ b/packages/core/test/lib/transports/base.test.ts @@ -20,11 +20,6 @@ const TRANSACTION_ENVELOPE = createEnvelope( ); describe('createTransport', () => { - it('has $ property', () => { - const transport = createTransport({}, _ => resolvedSyncPromise({ statusCode: 200 })); - expect(transport.$).toBeDefined(); - }); - it('flushes the buffer', async () => { const mockBuffer: PromiseBuffer = { $: [], From 26db35b27d678e83c39d28b9a1840259257b4b5d Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Wed, 23 Mar 2022 09:05:14 -0400 Subject: [PATCH 6/6] remove _setupNewTransport --- packages/core/src/basebackend.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/core/src/basebackend.ts b/packages/core/src/basebackend.ts index aba7919990b8..f770c5aa0d8c 100644 --- a/packages/core/src/basebackend.ts +++ b/packages/core/src/basebackend.ts @@ -157,6 +157,4 @@ export abstract class BaseBackend implements Backend { protected _setupTransport(): Transport { return new NoopTransport(); } - - protected abstract _setupNewTransport(): NewTransport; }