From 61db6f6355c4a6f4f4ff88b65d1b72ab434b378d Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Wed, 1 Feb 2023 10:31:32 +0100 Subject: [PATCH 1/5] fix(integrations): BaseClient reports integrations added after init --- packages/core/src/utils/prepareEvent.ts | 6 ++---- packages/core/test/lib/base.test.ts | 21 ++++++++++++++++++++- packages/core/test/mocks/integration.ts | 10 ++++++++++ 3 files changed, 32 insertions(+), 5 deletions(-) diff --git a/packages/core/src/utils/prepareEvent.ts b/packages/core/src/utils/prepareEvent.ts index 3aa0d86df3f6..c6ecf325ae22 100644 --- a/packages/core/src/utils/prepareEvent.ts +++ b/packages/core/src/utils/prepareEvent.ts @@ -1,6 +1,7 @@ import type { ClientOptions, Event, EventHint } from '@sentry/types'; import { dateTimestampInSeconds, normalize, resolvedSyncPromise, truncate, uuid4 } from '@sentry/utils'; +import { installedIntegrations } from '../integration'; import { Scope } from '../scope'; /** @@ -34,10 +35,7 @@ export function prepareEvent( }; applyClientOptions(prepared, options); - applyIntegrationsMetadata( - prepared, - options.integrations.map(i => i.name), - ); + applyIntegrationsMetadata(prepared, installedIntegrations); // 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. diff --git a/packages/core/test/lib/base.test.ts b/packages/core/test/lib/base.test.ts index c82014515067..c9a52b2a630d 100644 --- a/packages/core/test/lib/base.test.ts +++ b/packages/core/test/lib/base.test.ts @@ -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 { AdHockIntegration, TestIntegration } from '../mocks/integration'; import { makeFakeTransport } from '../mocks/transport'; const PUBLIC_DSN = 'https://username@domain/123'; @@ -1514,6 +1514,25 @@ describe('BaseClient', () => { expect(setupIntegrationsHelper).toHaveBeenCalledTimes(1); }); + + 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 AdHockIntegration()); + + client.captureException(new Error('test exception')); + + expect(TestClient.instance!.event).toEqual( + expect.objectContaining({ + sdk: expect.objectContaining({ + integrations: expect.arrayContaining(['TestIntegration', 'AdHockIntegration']), + }), + }), + ); + }); }); describe('flush/close', () => { diff --git a/packages/core/test/mocks/integration.ts b/packages/core/test/mocks/integration.ts index ff01158b0632..6bb36bd29e75 100644 --- a/packages/core/test/mocks/integration.ts +++ b/packages/core/test/mocks/integration.ts @@ -36,3 +36,13 @@ export class AddAttachmentTestIntegration implements Integration { }); } } + +export class AdHockIntegration implements Integration { + public static id: string = 'AdHockIntegration'; + + public name: string = 'AdHockIntegration'; + + public setupOnce(): void { + // Noop + } +} From 0d75590bdaafb81402a7d53d2ff96230e29fcfa7 Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Wed, 1 Feb 2023 12:14:26 +0100 Subject: [PATCH 2/5] Fix typo --- packages/core/test/lib/base.test.ts | 4 ++-- packages/core/test/mocks/integration.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/core/test/lib/base.test.ts b/packages/core/test/lib/base.test.ts index c9a52b2a630d..f1d979c2cbe0 100644 --- a/packages/core/test/lib/base.test.ts +++ b/packages/core/test/lib/base.test.ts @@ -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 { AdHockIntegration, TestIntegration } from '../mocks/integration'; +import { AdHocIntegration, TestIntegration } from '../mocks/integration'; import { makeFakeTransport } from '../mocks/transport'; const PUBLIC_DSN = 'https://username@domain/123'; @@ -1521,7 +1521,7 @@ describe('BaseClient', () => { const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, integrations: [new TestIntegration()] }); const client = new TestClient(options); client.setupIntegrations(); - client.addIntegration(new AdHockIntegration()); + client.addIntegration(new AdHocIntegration()); client.captureException(new Error('test exception')); diff --git a/packages/core/test/mocks/integration.ts b/packages/core/test/mocks/integration.ts index 6bb36bd29e75..ce95d04520a7 100644 --- a/packages/core/test/mocks/integration.ts +++ b/packages/core/test/mocks/integration.ts @@ -37,7 +37,7 @@ export class AddAttachmentTestIntegration implements Integration { } } -export class AdHockIntegration implements Integration { +export class AdHocIntegration implements Integration { public static id: string = 'AdHockIntegration'; public name: string = 'AdHockIntegration'; From 13fea73041ab1956e185c3045f0d745cbe06e4aa Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Wed, 1 Feb 2023 12:24:43 +0100 Subject: [PATCH 3/5] Fix not resetting installedIntegrations for each test --- packages/core/test/lib/base.test.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/core/test/lib/base.test.ts b/packages/core/test/lib/base.test.ts index f1d979c2cbe0..429ca0dbcf24 100644 --- a/packages/core/test/lib/base.test.ts +++ b/packages/core/test/lib/base.test.ts @@ -3,6 +3,7 @@ import { dsnToString, logger, SentryError, SyncPromise } from '@sentry/utils'; import { Hub, makeSession, Scope } from '../../src'; import * as integrationModule from '../../src/integration'; +import { installedIntegrations } from '../../src/integration'; import { getDefaultTestClientOptions, TestClient } from '../mocks/client'; import { AdHocIntegration, TestIntegration } from '../mocks/integration'; import { makeFakeTransport } from '../mocks/transport'; @@ -52,6 +53,7 @@ jest.mock('@sentry/utils', () => { describe('BaseClient', () => { beforeEach(() => { + installedIntegrations.splice(0); TestClient.sendEventCalled = undefined; TestClient.instance = undefined; }); From 957d27d5f50b176af6679f660d2e896f01b5ee7b Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Wed, 1 Feb 2023 16:13:23 +0100 Subject: [PATCH 4/5] Use `client._integrations`, fix replay missing integrations --- packages/core/src/baseclient.ts | 3 +- packages/core/src/utils/prepareEvent.ts | 4 +- packages/core/test/lib/base.test.ts | 39 +++++++++---------- .../replay/src/util/prepareReplayEvent.ts | 15 ++++++- rollup/plugins/bundlePlugins.js | 2 + 5 files changed, 38 insertions(+), 25 deletions(-) diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index 647d98b61f63..019304c28c45 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -435,7 +435,8 @@ export abstract class BaseClient implements Client { */ protected _prepareEvent(event: Event, hint: EventHint, scope?: Scope): PromiseLike { const options = this.getOptions(); - return prepareEvent(options, event, hint, scope); + const integrations = Object.keys(this._integrations); + return prepareEvent(options, integrations, event, hint, scope); } /** diff --git a/packages/core/src/utils/prepareEvent.ts b/packages/core/src/utils/prepareEvent.ts index c6ecf325ae22..40ce5519ab99 100644 --- a/packages/core/src/utils/prepareEvent.ts +++ b/packages/core/src/utils/prepareEvent.ts @@ -1,7 +1,6 @@ import type { ClientOptions, Event, EventHint } from '@sentry/types'; import { dateTimestampInSeconds, normalize, resolvedSyncPromise, truncate, uuid4 } from '@sentry/utils'; -import { installedIntegrations } from '../integration'; import { Scope } from '../scope'; /** @@ -23,6 +22,7 @@ import { Scope } from '../scope'; */ export function prepareEvent( options: ClientOptions, + integrations: string[], event: Event, hint: EventHint, scope?: Scope, @@ -35,7 +35,7 @@ export function prepareEvent( }; applyClientOptions(prepared, options); - applyIntegrationsMetadata(prepared, installedIntegrations); + applyIntegrationsMetadata(prepared, integrations); // 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. diff --git a/packages/core/test/lib/base.test.ts b/packages/core/test/lib/base.test.ts index 429ca0dbcf24..9c47a86523b4 100644 --- a/packages/core/test/lib/base.test.ts +++ b/packages/core/test/lib/base.test.ts @@ -53,7 +53,6 @@ jest.mock('@sentry/utils', () => { describe('BaseClient', () => { beforeEach(() => { - installedIntegrations.splice(0); TestClient.sendEventCalled = undefined; TestClient.instance = undefined; }); @@ -680,6 +679,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); @@ -1516,25 +1534,6 @@ describe('BaseClient', () => { expect(setupIntegrationsHelper).toHaveBeenCalledTimes(1); }); - - 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']), - }), - }), - ); - }); }); describe('flush/close', () => { diff --git a/packages/replay/src/util/prepareReplayEvent.ts b/packages/replay/src/util/prepareReplayEvent.ts index c2b645e92bdc..4d0f5bd66e1c 100644 --- a/packages/replay/src/util/prepareReplayEvent.ts +++ b/packages/replay/src/util/prepareReplayEvent.ts @@ -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'; /** @@ -11,12 +12,22 @@ export async function prepareReplayEvent({ replayId: event_id, event, }: { - client: Client; + client: Client & { _integrations?: IntegrationIndex }; scope: Scope; replayId: string; event: ReplayEvent; }): Promise { - const preparedEvent = (await prepareEvent(client.getOptions(), event, { event_id }, scope)) as ReplayEvent | null; + const integrations = + typeof client._integrations === 'object' && !Array.isArray(client._integrations) + ? Object.keys(client._integrations) + : client.getOptions().integrations.map(i => i.name); + const preparedEvent = (await prepareEvent( + client.getOptions(), + integrations, + event, + { event_id }, + scope, + )) as ReplayEvent | null; // If e.g. a global event processor returned null if (!preparedEvent) { diff --git a/rollup/plugins/bundlePlugins.js b/rollup/plugins/bundlePlugins.js index 5e483dfddc46..0fb662b7e3e4 100644 --- a/rollup/plugins/bundlePlugins.js +++ b/rollup/plugins/bundlePlugins.js @@ -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', ], }, }, From 93d9ab6c276c122f5540af08356ef28c385e8127 Mon Sep 17 00:00:00 2001 From: Krystof Woldrich Date: Wed, 1 Feb 2023 17:13:57 +0100 Subject: [PATCH 5/5] Use hint to pass integrations --- packages/core/src/baseclient.ts | 5 ++++- packages/core/src/utils/prepareEvent.ts | 2 +- packages/core/test/lib/base.test.ts | 1 - packages/replay/src/util/prepareReplayEvent.ts | 7 +++---- packages/types/src/event.ts | 1 + 5 files changed, 9 insertions(+), 7 deletions(-) diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index 019304c28c45..09200e30c503 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -436,7 +436,10 @@ export abstract class BaseClient implements Client { protected _prepareEvent(event: Event, hint: EventHint, scope?: Scope): PromiseLike { const options = this.getOptions(); const integrations = Object.keys(this._integrations); - return prepareEvent(options, integrations, event, hint, scope); + if (!hint.integrations && integrations.length > 0) { + hint.integrations = integrations; + } + return prepareEvent(options, event, hint, scope); } /** diff --git a/packages/core/src/utils/prepareEvent.ts b/packages/core/src/utils/prepareEvent.ts index 40ce5519ab99..73a71e138db2 100644 --- a/packages/core/src/utils/prepareEvent.ts +++ b/packages/core/src/utils/prepareEvent.ts @@ -22,7 +22,6 @@ import { Scope } from '../scope'; */ export function prepareEvent( options: ClientOptions, - integrations: string[], event: Event, hint: EventHint, scope?: Scope, @@ -33,6 +32,7 @@ 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, integrations); diff --git a/packages/core/test/lib/base.test.ts b/packages/core/test/lib/base.test.ts index 9c47a86523b4..2925c6e213c7 100644 --- a/packages/core/test/lib/base.test.ts +++ b/packages/core/test/lib/base.test.ts @@ -3,7 +3,6 @@ import { dsnToString, logger, SentryError, SyncPromise } from '@sentry/utils'; import { Hub, makeSession, Scope } from '../../src'; import * as integrationModule from '../../src/integration'; -import { installedIntegrations } from '../../src/integration'; import { getDefaultTestClientOptions, TestClient } from '../mocks/client'; import { AdHocIntegration, TestIntegration } from '../mocks/integration'; import { makeFakeTransport } from '../mocks/transport'; diff --git a/packages/replay/src/util/prepareReplayEvent.ts b/packages/replay/src/util/prepareReplayEvent.ts index 4d0f5bd66e1c..522e7e7689bc 100644 --- a/packages/replay/src/util/prepareReplayEvent.ts +++ b/packages/replay/src/util/prepareReplayEvent.ts @@ -18,14 +18,13 @@ export async function prepareReplayEvent({ event: ReplayEvent; }): Promise { const integrations = - typeof client._integrations === 'object' && !Array.isArray(client._integrations) + typeof client._integrations === 'object' && client._integrations !== null && !Array.isArray(client._integrations) ? Object.keys(client._integrations) - : client.getOptions().integrations.map(i => i.name); + : undefined; const preparedEvent = (await prepareEvent( client.getOptions(), - integrations, event, - { event_id }, + { event_id, integrations }, scope, )) as ReplayEvent | null; diff --git a/packages/types/src/event.ts b/packages/types/src/event.ts index 86280fc7e65b..7c00bcef4718 100644 --- a/packages/types/src/event.ts +++ b/packages/types/src/event.ts @@ -80,4 +80,5 @@ export interface EventHint { originalException?: Error | string | null; attachments?: Attachment[]; data?: any; + integrations?: string[]; }