From 0784296290f340cd42facac17fbd51aec5592b37 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Tue, 1 Mar 2022 14:56:46 -0500 Subject: [PATCH 1/3] ideas --- packages/core/src/transports/base.ts | 69 ++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) create mode 100644 packages/core/src/transports/base.ts diff --git a/packages/core/src/transports/base.ts b/packages/core/src/transports/base.ts new file mode 100644 index 000000000000..a93a818b55c4 --- /dev/null +++ b/packages/core/src/transports/base.ts @@ -0,0 +1,69 @@ +import { EventStatus, SentryRequestType, TransportOptions, DsnLike } from '@sentry/types'; + +export type TransportRequest = { + body: T; + type: SentryRequestType; +}; + +export type TransportMakeRequestResponse = { + body?: string; + headers?: Record; + reason?: string; + statusCode: number; +}; + +export type TransportResponse = { + status: EventStatus; + reason?: string; +}; + +interface INewTransport { + sendRequest(request: TransportRequest): PromiseLike; + flush(timeout: number): PromiseLike; +} + +export type TransportOptions = { + dsn: string; + /** Set a HTTP proxy that should be used for outbound requests. */ + proxy?: string; + /** HTTPS proxy certificates path */ + caCerts?: string; + /** Fetch API init parameters */ + credentials?: string; + headers?: Record; + bufferSize?: number; +}; + +/** JSDoc */ +export interface BaseTransportOptions { + /** Sentry DSN */ + dsn: DsnLike; + /** Define custom headers */ + headers?: { [key: string]: string }; + /** Set a HTTP proxy that should be used for outbound requests. */ + httpProxy?: string; // ONLY USED BY NODE SDK + /** Set a HTTPS proxy that should be used for outbound requests. */ + httpsProxy?: string; // ONLY USED BY NODE SDK + /** HTTPS proxy certificates path */ + caCerts?: string; // ONLY USED BY NODE SDK + /** Fetch API init parameters */ + fetchParameters?: { [key: string]: string }; // ONLY USED BY BROWSER SDK + /** The envelope tunnel to use. */ + tunnel?: string; + /** Send SDK Client Reports. Enabled by default. */ + sendClientReports?: boolean; // ONLY USED BY BROWSER SDK ATM + /** + * Set of metadata about the SDK that can be internally used to enhance envelopes and events, + * and provide additional data about every request. + * */ + _metadata?: SdkMetadata; +} + +/** + * + */ +export abstract class BaseTransport implements INewTransport { + public constructor(protected readonly _options: TransportOptions) {} + + protected abstract _makeRequest(request: TransportRequest): PromiseLike; +} From fa2b1520aa49076ede5b4017ecd996017b277e96 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Tue, 1 Mar 2022 17:39:09 -0500 Subject: [PATCH 2/3] more experiments --- packages/core/src/transports/base.ts | 129 ++++++++++++++++++--------- 1 file changed, 87 insertions(+), 42 deletions(-) diff --git a/packages/core/src/transports/base.ts b/packages/core/src/transports/base.ts index a93a818b55c4..0eec5f051ac0 100644 --- a/packages/core/src/transports/base.ts +++ b/packages/core/src/transports/base.ts @@ -1,7 +1,16 @@ -import { EventStatus, SentryRequestType, TransportOptions, DsnLike } from '@sentry/types'; +import { Envelope, EventStatus, SentryRequestType } from '@sentry/types'; +import { + eventStatusFromHttpCode, + makePromiseBuffer, + PromiseBuffer, + rejectedSyncPromise, + resolvedSyncPromise, + serializeEnvelope, + SentryError, +} from '@sentry/utils'; -export type TransportRequest = { - body: T; +export type TransportRequest = { + body: string; type: SentryRequestType; }; @@ -17,53 +26,89 @@ export type TransportResponse = { reason?: string; }; -interface INewTransport { - sendRequest(request: TransportRequest): PromiseLike; - flush(timeout: number): PromiseLike; +export interface BaseTransportOptions { + // url to send the event + // transport does not care about dsn specific - client should take care of + // parsing and figuring that out + url: string; + headers?: Record; + bufferSize?: number; // make transport buffer size configurable +} + +export interface BrowserTransportOptions extends BaseTransportOptions { + // options to pass into fetch request + fetchParams: Record; + sendClientReports?: boolean; } -export type TransportOptions = { - dsn: string; - /** Set a HTTP proxy that should be used for outbound requests. */ - proxy?: string; - /** HTTPS proxy certificates path */ +export interface NodeTransportOptions extends BaseTransportOptions { + // Set a HTTP proxy that should be used for outbound requests. + httpProxy?: string; + // Set a HTTPS proxy that should be used for outbound requests. + httpsProxy?: string; + // HTTPS proxy certificates path caCerts?: string; - /** Fetch API init parameters */ - credentials?: string; - headers?: Record; - bufferSize?: number; -}; +} -/** JSDoc */ -export interface BaseTransportOptions { - /** Sentry DSN */ - dsn: DsnLike; - /** Define custom headers */ - headers?: { [key: string]: string }; - /** Set a HTTP proxy that should be used for outbound requests. */ - httpProxy?: string; // ONLY USED BY NODE SDK - /** Set a HTTPS proxy that should be used for outbound requests. */ - httpsProxy?: string; // ONLY USED BY NODE SDK - /** HTTPS proxy certificates path */ - caCerts?: string; // ONLY USED BY NODE SDK - /** Fetch API init parameters */ - fetchParameters?: { [key: string]: string }; // ONLY USED BY BROWSER SDK - /** The envelope tunnel to use. */ - tunnel?: string; - /** Send SDK Client Reports. Enabled by default. */ - sendClientReports?: boolean; // ONLY USED BY BROWSER SDK ATM - /** - * Set of metadata about the SDK that can be internally used to enhance envelopes and events, - * and provide additional data about every request. - * */ - _metadata?: SdkMetadata; +interface INewTransport { + send(request: Envelope, type: SentryRequestType): PromiseLike; + flush(timeout: number): PromiseLike; } /** - * + * Heavily based on Kamil's work in + * https://github.com/getsentry/sentry-javascript/blob/v7-dev/packages/transport-base/src/transport.ts */ export abstract class BaseTransport implements INewTransport { - public constructor(protected readonly _options: TransportOptions) {} + protected readonly _buffer: PromiseBuffer; + private readonly _rateLimits: Record = {}; + + public constructor(protected readonly _options: BaseTransportOptions) { + this._buffer = makePromiseBuffer(this._options.bufferSize || 30); + } + + /** */ + public send(envelope: Envelope, type: SentryRequestType): PromiseLike { + const request: TransportRequest = { + // I'm undecided if the type API should work like this + // though we are a little stuck with this because of how + // minimal the envelopes implementation is + // perhaps there is a way we can expand it? + type, + body: serializeEnvelope(envelope), + }; + + if (isRateLimited(this._rateLimits, type)) { + return rejectedSyncPromise( + new SentryError(`oh no, disabled until: ${rateLimitDisableUntil(this._rateLimits, type)}`), + ); + } + + const requestTask = (): PromiseLike => + this._makeRequest(request).then(({ body, headers, reason, statusCode }): PromiseLike => { + if (headers) { + updateRateLimits(this._rateLimits, headers); + } + + // TODO: This is the happy path! + const status = eventStatusFromHttpCode(statusCode); + if (status === 'success') { + return resolvedSyncPromise({ status }); + } + + return rejectedSyncPromise(new SentryError(body || reason || 'Unknown transport error')); + }); + + return this._buffer.add(requestTask); + } + + /** */ + public flush(timeout?: number): PromiseLike { + return this._buffer.drain(timeout); + } - protected abstract _makeRequest(request: TransportRequest): PromiseLike; + // Each is up to each transport implementation to determine how to make a request -> return an according response + // `TransportMakeRequestResponse` is different than `TransportResponse` because the client doesn't care about + // these extra details + protected abstract _makeRequest(request: TransportRequest): PromiseLike; } From 578b16b79907cfd1ffa13ef5c5d38a0bf7cd2b36 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Wed, 2 Mar 2022 10:40:24 -0500 Subject: [PATCH 3/3] add functional approach --- packages/core/src/transports/base.ts | 45 ++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/packages/core/src/transports/base.ts b/packages/core/src/transports/base.ts index 0eec5f051ac0..7beb72cf97df 100644 --- a/packages/core/src/transports/base.ts +++ b/packages/core/src/transports/base.ts @@ -112,3 +112,48 @@ export abstract class BaseTransport implements INewTransport { // these extra details protected abstract _makeRequest(request: TransportRequest): PromiseLike; } + +/** */ +function createTransport( + options: O, + makeRequest: (request: TransportRequest) => PromiseLike, +): INewTransport { + const buffer = makePromiseBuffer(options.bufferSize || 30); + const rateLimits: Record = {}; + + const flush = (timeout?: number): PromiseLike => buffer.drain(timeout); + + function send(envelope: Envelope, type: SentryRequestType): PromiseLike { + const request: TransportRequest = { + // I'm undecided if the type API should work like this + // though we are a little stuck with this because of how + // minimal the envelopes implementation is + // perhaps there is a way we can expand it? + type, + body: serializeEnvelope(envelope), + }; + + if (isRateLimited(rateLimits, type)) { + return rejectedSyncPromise(new SentryError(`oh no, disabled until: ${rateLimitDisableUntil(rateLimits, type)}`)); + } + + const requestTask = (): PromiseLike => + makeRequest(request).then(({ body, headers, reason, statusCode }): PromiseLike => { + if (headers) { + updateRateLimits(rateLimits, headers); + } + + // TODO: This is the happy path! + const status = eventStatusFromHttpCode(statusCode); + if (status === 'success') { + return resolvedSyncPromise({ status }); + } + + return rejectedSyncPromise(new SentryError(body || reason || 'Unknown transport error')); + }); + + return buffer.add(requestTask); + } + + return { send, flush }; +}