fix: Ensure afterAllSetup is called when using addIntegration() (#…

…10372) Noticed that this is not really correct right now!
getsentry · Jan 26, 2024 · 9f9c51d · 9f9c51d
1 parent dd77951
commit 9f9c51d
Show file tree

Hide file tree

Showing 2 changed files with 64 additions and 0 deletions.
diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts
@@ -368,7 +368,14 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
    * @inheritDoc
    */
   public addIntegration(integration: Integration): void {
+    const isAlreadyInstalled = this._integrations[integration.name];
+
+    // This hook takes care of only installing if not already installed
     setupIntegration(this, integration, this._integrations);
+    // Here we need to check manually to make sure to not run this multiple times
+    if (!isAlreadyInstalled) {
+      afterSetupIntegrations(this, [integration]);
+    }
   }
 
   /**

diff --git a/packages/core/test/lib/integration.test.ts b/packages/core/test/lib/integration.test.ts
@@ -646,6 +646,63 @@ describe('addIntegration', () => {
     expect(warnings).toHaveBeenCalledTimes(1);
     expect(warnings).toHaveBeenCalledWith('Cannot add integration "test" because no SDK Client is available.');
   });
+
+  it('triggers all hooks', () => {
+    const setup = jest.fn();
+    const setupOnce = jest.fn();
+    const setupAfterAll = jest.fn();
+
+    class CustomIntegration implements Integration {
+      name = 'test';
+      setupOnce = setupOnce;
+      setup = setup;
+      afterAllSetup = setupAfterAll;
+    }
+
+    const client = getTestClient();
+    const hub = new Hub(client);
+    // eslint-disable-next-line deprecation/deprecation
+    makeMain(hub);
+
+    const integration = new CustomIntegration();
+    addIntegration(integration);
+
+    expect(setupOnce).toHaveBeenCalledTimes(1);
+    expect(setup).toHaveBeenCalledTimes(1);
+    expect(setupAfterAll).toHaveBeenCalledTimes(1);
+  });
+
+  it('does not trigger hooks if already installed', () => {
+    const logs = jest.spyOn(logger, 'log');
+
+    class CustomIntegration implements Integration {
+      name = 'test';
+      setupOnce = jest.fn();
+      setup = jest.fn();
+      afterAllSetup = jest.fn();
+    }
+
+    const client = getTestClient();
+    const hub = new Hub(client);
+    // eslint-disable-next-line deprecation/deprecation
+    makeMain(hub);
+
+    const integration1 = new CustomIntegration();
+    const integration2 = new CustomIntegration();
+    addIntegration(integration1);
+
+    expect(integration1.setupOnce).toHaveBeenCalledTimes(1);
+    expect(integration1.setup).toHaveBeenCalledTimes(1);
+    expect(integration1.afterAllSetup).toHaveBeenCalledTimes(1);
+
+    addIntegration(integration2);
+
+    expect(integration2.setupOnce).toHaveBeenCalledTimes(0);
+    expect(integration2.setup).toHaveBeenCalledTimes(0);
+    expect(integration2.afterAllSetup).toHaveBeenCalledTimes(0);
+
+    expect(logs).toHaveBeenCalledWith('Integration skipped because it was already installed: test');
+  });
 });
 
 describe('convertIntegrationFnToClass', () => {