From d53b30c64e9bd40a939d8a286c8ef2f95e64b070 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 13 Dec 2023 11:48:11 +0100 Subject: [PATCH 01/12] WIP: Add makeIntegrationFn util --- packages/core/src/integration.ts | 26 +++++++- packages/core/test/lib/integration.test.ts | 74 +++++++++++++++++++++- packages/types/src/index.ts | 2 +- packages/types/src/integration.ts | 44 ++++++++++++- 4 files changed, 142 insertions(+), 4 deletions(-) diff --git a/packages/core/src/integration.ts b/packages/core/src/integration.ts index 7be22a316e53..ca38adbd4513 100644 --- a/packages/core/src/integration.ts +++ b/packages/core/src/integration.ts @@ -1,4 +1,4 @@ -import type { Client, Event, EventHint, Integration, Options } from '@sentry/types'; +import type { Client, Event, EventHint, Integration, IntegrationFn, IntegrationFnResult, Options } from '@sentry/types'; import { arrayify, logger } from '@sentry/utils'; import { DEBUG_BUILD } from './debug-build'; @@ -155,3 +155,27 @@ function findIndex(arr: T[], callback: (item: T) => boolean): number { return -1; } + +/** + * Generate a full integration function from a simple function. + * This will ensure to add the given name both to the function definition, as well as to the integration return value. + */ +export function makeIntegrationFn< + Fn extends (...rest: any[]) => Partial, +>(name: string, fn: Fn): ((...rest: Parameters) => ReturnType & { name: string }) & { id: string } { + const patchedFn = addIdToIntegrationFnResult(name, fn); + + + return Object.assign(patchedFn, { id: name }); +} + +function addIdToIntegrationFnResult Partial>( + name: string, + fn: Fn, +): (...rest: Parameters) => ReturnType & { name: string } { + const patchedFn = ((...rest: Parameters): ReturnType & { name: string } => { + return { ...fn(...rest), name } as ReturnType & { name: string }; + }); + + return patchedFn; +} diff --git a/packages/core/test/lib/integration.test.ts b/packages/core/test/lib/integration.test.ts index 54bd426abb5c..9fd0c08040e8 100644 --- a/packages/core/test/lib/integration.test.ts +++ b/packages/core/test/lib/integration.test.ts @@ -2,7 +2,13 @@ import type { Integration, Options } from '@sentry/types'; import { logger } from '@sentry/utils'; import { Hub, makeMain } from '../../src/hub'; -import { addIntegration, getIntegrationsToSetup, installedIntegrations, setupIntegration } from '../../src/integration'; +import { + addIntegration, + getIntegrationsToSetup, + installedIntegrations, + makeIntegrationFn, + setupIntegration, +} from '../../src/integration'; import { TestClient, getDefaultTestClientOptions } from '../mocks/client'; function getTestClient(): TestClient { @@ -647,3 +653,69 @@ describe('addIntegration', () => { expect(warnings).toHaveBeenCalledWith('Cannot add integration "test" because no SDK Client is available.'); }); }); + +describe('makeIntegrationFn', () => { + it('works with a minimal integration', () => { + const myIntegration = makeIntegrationFn('testName', () => { + return {}; + }); + + expect(myIntegration.id).toBe('testName'); + + const integration = myIntegration(); + expect(integration).toEqual({ + name: 'testName', + }); + + // @ts-expect-error This SHOULD error + myIntegration({}); + }); + + it('works with integration options', () => { + const myIntegration = makeIntegrationFn('testName', (_options: { xxx: string }) => { + return {}; + }); + + expect(myIntegration.id).toBe('testName'); + + const integration = myIntegration({ xxx: 'aa' }); + expect(integration).toEqual({ + name: 'testName', + }); + + // @ts-expect-error This SHOULD error + myIntegration(); + // @ts-expect-error This SHOULD error + myIntegration({}); + }); + + it('works with integration hooks', () => { + const setup = jest.fn(); + const setupOnce = jest.fn(); + const processEvent = jest.fn(); + const preprocessEvent = jest.fn(); + + const myIntegration = makeIntegrationFn('testName', () => { + return { + setup, + setupOnce, + processEvent, + preprocessEvent, + }; + }); + + expect(myIntegration.id).toBe('testName'); + + const integration = myIntegration(); + expect(integration).toEqual({ + name: 'testName', + setup, + setupOnce, + processEvent, + preprocessEvent, + }); + + // @ts-expect-error This SHOULD error + makeIntegrationFn('testName', () => ({ other: 'aha' })); + }); +}); diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index 7d3531599aa3..a2d75a193414 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -52,7 +52,7 @@ export type { EventProcessor } from './eventprocessor'; export type { Exception } from './exception'; export type { Extra, Extras } from './extra'; export type { Hub } from './hub'; -export type { Integration, IntegrationClass } from './integration'; +export type { Integration, IntegrationClass, IntegrationFn, IntegrationFnResult } from './integration'; export type { Mechanism } from './mechanism'; export type { ExtractedNodeRequestData, HttpHeaderValue, Primitive, WorkerLocation } from './misc'; export type { ClientOptions, Options } from './options'; diff --git a/packages/types/src/integration.ts b/packages/types/src/integration.ts index 0c18845414e3..3e2f6e0dddec 100644 --- a/packages/types/src/integration.ts +++ b/packages/types/src/integration.ts @@ -10,7 +10,49 @@ export interface IntegrationClass { */ id: string; - new (...args: any[]): T; + new(...args: any[]): T; +} + +/** + * An integration in function form. + * This is expected to return an integration result, + * and also have a `name` property that holds the integration name (so we can e.g. filter these before actually calling them). + */ +export type IntegrationFn IntegrationFnResult)> = { id: string } & Fn; + +export interface IntegrationFnResult { + /** + * The name of the integration. + */ + name: string; + + /** + * This hook is only called once, even if multiple clients are created. + * It does not receives any arguments, and should only use for e.g. global monkey patching and similar things. + */ + setupOnce?(): void; + + /** + * Set up an integration for the given client. + * Receives the client as argument. + * + * Whenever possible, prefer this over `setupOnce`, as that is only run for the first client, + * whereas `setup` runs for each client. Only truly global things (e.g. registering global handlers) + * should be done in `setupOnce`. + */ + setup?(client: Client): void; + + /** + * An optional hook that allows to preprocess an event _before_ it is passed to all other event processors. + */ + preprocessEvent?(event: Event, hint: EventHint | undefined, client: Client): void; + + /** + * An optional hook that allows to process an event. + * Return `null` to drop the event, or mutate the event & return it. + * This receives the client that the integration was installed for as third argument. + */ + processEvent?(event: Event, hint: EventHint, client: Client): Event | null | PromiseLike; } /** Integration interface */ From ed7652c03a5b65a5f3d545232c497a6496e10c76 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 13 Dec 2023 13:34:57 +0100 Subject: [PATCH 02/12] WIP: add convertIntegrationFnToClass util --- packages/core/src/integration.ts | 45 +++++++++++++++++---- packages/core/test/lib/integration.test.ts | 46 ++++++++++++++++++++++ 2 files changed, 84 insertions(+), 7 deletions(-) diff --git a/packages/core/src/integration.ts b/packages/core/src/integration.ts index ca38adbd4513..d4fe3c2d8968 100644 --- a/packages/core/src/integration.ts +++ b/packages/core/src/integration.ts @@ -1,4 +1,13 @@ -import type { Client, Event, EventHint, Integration, IntegrationFn, IntegrationFnResult, Options } from '@sentry/types'; +import type { + Client, + Event, + EventHint, + Integration, + IntegrationClass, + IntegrationFn, + IntegrationFnResult, + Options, +} from '@sentry/types'; import { arrayify, logger } from '@sentry/utils'; import { DEBUG_BUILD } from './debug-build'; @@ -160,22 +169,44 @@ function findIndex(arr: T[], callback: (item: T) => boolean): number { * Generate a full integration function from a simple function. * This will ensure to add the given name both to the function definition, as well as to the integration return value. */ -export function makeIntegrationFn< - Fn extends (...rest: any[]) => Partial, ->(name: string, fn: Fn): ((...rest: Parameters) => ReturnType & { name: string }) & { id: string } { +export function makeIntegrationFn Partial>( + name: string, + fn: Fn, +): ((...rest: Parameters) => ReturnType & { name: string }) & { id: string } { const patchedFn = addIdToIntegrationFnResult(name, fn); - return Object.assign(patchedFn, { id: name }); } +/** + * Convert a new integration function to the legacy class syntax. + * In v8, we can remove this and instead export the integration functions directly. + */ +export function convertIntegrationFnToClass IntegrationFnResult>>( + fn: Fn, +): IntegrationClass { + return Object.assign( + function ConvertedIntegration(...rest: any[]) { + const res = { + // eslint-disable-next-line @typescript-eslint/no-empty-function + setupOnce: () => {}, + ...fn(...rest), + name: fn.id, + }; + + return res; + }, + { id: fn.id }, + ) as unknown as IntegrationClass; +} + function addIdToIntegrationFnResult Partial>( name: string, fn: Fn, ): (...rest: Parameters) => ReturnType & { name: string } { - const patchedFn = ((...rest: Parameters): ReturnType & { name: string } => { + const patchedFn = (...rest: Parameters): ReturnType & { name: string } => { return { ...fn(...rest), name } as ReturnType & { name: string }; - }); + }; return patchedFn; } diff --git a/packages/core/test/lib/integration.test.ts b/packages/core/test/lib/integration.test.ts index 9fd0c08040e8..97bf21de4a35 100644 --- a/packages/core/test/lib/integration.test.ts +++ b/packages/core/test/lib/integration.test.ts @@ -4,6 +4,7 @@ import { logger } from '@sentry/utils'; import { Hub, makeMain } from '../../src/hub'; import { addIntegration, + convertIntegrationFnToClass, getIntegrationsToSetup, installedIntegrations, makeIntegrationFn, @@ -719,3 +720,48 @@ describe('makeIntegrationFn', () => { makeIntegrationFn('testName', () => ({ other: 'aha' })); }); }); + +describe('convertIntegrationFnToClass', () => { + it('works with a minimal integration', () => { + const integrationFn = makeIntegrationFn('testName', () => ({})); + + const IntegrationClass = convertIntegrationFnToClass(integrationFn); + + expect(IntegrationClass.id).toBe('testName'); + + const integration = new IntegrationClass(); + expect(integration).toEqual({ + name: 'testName', + setupOnce: expect.any(Function), + }); + }); + + it('works with integration hooks', () => { + const setup = jest.fn(); + const setupOnce = jest.fn(); + const processEvent = jest.fn(); + const preprocessEvent = jest.fn(); + + const integrationFn = makeIntegrationFn('testName', () => { + return { + setup, + setupOnce, + processEvent, + preprocessEvent, + }; + }); + + const IntegrationClass = convertIntegrationFnToClass(integrationFn); + + expect(IntegrationClass.id).toBe('testName'); + + const integration = new IntegrationClass(); + expect(integration).toEqual({ + name: 'testName', + setupOnce, + setup, + processEvent, + preprocessEvent, + }); + }); +}); From a91624aab6b40b8730fee3f3793e3b442aa6a1f7 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 13 Dec 2023 13:37:32 +0100 Subject: [PATCH 03/12] export it --- packages/core/src/index.ts | 8 ++++++- packages/core/src/integration.ts | 26 +++++++++++++--------- packages/core/test/lib/integration.test.ts | 2 ++ 3 files changed, 24 insertions(+), 12 deletions(-) diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index ccf219031b8f..88ad4299e377 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -54,7 +54,13 @@ export { createTransport } from './transports/base'; export { makeOfflineTransport } from './transports/offline'; export { makeMultiplexedTransport } from './transports/multiplexed'; export { SDK_VERSION } from './version'; -export { getIntegrationsToSetup, addIntegration } from './integration'; +export { + getIntegrationsToSetup, + addIntegration, + // eslint-disable-next-line deprecation/deprecation + convertIntegrationFnToClass, + makeIntegrationFn, +} from './integration'; export { FunctionToString, InboundFilters, LinkedErrors } from './integrations'; export { prepareEvent } from './utils/prepareEvent'; export { createCheckInEnvelope } from './checkin'; diff --git a/packages/core/src/integration.ts b/packages/core/src/integration.ts index d4fe3c2d8968..68cca759f053 100644 --- a/packages/core/src/integration.ts +++ b/packages/core/src/integration.ts @@ -169,10 +169,10 @@ function findIndex(arr: T[], callback: (item: T) => boolean): number { * Generate a full integration function from a simple function. * This will ensure to add the given name both to the function definition, as well as to the integration return value. */ -export function makeIntegrationFn Partial>( - name: string, - fn: Fn, -): ((...rest: Parameters) => ReturnType & { name: string }) & { id: string } { +export function makeIntegrationFn< + // eslint-disable-next-line @typescript-eslint/no-explicit-any + Fn extends (...rest: any[]) => Partial, +>(name: string, fn: Fn): ((...rest: Parameters) => ReturnType & { name: string }) & { id: string } { const patchedFn = addIdToIntegrationFnResult(name, fn); return Object.assign(patchedFn, { id: name }); @@ -181,11 +181,15 @@ export function makeIntegrationFn Partial IntegrationFnResult>>( - fn: Fn, -): IntegrationClass { +export function convertIntegrationFnToClass< + // eslint-disable-next-line @typescript-eslint/no-explicit-any + Fn extends IntegrationFn<(...rest: any[]) => IntegrationFnResult>, +>(fn: Fn): IntegrationClass { return Object.assign( + // eslint-disable-next-line @typescript-eslint/no-explicit-any function ConvertedIntegration(...rest: any[]) { const res = { // eslint-disable-next-line @typescript-eslint/no-empty-function @@ -200,10 +204,10 @@ export function convertIntegrationFnToClass; } -function addIdToIntegrationFnResult Partial>( - name: string, - fn: Fn, -): (...rest: Parameters) => ReturnType & { name: string } { +function addIdToIntegrationFnResult< + // eslint-disable-next-line @typescript-eslint/no-explicit-any + Fn extends (...rest: any[]) => Partial, +>(name: string, fn: Fn): (...rest: Parameters) => ReturnType & { name: string } { const patchedFn = (...rest: Parameters): ReturnType & { name: string } => { return { ...fn(...rest), name } as ReturnType & { name: string }; }; diff --git a/packages/core/test/lib/integration.test.ts b/packages/core/test/lib/integration.test.ts index 97bf21de4a35..0907fb210e8f 100644 --- a/packages/core/test/lib/integration.test.ts +++ b/packages/core/test/lib/integration.test.ts @@ -722,6 +722,7 @@ describe('makeIntegrationFn', () => { }); describe('convertIntegrationFnToClass', () => { + /* eslint-disable deprecation/deprecation */ it('works with a minimal integration', () => { const integrationFn = makeIntegrationFn('testName', () => ({})); @@ -764,4 +765,5 @@ describe('convertIntegrationFnToClass', () => { preprocessEvent, }); }); + /* eslint-enable deprecation/deprecation */ }); From a12c4059cc27b43694a18fcc8e8da0add54e009a Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 13 Dec 2023 13:42:28 +0100 Subject: [PATCH 04/12] refactor one integration as an example --- .../core/src/integrations/inboundfilters.ts | 49 ++++++------------- 1 file changed, 14 insertions(+), 35 deletions(-) diff --git a/packages/core/src/integrations/inboundfilters.ts b/packages/core/src/integrations/inboundfilters.ts index 9d348a4b4d23..054679ba5ada 100644 --- a/packages/core/src/integrations/inboundfilters.ts +++ b/packages/core/src/integrations/inboundfilters.ts @@ -2,6 +2,7 @@ import type { Client, Event, EventHint, Integration, StackFrame } from '@sentry/ import { getEventDescription, logger, stringMatchesSomePattern } from '@sentry/utils'; import { DEBUG_BUILD } from '../debug-build'; +import { convertIntegrationFnToClass, makeIntegrationFn } from '../integration'; // "Script error." is hard coded into browsers for errors that it can't read. // this is the result of a script being pulled in from an external domain and CORS. @@ -28,42 +29,21 @@ export interface InboundFiltersOptions { disableTransactionDefaults: boolean; } -/** Inbound filters configurable by the user */ -export class InboundFilters implements Integration { - /** - * @inheritDoc - */ - public static id: string = 'InboundFilters'; - - /** - * @inheritDoc - */ - public name: string; - - private readonly _options: Partial; - - public constructor(options: Partial = {}) { - this.name = InboundFilters.id; - this._options = options; - } - - /** - * @inheritDoc - */ - public setupOnce(_addGlobalEventProcessor: unknown, _getCurrentHub: unknown): void { - // noop +const inboundFiltersIntegration = makeIntegrationFn('InboundFilters', (options: Partial) => { + return { + processEvent(event, _hint, client) { + const clientOptions = client.getOptions(); + const mergedOptions = _mergeOptions(options, clientOptions); + return _shouldDropEvent(event, mergedOptions) ? null : event; + } } +}); - /** @inheritDoc */ - public processEvent(event: Event, _eventHint: EventHint, client: Client): Event | null { - const clientOptions = client.getOptions(); - const options = _mergeOptions(this._options, clientOptions); - return _shouldDropEvent(event, options) ? null : event; - } -} +/** Inbound filters configurable by the user */ +// eslint-disable-next-line deprecation/deprecation +export const InboundFilters = convertIntegrationFnToClass(inboundFiltersIntegration); -/** JSDoc */ -export function _mergeOptions( +function _mergeOptions( internalOptions: Partial = {}, clientOptions: Partial = {}, ): Partial { @@ -84,8 +64,7 @@ export function _mergeOptions( }; } -/** JSDoc */ -export function _shouldDropEvent(event: Event, options: Partial): boolean { +function _shouldDropEvent(event: Event, options: Partial): boolean { if (options.ignoreInternal && _isSentryError(event)) { DEBUG_BUILD && logger.warn(`Event dropped due to being internal Sentry Error.\nEvent: ${getEventDescription(event)}`); From 67e2daac74f45d1b4cf4cc291422b4e33ec43ee4 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 13 Dec 2023 13:50:16 +0100 Subject: [PATCH 05/12] small doc --- packages/core/src/integration.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/core/src/integration.ts b/packages/core/src/integration.ts index 68cca759f053..df37530abe20 100644 --- a/packages/core/src/integration.ts +++ b/packages/core/src/integration.ts @@ -167,7 +167,8 @@ function findIndex(arr: T[], callback: (item: T) => boolean): number { /** * Generate a full integration function from a simple function. - * This will ensure to add the given name both to the function definition, as well as to the integration return value. + * This will ensure to add the given name both to the function definition (as id), + * as well as to the integration return value. */ export function makeIntegrationFn< // eslint-disable-next-line @typescript-eslint/no-explicit-any From 57ed4d22dc533c8fe9e90554985f24735d9151aa Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 13 Dec 2023 14:06:02 +0100 Subject: [PATCH 06/12] fix linting --- packages/core/src/integration.ts | 4 ++-- packages/core/src/integrations/inboundfilters.ts | 4 ++-- packages/types/src/integration.ts | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/core/src/integration.ts b/packages/core/src/integration.ts index df37530abe20..23dcb4eca9f5 100644 --- a/packages/core/src/integration.ts +++ b/packages/core/src/integration.ts @@ -174,7 +174,7 @@ export function makeIntegrationFn< // eslint-disable-next-line @typescript-eslint/no-explicit-any Fn extends (...rest: any[]) => Partial, >(name: string, fn: Fn): ((...rest: Parameters) => ReturnType & { name: string }) & { id: string } { - const patchedFn = addIdToIntegrationFnResult(name, fn); + const patchedFn = addNameToIntegrationFnResult(name, fn); return Object.assign(patchedFn, { id: name }); } @@ -205,7 +205,7 @@ export function convertIntegrationFnToClass< ) as unknown as IntegrationClass; } -function addIdToIntegrationFnResult< +function addNameToIntegrationFnResult< // eslint-disable-next-line @typescript-eslint/no-explicit-any Fn extends (...rest: any[]) => Partial, >(name: string, fn: Fn): (...rest: Parameters) => ReturnType & { name: string } { diff --git a/packages/core/src/integrations/inboundfilters.ts b/packages/core/src/integrations/inboundfilters.ts index 054679ba5ada..309315577296 100644 --- a/packages/core/src/integrations/inboundfilters.ts +++ b/packages/core/src/integrations/inboundfilters.ts @@ -35,8 +35,8 @@ const inboundFiltersIntegration = makeIntegrationFn('InboundFilters', (options: const clientOptions = client.getOptions(); const mergedOptions = _mergeOptions(options, clientOptions); return _shouldDropEvent(event, mergedOptions) ? null : event; - } - } + }, + }; }); /** Inbound filters configurable by the user */ diff --git a/packages/types/src/integration.ts b/packages/types/src/integration.ts index 3e2f6e0dddec..7f8816859010 100644 --- a/packages/types/src/integration.ts +++ b/packages/types/src/integration.ts @@ -10,7 +10,7 @@ export interface IntegrationClass { */ id: string; - new(...args: any[]): T; + new (...args: any[]): T; } /** @@ -18,7 +18,7 @@ export interface IntegrationClass { * This is expected to return an integration result, * and also have a `name` property that holds the integration name (so we can e.g. filter these before actually calling them). */ -export type IntegrationFn IntegrationFnResult)> = { id: string } & Fn; +export type IntegrationFn IntegrationFnResult> = { id: string } & Fn; export interface IntegrationFnResult { /** From b1a536d0516727983fae98f24959d4cbfcaf323f Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 13 Dec 2023 14:57:26 +0100 Subject: [PATCH 07/12] PR feedback --- packages/core/src/integration.ts | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/packages/core/src/integration.ts b/packages/core/src/integration.ts index 23dcb4eca9f5..3f3480cab2fd 100644 --- a/packages/core/src/integration.ts +++ b/packages/core/src/integration.ts @@ -174,9 +174,12 @@ export function makeIntegrationFn< // eslint-disable-next-line @typescript-eslint/no-explicit-any Fn extends (...rest: any[]) => Partial, >(name: string, fn: Fn): ((...rest: Parameters) => ReturnType & { name: string }) & { id: string } { - const patchedFn = addNameToIntegrationFnResult(name, fn); + const patchedFn = addNameToIntegrationFnResult(name, fn) as (( + ...rest: Parameters + ) => ReturnType & { name: string }) & { id: string }; - return Object.assign(patchedFn, { id: name }); + patchedFn.id = name; + return patchedFn; } /** @@ -192,14 +195,12 @@ export function convertIntegrationFnToClass< return Object.assign( // eslint-disable-next-line @typescript-eslint/no-explicit-any function ConvertedIntegration(...rest: any[]) { - const res = { + return { // eslint-disable-next-line @typescript-eslint/no-empty-function setupOnce: () => {}, ...fn(...rest), name: fn.id, }; - - return res; }, { id: fn.id }, ) as unknown as IntegrationClass; @@ -210,7 +211,9 @@ function addNameToIntegrationFnResult< Fn extends (...rest: any[]) => Partial, >(name: string, fn: Fn): (...rest: Parameters) => ReturnType & { name: string } { const patchedFn = (...rest: Parameters): ReturnType & { name: string } => { - return { ...fn(...rest), name } as ReturnType & { name: string }; + const result = fn(...rest); + result.name = name; + return result as ReturnType & { name: string }; }; return patchedFn; From a053678fdb4250d1369fe93a4ead9af23b2b47ac Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 13 Dec 2023 15:07:08 +0100 Subject: [PATCH 08/12] try make it preprocess?? --- packages/integrations/src/contextlines.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/integrations/src/contextlines.ts b/packages/integrations/src/contextlines.ts index d716ca2fbb5f..7812cea358fb 100644 --- a/packages/integrations/src/contextlines.ts +++ b/packages/integrations/src/contextlines.ts @@ -49,8 +49,8 @@ export class ContextLines implements Integration { } /** @inheritDoc */ - public processEvent(event: Event): Event { - return this.addSourceContext(event); + public preprocessEvent(event: Event): void { + this.addSourceContext(event); } /** From 8771e998c82469efd240aad312b2b0cd12ff623f Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Thu, 14 Dec 2023 14:54:26 +0100 Subject: [PATCH 09/12] Revert "try make it preprocess??" This reverts commit 31887622b297c094e37ccfa00b2a8708bd0d73c7. --- packages/integrations/src/contextlines.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/integrations/src/contextlines.ts b/packages/integrations/src/contextlines.ts index 7812cea358fb..d716ca2fbb5f 100644 --- a/packages/integrations/src/contextlines.ts +++ b/packages/integrations/src/contextlines.ts @@ -49,8 +49,8 @@ export class ContextLines implements Integration { } /** @inheritDoc */ - public preprocessEvent(event: Event): void { - this.addSourceContext(event); + public processEvent(event: Event): Event { + return this.addSourceContext(event); } /** From c5c8d82e51df816b482838bb3c8b5571654df2de Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Thu, 14 Dec 2023 15:13:31 +0100 Subject: [PATCH 10/12] reduce --- packages/core/src/integration.ts | 36 ++++--------------- .../core/src/integrations/inboundfilters.ts | 6 ++-- packages/core/test/lib/integration.test.ts | 30 ++++++++-------- packages/types/src/integration.ts | 3 +- 4 files changed, 27 insertions(+), 48 deletions(-) diff --git a/packages/core/src/integration.ts b/packages/core/src/integration.ts index 3f3480cab2fd..e2d265509754 100644 --- a/packages/core/src/integration.ts +++ b/packages/core/src/integration.ts @@ -170,16 +170,8 @@ function findIndex(arr: T[], callback: (item: T) => boolean): number { * This will ensure to add the given name both to the function definition (as id), * as well as to the integration return value. */ -export function makeIntegrationFn< - // eslint-disable-next-line @typescript-eslint/no-explicit-any - Fn extends (...rest: any[]) => Partial, ->(name: string, fn: Fn): ((...rest: Parameters) => ReturnType & { name: string }) & { id: string } { - const patchedFn = addNameToIntegrationFnResult(name, fn) as (( - ...rest: Parameters - ) => ReturnType & { name: string }) & { id: string }; - - patchedFn.id = name; - return patchedFn; +export function makeIntegrationFn(fn: Fn): Fn { + return fn; } /** @@ -188,10 +180,10 @@ export function makeIntegrationFn< * * @deprecated This will be removed in v8! */ -export function convertIntegrationFnToClass< - // eslint-disable-next-line @typescript-eslint/no-explicit-any - Fn extends IntegrationFn<(...rest: any[]) => IntegrationFnResult>, ->(fn: Fn): IntegrationClass { +export function convertIntegrationFnToClass( + name: string, + fn: Fn, +): IntegrationClass { return Object.assign( // eslint-disable-next-line @typescript-eslint/no-explicit-any function ConvertedIntegration(...rest: any[]) { @@ -199,22 +191,8 @@ export function convertIntegrationFnToClass< // eslint-disable-next-line @typescript-eslint/no-empty-function setupOnce: () => {}, ...fn(...rest), - name: fn.id, }; }, - { id: fn.id }, + { id: name }, ) as unknown as IntegrationClass; } - -function addNameToIntegrationFnResult< - // eslint-disable-next-line @typescript-eslint/no-explicit-any - Fn extends (...rest: any[]) => Partial, ->(name: string, fn: Fn): (...rest: Parameters) => ReturnType & { name: string } { - const patchedFn = (...rest: Parameters): ReturnType & { name: string } => { - const result = fn(...rest); - result.name = name; - return result as ReturnType & { name: string }; - }; - - return patchedFn; -} diff --git a/packages/core/src/integrations/inboundfilters.ts b/packages/core/src/integrations/inboundfilters.ts index 309315577296..af9bffbf9c8f 100644 --- a/packages/core/src/integrations/inboundfilters.ts +++ b/packages/core/src/integrations/inboundfilters.ts @@ -29,8 +29,10 @@ export interface InboundFiltersOptions { disableTransactionDefaults: boolean; } -const inboundFiltersIntegration = makeIntegrationFn('InboundFilters', (options: Partial) => { +const INTEGRATION_NAME = 'InboundFilters'; +const inboundFiltersIntegration = makeIntegrationFn((options: Partial) => { return { + name: INTEGRATION_NAME, processEvent(event, _hint, client) { const clientOptions = client.getOptions(); const mergedOptions = _mergeOptions(options, clientOptions); @@ -41,7 +43,7 @@ const inboundFiltersIntegration = makeIntegrationFn('InboundFilters', (options: /** Inbound filters configurable by the user */ // eslint-disable-next-line deprecation/deprecation -export const InboundFilters = convertIntegrationFnToClass(inboundFiltersIntegration); +export const InboundFilters = convertIntegrationFnToClass(INTEGRATION_NAME, inboundFiltersIntegration); function _mergeOptions( internalOptions: Partial = {}, diff --git a/packages/core/test/lib/integration.test.ts b/packages/core/test/lib/integration.test.ts index 0907fb210e8f..00efc3a7b1a0 100644 --- a/packages/core/test/lib/integration.test.ts +++ b/packages/core/test/lib/integration.test.ts @@ -657,12 +657,12 @@ describe('addIntegration', () => { describe('makeIntegrationFn', () => { it('works with a minimal integration', () => { - const myIntegration = makeIntegrationFn('testName', () => { - return {}; + const myIntegration = makeIntegrationFn(() => { + return { + name: 'testName' + }; }); - expect(myIntegration.id).toBe('testName'); - const integration = myIntegration(); expect(integration).toEqual({ name: 'testName', @@ -673,12 +673,12 @@ describe('makeIntegrationFn', () => { }); it('works with integration options', () => { - const myIntegration = makeIntegrationFn('testName', (_options: { xxx: string }) => { - return {}; + const myIntegration = makeIntegrationFn((_options: { xxx: string }) => { + return { + name: 'testName' + }; }); - expect(myIntegration.id).toBe('testName'); - const integration = myIntegration({ xxx: 'aa' }); expect(integration).toEqual({ name: 'testName', @@ -696,8 +696,9 @@ describe('makeIntegrationFn', () => { const processEvent = jest.fn(); const preprocessEvent = jest.fn(); - const myIntegration = makeIntegrationFn('testName', () => { + const myIntegration = makeIntegrationFn(() => { return { + name: 'testName', setup, setupOnce, processEvent, @@ -705,8 +706,6 @@ describe('makeIntegrationFn', () => { }; }); - expect(myIntegration.id).toBe('testName'); - const integration = myIntegration(); expect(integration).toEqual({ name: 'testName', @@ -724,9 +723,9 @@ describe('makeIntegrationFn', () => { describe('convertIntegrationFnToClass', () => { /* eslint-disable deprecation/deprecation */ it('works with a minimal integration', () => { - const integrationFn = makeIntegrationFn('testName', () => ({})); + const integrationFn = makeIntegrationFn(() => ({ name: 'testName'})); - const IntegrationClass = convertIntegrationFnToClass(integrationFn); + const IntegrationClass = convertIntegrationFnToClass('testName', integrationFn); expect(IntegrationClass.id).toBe('testName'); @@ -743,8 +742,9 @@ describe('convertIntegrationFnToClass', () => { const processEvent = jest.fn(); const preprocessEvent = jest.fn(); - const integrationFn = makeIntegrationFn('testName', () => { + const integrationFn = makeIntegrationFn( () => { return { + name: 'testName', setup, setupOnce, processEvent, @@ -752,7 +752,7 @@ describe('convertIntegrationFnToClass', () => { }; }); - const IntegrationClass = convertIntegrationFnToClass(integrationFn); + const IntegrationClass = convertIntegrationFnToClass('testName', integrationFn); expect(IntegrationClass.id).toBe('testName'); diff --git a/packages/types/src/integration.ts b/packages/types/src/integration.ts index 7f8816859010..a4108a60c749 100644 --- a/packages/types/src/integration.ts +++ b/packages/types/src/integration.ts @@ -16,9 +16,8 @@ export interface IntegrationClass { /** * An integration in function form. * This is expected to return an integration result, - * and also have a `name` property that holds the integration name (so we can e.g. filter these before actually calling them). */ -export type IntegrationFn IntegrationFnResult> = { id: string } & Fn; +export type IntegrationFn = (...rest: any[]) => IntegrationFnResult; export interface IntegrationFnResult { /** From 87446e955ceb5a487fcaaf4c4f10910065fc0bc8 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Fri, 15 Dec 2023 12:21:06 +0100 Subject: [PATCH 11/12] avoid unncessary function --- packages/core/src/index.ts | 1 - packages/core/src/integration.ts | 10 --- .../core/src/integrations/inboundfilters.ts | 8 +-- packages/core/test/lib/integration.test.ts | 70 +------------------ 4 files changed, 7 insertions(+), 82 deletions(-) diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 88ad4299e377..3c39921f495a 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -59,7 +59,6 @@ export { addIntegration, // eslint-disable-next-line deprecation/deprecation convertIntegrationFnToClass, - makeIntegrationFn, } from './integration'; export { FunctionToString, InboundFilters, LinkedErrors } from './integrations'; export { prepareEvent } from './utils/prepareEvent'; diff --git a/packages/core/src/integration.ts b/packages/core/src/integration.ts index e2d265509754..9316039e4917 100644 --- a/packages/core/src/integration.ts +++ b/packages/core/src/integration.ts @@ -5,7 +5,6 @@ import type { Integration, IntegrationClass, IntegrationFn, - IntegrationFnResult, Options, } from '@sentry/types'; import { arrayify, logger } from '@sentry/utils'; @@ -165,15 +164,6 @@ function findIndex(arr: T[], callback: (item: T) => boolean): number { return -1; } -/** - * Generate a full integration function from a simple function. - * This will ensure to add the given name both to the function definition (as id), - * as well as to the integration return value. - */ -export function makeIntegrationFn(fn: Fn): Fn { - return fn; -} - /** * Convert a new integration function to the legacy class syntax. * In v8, we can remove this and instead export the integration functions directly. diff --git a/packages/core/src/integrations/inboundfilters.ts b/packages/core/src/integrations/inboundfilters.ts index af9bffbf9c8f..df560175ad78 100644 --- a/packages/core/src/integrations/inboundfilters.ts +++ b/packages/core/src/integrations/inboundfilters.ts @@ -1,8 +1,8 @@ -import type { Client, Event, EventHint, Integration, StackFrame } from '@sentry/types'; +import type { Event, IntegrationFn, StackFrame } from '@sentry/types'; import { getEventDescription, logger, stringMatchesSomePattern } from '@sentry/utils'; import { DEBUG_BUILD } from '../debug-build'; -import { convertIntegrationFnToClass, makeIntegrationFn } from '../integration'; +import { convertIntegrationFnToClass } from '../integration'; // "Script error." is hard coded into browsers for errors that it can't read. // this is the result of a script being pulled in from an external domain and CORS. @@ -30,7 +30,7 @@ export interface InboundFiltersOptions { } const INTEGRATION_NAME = 'InboundFilters'; -const inboundFiltersIntegration = makeIntegrationFn((options: Partial) => { +const inboundFiltersIntegration: IntegrationFn = (options: Partial) => { return { name: INTEGRATION_NAME, processEvent(event, _hint, client) { @@ -39,7 +39,7 @@ const inboundFiltersIntegration = makeIntegrationFn((options: Partial { }); }); -describe('makeIntegrationFn', () => { - it('works with a minimal integration', () => { - const myIntegration = makeIntegrationFn(() => { - return { - name: 'testName' - }; - }); - - const integration = myIntegration(); - expect(integration).toEqual({ - name: 'testName', - }); - - // @ts-expect-error This SHOULD error - myIntegration({}); - }); - - it('works with integration options', () => { - const myIntegration = makeIntegrationFn((_options: { xxx: string }) => { - return { - name: 'testName' - }; - }); - - const integration = myIntegration({ xxx: 'aa' }); - expect(integration).toEqual({ - name: 'testName', - }); - - // @ts-expect-error This SHOULD error - myIntegration(); - // @ts-expect-error This SHOULD error - myIntegration({}); - }); - - it('works with integration hooks', () => { - const setup = jest.fn(); - const setupOnce = jest.fn(); - const processEvent = jest.fn(); - const preprocessEvent = jest.fn(); - const myIntegration = makeIntegrationFn(() => { - return { - name: 'testName', - setup, - setupOnce, - processEvent, - preprocessEvent, - }; - }); - - const integration = myIntegration(); - expect(integration).toEqual({ - name: 'testName', - setup, - setupOnce, - processEvent, - preprocessEvent, - }); - - // @ts-expect-error This SHOULD error - makeIntegrationFn('testName', () => ({ other: 'aha' })); - }); -}); describe('convertIntegrationFnToClass', () => { /* eslint-disable deprecation/deprecation */ it('works with a minimal integration', () => { - const integrationFn = makeIntegrationFn(() => ({ name: 'testName'})); + const integrationFn = () => ({ name: 'testName'}); const IntegrationClass = convertIntegrationFnToClass('testName', integrationFn); @@ -742,7 +678,7 @@ describe('convertIntegrationFnToClass', () => { const processEvent = jest.fn(); const preprocessEvent = jest.fn(); - const integrationFn = makeIntegrationFn( () => { + const integrationFn = () => { return { name: 'testName', setup, @@ -750,7 +686,7 @@ describe('convertIntegrationFnToClass', () => { processEvent, preprocessEvent, }; - }); + }; const IntegrationClass = convertIntegrationFnToClass('testName', integrationFn); From a2901ce513856e5b319ae1d652b987218bb9d445 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Fri, 15 Dec 2023 12:50:21 +0100 Subject: [PATCH 12/12] fix linting --- packages/core/src/integration.ts | 10 +--------- packages/core/src/integrations/inboundfilters.ts | 4 ++-- packages/core/test/lib/integration.test.ts | 4 +--- 3 files changed, 4 insertions(+), 14 deletions(-) diff --git a/packages/core/src/integration.ts b/packages/core/src/integration.ts index 9316039e4917..57adc3d33c36 100644 --- a/packages/core/src/integration.ts +++ b/packages/core/src/integration.ts @@ -1,12 +1,4 @@ -import type { - Client, - Event, - EventHint, - Integration, - IntegrationClass, - IntegrationFn, - Options, -} from '@sentry/types'; +import type { Client, Event, EventHint, Integration, IntegrationClass, IntegrationFn, Options } from '@sentry/types'; import { arrayify, logger } from '@sentry/utils'; import { DEBUG_BUILD } from './debug-build'; diff --git a/packages/core/src/integrations/inboundfilters.ts b/packages/core/src/integrations/inboundfilters.ts index df560175ad78..57c0387b25e4 100644 --- a/packages/core/src/integrations/inboundfilters.ts +++ b/packages/core/src/integrations/inboundfilters.ts @@ -1,4 +1,4 @@ -import type { Event, IntegrationFn, StackFrame } from '@sentry/types'; +import type { Event, IntegrationFn, StackFrame } from '@sentry/types'; import { getEventDescription, logger, stringMatchesSomePattern } from '@sentry/utils'; import { DEBUG_BUILD } from '../debug-build'; @@ -39,7 +39,7 @@ const inboundFiltersIntegration: IntegrationFn = (options: Partial { }); }); - - describe('convertIntegrationFnToClass', () => { /* eslint-disable deprecation/deprecation */ it('works with a minimal integration', () => { - const integrationFn = () => ({ name: 'testName'}); + const integrationFn = () => ({ name: 'testName' }); const IntegrationClass = convertIntegrationFnToClass('testName', integrationFn);