From 5515dd849d5149d9c1550c3114821a8b867e2861 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Mon, 31 Jan 2022 15:28:25 -0500 Subject: [PATCH 1/9] ref(core): Reduce inboundfilters bundle size --- .../core/src/integrations/inboundfilters.ts | 300 +++++++++--------- 1 file changed, 143 insertions(+), 157 deletions(-) diff --git a/packages/core/src/integrations/inboundfilters.ts b/packages/core/src/integrations/inboundfilters.ts index f779c3be6266..5be2ffaf30b3 100644 --- a/packages/core/src/integrations/inboundfilters.ts +++ b/packages/core/src/integrations/inboundfilters.ts @@ -39,195 +39,181 @@ export class InboundFilters implements Integration { public setupOnce(): void { addGlobalEventProcessor((event: Event) => { const hub = getCurrentHub(); - if (!hub) { - return event; - } - const self = hub.getIntegration(InboundFilters); - if (self) { - const client = hub.getClient(); - const clientOptions = client ? client.getOptions() : {}; - // This checks prevents most of the occurrences of the bug linked below: - // https://github.com/getsentry/sentry-javascript/issues/2622 - // The bug is caused by multiple SDK instances, where one is minified and one is using non-mangled code. - // Unfortunatelly we cannot fix it reliably (thus reserved property in rollup's terser config), - // as we cannot force people using multiple instances in their apps to sync SDK versions. - const options = typeof self._mergeOptions === 'function' ? self._mergeOptions(clientOptions) : {}; - if (typeof self._shouldDropEvent !== 'function') { - return event; + if (hub) { + const self = hub.getIntegration(InboundFilters); + if (self) { + const client = hub.getClient(); + const clientOptions = client ? client.getOptions() : {}; + const options = _mergeOptions(self._options, clientOptions); + return _shouldDropEvent(event, options) ? null : event; } - return self._shouldDropEvent(event, options) ? null : event; } return event; }); } +} - /** JSDoc */ - private _shouldDropEvent(event: Event, options: Partial): boolean { - if (this._isSentryError(event, options)) { - if (isDebugBuild()) { - logger.warn(`Event dropped due to being internal Sentry Error.\nEvent: ${getEventDescription(event)}`); - } - return true; +/** JSDoc */ +export function _mergeOptions( + intOptions: Partial, + clientOptions: Partial = {}, +): Partial { + return { + allowUrls: [ + // eslint-disable-next-line deprecation/deprecation + ...(intOptions.whitelistUrls || []), + ...(intOptions.allowUrls || []), + // eslint-disable-next-line deprecation/deprecation + ...(clientOptions.whitelistUrls || []), + ...(clientOptions.allowUrls || []), + ], + denyUrls: [ + // eslint-disable-next-line deprecation/deprecation + ...(intOptions.blacklistUrls || []), + ...(intOptions.denyUrls || []), + // eslint-disable-next-line deprecation/deprecation + ...(clientOptions.blacklistUrls || []), + ...(clientOptions.denyUrls || []), + ], + ignoreErrors: [...(intOptions.ignoreErrors || []), ...(clientOptions.ignoreErrors || []), ...DEFAULT_IGNORE_ERRORS], + ignoreInternal: typeof intOptions.ignoreInternal !== 'undefined' ? intOptions.ignoreInternal : true, + }; +} + +/** JSDoc */ +export function _shouldDropEvent(event: Event, options: Partial): boolean { + if (_isSentryError(event, options.ignoreInternal)) { + if (isDebugBuild()) { + logger.warn(`Event dropped due to being internal Sentry Error.\nEvent: ${getEventDescription(event)}`); } - if (this._isIgnoredError(event, options)) { - if (isDebugBuild()) { - logger.warn( - `Event dropped due to being matched by \`ignoreErrors\` option.\nEvent: ${getEventDescription(event)}`, - ); - } - return true; + return true; + } + if (_isIgnoredError(event, options.ignoreErrors)) { + if (isDebugBuild()) { + logger.warn( + `Event dropped due to being matched by \`ignoreErrors\` option.\nEvent: ${getEventDescription(event)}`, + ); } - if (this._isDeniedUrl(event, options)) { - if (isDebugBuild()) { - logger.warn( - `Event dropped due to being matched by \`denyUrls\` option.\nEvent: ${getEventDescription( - event, - )}.\nUrl: ${this._getEventFilterUrl(event)}`, - ); - } - return true; + return true; + } + if (_isDeniedUrl(event, options.denyUrls)) { + if (isDebugBuild()) { + logger.warn( + `Event dropped due to being matched by \`denyUrls\` option.\nEvent: ${getEventDescription( + event, + )}.\nUrl: ${_getEventFilterUrl(event)}`, + ); } - if (!this._isAllowedUrl(event, options)) { - if (isDebugBuild()) { - logger.warn( - `Event dropped due to not being matched by \`allowUrls\` option.\nEvent: ${getEventDescription( - event, - )}.\nUrl: ${this._getEventFilterUrl(event)}`, - ); - } - return true; + return true; + } + if (!_isAllowedUrl(event, options.allowUrls)) { + if (isDebugBuild()) { + logger.warn( + `Event dropped due to not being matched by \`allowUrls\` option.\nEvent: ${getEventDescription( + event, + )}.\nUrl: ${_getEventFilterUrl(event)}`, + ); } - return false; + return true; } + return false; +} - /** JSDoc */ - private _isSentryError(event: Event, options: Partial): boolean { - if (!options.ignoreInternal) { - return false; - } +/** JSDoc */ +function _isIgnoredError(event: Event, ignoreErrors: Partial['ignoreErrors']): boolean { + if (!ignoreErrors || !ignoreErrors.length) { + return false; + } - try { - // @ts-ignore can't be a sentry error if undefined - // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - return event.exception.values[0].type === 'SentryError'; - } catch (e) { - // ignore - } + return _getPossibleEventMessages(event).some(message => + ignoreErrors.some(pattern => isMatchingPattern(message, pattern)), + ); +} +/** JSDoc */ +function _isDeniedUrl(event: Event, denyUrls: Partial['denyUrls']): boolean { + // TODO: Use Glob instead? + if (!denyUrls || !denyUrls.length) { return false; } + const url = _getEventFilterUrl(event); + return !url ? false : denyUrls.some(pattern => isMatchingPattern(url, pattern)); +} - /** JSDoc */ - private _isIgnoredError(event: Event, options: Partial): boolean { - if (!options.ignoreErrors || !options.ignoreErrors.length) { - return false; - } - - return this._getPossibleEventMessages(event).some(message => - // Not sure why TypeScript complains here... - (options.ignoreErrors as Array).some(pattern => isMatchingPattern(message, pattern)), - ); +/** JSDoc */ +function _isAllowedUrl(event: Event, allowUrls: Partial['allowUrls']): boolean { + // TODO: Use Glob instead? + if (!allowUrls || !allowUrls.length) { + return true; } + const url = _getEventFilterUrl(event); + return !url ? true : allowUrls.some(pattern => isMatchingPattern(url, pattern)); +} - /** JSDoc */ - private _isDeniedUrl(event: Event, options: Partial): boolean { - // TODO: Use Glob instead? - if (!options.denyUrls || !options.denyUrls.length) { - return false; +/** JSDoc */ +function _getPossibleEventMessages(event: Event): string[] { + if (event.message) { + return [event.message]; + } + if (event.exception) { + try { + const { type = '', value = '' } = (event.exception.values && event.exception.values[0]) || {}; + return [`${value}`, `${type}: ${value}`]; + } catch (oO) { + if (isDebugBuild()) { + logger.error(`Cannot extract message for event ${getEventDescription(event)}`); + } + return []; } - const url = this._getEventFilterUrl(event); - return !url ? false : options.denyUrls.some(pattern => isMatchingPattern(url, pattern)); } + return []; +} - /** JSDoc */ - private _isAllowedUrl(event: Event, options: Partial): boolean { - // TODO: Use Glob instead? - if (!options.allowUrls || !options.allowUrls.length) { - return true; +/** JSDoc */ +function _isSentryError(event: Event, ignoreInternal: Partial['ignoreInternal']): boolean { + if (ignoreInternal) { + try { + // @ts-ignore can't be a sentry error if undefined + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access + return event.exception.values[0].type === 'SentryError'; + } catch (e) { + // ignore } - const url = this._getEventFilterUrl(event); - return !url ? true : options.allowUrls.some(pattern => isMatchingPattern(url, pattern)); } + return false; +} - /** JSDoc */ - private _mergeOptions(clientOptions: Partial = {}): Partial { - return { - allowUrls: [ - // eslint-disable-next-line deprecation/deprecation - ...(this._options.whitelistUrls || []), - ...(this._options.allowUrls || []), - // eslint-disable-next-line deprecation/deprecation - ...(clientOptions.whitelistUrls || []), - ...(clientOptions.allowUrls || []), - ], - denyUrls: [ - // eslint-disable-next-line deprecation/deprecation - ...(this._options.blacklistUrls || []), - ...(this._options.denyUrls || []), - // eslint-disable-next-line deprecation/deprecation - ...(clientOptions.blacklistUrls || []), - ...(clientOptions.denyUrls || []), - ], - ignoreErrors: [ - ...(this._options.ignoreErrors || []), - ...(clientOptions.ignoreErrors || []), - ...DEFAULT_IGNORE_ERRORS, - ], - ignoreInternal: typeof this._options.ignoreInternal !== 'undefined' ? this._options.ignoreInternal : true, - }; - } +/** JSDoc */ +function _getLastValidUrl(frames: StackFrame[] = []): string | null { + for (let i = frames.length - 1; i >= 0; i--) { + const frame = frames[i]; - /** JSDoc */ - private _getPossibleEventMessages(event: Event): string[] { - if (event.message) { - return [event.message]; - } - if (event.exception) { - try { - const { type = '', value = '' } = (event.exception.values && event.exception.values[0]) || {}; - return [`${value}`, `${type}: ${value}`]; - } catch (oO) { - if (isDebugBuild()) { - logger.error(`Cannot extract message for event ${getEventDescription(event)}`); - } - return []; - } + if (frame && frame.filename !== '' && frame.filename !== '[native code]') { + return frame.filename || null; } - return []; } - /** JSDoc */ - private _getLastValidUrl(frames: StackFrame[] = []): string | null { - for (let i = frames.length - 1; i >= 0; i--) { - const frame = frames[i]; + return null; +} - if (frame && frame.filename !== '' && frame.filename !== '[native code]') { - return frame.filename || null; - } +/** JSDoc */ +function _getEventFilterUrl(event: Event): string | null { + try { + if (event.stacktrace) { + return _getLastValidUrl(event.stacktrace.frames); } - - return null; - } - - /** JSDoc */ - private _getEventFilterUrl(event: Event): string | null { + let frames; try { - if (event.stacktrace) { - return this._getLastValidUrl(event.stacktrace.frames); - } - let frames; - try { - // @ts-ignore we only care about frames if the whole thing here is defined - frames = event.exception.values[0].stacktrace.frames; - } catch (e) { - // ignore - } - return frames ? this._getLastValidUrl(frames) : null; - } catch (oO) { - if (isDebugBuild()) { - logger.error(`Cannot extract url for event ${getEventDescription(event)}`); - } - return null; + // @ts-ignore we only care about frames if the whole thing here is defined + frames = event.exception.values[0].stacktrace.frames; + } catch (e) { + // ignore } + return frames ? _getLastValidUrl(frames) : null; + } catch (oO) { + if (isDebugBuild()) { + logger.error(`Cannot extract url for event ${getEventDescription(event)}`); + } + return null; } } From a4e6425c397305bb1cb1f34f1fb2a4da1e6cae13 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Wed, 23 Feb 2022 15:33:43 -0500 Subject: [PATCH 2/9] add tests --- .../core/src/integrations/inboundfilters.ts | 9 +- .../lib/integrations/inboundfilters.test.ts | 500 ++++++------------ 2 files changed, 154 insertions(+), 355 deletions(-) diff --git a/packages/core/src/integrations/inboundfilters.ts b/packages/core/src/integrations/inboundfilters.ts index 5be2ffaf30b3..d764cf6ccc2a 100644 --- a/packages/core/src/integrations/inboundfilters.ts +++ b/packages/core/src/integrations/inboundfilters.ts @@ -1,5 +1,4 @@ -import { addGlobalEventProcessor, getCurrentHub } from '@sentry/hub'; -import { Event, Integration, StackFrame } from '@sentry/types'; +import { Event, EventProcessor, Hub, Integration, StackFrame } from '@sentry/types'; import { getEventDescription, isDebugBuild, isMatchingPattern, logger } from '@sentry/utils'; // "Script error." is hard coded into browsers for errors that it can't read. @@ -7,7 +6,7 @@ import { getEventDescription, isDebugBuild, isMatchingPattern, logger } from '@s const DEFAULT_IGNORE_ERRORS = [/^Script error\.?$/, /^Javascript error: Script error\.? on line 0$/]; /** JSDoc */ -interface InboundFiltersOptions { +export interface InboundFiltersOptions { allowUrls: Array; denyUrls: Array; ignoreErrors: Array; @@ -36,7 +35,7 @@ export class InboundFilters implements Integration { /** * @inheritDoc */ - public setupOnce(): void { + public setupOnce(addGlobalEventProcessor: (callback: EventProcessor) => void, getCurrentHub: () => Hub): void { addGlobalEventProcessor((event: Event) => { const hub = getCurrentHub(); if (hub) { @@ -55,7 +54,7 @@ export class InboundFilters implements Integration { /** JSDoc */ export function _mergeOptions( - intOptions: Partial, + intOptions: Partial = {}, clientOptions: Partial = {}, ): Partial { return { diff --git a/packages/core/test/lib/integrations/inboundfilters.test.ts b/packages/core/test/lib/integrations/inboundfilters.test.ts index f711f4725a0b..156d1ead7cc2 100644 --- a/packages/core/test/lib/integrations/inboundfilters.test.ts +++ b/packages/core/test/lib/integrations/inboundfilters.test.ts @@ -1,59 +1,39 @@ -import { InboundFilters } from '../../../src/integrations/inboundfilters'; - -let inboundFilters: any; - -describe('InboundFilters', () => { - beforeEach(() => { - inboundFilters = new InboundFilters(); - }); - - describe('shouldDropEvent', () => { - it('should drop when error is internal one', () => { - inboundFilters._isSentryError = () => true; - expect(inboundFilters._shouldDropEvent({}, inboundFilters._mergeOptions())).toBe(true); - }); - - it('should drop when error is ignored', () => { - inboundFilters._isIgnoredError = () => true; - expect(inboundFilters._shouldDropEvent({}, inboundFilters._mergeOptions())).toBe(true); - }); - - it('should drop when url is denied', () => { - inboundFilters._isDeniedUrl = () => true; - expect(inboundFilters._shouldDropEvent({}, inboundFilters._mergeOptions())).toBe(true); - }); - - it('should drop when url is not allowed', () => { - inboundFilters._isAllowedUrl = () => false; - expect(inboundFilters._shouldDropEvent({}, inboundFilters._mergeOptions())).toBe(true); - }); - - it('should drop when url is not denied, but also not allowed', () => { - inboundFilters._isDeniedUrl = () => false; - inboundFilters._isAllowedUrl = () => false; - expect(inboundFilters._shouldDropEvent({}, inboundFilters._mergeOptions())).toBe(true); - }); - - it('should drop when url is denied and allowed at the same time', () => { - inboundFilters._isDeniedUrl = () => true; - inboundFilters._isAllowedUrl = () => true; - expect(inboundFilters._shouldDropEvent({}, inboundFilters._mergeOptions())).toBe(true); - }); +import { InboundFilters, InboundFiltersOptions } from '../../../src/integrations/inboundfilters'; +import { EventProcessor } from '@sentry/types'; + +/** JSDoc */ +function createInboundFilters( + options: Partial = {}, + clientOptions: Partial = {}, +): EventProcessor { + const eventProcessors: EventProcessor[] = []; + const inboundFilters = new InboundFilters(options); + + function addGlobalEventProcessor(callback: EventProcessor): void { + eventProcessors.push(callback); + expect(eventProcessors).toHaveLength(1); + } + + function getCurrentHub(): any { + return { + getIntegration(_integration: any): any { + // pretend integration is enabled + return inboundFilters; + }, + getClient(): any { + return { + getOptions: () => clientOptions, + }; + }, + }; + } - it('should not drop when url is not denied, but allowed', () => { - inboundFilters._isDeniedUrl = () => false; - inboundFilters._isAllowedUrl = () => true; - expect(inboundFilters._shouldDropEvent({}, inboundFilters._mergeOptions())).toBe(false); - }); + inboundFilters.setupOnce(addGlobalEventProcessor, getCurrentHub); - it('should not drop when any of checks dont match', () => { - inboundFilters._isIgnoredError = () => false; - inboundFilters._isDeniedUrl = () => false; - inboundFilters._isAllowedUrl = () => true; - expect(inboundFilters._shouldDropEvent({}, inboundFilters._mergeOptions())).toBe(false); - }); - }); + return eventProcessors[0]; +} +describe('InboundFilters', () => { describe('isSentryError', () => { const messageEvent = { message: 'captureMessage', @@ -80,18 +60,17 @@ describe('InboundFilters', () => { }; it('should work as expected', () => { - expect(inboundFilters._isSentryError(messageEvent, inboundFilters._mergeOptions())).toBe(false); - expect(inboundFilters._isSentryError(exceptionEvent, inboundFilters._mergeOptions())).toBe(false); - expect(inboundFilters._isSentryError(sentryEvent, inboundFilters._mergeOptions())).toBe(true); + const eventProcessor = createInboundFilters(); + expect(eventProcessor(messageEvent)).toBe(messageEvent); + expect(eventProcessor(exceptionEvent)).toBe(exceptionEvent); + expect(eventProcessor(sentryEvent)).toBe(null); }); it('should be configurable', () => { - inboundFilters = new InboundFilters({ - ignoreInternal: false, - }); - expect(inboundFilters._isSentryError(messageEvent, inboundFilters._mergeOptions())).toBe(false); - expect(inboundFilters._isSentryError(exceptionEvent, inboundFilters._mergeOptions())).toBe(false); - expect(inboundFilters._isSentryError(sentryEvent, inboundFilters._mergeOptions())).toBe(false); + const eventProcessor = createInboundFilters({ ignoreInternal: false }); + expect(eventProcessor(messageEvent)).toBe(messageEvent); + expect(eventProcessor(exceptionEvent)).toBe(exceptionEvent); + expect(eventProcessor(sentryEvent)).toBe(sentryEvent); }); }); @@ -99,6 +78,9 @@ describe('InboundFilters', () => { const messageEvent = { message: 'captureMessage', }; + const captureMessageSomethingEvent = { + message: 'captureMessageSomething', + }; const exceptionEvent = { exception: { values: [ @@ -111,142 +93,88 @@ describe('InboundFilters', () => { }; it('string filter with partial match', () => { - expect( - inboundFilters._isIgnoredError( - messageEvent, - inboundFilters._mergeOptions({ - ignoreErrors: ['capture'], - }), - ), - ).toBe(true); + const eventProcessor = createInboundFilters({ + ignoreErrors: ['capture'], + }); + expect(eventProcessor(messageEvent)).toBe(null); }); it('string filter with exact match', () => { - expect( - inboundFilters._isIgnoredError( - messageEvent, - inboundFilters._mergeOptions({ - ignoreErrors: ['captureMessage'], - }), - ), - ).toBe(true); + const eventProcessor = createInboundFilters({ + ignoreErrors: ['captureMessage'], + }); + expect(eventProcessor(messageEvent)).toBe(null); }); it('regexp filter with partial match', () => { - expect( - inboundFilters._isIgnoredError( - messageEvent, - inboundFilters._mergeOptions({ - ignoreErrors: [/capture/], - }), - ), - ).toBe(true); + const eventProcessor = createInboundFilters({ + ignoreErrors: [/capture/], + }); + expect(eventProcessor(messageEvent)).toBe(null); }); it('regexp filter with exact match', () => { - expect( - inboundFilters._isIgnoredError( - messageEvent, - inboundFilters._mergeOptions({ - ignoreErrors: [/^captureMessage$/], - }), - ), - ).toBe(true); - expect( - inboundFilters._isIgnoredError( - { - message: 'captureMessageSomething', - }, - inboundFilters._mergeOptions({ - ignoreErrors: [/^captureMessage$/], - }), - ), - ).toBe(false); + const eventProcessor = createInboundFilters({ + ignoreErrors: [/^captureMessage$/], + }); + expect(eventProcessor(messageEvent)).toBe(null); + expect(eventProcessor(captureMessageSomethingEvent)).toBe(captureMessageSomethingEvent); }); it('uses message when both, message and exception are available', () => { - expect( - inboundFilters._isIgnoredError( - { - ...exceptionEvent, - ...messageEvent, - }, - inboundFilters._mergeOptions({ - ignoreErrors: [/captureMessage/], - }), - ), - ).toBe(true); + const eventProcessor = createInboundFilters({ + ignoreErrors: [/captureMessage/], + }); + const event = { + ...exceptionEvent, + ...messageEvent, + }; + expect(eventProcessor(event)).toBe(null); }); it('can use multiple filters', () => { - expect( - inboundFilters._isIgnoredError( - messageEvent, - inboundFilters._mergeOptions({ - ignoreErrors: ['captureMessage', /SyntaxError/], - }), - ), - ).toBe(true); - expect( - inboundFilters._isIgnoredError( - exceptionEvent, - inboundFilters._mergeOptions({ - ignoreErrors: ['captureMessage', /SyntaxError/], - }), - ), - ).toBe(true); + const eventProcessor = createInboundFilters({ + ignoreErrors: ['captureMessage', /SyntaxError/], + }); + expect(eventProcessor(messageEvent)).toBe(null); + expect(eventProcessor(exceptionEvent)).toBe(null); }); it('uses default filters', () => { - expect( - inboundFilters._isIgnoredError( - { - exception: { - values: [ - { - type: '[undefined]', - value: 'Script error.', - }, - ], + const eventProcessor = createInboundFilters(); + const defaultEvent = { + exception: { + values: [ + { + type: '[undefined]', + value: 'Script error.', }, - }, - inboundFilters._mergeOptions(), - ), - ).toBe(true); + ], + }, + }; + expect(eventProcessor(defaultEvent)).toBe(null); }); describe('on exception', () => { it('uses exceptions data when message is unavailable', () => { - expect( - inboundFilters._isIgnoredError( - exceptionEvent, - inboundFilters._mergeOptions({ - ignoreErrors: ['SyntaxError: unidentified ? at line 1337'], - }), - ), - ).toBe(true); + const eventProcessor = createInboundFilters({ + ignoreErrors: ['SyntaxError: unidentified ? at line 1337'], + }); + expect(eventProcessor(exceptionEvent)).toBe(null); }); it('can match on exception value', () => { - expect( - inboundFilters._isIgnoredError( - exceptionEvent, - inboundFilters._mergeOptions({ - ignoreErrors: [/unidentified \?/], - }), - ), - ).toBe(true); + const eventProcessor = createInboundFilters({ + ignoreErrors: [/unidentified \?/], + }); + expect(eventProcessor(exceptionEvent)).toBe(null); }); it('can match on exception type', () => { - expect( - inboundFilters._isIgnoredError( - exceptionEvent, - inboundFilters._mergeOptions({ - ignoreErrors: [/^SyntaxError/], - }), - ), - ).toBe(true); + const eventProcessor = createInboundFilters({ + ignoreErrors: [/^SyntaxError/], + }); + expect(eventProcessor(exceptionEvent)).toBe(null); }); }); }); @@ -283,178 +211,76 @@ describe('InboundFilters', () => { }; it('should filter captured message based on its stack trace using string filter', () => { - expect( - inboundFilters._isDeniedUrl( - messageEvent, - inboundFilters._mergeOptions({ - allowUrls: ['https://awesome-analytics.io'], - denyUrls: ['https://awesome-analytics.io'], - }), - ), - ).toBe(true); - expect( - inboundFilters._isAllowedUrl( - messageEvent, - inboundFilters._mergeOptions({ - allowUrls: ['https://awesome-analytics.io'], - denyUrls: ['https://awesome-analytics.io'], - }), - ), - ).toBe(true); + const eventProcessorBoth = createInboundFilters({ + allowUrls: ['https://awesome-analytics.io'], + denyUrls: ['https://awesome-analytics.io'], + }); + expect(eventProcessorBoth(messageEvent)).toBe(null); + const eventProcessorAllow = createInboundFilters({ + allowUrls: ['https://awesome-analytics.io'], + }); + expect(eventProcessorAllow(messageEvent)).toBe(messageEvent); + const eventProcessorDeny = createInboundFilters({ + denyUrls: ['https://awesome-analytics.io'], + }); + expect(eventProcessorDeny(messageEvent)).toBe(null); }); it('should filter captured message based on its stack trace using regexp filter', () => { - expect( - inboundFilters._isDeniedUrl( - messageEvent, - inboundFilters._mergeOptions({ - denyUrls: [/awesome-analytics\.io/], - }), - ), - ).toBe(true); - expect( - inboundFilters._isAllowedUrl( - messageEvent, - inboundFilters._mergeOptions({ - denyUrls: [/awesome-analytics\.io/], - }), - ), - ).toBe(true); + const eventProcessorDeny = createInboundFilters({ + denyUrls: [/awesome-analytics\.io/], + }); + expect(eventProcessorDeny(messageEvent)).toBe(null); }); it('should not filter captured messages with no stacktraces', () => { - expect( - inboundFilters._isDeniedUrl( - { - message: 'any', - }, - inboundFilters._mergeOptions({ - allowUrls: ['https://awesome-analytics.io'], - denyUrls: ['https://awesome-analytics.io'], - }), - ), - ).toBe(false); - expect( - inboundFilters._isAllowedUrl( - { - message: 'any', - }, - inboundFilters._mergeOptions({ - allowUrls: ['https://awesome-analytics.io'], - denyUrls: ['https://awesome-analytics.io'], - }), - ), - ).toBe(true); + const simpleMessage = { + message: 'any', + }; + const eventProcessor = createInboundFilters({ + denyUrls: ['https://awesome-analytics.io'], + }); + expect(eventProcessor(simpleMessage)).toBe(simpleMessage); }); it('should filter captured exception based on its stack trace using string filter', () => { - expect( - inboundFilters._isDeniedUrl( - exceptionEvent, - inboundFilters._mergeOptions({ - allowUrls: ['https://awesome-analytics.io'], - denyUrls: ['https://awesome-analytics.io'], - }), - ), - ).toBe(true); - expect( - inboundFilters._isAllowedUrl( - exceptionEvent, - inboundFilters._mergeOptions({ - allowUrls: ['https://awesome-analytics.io'], - denyUrls: ['https://awesome-analytics.io'], - }), - ), - ).toBe(true); + const eventProcessor = createInboundFilters({ + denyUrls: ['https://awesome-analytics.io'], + }); + expect(eventProcessor(exceptionEvent)).toBe(null); }); it('should filter captured exceptions based on its stack trace using regexp filter', () => { - expect( - inboundFilters._isDeniedUrl( - exceptionEvent, - inboundFilters._mergeOptions({ - allowUrls: [/awesome-analytics\.io/], - denyUrls: [/awesome-analytics\.io/], - }), - ), - ).toBe(true); - expect( - inboundFilters._isAllowedUrl( - exceptionEvent, - inboundFilters._mergeOptions({ - allowUrls: [/awesome-analytics\.io/], - denyUrls: [/awesome-analytics\.io/], - }), - ), - ).toBe(true); + const eventProcessor = createInboundFilters({ + denyUrls: [/awesome-analytics\.io/], + }); + expect(eventProcessor(exceptionEvent)).toBe(null); }); it('should not filter events that doesnt pass the test', () => { - expect( - inboundFilters._isDeniedUrl( - exceptionEvent, - inboundFilters._mergeOptions({ - allowUrls: ['some-other-domain.com'], - denyUrls: ['some-other-domain.com'], - }), - ), - ).toBe(false); - expect( - inboundFilters._isAllowedUrl( - exceptionEvent, - inboundFilters._mergeOptions({ - allowUrls: ['some-other-domain.com'], - denyUrls: ['some-other-domain.com'], - }), - ), - ).toBe(false); + const eventProcessor = createInboundFilters({ + denyUrls: ['some-other-domain.com'], + }); + expect(eventProcessor(exceptionEvent)).toBe(exceptionEvent); }); it('should be able to use multiple filters', () => { - expect( - inboundFilters._isDeniedUrl( - exceptionEvent, - inboundFilters._mergeOptions({ - allowUrls: ['some-other-domain.com', /awesome-analytics\.io/], - denyUrls: ['some-other-domain.com', /awesome-analytics\.io/], - }), - ), - ).toBe(true); - expect( - inboundFilters._isAllowedUrl( - exceptionEvent, - inboundFilters._mergeOptions({ - allowUrls: ['some-other-domain.com', /awesome-analytics\.io/], - denyUrls: ['some-other-domain.com', /awesome-analytics\.io/], - }), - ), - ).toBe(true); + const eventProcessor = createInboundFilters({ + denyUrls: ['some-other-domain.com', /awesome-analytics\.io/], + }); + expect(eventProcessor(exceptionEvent)).toBe(null); }); - it('should not fail with malformed event event and default to false for isdeniedUrl and true for isallowedUrl', () => { + it('should not fail with malformed event event', () => { const malformedEvent = { stacktrace: { frames: undefined, }, }; - expect( - inboundFilters._isDeniedUrl( - malformedEvent, - inboundFilters._mergeOptions({ - allowUrls: ['https://awesome-analytics.io'], - denyUrls: ['https://awesome-analytics.io'], - }), - ), - ).toBe(false); - expect( - inboundFilters._isAllowedUrl( - malformedEvent, - inboundFilters._mergeOptions({ - allowUrls: ['https://awesome-analytics.io'], - denyUrls: ['https://awesome-analytics.io'], - }), - ), - ).toBe(true); + const eventProcessor = createInboundFilters({ + denyUrls: ['https://awesome-analytics.io'], + }); + expect(eventProcessor(malformedEvent)).toBe(malformedEvent); }); it('should search for script names when there is an anonymous callback at the last frame', () => { @@ -469,23 +295,10 @@ describe('InboundFilters', () => { }, }; - expect( - inboundFilters._isAllowedUrl( - messageEvent, - inboundFilters._mergeOptions({ - allowUrls: ['https://awesome-analytics.io/some/file.js'], - }), - ), - ).toBe(true); - - expect( - inboundFilters._isDeniedUrl( - messageEvent, - inboundFilters._mergeOptions({ - denyUrls: ['https://awesome-analytics.io/some/file.js'], - }), - ), - ).toBe(true); + const eventProcessor = createInboundFilters({ + denyUrls: ['https://awesome-analytics.io/some/file.js'], + }); + expect(eventProcessor(messageEvent)).toBe(null); }); it('should search for script names when the last frame is from native code', () => { @@ -500,23 +313,10 @@ describe('InboundFilters', () => { }, }; - expect( - inboundFilters._isAllowedUrl( - messageEvent, - inboundFilters._mergeOptions({ - allowUrls: ['https://awesome-analytics.io/some/file.js'], - }), - ), - ).toBe(true); - - expect( - inboundFilters._isDeniedUrl( - messageEvent, - inboundFilters._mergeOptions({ - denyUrls: ['https://awesome-analytics.io/some/file.js'], - }), - ), - ).toBe(true); + const eventProcessor = createInboundFilters({ + denyUrls: ['https://awesome-analytics.io/some/file.js'], + }); + expect(eventProcessor(messageEvent)).toBe(null); }); }); }); From 48134b2e66188c8123c1705c75cfa3769ce1f5c0 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Wed, 23 Feb 2022 15:40:06 -0500 Subject: [PATCH 3/9] move into fixtures --- .../lib/integrations/inboundfilters.test.ts | 323 +++++++++--------- 1 file changed, 157 insertions(+), 166 deletions(-) diff --git a/packages/core/test/lib/integrations/inboundfilters.test.ts b/packages/core/test/lib/integrations/inboundfilters.test.ts index 156d1ead7cc2..dfa326c84a10 100644 --- a/packages/core/test/lib/integrations/inboundfilters.test.ts +++ b/packages/core/test/lib/integrations/inboundfilters.test.ts @@ -2,7 +2,7 @@ import { InboundFilters, InboundFiltersOptions } from '../../../src/integrations import { EventProcessor } from '@sentry/types'; /** JSDoc */ -function createInboundFilters( +function createInboundFiltersEventProcessor( options: Partial = {}, clientOptions: Partial = {}, ): EventProcessor { @@ -35,288 +35,279 @@ function createInboundFilters( describe('InboundFilters', () => { describe('isSentryError', () => { - const messageEvent = { - message: 'captureMessage', - }; - const exceptionEvent = { - exception: { - values: [ - { - type: 'SyntaxError', - value: 'unidentified ? at line 1337', - }, - ], - }, - }; - const sentryEvent = { - exception: { - values: [ - { - type: 'SentryError', - value: 'something something server connection', - }, - ], - }, - }; - it('should work as expected', () => { - const eventProcessor = createInboundFilters(); - expect(eventProcessor(messageEvent)).toBe(messageEvent); - expect(eventProcessor(exceptionEvent)).toBe(exceptionEvent); - expect(eventProcessor(sentryEvent)).toBe(null); + const eventProcessor = createInboundFiltersEventProcessor(); + expect(eventProcessor(MESSAGE_EVENT)).toBe(MESSAGE_EVENT); + expect(eventProcessor(EXCEPTION_EVENT)).toBe(EXCEPTION_EVENT); + expect(eventProcessor(SENTRY_EVENT)).toBe(null); }); it('should be configurable', () => { - const eventProcessor = createInboundFilters({ ignoreInternal: false }); - expect(eventProcessor(messageEvent)).toBe(messageEvent); - expect(eventProcessor(exceptionEvent)).toBe(exceptionEvent); - expect(eventProcessor(sentryEvent)).toBe(sentryEvent); + const eventProcessor = createInboundFiltersEventProcessor({ ignoreInternal: false }); + expect(eventProcessor(MESSAGE_EVENT)).toBe(MESSAGE_EVENT); + expect(eventProcessor(EXCEPTION_EVENT)).toBe(EXCEPTION_EVENT); + expect(eventProcessor(SENTRY_EVENT)).toBe(SENTRY_EVENT); }); }); describe('ignoreErrors', () => { - const messageEvent = { - message: 'captureMessage', - }; - const captureMessageSomethingEvent = { - message: 'captureMessageSomething', - }; - const exceptionEvent = { - exception: { - values: [ - { - type: 'SyntaxError', - value: 'unidentified ? at line 1337', - }, - ], - }, - }; - it('string filter with partial match', () => { - const eventProcessor = createInboundFilters({ + const eventProcessor = createInboundFiltersEventProcessor({ ignoreErrors: ['capture'], }); - expect(eventProcessor(messageEvent)).toBe(null); + expect(eventProcessor(MESSAGE_EVENT)).toBe(null); }); it('string filter with exact match', () => { - const eventProcessor = createInboundFilters({ + const eventProcessor = createInboundFiltersEventProcessor({ ignoreErrors: ['captureMessage'], }); - expect(eventProcessor(messageEvent)).toBe(null); + expect(eventProcessor(MESSAGE_EVENT)).toBe(null); }); it('regexp filter with partial match', () => { - const eventProcessor = createInboundFilters({ + const eventProcessor = createInboundFiltersEventProcessor({ ignoreErrors: [/capture/], }); - expect(eventProcessor(messageEvent)).toBe(null); + expect(eventProcessor(MESSAGE_EVENT)).toBe(null); }); it('regexp filter with exact match', () => { - const eventProcessor = createInboundFilters({ + const eventProcessor = createInboundFiltersEventProcessor({ ignoreErrors: [/^captureMessage$/], }); - expect(eventProcessor(messageEvent)).toBe(null); - expect(eventProcessor(captureMessageSomethingEvent)).toBe(captureMessageSomethingEvent); + expect(eventProcessor(MESSAGE_EVENT)).toBe(null); + expect(eventProcessor(MESSAGE_EVENT_2)).toBe(MESSAGE_EVENT_2); }); it('uses message when both, message and exception are available', () => { - const eventProcessor = createInboundFilters({ + const eventProcessor = createInboundFiltersEventProcessor({ ignoreErrors: [/captureMessage/], }); const event = { - ...exceptionEvent, - ...messageEvent, + ...EXCEPTION_EVENT, + ...MESSAGE_EVENT, }; expect(eventProcessor(event)).toBe(null); }); it('can use multiple filters', () => { - const eventProcessor = createInboundFilters({ + const eventProcessor = createInboundFiltersEventProcessor({ ignoreErrors: ['captureMessage', /SyntaxError/], }); - expect(eventProcessor(messageEvent)).toBe(null); - expect(eventProcessor(exceptionEvent)).toBe(null); + expect(eventProcessor(MESSAGE_EVENT)).toBe(null); + expect(eventProcessor(EXCEPTION_EVENT)).toBe(null); }); it('uses default filters', () => { - const eventProcessor = createInboundFilters(); - const defaultEvent = { - exception: { - values: [ - { - type: '[undefined]', - value: 'Script error.', - }, - ], - }, - }; - expect(eventProcessor(defaultEvent)).toBe(null); + const eventProcessor = createInboundFiltersEventProcessor(); + expect(eventProcessor(DEFAULT_EVENT)).toBe(null); }); describe('on exception', () => { it('uses exceptions data when message is unavailable', () => { - const eventProcessor = createInboundFilters({ + const eventProcessor = createInboundFiltersEventProcessor({ ignoreErrors: ['SyntaxError: unidentified ? at line 1337'], }); - expect(eventProcessor(exceptionEvent)).toBe(null); + expect(eventProcessor(EXCEPTION_EVENT)).toBe(null); }); it('can match on exception value', () => { - const eventProcessor = createInboundFilters({ + const eventProcessor = createInboundFiltersEventProcessor({ ignoreErrors: [/unidentified \?/], }); - expect(eventProcessor(exceptionEvent)).toBe(null); + expect(eventProcessor(EXCEPTION_EVENT)).toBe(null); }); it('can match on exception type', () => { - const eventProcessor = createInboundFilters({ + const eventProcessor = createInboundFiltersEventProcessor({ ignoreErrors: [/^SyntaxError/], }); - expect(eventProcessor(exceptionEvent)).toBe(null); + expect(eventProcessor(EXCEPTION_EVENT)).toBe(null); }); }); }); describe('denyUrls/allowUrls', () => { - const messageEvent = { - message: 'wat', - stacktrace: { - // Frames are always in the reverse order, as this is how Sentry expect them to come. - // Frame that crashed is the last one, the one from awesome-analytics - frames: [ - { filename: 'https://our-side.com/js/bundle.js' }, - { filename: 'https://our-side.com/js/bundle.js' }, - { filename: 'https://awesome-analytics.io/some/file.js' }, - ], - }, - }; - const exceptionEvent = { - exception: { - values: [ - { - stacktrace: { - // Frames are always in the reverse order, as this is how Sentry expect them to come. - // Frame that crashed is the last one, the one from awesome-analytics - frames: [ - { filename: 'https://our-side.com/js/bundle.js' }, - { filename: 'https://our-side.com/js/bundle.js' }, - { filename: 'https://awesome-analytics.io/some/file.js' }, - ], - }, - }, - ], - }, - }; - it('should filter captured message based on its stack trace using string filter', () => { - const eventProcessorBoth = createInboundFilters({ + const eventProcessorBoth = createInboundFiltersEventProcessor({ allowUrls: ['https://awesome-analytics.io'], denyUrls: ['https://awesome-analytics.io'], }); - expect(eventProcessorBoth(messageEvent)).toBe(null); - const eventProcessorAllow = createInboundFilters({ + expect(eventProcessorBoth(MESSAGE_EVENT_WITH_STACKTRACE)).toBe(null); + const eventProcessorAllow = createInboundFiltersEventProcessor({ allowUrls: ['https://awesome-analytics.io'], }); - expect(eventProcessorAllow(messageEvent)).toBe(messageEvent); - const eventProcessorDeny = createInboundFilters({ + expect(eventProcessorAllow(MESSAGE_EVENT_WITH_STACKTRACE)).toBe(MESSAGE_EVENT_WITH_STACKTRACE); + const eventProcessorDeny = createInboundFiltersEventProcessor({ denyUrls: ['https://awesome-analytics.io'], }); - expect(eventProcessorDeny(messageEvent)).toBe(null); + expect(eventProcessorDeny(MESSAGE_EVENT_WITH_STACKTRACE)).toBe(null); }); it('should filter captured message based on its stack trace using regexp filter', () => { - const eventProcessorDeny = createInboundFilters({ + const eventProcessorDeny = createInboundFiltersEventProcessor({ denyUrls: [/awesome-analytics\.io/], }); - expect(eventProcessorDeny(messageEvent)).toBe(null); + expect(eventProcessorDeny(MESSAGE_EVENT_WITH_STACKTRACE)).toBe(null); }); it('should not filter captured messages with no stacktraces', () => { - const simpleMessage = { - message: 'any', - }; - const eventProcessor = createInboundFilters({ + const eventProcessor = createInboundFiltersEventProcessor({ denyUrls: ['https://awesome-analytics.io'], }); - expect(eventProcessor(simpleMessage)).toBe(simpleMessage); + expect(eventProcessor(MESSAGE_EVENT)).toBe(MESSAGE_EVENT); }); it('should filter captured exception based on its stack trace using string filter', () => { - const eventProcessor = createInboundFilters({ + const eventProcessor = createInboundFiltersEventProcessor({ denyUrls: ['https://awesome-analytics.io'], }); - expect(eventProcessor(exceptionEvent)).toBe(null); + expect(eventProcessor(EXCEPTION_EVENT_WITH_FRAMES)).toBe(null); }); it('should filter captured exceptions based on its stack trace using regexp filter', () => { - const eventProcessor = createInboundFilters({ + const eventProcessor = createInboundFiltersEventProcessor({ denyUrls: [/awesome-analytics\.io/], }); - expect(eventProcessor(exceptionEvent)).toBe(null); + expect(eventProcessor(EXCEPTION_EVENT_WITH_FRAMES)).toBe(null); }); it('should not filter events that doesnt pass the test', () => { - const eventProcessor = createInboundFilters({ + const eventProcessor = createInboundFiltersEventProcessor({ denyUrls: ['some-other-domain.com'], }); - expect(eventProcessor(exceptionEvent)).toBe(exceptionEvent); + expect(eventProcessor(EXCEPTION_EVENT_WITH_FRAMES)).toBe(EXCEPTION_EVENT_WITH_FRAMES); }); it('should be able to use multiple filters', () => { - const eventProcessor = createInboundFilters({ + const eventProcessor = createInboundFiltersEventProcessor({ denyUrls: ['some-other-domain.com', /awesome-analytics\.io/], }); - expect(eventProcessor(exceptionEvent)).toBe(null); + expect(eventProcessor(EXCEPTION_EVENT_WITH_FRAMES)).toBe(null); }); it('should not fail with malformed event event', () => { - const malformedEvent = { - stacktrace: { - frames: undefined, - }, - }; - const eventProcessor = createInboundFilters({ + const eventProcessor = createInboundFiltersEventProcessor({ denyUrls: ['https://awesome-analytics.io'], }); - expect(eventProcessor(malformedEvent)).toBe(malformedEvent); + expect(eventProcessor(MALFORMED_EVENT)).toBe(MALFORMED_EVENT); }); it('should search for script names when there is an anonymous callback at the last frame', () => { - const messageEvent = { - message: 'any', - stacktrace: { - frames: [ - { filename: 'https://our-side.com/js/bundle.js' }, - { filename: 'https://awesome-analytics.io/some/file.js' }, - { filename: '' }, - ], - }, - }; - - const eventProcessor = createInboundFilters({ + const eventProcessor = createInboundFiltersEventProcessor({ denyUrls: ['https://awesome-analytics.io/some/file.js'], }); - expect(eventProcessor(messageEvent)).toBe(null); + expect(eventProcessor(MESSAGE_EVENT_WITH_ANON_FRAME)).toBe(null); }); it('should search for script names when the last frame is from native code', () => { - const messageEvent = { - message: 'any', + const eventProcessor = createInboundFiltersEventProcessor({ + denyUrls: ['https://awesome-analytics.io/some/file.js'], + }); + expect(eventProcessor(MESSAGE_EVENT_WITH_NATIVE_FRAME)).toBe(null); + }); + }); +}); + +// Fixtures + +const MESSAGE_EVENT = { + message: 'captureMessage', +}; + +const MESSAGE_EVENT_2 = { + message: 'captureMessageSomething', +}; + +const MESSAGE_EVENT_WITH_STACKTRACE = { + message: 'wat', + stacktrace: { + // Frames are always in the reverse order, as this is how Sentry expect them to come. + // Frame that crashed is the last one, the one from awesome-analytics + frames: [ + { filename: 'https://our-side.com/js/bundle.js' }, + { filename: 'https://our-side.com/js/bundle.js' }, + { filename: 'https://awesome-analytics.io/some/file.js' }, + ], + }, +}; + +const MESSAGE_EVENT_WITH_ANON_FRAME = { + message: 'any', + stacktrace: { + frames: [ + { filename: 'https://our-side.com/js/bundle.js' }, + { filename: 'https://awesome-analytics.io/some/file.js' }, + { filename: '' }, + ], + }, +}; + +const MESSAGE_EVENT_WITH_NATIVE_FRAME = { + message: 'any', + stacktrace: { + frames: [ + { filename: 'https://our-side.com/js/bundle.js' }, + { filename: 'https://awesome-analytics.io/some/file.js' }, + { filename: '[native code]' }, + ], + }, +}; + +const EXCEPTION_EVENT = { + exception: { + values: [ + { + type: 'SyntaxError', + value: 'unidentified ? at line 1337', + }, + ], + }, +}; + +const EXCEPTION_EVENT_WITH_FRAMES = { + exception: { + values: [ + { stacktrace: { + // Frames are always in the reverse order, as this is how Sentry expect them to come. + // Frame that crashed is the last one, the one from awesome-analytics frames: [ + { filename: 'https://our-side.com/js/bundle.js' }, { filename: 'https://our-side.com/js/bundle.js' }, { filename: 'https://awesome-analytics.io/some/file.js' }, - { filename: '[native code]' }, ], }, - }; - - const eventProcessor = createInboundFilters({ - denyUrls: ['https://awesome-analytics.io/some/file.js'], - }); - expect(eventProcessor(messageEvent)).toBe(null); - }); - }); -}); + }, + ], + }, +}; + +const SENTRY_EVENT = { + exception: { + values: [ + { + type: 'SentryError', + value: 'something something server connection', + }, + ], + }, +}; + +const DEFAULT_EVENT = { + exception: { + values: [ + { + type: '[undefined]', + value: 'Script error.', + }, + ], + }, +}; + +const MALFORMED_EVENT = { + stacktrace: { + frames: undefined, + }, +}; From 174ffe2f16aed461ab2096c14e64903e5c7d3847 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Wed, 23 Feb 2022 15:59:38 -0500 Subject: [PATCH 4/9] yarn fix --- packages/core/test/lib/integrations/inboundfilters.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/core/test/lib/integrations/inboundfilters.test.ts b/packages/core/test/lib/integrations/inboundfilters.test.ts index dfa326c84a10..a90c5dbea80b 100644 --- a/packages/core/test/lib/integrations/inboundfilters.test.ts +++ b/packages/core/test/lib/integrations/inboundfilters.test.ts @@ -1,6 +1,7 @@ -import { InboundFilters, InboundFiltersOptions } from '../../../src/integrations/inboundfilters'; import { EventProcessor } from '@sentry/types'; +import { InboundFilters, InboundFiltersOptions } from '../../../src/integrations/inboundfilters'; + /** JSDoc */ function createInboundFiltersEventProcessor( options: Partial = {}, From c1e171da28128adc6084eb26727bf578f1bdfb19 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Thu, 24 Feb 2022 12:32:41 -0500 Subject: [PATCH 5/9] address PR review --- .../core/src/integrations/inboundfilters.ts | 20 +++--- .../lib/integrations/inboundfilters.test.ts | 65 ++++++++++++------- 2 files changed, 52 insertions(+), 33 deletions(-) diff --git a/packages/core/src/integrations/inboundfilters.ts b/packages/core/src/integrations/inboundfilters.ts index d764cf6ccc2a..c906efa2d880 100644 --- a/packages/core/src/integrations/inboundfilters.ts +++ b/packages/core/src/integrations/inboundfilters.ts @@ -35,7 +35,7 @@ export class InboundFilters implements Integration { /** * @inheritDoc */ - public setupOnce(addGlobalEventProcessor: (callback: EventProcessor) => void, getCurrentHub: () => Hub): void { + public setupOnce(addGlobalEventProcessor: (processor: EventProcessor) => void, getCurrentHub: () => Hub): void { addGlobalEventProcessor((event: Event) => { const hub = getCurrentHub(); if (hub) { @@ -54,28 +54,32 @@ export class InboundFilters implements Integration { /** JSDoc */ export function _mergeOptions( - intOptions: Partial = {}, + internalOptions: Partial = {}, clientOptions: Partial = {}, ): Partial { return { allowUrls: [ // eslint-disable-next-line deprecation/deprecation - ...(intOptions.whitelistUrls || []), - ...(intOptions.allowUrls || []), + ...(internalOptions.whitelistUrls || []), + ...(internalOptions.allowUrls || []), // eslint-disable-next-line deprecation/deprecation ...(clientOptions.whitelistUrls || []), ...(clientOptions.allowUrls || []), ], denyUrls: [ // eslint-disable-next-line deprecation/deprecation - ...(intOptions.blacklistUrls || []), - ...(intOptions.denyUrls || []), + ...(internalOptions.blacklistUrls || []), + ...(internalOptions.denyUrls || []), // eslint-disable-next-line deprecation/deprecation ...(clientOptions.blacklistUrls || []), ...(clientOptions.denyUrls || []), ], - ignoreErrors: [...(intOptions.ignoreErrors || []), ...(clientOptions.ignoreErrors || []), ...DEFAULT_IGNORE_ERRORS], - ignoreInternal: typeof intOptions.ignoreInternal !== 'undefined' ? intOptions.ignoreInternal : true, + ignoreErrors: [ + ...(internalOptions.ignoreErrors || []), + ...(clientOptions.ignoreErrors || []), + ...DEFAULT_IGNORE_ERRORS, + ], + ignoreInternal: internalOptions.ignoreInternal !== undefined ? internalOptions.ignoreInternal : true, }; } diff --git a/packages/core/test/lib/integrations/inboundfilters.test.ts b/packages/core/test/lib/integrations/inboundfilters.test.ts index a90c5dbea80b..b7acfb41d7c2 100644 --- a/packages/core/test/lib/integrations/inboundfilters.test.ts +++ b/packages/core/test/lib/integrations/inboundfilters.test.ts @@ -2,16 +2,33 @@ import { EventProcessor } from '@sentry/types'; import { InboundFilters, InboundFiltersOptions } from '../../../src/integrations/inboundfilters'; -/** JSDoc */ +/** + * Creates an instance of the InboundFilters integration and returns + * the event processor that the InboundFilters integration creates. + * + * To test the InboundFilters integration, call this function and assert on + * how the event processor handles an event. For example, if you set up the + * InboundFilters to filter out an SOME_EXCEPTION_EVENT. + * + * ``` + * // some options that cause SOME_EXCEPTION_EVENT to be filtered + * const eventProcessor = createInboundFiltersEventProcessor(options); + * + * expect(eventProcessor(SOME_EXCEPTION_EVENT)).toBe(null); + * ``` + * + * @param options options passed into the InboundFilters integration + * @param clientOptions options passed into the mock Sentry client + */ function createInboundFiltersEventProcessor( options: Partial = {}, clientOptions: Partial = {}, ): EventProcessor { const eventProcessors: EventProcessor[] = []; - const inboundFilters = new InboundFilters(options); + const inboundFiltersInstance = new InboundFilters(options); - function addGlobalEventProcessor(callback: EventProcessor): void { - eventProcessors.push(callback); + function addGlobalEventProcessor(processor: EventProcessor): void { + eventProcessors.push(processor); expect(eventProcessors).toHaveLength(1); } @@ -19,7 +36,7 @@ function createInboundFiltersEventProcessor( return { getIntegration(_integration: any): any { // pretend integration is enabled - return inboundFilters; + return inboundFiltersInstance; }, getClient(): any { return { @@ -29,13 +46,12 @@ function createInboundFiltersEventProcessor( }; } - inboundFilters.setupOnce(addGlobalEventProcessor, getCurrentHub); - + inboundFiltersInstance.setupOnce(addGlobalEventProcessor, getCurrentHub); return eventProcessors[0]; } describe('InboundFilters', () => { - describe('isSentryError', () => { + describe('_isSentryError', () => { it('should work as expected', () => { const eventProcessor = createInboundFiltersEventProcessor(); expect(eventProcessor(MESSAGE_EVENT)).toBe(MESSAGE_EVENT); @@ -81,7 +97,7 @@ describe('InboundFilters', () => { expect(eventProcessor(MESSAGE_EVENT_2)).toBe(MESSAGE_EVENT_2); }); - it('uses message when both, message and exception are available', () => { + it('prefers message when both message and exception are available', () => { const eventProcessor = createInboundFiltersEventProcessor({ ignoreErrors: [/captureMessage/], }); @@ -102,11 +118,11 @@ describe('InboundFilters', () => { it('uses default filters', () => { const eventProcessor = createInboundFiltersEventProcessor(); - expect(eventProcessor(DEFAULT_EVENT)).toBe(null); + expect(eventProcessor(SCRIPT_ERROR_EVENT)).toBe(null); }); describe('on exception', () => { - it('uses exceptions data when message is unavailable', () => { + it('uses exception data when message is unavailable', () => { const eventProcessor = createInboundFiltersEventProcessor({ ignoreErrors: ['SyntaxError: unidentified ? at line 1337'], }); @@ -131,19 +147,18 @@ describe('InboundFilters', () => { describe('denyUrls/allowUrls', () => { it('should filter captured message based on its stack trace using string filter', () => { - const eventProcessorBoth = createInboundFiltersEventProcessor({ - allowUrls: ['https://awesome-analytics.io'], + const eventProcessorDeny = createInboundFiltersEventProcessor({ denyUrls: ['https://awesome-analytics.io'], }); - expect(eventProcessorBoth(MESSAGE_EVENT_WITH_STACKTRACE)).toBe(null); - const eventProcessorAllow = createInboundFiltersEventProcessor({ + expect(eventProcessorDeny(MESSAGE_EVENT_WITH_STACKTRACE)).toBe(null); + }); + + it('should allow denyUrls to take precedence', () => { + const eventProcessorBoth = createInboundFiltersEventProcessor({ allowUrls: ['https://awesome-analytics.io'], - }); - expect(eventProcessorAllow(MESSAGE_EVENT_WITH_STACKTRACE)).toBe(MESSAGE_EVENT_WITH_STACKTRACE); - const eventProcessorDeny = createInboundFiltersEventProcessor({ denyUrls: ['https://awesome-analytics.io'], }); - expect(eventProcessorDeny(MESSAGE_EVENT_WITH_STACKTRACE)).toBe(null); + expect(eventProcessorBoth(MESSAGE_EVENT_WITH_STACKTRACE)).toBe(null); }); it('should filter captured message based on its stack trace using regexp filter', () => { @@ -167,7 +182,7 @@ describe('InboundFilters', () => { expect(eventProcessor(EXCEPTION_EVENT_WITH_FRAMES)).toBe(null); }); - it('should filter captured exceptions based on its stack trace using regexp filter', () => { + it('should filter captured exception based on its stack trace using regexp filter', () => { const eventProcessor = createInboundFiltersEventProcessor({ denyUrls: [/awesome-analytics\.io/], }); @@ -199,14 +214,14 @@ describe('InboundFilters', () => { const eventProcessor = createInboundFiltersEventProcessor({ denyUrls: ['https://awesome-analytics.io/some/file.js'], }); - expect(eventProcessor(MESSAGE_EVENT_WITH_ANON_FRAME)).toBe(null); + expect(eventProcessor(MESSAGE_EVENT_WITH_ANON_LAST_FRAME)).toBe(null); }); it('should search for script names when the last frame is from native code', () => { const eventProcessor = createInboundFiltersEventProcessor({ denyUrls: ['https://awesome-analytics.io/some/file.js'], }); - expect(eventProcessor(MESSAGE_EVENT_WITH_NATIVE_FRAME)).toBe(null); + expect(eventProcessor(MESSAGE_EVENT_WITH_NATIVE_LAST_FRAME)).toBe(null); }); }); }); @@ -234,7 +249,7 @@ const MESSAGE_EVENT_WITH_STACKTRACE = { }, }; -const MESSAGE_EVENT_WITH_ANON_FRAME = { +const MESSAGE_EVENT_WITH_ANON_LAST_FRAME = { message: 'any', stacktrace: { frames: [ @@ -245,7 +260,7 @@ const MESSAGE_EVENT_WITH_ANON_FRAME = { }, }; -const MESSAGE_EVENT_WITH_NATIVE_FRAME = { +const MESSAGE_EVENT_WITH_NATIVE_LAST_FRAME = { message: 'any', stacktrace: { frames: [ @@ -296,7 +311,7 @@ const SENTRY_EVENT = { }, }; -const DEFAULT_EVENT = { +const SCRIPT_ERROR_EVENT = { exception: { values: [ { From 7f015992421a1476951976dbd8af7ec712cd9911 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Thu, 24 Feb 2022 12:36:32 -0500 Subject: [PATCH 6/9] move fixtures around --- .../lib/integrations/inboundfilters.test.ts | 204 +++++++++--------- 1 file changed, 102 insertions(+), 102 deletions(-) diff --git a/packages/core/test/lib/integrations/inboundfilters.test.ts b/packages/core/test/lib/integrations/inboundfilters.test.ts index b7acfb41d7c2..603538eade0f 100644 --- a/packages/core/test/lib/integrations/inboundfilters.test.ts +++ b/packages/core/test/lib/integrations/inboundfilters.test.ts @@ -50,6 +50,108 @@ function createInboundFiltersEventProcessor( return eventProcessors[0]; } +// Fixtures + +const MESSAGE_EVENT = { + message: 'captureMessage', +}; + +const MESSAGE_EVENT_2 = { + message: 'captureMessageSomething', +}; + +const MESSAGE_EVENT_WITH_STACKTRACE = { + message: 'wat', + stacktrace: { + // Frames are always in the reverse order, as this is how Sentry expect them to come. + // Frame that crashed is the last one, the one from awesome-analytics + frames: [ + { filename: 'https://our-side.com/js/bundle.js' }, + { filename: 'https://our-side.com/js/bundle.js' }, + { filename: 'https://awesome-analytics.io/some/file.js' }, + ], + }, +}; + +const MESSAGE_EVENT_WITH_ANON_LAST_FRAME = { + message: 'any', + stacktrace: { + frames: [ + { filename: 'https://our-side.com/js/bundle.js' }, + { filename: 'https://awesome-analytics.io/some/file.js' }, + { filename: '' }, + ], + }, +}; + +const MESSAGE_EVENT_WITH_NATIVE_LAST_FRAME = { + message: 'any', + stacktrace: { + frames: [ + { filename: 'https://our-side.com/js/bundle.js' }, + { filename: 'https://awesome-analytics.io/some/file.js' }, + { filename: '[native code]' }, + ], + }, +}; + +const EXCEPTION_EVENT = { + exception: { + values: [ + { + type: 'SyntaxError', + value: 'unidentified ? at line 1337', + }, + ], + }, +}; + +const EXCEPTION_EVENT_WITH_FRAMES = { + exception: { + values: [ + { + stacktrace: { + // Frames are always in the reverse order, as this is how Sentry expect them to come. + // Frame that crashed is the last one, the one from awesome-analytics + frames: [ + { filename: 'https://our-side.com/js/bundle.js' }, + { filename: 'https://our-side.com/js/bundle.js' }, + { filename: 'https://awesome-analytics.io/some/file.js' }, + ], + }, + }, + ], + }, +}; + +const SENTRY_EVENT = { + exception: { + values: [ + { + type: 'SentryError', + value: 'something something server connection', + }, + ], + }, +}; + +const SCRIPT_ERROR_EVENT = { + exception: { + values: [ + { + type: '[undefined]', + value: 'Script error.', + }, + ], + }, +}; + +const MALFORMED_EVENT = { + stacktrace: { + frames: undefined, + }, +}; + describe('InboundFilters', () => { describe('_isSentryError', () => { it('should work as expected', () => { @@ -225,105 +327,3 @@ describe('InboundFilters', () => { }); }); }); - -// Fixtures - -const MESSAGE_EVENT = { - message: 'captureMessage', -}; - -const MESSAGE_EVENT_2 = { - message: 'captureMessageSomething', -}; - -const MESSAGE_EVENT_WITH_STACKTRACE = { - message: 'wat', - stacktrace: { - // Frames are always in the reverse order, as this is how Sentry expect them to come. - // Frame that crashed is the last one, the one from awesome-analytics - frames: [ - { filename: 'https://our-side.com/js/bundle.js' }, - { filename: 'https://our-side.com/js/bundle.js' }, - { filename: 'https://awesome-analytics.io/some/file.js' }, - ], - }, -}; - -const MESSAGE_EVENT_WITH_ANON_LAST_FRAME = { - message: 'any', - stacktrace: { - frames: [ - { filename: 'https://our-side.com/js/bundle.js' }, - { filename: 'https://awesome-analytics.io/some/file.js' }, - { filename: '' }, - ], - }, -}; - -const MESSAGE_EVENT_WITH_NATIVE_LAST_FRAME = { - message: 'any', - stacktrace: { - frames: [ - { filename: 'https://our-side.com/js/bundle.js' }, - { filename: 'https://awesome-analytics.io/some/file.js' }, - { filename: '[native code]' }, - ], - }, -}; - -const EXCEPTION_EVENT = { - exception: { - values: [ - { - type: 'SyntaxError', - value: 'unidentified ? at line 1337', - }, - ], - }, -}; - -const EXCEPTION_EVENT_WITH_FRAMES = { - exception: { - values: [ - { - stacktrace: { - // Frames are always in the reverse order, as this is how Sentry expect them to come. - // Frame that crashed is the last one, the one from awesome-analytics - frames: [ - { filename: 'https://our-side.com/js/bundle.js' }, - { filename: 'https://our-side.com/js/bundle.js' }, - { filename: 'https://awesome-analytics.io/some/file.js' }, - ], - }, - }, - ], - }, -}; - -const SENTRY_EVENT = { - exception: { - values: [ - { - type: 'SentryError', - value: 'something something server connection', - }, - ], - }, -}; - -const SCRIPT_ERROR_EVENT = { - exception: { - values: [ - { - type: '[undefined]', - value: 'Script error.', - }, - ], - }, -}; - -const MALFORMED_EVENT = { - stacktrace: { - frames: undefined, - }, -}; From ecb64603c73d4c1439cd798397d523960c355a2d Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Thu, 24 Feb 2022 12:38:22 -0500 Subject: [PATCH 7/9] review comment --- packages/core/test/lib/integrations/inboundfilters.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/test/lib/integrations/inboundfilters.test.ts b/packages/core/test/lib/integrations/inboundfilters.test.ts index 603538eade0f..fd6530cd75e3 100644 --- a/packages/core/test/lib/integrations/inboundfilters.test.ts +++ b/packages/core/test/lib/integrations/inboundfilters.test.ts @@ -291,7 +291,7 @@ describe('InboundFilters', () => { expect(eventProcessor(EXCEPTION_EVENT_WITH_FRAMES)).toBe(null); }); - it('should not filter events that doesnt pass the test', () => { + it("should not filter events that don't match the filtered values", () => { const eventProcessor = createInboundFiltersEventProcessor({ denyUrls: ['some-other-domain.com'], }); From f2bfe03842a59b22ef2d5e11c8b2d00404a054f9 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Fri, 18 Mar 2022 09:14:24 +0100 Subject: [PATCH 8/9] remove type alias --- packages/core/src/integrations/inboundfilters.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/core/src/integrations/inboundfilters.ts b/packages/core/src/integrations/inboundfilters.ts index 527cc1a1ab98..f5993867b0d0 100644 --- a/packages/core/src/integrations/inboundfilters.ts +++ b/packages/core/src/integrations/inboundfilters.ts @@ -119,7 +119,7 @@ export function _shouldDropEvent(event: Event, options: Partial['ignoreErrors']): boolean { +function _isIgnoredError(event: Event, ignoreErrors?: Array): boolean { if (!ignoreErrors || !ignoreErrors.length) { return false; } @@ -130,7 +130,7 @@ function _isIgnoredError(event: Event, ignoreErrors: Partial['denyUrls']): boolean { +function _isDeniedUrl(event: Event, denyUrls?: Array): boolean { // TODO: Use Glob instead? if (!denyUrls || !denyUrls.length) { return false; @@ -140,7 +140,7 @@ function _isDeniedUrl(event: Event, denyUrls: Partial['de } /** JSDoc */ -function _isAllowedUrl(event: Event, allowUrls: Partial['allowUrls']): boolean { +function _isAllowedUrl(event: Event, allowUrls?: Array): boolean { // TODO: Use Glob instead? if (!allowUrls || !allowUrls.length) { return true; @@ -167,7 +167,7 @@ function _getPossibleEventMessages(event: Event): string[] { } /** JSDoc */ -function _isSentryError(event: Event, ignoreInternal: Partial['ignoreInternal']): boolean { +function _isSentryError(event: Event, ignoreInternal?: boolean): boolean { if (ignoreInternal) { try { // @ts-ignore can't be a sentry error if undefined From 94eac238054b7b7910d5c71485b882481a76bc52 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Wed, 23 Mar 2022 13:28:40 -0400 Subject: [PATCH 9/9] address PR review feedback --- .../core/src/integrations/inboundfilters.ts | 27 +++++++------------ 1 file changed, 9 insertions(+), 18 deletions(-) diff --git a/packages/core/src/integrations/inboundfilters.ts b/packages/core/src/integrations/inboundfilters.ts index f5993867b0d0..04d395b69534 100644 --- a/packages/core/src/integrations/inboundfilters.ts +++ b/packages/core/src/integrations/inboundfilters.ts @@ -5,7 +5,7 @@ import { getEventDescription, isDebugBuild, isMatchingPattern, logger } from '@s // this is the result of a script being pulled in from an external domain and CORS. const DEFAULT_IGNORE_ERRORS = [/^Script error\.?$/, /^Javascript error: Script error\.? on line 0$/]; -/** JSDoc */ +/** Options for the InboundFilters integration */ export interface InboundFiltersOptions { allowUrls: Array; denyUrls: Array; @@ -85,7 +85,7 @@ export function _mergeOptions( /** JSDoc */ export function _shouldDropEvent(event: Event, options: Partial): boolean { - if (_isSentryError(event, options.ignoreInternal)) { + if (options.ignoreInternal && _isSentryError(event)) { isDebugBuild() && logger.warn(`Event dropped due to being internal Sentry Error.\nEvent: ${getEventDescription(event)}`); return true; @@ -118,7 +118,6 @@ export function _shouldDropEvent(event: Event, options: Partial): boolean { if (!ignoreErrors || !ignoreErrors.length) { return false; @@ -129,7 +128,6 @@ function _isIgnoredError(event: Event, ignoreErrors?: Array): b ); } -/** JSDoc */ function _isDeniedUrl(event: Event, denyUrls?: Array): boolean { // TODO: Use Glob instead? if (!denyUrls || !denyUrls.length) { @@ -139,7 +137,6 @@ function _isDeniedUrl(event: Event, denyUrls?: Array): boolean return !url ? false : denyUrls.some(pattern => isMatchingPattern(url, pattern)); } -/** JSDoc */ function _isAllowedUrl(event: Event, allowUrls?: Array): boolean { // TODO: Use Glob instead? if (!allowUrls || !allowUrls.length) { @@ -149,7 +146,6 @@ function _isAllowedUrl(event: Event, allowUrls?: Array): boolea return !url ? true : allowUrls.some(pattern => isMatchingPattern(url, pattern)); } -/** JSDoc */ function _getPossibleEventMessages(event: Event): string[] { if (event.message) { return [event.message]; @@ -166,21 +162,17 @@ function _getPossibleEventMessages(event: Event): string[] { return []; } -/** JSDoc */ -function _isSentryError(event: Event, ignoreInternal?: boolean): boolean { - if (ignoreInternal) { - try { - // @ts-ignore can't be a sentry error if undefined - // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - return event.exception.values[0].type === 'SentryError'; - } catch (e) { - // ignore - } +function _isSentryError(event: Event): boolean { + try { + // @ts-ignore can't be a sentry error if undefined + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access + return event.exception.values[0].type === 'SentryError'; + } catch (e) { + // ignore } return false; } -/** JSDoc */ function _getLastValidUrl(frames: StackFrame[] = []): string | null { for (let i = frames.length - 1; i >= 0; i--) { const frame = frames[i]; @@ -193,7 +185,6 @@ function _getLastValidUrl(frames: StackFrame[] = []): string | null { return null; } -/** JSDoc */ function _getEventFilterUrl(event: Event): string | null { try { if (event.stacktrace) {