fix(nextjs): Allow onUncaughtException integration to remain excluded

packages/nextjs/src/index.server.ts

@@ -10,7 +10,7 @@ import * as path from 'path';
 import { isBuild } from './utils/isBuild';
 import { buildMetadata } from './utils/metadata';
 import { NextjsOptions } from './utils/nextjsOptions';
-import { addOrUpdateIntegration } from './utils/userIntegrations';
+import { addOrUpdateIntegration, IntegrationWithExclusionOption } from './utils/userIntegrations';
 
 export * from '@sentry/node';
 export { captureUnderscoreErrorException } from './utils/_error';
@@ -118,8 +118,11 @@ function addServerIntegrations(options: NextjsOptions): void {
   });
   integrations = addOrUpdateIntegration(defaultRewriteFramesIntegration, integrations);
 
-  const nativeBehaviourOnUncaughtException = new Integrations.OnUncaughtException();
-  integrations = addOrUpdateIntegration(nativeBehaviourOnUncaughtException, integrations, {
+  const defaultOnUncaughtExceptionIntegration: IntegrationWithExclusionOption = new Integrations.OnUncaughtException({
+    exitEvenIfOtherHandlersAreRegistered: false,
+  });
+  defaultOnUncaughtExceptionIntegration.allowExclusionByUser = true;
+  integrations = addOrUpdateIntegration(defaultOnUncaughtExceptionIntegration, integrations, {
     _options: { exitEvenIfOtherHandlersAreRegistered: false },
   });
 

packages/nextjs/src/utils/userIntegrations.ts

@@ -7,6 +7,15 @@ type ForcedIntegrationOptions = {
   [keyPath: string]: unknown;
 };
 
+export type IntegrationWithExclusionOption = Integration & {
+  /**
+   * Allow the user to exclude this integration by not returning it from a function provided as the `integrations` option
+   * in `Sentry.init()`. Meant to be used with default integrations, the idea being that if a user has actively filtered
+   * an integration out, we should be able to respect that choice if we wish.
+   */
+  allowExclusionByUser?: boolean;
+};
+
 /**
  * Recursively traverses an object to update an existing nested key.
  * Note: The provided key path must include existing properties,
@@ -43,14 +52,21 @@ function setNestedKey(obj: Record<string, any>, keyPath: string, value: unknown)
  * @param forcedOptions Options with which to patch an existing user-derived instance on the integration.
  * @returns A final integrations array.
  */
-export function addOrUpdateIntegration(
+export function addOrUpdateIntegration<T extends UserIntegrations>(
   defaultIntegrationInstance: Integration,
-  userIntegrations: UserIntegrations,
+  userIntegrations: T,
   forcedOptions: ForcedIntegrationOptions = {},
-): UserIntegrations {
-  return Array.isArray(userIntegrations)
-    ? addOrUpdateIntegrationInArray(defaultIntegrationInstance, userIntegrations, forcedOptions)
-    : addOrUpdateIntegrationInFunction(defaultIntegrationInstance, userIntegrations, forcedOptions);
+): T {
+  return (
+    Array.isArray(userIntegrations)
+      ? addOrUpdateIntegrationInArray(defaultIntegrationInstance, userIntegrations, forcedOptions)
+      : addOrUpdateIntegrationInFunction(
+          defaultIntegrationInstance,
+          // Somehow TS can't figure out that not being an array makes this necessarily a function
+          userIntegrations as UserIntegrationsFunction,
+          forcedOptions,
+        )
+  ) as T;
 }
 
 function addOrUpdateIntegrationInArray(
@@ -72,13 +88,27 @@ function addOrUpdateIntegrationInArray(
 }
 
 function addOrUpdateIntegrationInFunction(
-  defaultIntegrationInstance: Integration,
+  defaultIntegrationInstance: IntegrationWithExclusionOption,
   userIntegrationsFunc: UserIntegrationsFunction,
   forcedOptions: ForcedIntegrationOptions,
 ): UserIntegrationsFunction {
   const wrapper: UserIntegrationsFunction = defaultIntegrations => {
     const userFinalIntegrations = userIntegrationsFunc(defaultIntegrations);
+
+    // There are instances where we want the user to be able to prevent an integration from appearing at all, which they
+    // would do by providing a function which filters out the integration in question. If that's happened in one of
+    // those cases, don't add our default back in.
+    if (defaultIntegrationInstance.allowExclusionByUser) {
+      const userFinalInstance = userFinalIntegrations.find(
+        integration => integration.name === defaultIntegrationInstance.name,
+      );
+      if (!userFinalInstance) {
+        return userFinalIntegrations;
+      }
+    }
+
     return addOrUpdateIntegrationInArray(defaultIntegrationInstance, userFinalIntegrations, forcedOptions);
   };
+
   return wrapper;
 }

packages/nextjs/test/utils/userIntegrations.test.ts

@@ -1,50 +1,215 @@
-import { RewriteFrames } from '@sentry/integrations';
-import { Integration } from '@sentry/types';
-
-import { addOrUpdateIntegration, UserIntegrationsFunction } from '../../src/utils/userIntegrations';
-
-const testIntegration = new RewriteFrames();
-
-describe('user integrations without any integrations', () => {
-  test('as an array', () => {
-    const userIntegrations: Integration[] = [];
-    // Should get a single integration
-    let finalIntegrations = addOrUpdateIntegration(testIntegration, userIntegrations);
-    expect(finalIntegrations).toBeInstanceOf(Array);
-    finalIntegrations = finalIntegrations as Integration[];
-    expect(finalIntegrations).toHaveLength(1);
-    expect(finalIntegrations[0]).toMatchObject(testIntegration);
-  });
+import type { IntegrationWithExclusionOption as Integration } from '../../src/utils/userIntegrations';
+import { addOrUpdateIntegration, UserIntegrations } from '../../src/utils/userIntegrations';
+
+type MockIntegrationOptions = {
+  name: string;
+  descriptor: string;
+  age?: number;
+};
+
+class DogIntegration implements Integration {
+  public static id: string = 'Dog';
+  public name: string = DogIntegration.id;
+
+  public dogName: string;
+  public descriptor: string;
+  public age?: number;
+
+  public allowExclusionByUser?: boolean;
+
+  constructor(options: MockIntegrationOptions) {
+    this.dogName = options.name;
+    this.descriptor = options.descriptor;
+    this.age = options.age;
+  }
+
+  setupOnce() {
+    return undefined;
+  }
+}
+
+class CatIntegration implements Integration {
+  public static id: string = 'Cat';
+  public name: string = CatIntegration.id;
+
+  public catName: string;
+  public descriptor: string;
+  public age?: number;
+
+  constructor(options: MockIntegrationOptions) {
+    this.catName = options.name;
+    this.descriptor = options.descriptor;
+    this.age = options.age;
+  }
+
+  setupOnce() {
+    return undefined;
+  }
+}
+
+const defaultDogIntegration = new DogIntegration({ name: 'Maisey', descriptor: 'silly' });
+const defaultCatIntegration = new CatIntegration({ name: 'Piper', descriptor: 'fluffy' });
+const forcedDogIntegration = new DogIntegration({ name: 'Charlie', descriptor: 'goofy' });
+const forcedDogIntegrationProperties = { dogName: 'Charlie', descriptor: 'goofy' };
+
+// Note: This is essentially the implementation of a `test.each()` test. Making it a separate function called in
+// individual tests instead allows the various `describe` clauses to be nested, which is helpful here given how many
+// different combinations of factors come into play.
+function runTest(testOptions: {
+  userIntegrations: UserIntegrations;
+  forcedDogIntegrationInstance: DogIntegration;
+  underlyingDefaultIntegrations?: Integration[];
+  allowIntegrationExclusion?: boolean;
+  expectedDogIntegrationProperties: Partial<DogIntegration> | undefined;
+}): void {
+  const {
+    userIntegrations,
+    forcedDogIntegrationInstance,
+    underlyingDefaultIntegrations = [],
+    allowIntegrationExclusion = false,
+    expectedDogIntegrationProperties,
+  } = testOptions;
+
+  if (allowIntegrationExclusion) {
+    forcedDogIntegrationInstance.allowExclusionByUser = true;
+  }
+
+  let integrations;
+  if (typeof userIntegrations === 'function') {
+    const wrappedUserIntegrationsFunction = addOrUpdateIntegration(forcedDogIntegrationInstance, userIntegrations, {
+      dogName: 'Charlie',
+      descriptor: 'goofy',
+    });
+    integrations = wrappedUserIntegrationsFunction(underlyingDefaultIntegrations);
+  } else {
+    integrations = addOrUpdateIntegration(
+      forcedDogIntegrationInstance,
+      userIntegrations,
+      forcedDogIntegrationProperties,
+    );
+  }
+
+  const finalDogIntegrationInstance = integrations.find(integration => integration.name === 'Dog') as DogIntegration;
+
+  if (expectedDogIntegrationProperties) {
+    expect(finalDogIntegrationInstance).toMatchObject(expectedDogIntegrationProperties);
+  } else {
+    expect(finalDogIntegrationInstance).toBeUndefined();
+  }
 
-  test('as a function', () => {
-    const userIntegrationFnc: UserIntegrationsFunction = (): Integration[] => [];
-    // Should get a single integration
-    const integrationWrapper = addOrUpdateIntegration(testIntegration, userIntegrationFnc);
-    expect(integrationWrapper).toBeInstanceOf(Function);
-    const finalIntegrations = (integrationWrapper as UserIntegrationsFunction)([]);
-    expect(finalIntegrations).toHaveLength(1);
-    expect(finalIntegrations[0]).toMatchObject(testIntegration);
+  delete forcedDogIntegrationInstance.allowExclusionByUser;
+}
+
+describe('addOrUpdateIntegration()', () => {
+  describe('user provides no `integrations` option', () => {
+    it('adds forced integration instance', () => {
+      expect.assertions(1);
+
+      runTest({
+        userIntegrations: [], // default if no option is provided
+        forcedDogIntegrationInstance: forcedDogIntegration,
+        expectedDogIntegrationProperties: forcedDogIntegrationProperties,
+      });
+    });
   });
-});
 
-describe('user integrations with integrations', () => {
-  test('as an array', () => {
-    const userIntegrations = [new RewriteFrames()];
-    // Should get the same array (with no patches)
-    const finalIntegrations = addOrUpdateIntegration(testIntegration, userIntegrations);
-    expect(finalIntegrations).toMatchObject(userIntegrations);
+  describe('user provides `integrations` array', () => {
+    describe('array contains forced integration type', () => {
+      it('updates user instance with forced options', () => {
+        expect.assertions(1);
+
+        runTest({
+          userIntegrations: [{ ...defaultDogIntegration, age: 9 } as unknown as Integration],
+          forcedDogIntegrationInstance: forcedDogIntegration,
+          expectedDogIntegrationProperties: { ...forcedDogIntegrationProperties, age: 9 },
+        });
+      });
+    });
+
+    describe('array does not contain forced integration type', () => {
+      it('adds forced integration instance', () => {
+        expect.assertions(1);
+
+        runTest({
+          userIntegrations: [defaultCatIntegration],
+          forcedDogIntegrationInstance: forcedDogIntegration,
+          expectedDogIntegrationProperties: forcedDogIntegrationProperties,
+        });
+      });
+    });
   });
 
-  test('as a function', () => {
-    const userIntegrations = [new RewriteFrames()];
-    const integrationsFnc: UserIntegrationsFunction = (_integrations: Integration[]): Integration[] => {
-      return userIntegrations;
-    };
-    // Should get a function that returns the test integration
-    let finalIntegrations = addOrUpdateIntegration(testIntegration, integrationsFnc);
-    expect(typeof finalIntegrations === 'function').toBe(true);
-    expect(finalIntegrations).toBeInstanceOf(Function);
-    finalIntegrations = finalIntegrations as UserIntegrationsFunction;
-    expect(finalIntegrations([])).toMatchObject(userIntegrations);
+  describe('user provides `integrations` function', () => {
+    describe('forced integration in `defaultIntegrations`', () => {
+      const underlyingDefaultIntegrations = [defaultDogIntegration, defaultCatIntegration];
+
+      describe('function filters out forced integration type', () => {
+        it('adds forced integration instance by default', () => {
+          expect.assertions(1);
+
+          runTest({
+            userIntegrations: _defaults => [defaultCatIntegration],
+            forcedDogIntegrationInstance: forcedDogIntegration,
+            underlyingDefaultIntegrations,
+            expectedDogIntegrationProperties: forcedDogIntegrationProperties,
+          });
+        });
+
+        it('does not add forced integration instance if integration exclusion is allowed', () => {
+          expect.assertions(1);
+
+          runTest({
+            userIntegrations: _defaults => [defaultCatIntegration],
+            forcedDogIntegrationInstance: forcedDogIntegration,
+            underlyingDefaultIntegrations,
+            allowIntegrationExclusion: true,
+            expectedDogIntegrationProperties: undefined, // this means no instance was found
+          });
+        });
+      });
+
+      describe("function doesn't filter out forced integration type", () => {
+        it('updates user instance with forced options', () => {
+          expect.assertions(1);
+
+          runTest({
+            userIntegrations: _defaults => [{ ...defaultDogIntegration, age: 9 } as unknown as Integration],
+            forcedDogIntegrationInstance: forcedDogIntegration,
+            underlyingDefaultIntegrations,
+            expectedDogIntegrationProperties: { ...forcedDogIntegrationProperties, age: 9 },
+          });
+        });
+      });
+    });
+
+    describe('forced integration not in `defaultIntegrations`', () => {
+      const underlyingDefaultIntegrations = [defaultCatIntegration];
+
+      describe('function returns forced integration type', () => {
+        it('updates user instance with forced options', () => {
+          expect.assertions(1);
+
+          runTest({
+            userIntegrations: _defaults => [{ ...defaultDogIntegration, age: 9 } as unknown as Integration],
+            forcedDogIntegrationInstance: forcedDogIntegration,
+            underlyingDefaultIntegrations,
+            expectedDogIntegrationProperties: { ...forcedDogIntegrationProperties, age: 9 },
+          });
+        });
+      });
+
+      describe("function doesn't return forced integration type", () => {
+        it('adds forced integration instance', () => {
+          expect.assertions(1);
+
+          runTest({
+            userIntegrations: _defaults => [{ ...defaultCatIntegration, age: 1 } as unknown as Integration],
+            forcedDogIntegrationInstance: forcedDogIntegration,
+            underlyingDefaultIntegrations,
+            expectedDogIntegrationProperties: forcedDogIntegrationProperties,
+          });
+        });
+      });
+    });
   });
 });