diff --git a/packages/core/src/integration.ts b/packages/core/src/integration.ts index 811a3852c410..153bb0cd0d65 100644 --- a/packages/core/src/integration.ts +++ b/packages/core/src/integration.ts @@ -1,6 +1,12 @@ import { addGlobalEventProcessor, getCurrentHub } from '@sentry/hub'; import { Integration, Options } from '@sentry/types'; -import { logger } from '@sentry/utils'; +import { arrayify, logger } from '@sentry/utils'; + +declare module '@sentry/types' { + interface Integration { + isDefaultInstance?: boolean; + } +} export const installedIntegrations: string[] = []; @@ -10,46 +16,64 @@ export type IntegrationIndex = { }; /** + * Remove duplicates from the given array, preferring the last instance of any duplicate. Not guaranteed to + * preseve the order of integrations in the array. + * * @private */ function filterDuplicates(integrations: Integration[]): Integration[] { - return integrations.reduce((acc, integrations) => { - if (acc.every(accIntegration => integrations.name !== accIntegration.name)) { - acc.push(integrations); + const integrationsByName: { [key: string]: Integration } = {}; + + integrations.forEach(currentInstance => { + const { name } = currentInstance; + + const existingInstance = integrationsByName[name]; + + // We want integrations later in the array to overwrite earlier ones of the same type, except that we never want a + // default instance to overwrite an existing user instance + if (existingInstance && !existingInstance.isDefaultInstance && currentInstance.isDefaultInstance) { + return; } - return acc; - }, [] as Integration[]); + + integrationsByName[name] = currentInstance; + }); + + return Object.values(integrationsByName); } -/** Gets integration to install */ +/** Gets integrations to install */ export function getIntegrationsToSetup(options: Options): Integration[] { - const defaultIntegrations = (options.defaultIntegrations && [...options.defaultIntegrations]) || []; + const defaultIntegrations = options.defaultIntegrations || []; const userIntegrations = options.integrations; - let integrations: Integration[] = [...filterDuplicates(defaultIntegrations)]; + // We flag default instances, so that later we can tell them apart from any user-created instances of the same class + defaultIntegrations.forEach(integration => { + integration.isDefaultInstance = true; + }); + + let integrations: Integration[]; if (Array.isArray(userIntegrations)) { - // Filter out integrations that are also included in user options - integrations = [ - ...integrations.filter(integrations => - userIntegrations.every(userIntegration => userIntegration.name !== integrations.name), - ), - // And filter out duplicated user options integrations - ...filterDuplicates(userIntegrations), - ]; + integrations = [...defaultIntegrations, ...userIntegrations]; } else if (typeof userIntegrations === 'function') { - integrations = userIntegrations(integrations); - integrations = Array.isArray(integrations) ? integrations : [integrations]; + integrations = arrayify(userIntegrations(defaultIntegrations)); + } else { + integrations = defaultIntegrations; } - // Make sure that if present, `Debug` integration will always run last - const integrationsNames = integrations.map(i => i.name); - const alwaysLastToRun = 'Debug'; - if (integrationsNames.indexOf(alwaysLastToRun) !== -1) { - integrations.push(...integrations.splice(integrationsNames.indexOf(alwaysLastToRun), 1)); + const finalIntegrations = filterDuplicates(integrations); + + // The `Debug` integration prints copies of the `event` and `hint` which will be passed to `beforeSend`. It therefore + // has to run after all other integrations, so that the changes of all event processors will be reflected in the + // printed values. For lack of a more elegant way to guarantee that, we therefore locate it and, assuming it exists, + // pop it out of its current spot and shove it onto the end of the array. + const debugIndex = finalIntegrations.findIndex(integration => integration.name === 'Debug'); + if (debugIndex !== -1) { + const [debugInstance] = finalIntegrations.splice(debugIndex, 1); + finalIntegrations.push(debugInstance); } - return integrations; + return finalIntegrations; } /** diff --git a/packages/core/test/lib/integration.test.ts b/packages/core/test/lib/integration.test.ts index 606c09ecb01f..1b5b161b3d86 100644 --- a/packages/core/test/lib/integration.test.ts +++ b/packages/core/test/lib/integration.test.ts @@ -1,4 +1,4 @@ -import { Integration } from '@sentry/types'; +import { Integration, Options } from '@sentry/types'; import { getIntegrationsToSetup } from '../../src/integration'; @@ -6,8 +6,12 @@ import { getIntegrationsToSetup } from '../../src/integration'; class MockIntegration implements Integration { public name: string; - public constructor(name: string) { + // Only for testing - tag to keep separate instances straight when testing deduplication + public tag?: string; + + public constructor(name: string, tag?: string) { this.name = name; + this.tag = tag; } public setupOnce(): void { @@ -15,7 +19,187 @@ class MockIntegration implements Integration { } } +type TestCase = [ + string, // test name + Options['defaultIntegrations'], // default integrations + Options['integrations'], // user-provided integrations + Array, // expected resulst +]; + describe('getIntegrationsToSetup', () => { + describe('no duplicate integrations', () => { + const defaultIntegrations = [new MockIntegration('ChaseSquirrels')]; + const userIntegrationsArray = [new MockIntegration('CatchTreats')]; + const userIntegrationsFunction = (defaults: Integration[]) => [...defaults, ...userIntegrationsArray]; + + const testCases: TestCase[] = [ + // each test case is [testName, defaultIntegrations, userIntegrations, expectedResult] + ['no default integrations, no user integrations provided', false, undefined, []], + ['no default integrations, empty user-provided array', false, [], []], + ['no default integrations, user-provided array', false, userIntegrationsArray, ['CatchTreats']], + ['no default integrations, user-provided function', false, userIntegrationsFunction, ['CatchTreats']], + ['with default integrations, no user integrations provided', defaultIntegrations, undefined, ['ChaseSquirrels']], + ['with default integrations, empty user-provided array', defaultIntegrations, [], ['ChaseSquirrels']], + [ + 'with default integrations, user-provided array', + defaultIntegrations, + userIntegrationsArray, + ['ChaseSquirrels', 'CatchTreats'], + ], + [ + 'with default integrations, user-provided function', + defaultIntegrations, + userIntegrationsFunction, + ['ChaseSquirrels', 'CatchTreats'], + ], + ]; + + test.each(testCases)('%s', (_, defaultIntegrations, userIntegrations, expected) => { + const integrations = getIntegrationsToSetup({ + defaultIntegrations, + integrations: userIntegrations, + }); + expect(integrations.map(i => i.name)).toEqual(expected); + }); + }); + + describe('deduping', () => { + // No duplicates + const defaultIntegrations = [new MockIntegration('ChaseSquirrels', 'defaultA')]; + const userIntegrationsArray = [new MockIntegration('CatchTreats', 'userA')]; + + // Duplicates within either default or user integrations, but no overlap between them (to show that last one wins) + const duplicateDefaultIntegrations = [ + new MockIntegration('ChaseSquirrels', 'defaultA'), + new MockIntegration('ChaseSquirrels', 'defaultB'), + ]; + const duplicateUserIntegrationsArray = [ + new MockIntegration('CatchTreats', 'userA'), + new MockIntegration('CatchTreats', 'userB'), + ]; + const duplicateUserIntegrationsFunctionDefaultsFirst = (defaults: Integration[]) => [ + ...defaults, + ...duplicateUserIntegrationsArray, + ]; + const duplicateUserIntegrationsFunctionDefaultsSecond = (defaults: Integration[]) => [ + ...duplicateUserIntegrationsArray, + ...defaults, + ]; + + // User integrations containing new instances of default integrations (to show that user integration wins) + const userIntegrationsMatchingDefaultsArray = [ + new MockIntegration('ChaseSquirrels', 'userA'), + new MockIntegration('CatchTreats', 'userA'), + ]; + const userIntegrationsMatchingDefaultsFunctionDefaultsFirst = (defaults: Integration[]) => [ + ...defaults, + ...userIntegrationsMatchingDefaultsArray, + ]; + const userIntegrationsMatchingDefaultsFunctionDefaultsSecond = (defaults: Integration[]) => [ + ...userIntegrationsMatchingDefaultsArray, + ...defaults, + ]; + + const testCases: TestCase[] = [ + // each test case is [testName, defaultIntegrations, userIntegrations, expectedResult] + [ + 'duplicate default integrations', + duplicateDefaultIntegrations, + userIntegrationsArray, + [ + ['ChaseSquirrels', 'defaultB'], + ['CatchTreats', 'userA'], + ], + ], + [ + 'duplicate user integrations, user-provided array', + defaultIntegrations, + duplicateUserIntegrationsArray, + [ + ['ChaseSquirrels', 'defaultA'], + ['CatchTreats', 'userB'], + ], + ], + [ + 'duplicate user integrations, user-provided function with defaults first', + defaultIntegrations, + duplicateUserIntegrationsFunctionDefaultsFirst, + [ + ['ChaseSquirrels', 'defaultA'], + ['CatchTreats', 'userB'], + ], + ], + [ + 'duplicate user integrations, user-provided function with defaults second', + defaultIntegrations, + duplicateUserIntegrationsFunctionDefaultsSecond, + [ + ['CatchTreats', 'userB'], + ['ChaseSquirrels', 'defaultA'], + ], + ], + [ + 'same integration in default and user integrations, user-provided array', + defaultIntegrations, + userIntegrationsMatchingDefaultsArray, + [ + ['ChaseSquirrels', 'userA'], + ['CatchTreats', 'userA'], + ], + ], + [ + 'same integration in default and user integrations, user-provided function with defaults first', + defaultIntegrations, + userIntegrationsMatchingDefaultsFunctionDefaultsFirst, + [ + ['ChaseSquirrels', 'userA'], + ['CatchTreats', 'userA'], + ], + ], + [ + 'same integration in default and user integrations, user-provided function with defaults second', + defaultIntegrations, + userIntegrationsMatchingDefaultsFunctionDefaultsSecond, + [ + ['ChaseSquirrels', 'userA'], + ['CatchTreats', 'userA'], + ], + ], + ]; + + test.each(testCases)('%s', (_, defaultIntegrations, userIntegrations, expected) => { + const integrations = getIntegrationsToSetup({ + defaultIntegrations: defaultIntegrations, + integrations: userIntegrations, + }) as MockIntegration[]; + + expect(integrations.map(i => [i.name, i.tag])).toEqual(expected); + }); + }); + + describe('puts `Debug` integration last', () => { + // No variations here (default vs user, duplicates, user array vs user function, etc) because by the time we're + // dealing with the `Debug` integration, all of the combining and deduping has already been done + const noDebug = [new MockIntegration('ChaseSquirrels')]; + const debugNotLast = [new MockIntegration('Debug'), new MockIntegration('CatchTreats')]; + const debugAlreadyLast = [new MockIntegration('ChaseSquirrels'), new MockIntegration('Debug')]; + + const testCases: TestCase[] = [ + // each test case is [testName, defaultIntegrations, userIntegrations, expectedResult] + ['`Debug` not present', false, noDebug, ['ChaseSquirrels']], + ['`Debug` not originally last', false, debugNotLast, ['CatchTreats', 'Debug']], + ['`Debug` already last', false, debugAlreadyLast, ['ChaseSquirrels', 'Debug']], + ]; + + test.each(testCases)('%s', (_, defaultIntegrations, userIntegrations, expected) => { + const integrations = getIntegrationsToSetup({ + defaultIntegrations, + integrations: userIntegrations, + }); + expect(integrations.map(i => i.name)).toEqual(expected); + }); + }); + it('works with empty array', () => { const integrations = getIntegrationsToSetup({ integrations: [], @@ -48,20 +232,6 @@ describe('getIntegrationsToSetup', () => { expect(integrations.map(i => i.name)).toEqual(['foo', 'bar']); }); - it('filter duplicated items and always let first win', () => { - const first = new MockIntegration('foo'); - (first as any).order = 'first'; - const second = new MockIntegration('foo'); - (second as any).order = 'second'; - - const integrations = getIntegrationsToSetup({ - integrations: [first, second, new MockIntegration('bar')], - }); - - expect(integrations.map(i => i.name)).toEqual(['foo', 'bar']); - expect((integrations[0] as any).order).toEqual('first'); - }); - it('work with empty defaults', () => { const integrations = getIntegrationsToSetup({ defaultIntegrations: [],