From d4f67a27ebfbd2bd59428cd85d6ee1864c8cad82 Mon Sep 17 00:00:00 2001 From: Daniel Griesser Date: Wed, 27 Sep 2023 15:33:40 +0200 Subject: [PATCH] feat: Always assemble Envelopes (#9101) --- packages/browser/src/client.ts | 1 + packages/core/src/baseclient.ts | 59 ++++++++----------- packages/core/src/envelope.ts | 6 +- packages/core/test/lib/base.test.ts | 45 ++------------ packages/sveltekit/test/server/handle.test.ts | 2 +- packages/types/src/client.ts | 2 +- packages/utils/src/envelope.ts | 4 +- 7 files changed, 37 insertions(+), 82 deletions(-) diff --git a/packages/browser/src/client.ts b/packages/browser/src/client.ts index 5027c4f0f1a4..4c0ace57547b 100644 --- a/packages/browser/src/client.ts +++ b/packages/browser/src/client.ts @@ -127,6 +127,7 @@ export class BrowserClient extends BaseClient { return; } + // This is really the only place where we want to check for a DSN and only send outcomes then if (!this._dsn) { __DEBUG_BUILD__ && logger.log('No dsn provided, will not send outcomes'); return; diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index 1a15c4bc37bd..c811c4f827a3 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -127,7 +127,7 @@ export abstract class BaseClient implements Client { if (options.dsn) { this._dsn = makeDsn(options.dsn); } else { - __DEBUG_BUILD__ && logger.warn('No DSN provided, client will not do anything.'); + __DEBUG_BUILD__ && logger.warn('No DSN provided, client will not send events.'); } if (this._dsn) { @@ -216,11 +216,6 @@ export abstract class BaseClient implements Client { * @inheritDoc */ public captureSession(session: Session): void { - if (!this._isEnabled()) { - __DEBUG_BUILD__ && logger.warn('SDK not enabled, will not capture session.'); - return; - } - if (!(typeof session.release === 'string')) { __DEBUG_BUILD__ && logger.warn('Discarded session because of missing or non-string release'); } else { @@ -297,8 +292,8 @@ export abstract class BaseClient implements Client { /** * Sets up the integrations */ - public setupIntegrations(): void { - if (this._isEnabled() && !this._integrationsInitialized) { + public setupIntegrations(forceInitialize?: boolean): void { + if ((forceInitialize && !this._integrationsInitialized) || (this._isEnabled() && !this._integrationsInitialized)) { this._integrations = setupIntegrations(this, this._options.integrations); this._integrationsInitialized = true; } @@ -338,23 +333,21 @@ export abstract class BaseClient implements Client { public sendEvent(event: Event, hint: EventHint = {}): void { this.emit('beforeSendEvent', event, hint); - if (this._dsn) { - let env = createEventEnvelope(event, this._dsn, this._options._metadata, this._options.tunnel); - - for (const attachment of hint.attachments || []) { - env = addItemToEnvelope( - env, - createAttachmentEnvelopeItem( - attachment, - this._options.transportOptions && this._options.transportOptions.textEncoder, - ), - ); - } + let env = createEventEnvelope(event, this._dsn, this._options._metadata, this._options.tunnel); - const promise = this._sendEnvelope(env); - if (promise) { - promise.then(sendResponse => this.emit('afterSendEvent', event, sendResponse), null); - } + for (const attachment of hint.attachments || []) { + env = addItemToEnvelope( + env, + createAttachmentEnvelopeItem( + attachment, + this._options.transportOptions && this._options.transportOptions.textEncoder, + ), + ); + } + + const promise = this._sendEnvelope(env); + if (promise) { + promise.then(sendResponse => this.emit('afterSendEvent', event, sendResponse), null); } } @@ -362,10 +355,8 @@ export abstract class BaseClient implements Client { * @inheritDoc */ public sendSession(session: Session | SessionAggregates): void { - if (this._dsn) { - const env = createSessionEnvelope(session, this._dsn, this._options._metadata, this._options.tunnel); - void this._sendEnvelope(env); - } + const env = createSessionEnvelope(session, this._dsn, this._options._metadata, this._options.tunnel); + void this._sendEnvelope(env); } /** @@ -531,9 +522,9 @@ export abstract class BaseClient implements Client { }); } - /** Determines whether this SDK is enabled and a valid Dsn is present. */ + /** Determines whether this SDK is enabled and a transport is present. */ protected _isEnabled(): boolean { - return this.getOptions().enabled !== false && this._dsn !== undefined; + return this.getOptions().enabled !== false && this._transport !== undefined; } /** @@ -635,10 +626,6 @@ export abstract class BaseClient implements Client { const options = this.getOptions(); const { sampleRate } = options; - if (!this._isEnabled()) { - return rejectedSyncPromise(new SentryError('SDK not enabled, will not capture event.', 'log')); - } - const isTransaction = isTransactionEvent(event); const isError = isErrorEvent(event); const eventType = event.type || 'error'; @@ -738,9 +725,9 @@ export abstract class BaseClient implements Client { * @inheritdoc */ protected _sendEnvelope(envelope: Envelope): PromiseLike | void { - if (this._transport && this._dsn) { - this.emit('beforeEnvelope', envelope); + this.emit('beforeEnvelope', envelope); + if (this._isEnabled() && this._transport) { return this._transport.send(envelope).then(null, reason => { __DEBUG_BUILD__ && logger.error('Error while sending event:', reason); }); diff --git a/packages/core/src/envelope.ts b/packages/core/src/envelope.ts index 5e79d1707d67..9ec29c9d2a7e 100644 --- a/packages/core/src/envelope.ts +++ b/packages/core/src/envelope.ts @@ -36,7 +36,7 @@ function enhanceEventWithSdkInfo(event: Event, sdkInfo?: SdkInfo): Event { /** Creates an envelope from a Session */ export function createSessionEnvelope( session: Session | SessionAggregates, - dsn: DsnComponents, + dsn?: DsnComponents, metadata?: SdkMetadata, tunnel?: string, ): SessionEnvelope { @@ -44,7 +44,7 @@ export function createSessionEnvelope( const envelopeHeaders = { sent_at: new Date().toISOString(), ...(sdkInfo && { sdk: sdkInfo }), - ...(!!tunnel && { dsn: dsnToString(dsn) }), + ...(!!tunnel && dsn && { dsn: dsnToString(dsn) }), }; const envelopeItem: SessionItem = @@ -58,7 +58,7 @@ export function createSessionEnvelope( */ export function createEventEnvelope( event: Event, - dsn: DsnComponents, + dsn?: DsnComponents, metadata?: SdkMetadata, tunnel?: string, ): EventEnvelope { diff --git a/packages/core/test/lib/base.test.ts b/packages/core/test/lib/base.test.ts index dd0906921fa3..27e34a1402d4 100644 --- a/packages/core/test/lib/base.test.ts +++ b/packages/core/test/lib/base.test.ts @@ -398,30 +398,6 @@ describe('BaseClient', () => { }); describe('captureEvent() / prepareEvent()', () => { - test('skips when disabled', () => { - expect.assertions(1); - - const options = getDefaultTestClientOptions({ enabled: false, dsn: PUBLIC_DSN }); - const client = new TestClient(options); - const scope = new Scope(); - - client.captureEvent({}, undefined, scope); - - expect(TestClient.instance!.event).toBeUndefined(); - }); - - test('skips without a Dsn', () => { - expect.assertions(1); - - const options = getDefaultTestClientOptions({}); - const client = new TestClient(options); - const scope = new Scope(); - - client.captureEvent({}, undefined, scope); - - expect(TestClient.instance!.event).toBeUndefined(); - }); - test.each([ ['`Error` instance', new Error('Will I get caught twice?')], ['plain object', { 'Will I': 'get caught twice?' }], @@ -1616,9 +1592,9 @@ describe('BaseClient', () => { test('close', async () => { jest.useRealTimers(); - expect.assertions(2); + expect.assertions(4); - const { makeTransport, delay } = makeFakeTransport(300); + const { makeTransport, delay, getSentCount } = makeFakeTransport(300); const client = new TestClient( getDefaultTestClientOptions({ @@ -1630,9 +1606,12 @@ describe('BaseClient', () => { expect(client.captureMessage('test')).toBeTruthy(); await client.close(delay); + expect(getSentCount()).toBe(1); + expect(client.captureMessage('test')).toBeTruthy(); + await client.close(delay); // Sends after close shouldn't work anymore - expect(client.captureMessage('test')).toBeFalsy(); + expect(getSentCount()).toBe(1); }); test('multiple concurrent flush calls should just work', async () => { @@ -1798,18 +1777,6 @@ describe('BaseClient', () => { expect(TestClient.instance!.session).toEqual(session); }); - - test('skips when disabled', () => { - expect.assertions(1); - - const options = getDefaultTestClientOptions({ enabled: false, dsn: PUBLIC_DSN }); - const client = new TestClient(options); - const session = makeSession({ release: 'test' }); - - client.captureSession(session); - - expect(TestClient.instance!.session).toBeUndefined(); - }); }); describe('recordDroppedEvent()/_clearOutcomes()', () => { diff --git a/packages/sveltekit/test/server/handle.test.ts b/packages/sveltekit/test/server/handle.test.ts index eb0276b7f95d..23528dcf6870 100644 --- a/packages/sveltekit/test/server/handle.test.ts +++ b/packages/sveltekit/test/server/handle.test.ts @@ -296,7 +296,7 @@ describe('handleSentry', () => { } catch (e) { expect(mockCaptureException).toBeCalledTimes(1); expect(addEventProcessorSpy).toBeCalledTimes(1); - expect(mockAddExceptionMechanism).toBeCalledTimes(1); + expect(mockAddExceptionMechanism).toBeCalledTimes(2); expect(mockAddExceptionMechanism).toBeCalledWith( {}, { handled: false, type: 'sveltekit', data: { function: 'handle' } }, diff --git a/packages/types/src/client.ts b/packages/types/src/client.ts index 1b7b78066f0c..8aeabaa6cc8d 100644 --- a/packages/types/src/client.ts +++ b/packages/types/src/client.ts @@ -149,7 +149,7 @@ export interface Client { addIntegration?(integration: Integration): void; /** This is an internal function to setup all integrations that should run on the client */ - setupIntegrations(): void; + setupIntegrations(forceInitialize?: boolean): void; /** Creates an {@link Event} from all inputs to `captureException` and non-primitive inputs to `captureMessage`. */ // eslint-disable-next-line @typescript-eslint/no-explicit-any diff --git a/packages/utils/src/envelope.ts b/packages/utils/src/envelope.ts index e91aefdbab5b..e249564eca51 100644 --- a/packages/utils/src/envelope.ts +++ b/packages/utils/src/envelope.ts @@ -234,14 +234,14 @@ export function createEventEnvelopeHeaders( event: Event, sdkInfo: SdkInfo | undefined, tunnel: string | undefined, - dsn: DsnComponents, + dsn?: DsnComponents, ): EventEnvelopeHeaders { const dynamicSamplingContext = event.sdkProcessingMetadata && event.sdkProcessingMetadata.dynamicSamplingContext; return { event_id: event.event_id as string, sent_at: new Date().toISOString(), ...(sdkInfo && { sdk: sdkInfo }), - ...(!!tunnel && { dsn: dsnToString(dsn) }), + ...(!!tunnel && dsn && { dsn: dsnToString(dsn) }), ...(dynamicSamplingContext && { trace: dropUndefinedKeys({ ...dynamicSamplingContext }), }),