Skip to content

Commit 0b10294

Browse files
mydeamydea
committed
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.
1 parent d41e39e commit 0b10294

File tree

2 files changed

+35
-39
lines changed

2 files changed

+35
-39
lines changed

‎packages/core/src/integration.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,10 @@ export function setupIntegrations(client: Client, integrations: Integration[]):
101101

102102
/** Setup a single integration. */
103103
export function setupIntegration(client: Client, integration: Integration, integrationIndex: IntegrationIndex): void {
104+
if (integrationIndex[integration.name]) {
105+
DEBUG_BUILD && logger.log(`Integration skipped because it was already installed: ${integration.name}`);
106+
return;
107+
}
104108
integrationIndex[integration.name] = integration;
105109

106110
// `setupOnce` is only called the first time

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

Lines changed: 31 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -377,7 +377,7 @@ describe('setupIntegration', () => {
377377
setupIntegration(client2, integration3, integrationIndex);
378378
setupIntegration(client2, integration4, integrationIndex);
379379

380-
expect(integrationIndex).toEqual({ test: integration4 });
380+
expect(integrationIndex).toEqual({ test: integration1 });
381381
expect(integration1.setupOnce).toHaveBeenCalledTimes(1);
382382
expect(integration2.setupOnce).not.toHaveBeenCalled();
383383
expect(integration3.setupOnce).not.toHaveBeenCalled();
@@ -394,32 +394,32 @@ describe('setupIntegration', () => {
394394
const client1 = getTestClient();
395395
const client2 = getTestClient();
396396

397-
const integrationIndex = {};
397+
const integrationIndex1 = {};
398+
const integrationIndex2 = {};
398399
const integration1 = new CustomIntegration();
399400
const integration2 = new CustomIntegration();
400401
const integration3 = new CustomIntegration();
401402
const integration4 = new CustomIntegration();
402403

403-
setupIntegration(client1, integration1, integrationIndex);
404-
setupIntegration(client1, integration2, integrationIndex);
405-
setupIntegration(client2, integration3, integrationIndex);
406-
setupIntegration(client2, integration4, integrationIndex);
404+
setupIntegration(client1, integration1, integrationIndex1);
405+
setupIntegration(client1, integration2, integrationIndex1);
406+
setupIntegration(client2, integration3, integrationIndex2);
407+
setupIntegration(client2, integration4, integrationIndex2);
407408

408-
expect(integrationIndex).toEqual({ test: integration4 });
409+
expect(integrationIndex1).toEqual({ test: integration1 });
410+
expect(integrationIndex2).toEqual({ test: integration3 });
409411
expect(integration1.setupOnce).toHaveBeenCalledTimes(1);
410412
expect(integration2.setupOnce).not.toHaveBeenCalled();
411413
expect(integration3.setupOnce).not.toHaveBeenCalled();
412414
expect(integration4.setupOnce).not.toHaveBeenCalled();
413415

414416
expect(integration1.setup).toHaveBeenCalledTimes(1);
415-
expect(integration2.setup).toHaveBeenCalledTimes(1);
417+
expect(integration2.setup).toHaveBeenCalledTimes(0);
416418
expect(integration3.setup).toHaveBeenCalledTimes(1);
417-
expect(integration4.setup).toHaveBeenCalledTimes(1);
419+
expect(integration4.setup).toHaveBeenCalledTimes(0);
418420

419421
expect(integration1.setup).toHaveBeenCalledWith(client1);
420-
expect(integration2.setup).toHaveBeenCalledWith(client1);
421422
expect(integration3.setup).toHaveBeenCalledWith(client2);
422-
expect(integration4.setup).toHaveBeenCalledWith(client2);
423423
});
424424

425425
it('binds preprocessEvent for each client', () => {
@@ -432,18 +432,20 @@ describe('setupIntegration', () => {
432432
const client1 = getTestClient();
433433
const client2 = getTestClient();
434434

435-
const integrationIndex = {};
435+
const integrationIndex1 = {};
436+
const integrationIndex2 = {};
436437
const integration1 = new CustomIntegration();
437438
const integration2 = new CustomIntegration();
438439
const integration3 = new CustomIntegration();
439440
const integration4 = new CustomIntegration();
440441

441-
setupIntegration(client1, integration1, integrationIndex);
442-
setupIntegration(client1, integration2, integrationIndex);
443-
setupIntegration(client2, integration3, integrationIndex);
444-
setupIntegration(client2, integration4, integrationIndex);
442+
setupIntegration(client1, integration1, integrationIndex1);
443+
setupIntegration(client1, integration2, integrationIndex1);
444+
setupIntegration(client2, integration3, integrationIndex2);
445+
setupIntegration(client2, integration4, integrationIndex2);
445446

446-
expect(integrationIndex).toEqual({ test: integration4 });
447+
expect(integrationIndex1).toEqual({ test: integration1 });
448+
expect(integrationIndex2).toEqual({ test: integration3 });
447449
expect(integration1.setupOnce).toHaveBeenCalledTimes(1);
448450
expect(integration2.setupOnce).not.toHaveBeenCalled();
449451
expect(integration3.setupOnce).not.toHaveBeenCalled();
@@ -456,14 +458,12 @@ describe('setupIntegration', () => {
456458
client2.captureEvent({ event_id: '2c' });
457459

458460
expect(integration1.preprocessEvent).toHaveBeenCalledTimes(2);
459-
expect(integration2.preprocessEvent).toHaveBeenCalledTimes(2);
461+
expect(integration2.preprocessEvent).toHaveBeenCalledTimes(0);
460462
expect(integration3.preprocessEvent).toHaveBeenCalledTimes(3);
461-
expect(integration4.preprocessEvent).toHaveBeenCalledTimes(3);
463+
expect(integration4.preprocessEvent).toHaveBeenCalledTimes(0);
462464

463465
expect(integration1.preprocessEvent).toHaveBeenLastCalledWith({ event_id: '1b' }, {}, client1);
464-
expect(integration2.preprocessEvent).toHaveBeenLastCalledWith({ event_id: '1b' }, {}, client1);
465466
expect(integration3.preprocessEvent).toHaveBeenLastCalledWith({ event_id: '2c' }, {}, client2);
466-
expect(integration4.preprocessEvent).toHaveBeenLastCalledWith({ event_id: '2c' }, {}, client2);
467467
});
468468

469469
it('allows to mutate events in preprocessEvent', async () => {
@@ -504,18 +504,20 @@ describe('setupIntegration', () => {
504504
const client1 = getTestClient();
505505
const client2 = getTestClient();
506506

507-
const integrationIndex = {};
507+
const integrationIndex1 = {};
508+
const integrationIndex2 = {};
508509
const integration1 = new CustomIntegration();
509510
const integration2 = new CustomIntegration();
510511
const integration3 = new CustomIntegration();
511512
const integration4 = new CustomIntegration();
512513

513-
setupIntegration(client1, integration1, integrationIndex);
514-
setupIntegration(client1, integration2, integrationIndex);
515-
setupIntegration(client2, integration3, integrationIndex);
516-
setupIntegration(client2, integration4, integrationIndex);
514+
setupIntegration(client1, integration1, integrationIndex1);
515+
setupIntegration(client1, integration2, integrationIndex1);
516+
setupIntegration(client2, integration3, integrationIndex2);
517+
setupIntegration(client2, integration4, integrationIndex2);
517518

518-
expect(integrationIndex).toEqual({ test: integration4 });
519+
expect(integrationIndex1).toEqual({ test: integration1 });
520+
expect(integrationIndex2).toEqual({ test: integration3 });
519521
expect(integration1.setupOnce).toHaveBeenCalledTimes(1);
520522
expect(integration2.setupOnce).not.toHaveBeenCalled();
521523
expect(integration3.setupOnce).not.toHaveBeenCalled();
@@ -528,30 +530,20 @@ describe('setupIntegration', () => {
528530
client2.captureEvent({ event_id: '2c' });
529531

530532
expect(integration1.processEvent).toHaveBeenCalledTimes(2);
531-
expect(integration2.processEvent).toHaveBeenCalledTimes(2);
533+
expect(integration2.processEvent).toHaveBeenCalledTimes(0);
532534
expect(integration3.processEvent).toHaveBeenCalledTimes(3);
533-
expect(integration4.processEvent).toHaveBeenCalledTimes(3);
535+
expect(integration4.processEvent).toHaveBeenCalledTimes(0);
534536

535537
expect(integration1.processEvent).toHaveBeenLastCalledWith(
536538
expect.objectContaining({ event_id: '1b' }),
537539
{},
538540
client1,
539541
);
540-
expect(integration2.processEvent).toHaveBeenLastCalledWith(
541-
expect.objectContaining({ event_id: '1b' }),
542-
{},
543-
client1,
544-
);
545542
expect(integration3.processEvent).toHaveBeenLastCalledWith(
546543
expect.objectContaining({ event_id: '2c' }),
547544
{},
548545
client2,
549546
);
550-
expect(integration4.processEvent).toHaveBeenLastCalledWith(
551-
expect.objectContaining({ event_id: '2c' }),
552-
{},
553-
client2,
554-
);
555547
});
556548

557549
it('allows to mutate events in processEvent', async () => {

0 commit comments

Comments
 (0)