-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
feat(core): Add type & utility for function-based integrations #9818
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
d53b30c
ed7652c
a91624a
a12c405
67e2daa
57ed4d2
b1a536d
a053678
8771e99
c5c8d82
87446e9
a2901ce
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +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 } 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,23 @@ 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<InboundFiltersOptions>; | ||
|
||
public constructor(options: Partial<InboundFiltersOptions> = {}) { | ||
this.name = InboundFilters.id; | ||
this._options = options; | ||
} | ||
|
||
/** | ||
* @inheritDoc | ||
*/ | ||
public setupOnce(_addGlobalEventProcessor: unknown, _getCurrentHub: unknown): void { | ||
// noop | ||
} | ||
const INTEGRATION_NAME = 'InboundFilters'; | ||
const inboundFiltersIntegration: IntegrationFn = (options: Partial<InboundFiltersOptions>) => { | ||
return { | ||
name: INTEGRATION_NAME, | ||
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(INTEGRATION_NAME, inboundFiltersIntegration); | ||
|
||
/** JSDoc */ | ||
export function _mergeOptions( | ||
function _mergeOptions( | ||
internalOptions: Partial<InboundFiltersOptions> = {}, | ||
clientOptions: Partial<InboundFiltersOptions> = {}, | ||
): Partial<InboundFiltersOptions> { | ||
|
@@ -84,8 +66,7 @@ export function _mergeOptions( | |
}; | ||
} | ||
|
||
/** JSDoc */ | ||
export function _shouldDropEvent(event: Event, options: Partial<InboundFiltersOptions>): boolean { | ||
function _shouldDropEvent(event: Event, options: Partial<InboundFiltersOptions>): boolean { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why does this function start with underscore? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. they are "private" - we are not super consistent there, sometimes we prefix them and sometimes not. but specifically here I did not change that, just removed the |
||
if (options.ignoreInternal && _isSentryError(event)) { | ||
DEBUG_BUILD && | ||
logger.warn(`Event dropped due to being internal Sentry Error.\nEvent: ${getEventDescription(event)}`); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we are using deprecated functions, I think we should disable this rule instead of overriding it everywhere
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea here was to make it easier to spot that this should be removed in v8 🤔 no strong feelings, I just wanted to make it clear that this is a temporary thing that will be removed in v8 again - generally, we should avoid using deprecated stuff ourselves, so I think the rule is good.