Skip to content

Commit

Permalink
feat: Always assemble Envelopes (#9101)
Browse files Browse the repository at this point in the history
  • Loading branch information
HazAT committed Sep 27, 2023
1 parent d47eb1a commit d4f67a2
Show file tree
Hide file tree
Showing 7 changed files with 37 additions and 82 deletions.
1 change: 1 addition & 0 deletions packages/browser/src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ export class BrowserClient extends BaseClient<BrowserClientOptions> {
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;
Expand Down
59 changes: 23 additions & 36 deletions packages/core/src/baseclient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
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) {
Expand Down Expand Up @@ -216,11 +216,6 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
* @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 {
Expand Down Expand Up @@ -297,8 +292,8 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
/**
* 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;
}
Expand Down Expand Up @@ -338,34 +333,30 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
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);
}
}

/**
* @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);
}

/**
Expand Down Expand Up @@ -531,9 +522,9 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
});
}

/** 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;
}

/**
Expand Down Expand Up @@ -635,10 +626,6 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
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';
Expand Down Expand Up @@ -738,9 +725,9 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
* @inheritdoc
*/
protected _sendEnvelope(envelope: Envelope): PromiseLike<void | TransportMakeRequestResponse> | 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);
});
Expand Down
6 changes: 3 additions & 3 deletions packages/core/src/envelope.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,15 +36,15 @@ 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 {
const sdkInfo = getSdkMetadataForEnvelopeHeader(metadata);
const envelopeHeaders = {
sent_at: new Date().toISOString(),
...(sdkInfo && { sdk: sdkInfo }),
...(!!tunnel && { dsn: dsnToString(dsn) }),
...(!!tunnel && dsn && { dsn: dsnToString(dsn) }),
};

const envelopeItem: SessionItem =
Expand All @@ -58,7 +58,7 @@ export function createSessionEnvelope(
*/
export function createEventEnvelope(
event: Event,
dsn: DsnComponents,
dsn?: DsnComponents,
metadata?: SdkMetadata,
tunnel?: string,
): EventEnvelope {
Expand Down
45 changes: 6 additions & 39 deletions packages/core/test/lib/base.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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?' }],
Expand Down Expand Up @@ -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({
Expand All @@ -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 () => {
Expand Down Expand Up @@ -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()', () => {
Expand Down
2 changes: 1 addition & 1 deletion packages/sveltekit/test/server/handle.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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' } },
Expand Down
2 changes: 1 addition & 1 deletion packages/types/src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ export interface Client<O extends ClientOptions = ClientOptions> {
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
Expand Down
4 changes: 2 additions & 2 deletions packages/utils/src/envelope.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 }),
}),
Expand Down

0 comments on commit d4f67a2

Please sign in to comment.