Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(integrations): BaseClient reports integrations added after init #7011

Merged
merged 10 commits into from
Feb 2, 2023
4 changes: 4 additions & 0 deletions packages/core/src/baseclient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -435,6 +435,10 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
*/
protected _prepareEvent(event: Event, hint: EventHint, scope?: Scope): PromiseLike<Event | null> {
const options = this.getOptions();
const integrations = Object.keys(this._integrations);
if (!hint.integrations && integrations.length > 0) {
hint.integrations = integrations;
}
return prepareEvent(options, event, hint, scope);
}

Expand Down
6 changes: 2 additions & 4 deletions packages/core/src/utils/prepareEvent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,10 @@ export function prepareEvent(
event_id: event.event_id || hint.event_id || uuid4(),
timestamp: event.timestamp || dateTimestampInSeconds(),
};
const integrations = hint.integrations || options.integrations.map(i => i.name);

applyClientOptions(prepared, options);
applyIntegrationsMetadata(
prepared,
options.integrations.map(i => i.name),
);
applyIntegrationsMetadata(prepared, integrations);
krystofwoldrich marked this conversation as resolved.
Show resolved Hide resolved

// If we have scope given to us, use it as the base for further modifications.
// This allows us to prevent unnecessary copying of data if `captureContext` is not provided.
Expand Down
21 changes: 20 additions & 1 deletion packages/core/test/lib/base.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { dsnToString, logger, SentryError, SyncPromise } from '@sentry/utils';
import { Hub, makeSession, Scope } from '../../src';
import * as integrationModule from '../../src/integration';
import { getDefaultTestClientOptions, TestClient } from '../mocks/client';
import { TestIntegration } from '../mocks/integration';
import { AdHocIntegration, TestIntegration } from '../mocks/integration';
import { makeFakeTransport } from '../mocks/transport';

const PUBLIC_DSN = 'https://username@domain/123';
Expand Down Expand Up @@ -678,6 +678,25 @@ describe('BaseClient', () => {
});
});

test('send all installed integrations in event sdk metadata', () => {
expect.assertions(1);

const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, integrations: [new TestIntegration()] });
const client = new TestClient(options);
client.setupIntegrations();
client.addIntegration(new AdHocIntegration());

client.captureException(new Error('test exception'));

expect(TestClient.instance!.event).toEqual(
expect.objectContaining({
sdk: expect.objectContaining({
integrations: expect.arrayContaining(['TestIntegration', 'AdHockIntegration']),
}),
}),
);
});

test('normalizes event with default depth of 3', () => {
expect.assertions(1);

Expand Down
10 changes: 10 additions & 0 deletions packages/core/test/mocks/integration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,3 +36,13 @@ export class AddAttachmentTestIntegration implements Integration {
});
}
}

export class AdHocIntegration implements Integration {
public static id: string = 'AdHockIntegration';

public name: string = 'AdHockIntegration';

public setupOnce(): void {
// Noop
}
}
14 changes: 12 additions & 2 deletions packages/replay/src/util/prepareReplayEvent.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import type { Scope } from '@sentry/core';
import { prepareEvent } from '@sentry/core';
import type { IntegrationIndex } from '@sentry/core/build/types/integration';
import type { Client, ReplayEvent } from '@sentry/types';

/**
Expand All @@ -11,12 +12,21 @@ export async function prepareReplayEvent({
replayId: event_id,
event,
}: {
client: Client;
client: Client & { _integrations?: IntegrationIndex };
scope: Scope;
replayId: string;
event: ReplayEvent;
}): Promise<ReplayEvent | null> {
const preparedEvent = (await prepareEvent(client.getOptions(), event, { event_id }, scope)) as ReplayEvent | null;
const integrations =
typeof client._integrations === 'object' && client._integrations !== null && !Array.isArray(client._integrations)
? Object.keys(client._integrations)
: undefined;
const preparedEvent = (await prepareEvent(
client.getOptions(),
event,
{ event_id, integrations },
scope,
)) as ReplayEvent | null;

// If e.g. a global event processor returned null
if (!preparedEvent) {
Expand Down
1 change: 1 addition & 0 deletions packages/types/src/event.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,4 +80,5 @@ export interface EventHint {
originalException?: Error | string | null;
attachments?: Attachment[];
data?: any;
integrations?: string[];
}
2 changes: 2 additions & 0 deletions rollup/plugins/bundlePlugins.js
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,8 @@ export function makeTerserPlugin() {
// We want to keept he _replay and _isEnabled variable unmangled to enable integration tests to access it
'_replay',
'_isEnabled',
// We want to keep the _integrations variable unmangled to send all installed integrations from replay
'_integrations',
krystofwoldrich marked this conversation as resolved.
Show resolved Hide resolved
],
},
},
Expand Down