fix(core): Do not run setup for integration on client multiple times

Currently, if you do:

```js
const myIntegration = new InboundFilters();
const myIntegration2 = new InboundFilters();
const myIntegration3 = new InboundFilters();

client.addIntegration(myIntegration);
client.addIntegration(myIntegration2);
client.addIntegration(myIntegration3);
```

All of these will have their `setup` hooks called, and thus they will be initialized multiple times. However, all but the last will be discarded from the client and not be accessible anymore via e.g. `getIntegration()` or similar.

This used to not matter because everything was guarded through `setupOnce()` anyhow, but i would say this is more of a bug and not really expected.

With this change, an integration can only be added to a client once, if you try to add it again it will noop.

Author: mydea

MIGRATION.md

@@ -8,6 +8,11 @@ npx @sentry/migr8@latest
 
 This will let you select which updates to run, and automatically update your code. Make sure to still review all code changes!
 
+## Deprecate `client.setupIntegrations()`
+
+Instead, use the new `client.init()` method. You should probably not use this directly and instead use `Sentry.init()`, which calls this under the hood. But if you have a special use case that requires that, you can call `client.init()` instead now.
+Note that this has a bit of a different semantic, as it does not check anymore if the SDK is enabled - it will install the integrations anyhow. You need to check if you want to initialize the client yourself.
+
 ## Deprecate `scope.setTransactionName()`
 
 Instead, either set this as attributes or tags, or use an event processor to set `event.transaction`.

packages/core/src/integration.ts

@@ -101,6 +101,10 @@ export function setupIntegrations(client: Client, integrations: Integration[]):
 
 /** Setup a single integration.  */
 export function setupIntegration(client: Client, integration: Integration, integrationIndex: IntegrationIndex): void {
+  if (integrationIndex[integration.name]) {
+    DEBUG_BUILD && logger.log(`Integration skipped because it was already installed: ${integration.name}`);
+    return;
+  }
   integrationIndex[integration.name] = integration;
 
   // `setupOnce` is only called the first time

packages/core/test/lib/integration.test.ts

@@ -377,7 +377,7 @@ describe('setupIntegration', () => {
     setupIntegration(client2, integration3, integrationIndex);
     setupIntegration(client2, integration4, integrationIndex);
 
-    expect(integrationIndex).toEqual({ test: integration4 });
+    expect(integrationIndex).toEqual({ test: integration1 });
     expect(integration1.setupOnce).toHaveBeenCalledTimes(1);
     expect(integration2.setupOnce).not.toHaveBeenCalled();
     expect(integration3.setupOnce).not.toHaveBeenCalled();
@@ -394,32 +394,32 @@ describe('setupIntegration', () => {
     const client1 = getTestClient();
     const client2 = getTestClient();
 
-    const integrationIndex = {};
+    const integrationIndex1 = {};
+    const integrationIndex2 = {};
     const integration1 = new CustomIntegration();
     const integration2 = new CustomIntegration();
     const integration3 = new CustomIntegration();
     const integration4 = new CustomIntegration();
 
-    setupIntegration(client1, integration1, integrationIndex);
-    setupIntegration(client1, integration2, integrationIndex);
-    setupIntegration(client2, integration3, integrationIndex);
-    setupIntegration(client2, integration4, integrationIndex);
+    setupIntegration(client1, integration1, integrationIndex1);
+    setupIntegration(client1, integration2, integrationIndex1);
+    setupIntegration(client2, integration3, integrationIndex2);
+    setupIntegration(client2, integration4, integrationIndex2);
 
-    expect(integrationIndex).toEqual({ test: integration4 });
+    expect(integrationIndex1).toEqual({ test: integration1 });
+    expect(integrationIndex2).toEqual({ test: integration3 });
     expect(integration1.setupOnce).toHaveBeenCalledTimes(1);
     expect(integration2.setupOnce).not.toHaveBeenCalled();
     expect(integration3.setupOnce).not.toHaveBeenCalled();
     expect(integration4.setupOnce).not.toHaveBeenCalled();
 
     expect(integration1.setup).toHaveBeenCalledTimes(1);
-    expect(integration2.setup).toHaveBeenCalledTimes(1);
+    expect(integration2.setup).toHaveBeenCalledTimes(0);
     expect(integration3.setup).toHaveBeenCalledTimes(1);
-    expect(integration4.setup).toHaveBeenCalledTimes(1);
+    expect(integration4.setup).toHaveBeenCalledTimes(0);
 
     expect(integration1.setup).toHaveBeenCalledWith(client1);
-    expect(integration2.setup).toHaveBeenCalledWith(client1);
     expect(integration3.setup).toHaveBeenCalledWith(client2);
-    expect(integration4.setup).toHaveBeenCalledWith(client2);
   });
 
   it('binds preprocessEvent for each client', () => {
@@ -432,18 +432,20 @@ describe('setupIntegration', () => {
     const client1 = getTestClient();
     const client2 = getTestClient();
 
-    const integrationIndex = {};
+    const integrationIndex1 = {};
+    const integrationIndex2 = {};
     const integration1 = new CustomIntegration();
     const integration2 = new CustomIntegration();
     const integration3 = new CustomIntegration();
     const integration4 = new CustomIntegration();
 
-    setupIntegration(client1, integration1, integrationIndex);
-    setupIntegration(client1, integration2, integrationIndex);
-    setupIntegration(client2, integration3, integrationIndex);
-    setupIntegration(client2, integration4, integrationIndex);
+    setupIntegration(client1, integration1, integrationIndex1);
+    setupIntegration(client1, integration2, integrationIndex1);
+    setupIntegration(client2, integration3, integrationIndex2);
+    setupIntegration(client2, integration4, integrationIndex2);
 
-    expect(integrationIndex).toEqual({ test: integration4 });
+    expect(integrationIndex1).toEqual({ test: integration1 });
+    expect(integrationIndex2).toEqual({ test: integration3 });
     expect(integration1.setupOnce).toHaveBeenCalledTimes(1);
     expect(integration2.setupOnce).not.toHaveBeenCalled();
     expect(integration3.setupOnce).not.toHaveBeenCalled();
@@ -456,14 +458,12 @@ describe('setupIntegration', () => {
     client2.captureEvent({ event_id: '2c' });
 
     expect(integration1.preprocessEvent).toHaveBeenCalledTimes(2);
-    expect(integration2.preprocessEvent).toHaveBeenCalledTimes(2);
+    expect(integration2.preprocessEvent).toHaveBeenCalledTimes(0);
     expect(integration3.preprocessEvent).toHaveBeenCalledTimes(3);
-    expect(integration4.preprocessEvent).toHaveBeenCalledTimes(3);
+    expect(integration4.preprocessEvent).toHaveBeenCalledTimes(0);
 
     expect(integration1.preprocessEvent).toHaveBeenLastCalledWith({ event_id: '1b' }, {}, client1);
-    expect(integration2.preprocessEvent).toHaveBeenLastCalledWith({ event_id: '1b' }, {}, client1);
     expect(integration3.preprocessEvent).toHaveBeenLastCalledWith({ event_id: '2c' }, {}, client2);
-    expect(integration4.preprocessEvent).toHaveBeenLastCalledWith({ event_id: '2c' }, {}, client2);
   });
 
   it('allows to mutate events in preprocessEvent', async () => {
@@ -504,18 +504,20 @@ describe('setupIntegration', () => {
     const client1 = getTestClient();
     const client2 = getTestClient();
 
-    const integrationIndex = {};
+    const integrationIndex1 = {};
+    const integrationIndex2 = {};
     const integration1 = new CustomIntegration();
     const integration2 = new CustomIntegration();
     const integration3 = new CustomIntegration();
     const integration4 = new CustomIntegration();
 
-    setupIntegration(client1, integration1, integrationIndex);
-    setupIntegration(client1, integration2, integrationIndex);
-    setupIntegration(client2, integration3, integrationIndex);
-    setupIntegration(client2, integration4, integrationIndex);
+    setupIntegration(client1, integration1, integrationIndex1);
+    setupIntegration(client1, integration2, integrationIndex1);
+    setupIntegration(client2, integration3, integrationIndex2);
+    setupIntegration(client2, integration4, integrationIndex2);
 
-    expect(integrationIndex).toEqual({ test: integration4 });
+    expect(integrationIndex1).toEqual({ test: integration1 });
+    expect(integrationIndex2).toEqual({ test: integration3 });
     expect(integration1.setupOnce).toHaveBeenCalledTimes(1);
     expect(integration2.setupOnce).not.toHaveBeenCalled();
     expect(integration3.setupOnce).not.toHaveBeenCalled();
@@ -528,30 +530,20 @@ describe('setupIntegration', () => {
     client2.captureEvent({ event_id: '2c' });
 
     expect(integration1.processEvent).toHaveBeenCalledTimes(2);
-    expect(integration2.processEvent).toHaveBeenCalledTimes(2);
+    expect(integration2.processEvent).toHaveBeenCalledTimes(0);
     expect(integration3.processEvent).toHaveBeenCalledTimes(3);
-    expect(integration4.processEvent).toHaveBeenCalledTimes(3);
+    expect(integration4.processEvent).toHaveBeenCalledTimes(0);
 
     expect(integration1.processEvent).toHaveBeenLastCalledWith(
       expect.objectContaining({ event_id: '1b' }),
       {},
       client1,
     );
-    expect(integration2.processEvent).toHaveBeenLastCalledWith(
-      expect.objectContaining({ event_id: '1b' }),
-      {},
-      client1,
-    );
     expect(integration3.processEvent).toHaveBeenLastCalledWith(
       expect.objectContaining({ event_id: '2c' }),
       {},
       client2,
     );
-    expect(integration4.processEvent).toHaveBeenLastCalledWith(
-      expect.objectContaining({ event_id: '2c' }),
-      {},
-      client2,
-    );
   });
 
   it('allows to mutate events in processEvent', async () => {