diff --git a/packages/nextjs/src/index.server.ts b/packages/nextjs/src/index.server.ts index a61e49fe758f..0191a5334b2d 100644 --- a/packages/nextjs/src/index.server.ts +++ b/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 }, }); diff --git a/packages/nextjs/src/utils/userIntegrations.ts b/packages/nextjs/src/utils/userIntegrations.ts index 5f9e21b6b0f2..3f84a58fa41b 100644 --- a/packages/nextjs/src/utils/userIntegrations.ts +++ b/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, 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( 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; } diff --git a/packages/nextjs/test/utils/userIntegrations.test.ts b/packages/nextjs/test/utils/userIntegrations.test.ts index aa2aeca12e61..7a8693a8a524 100644 --- a/packages/nextjs/test/utils/userIntegrations.test.ts +++ b/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 | 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, + }); + }); + }); + }); }); });