Skip to content

Commit 59a0cb6

Browse files
AbhiPrasadAbhiPrasad
authored
ref(core): Reduce inboundfilters bundle size (#4625)
- Move all private methods from `InboundFilters` into their own standalone functions - Pass in options explicitly so you can do `allowUrls.length` instead of `options.allowUrls.length` (can't minify `allowUrls` in the 2nd option) - Remove `typeof self` logic since now we explicitly pass everything in - Rewrite tests to stop relying on private methods but instead assert on event processor
1 parent 57a82f3 commit 59a0cb6

File tree

2 files changed

+396
-609
lines changed

2 files changed

+396
-609
lines changed

‎packages/core/src/integrations/inboundfilters.ts‎

Lines changed: 139 additions & 159 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,12 @@
1-
import { addGlobalEventProcessor, getCurrentHub } from '@sentry/hub';
2-
import { Event, Integration, StackFrame } from '@sentry/types';
1+
import { Event, EventProcessor, Hub, Integration, StackFrame } from '@sentry/types';
32
import { getEventDescription, isDebugBuild, isMatchingPattern, logger } from '@sentry/utils';
43

54
// "Script error." is hard coded into browsers for errors that it can't read.
65
// this is the result of a script being pulled in from an external domain and CORS.
76
const DEFAULT_IGNORE_ERRORS = [/^Script error\.?$/, /^Javascript error: Script error\.? on line 0$/];
87

9-
/** JSDoc */
10-
interface InboundFiltersOptions {
8+
/** Options for the InboundFilters integration */
9+
export interface InboundFiltersOptions {
1110
allowUrls: Array<string | RegExp>;
1211
denyUrls: Array<string | RegExp>;
1312
ignoreErrors: Array<string | RegExp>;
@@ -36,190 +35,171 @@ export class InboundFilters implements Integration {
3635
/**
3736
* @inheritDoc
3837
*/
39-
public setupOnce(): void {
38+
public setupOnce(addGlobalEventProcessor: (processor: EventProcessor) => void, getCurrentHub: () => Hub): void {
4039
addGlobalEventProcessor((event: Event) => {
4140
const hub = getCurrentHub();
42-
if (!hub) {
43-
return event;
44-
}
45-
const self = hub.getIntegration(InboundFilters);
46-
if (self) {
47-
const client = hub.getClient();
48-
const clientOptions = client ? client.getOptions() : {};
49-
// This checks prevents most of the occurrences of the bug linked below:
50-
// https://github.com/getsentry/sentry-javascript/issues/2622
51-
// The bug is caused by multiple SDK instances, where one is minified and one is using non-mangled code.
52-
// Unfortunatelly we cannot fix it reliably (thus reserved property in rollup's terser config),
53-
// as we cannot force people using multiple instances in their apps to sync SDK versions.
54-
const options = typeof self._mergeOptions === 'function' ? self._mergeOptions(clientOptions) : {};
55-
if (typeof self._shouldDropEvent !== 'function') {
56-
return event;
41+
if (hub) {
42+
const self = hub.getIntegration(InboundFilters);
43+
if (self) {
44+
const client = hub.getClient();
45+
const clientOptions = client ? client.getOptions() : {};
46+
const options = _mergeOptions(self._options, clientOptions);
47+
return _shouldDropEvent(event, options) ? null : event;
5748
}
58-
return self._shouldDropEvent(event, options) ? null : event;
5949
}
6050
return event;
6151
});
6252
}
53+
}
6354

64-
/** JSDoc */
65-
private _shouldDropEvent(event: Event, options: Partial<InboundFiltersOptions>): boolean {
66-
if (this._isSentryError(event, options)) {
67-
isDebugBuild() &&
68-
logger.warn(`Event dropped due to being internal Sentry Error.\nEvent: ${getEventDescription(event)}`);
69-
return true;
70-
}
71-
if (this._isIgnoredError(event, options)) {
72-
isDebugBuild() &&
73-
logger.warn(
74-
`Event dropped due to being matched by \`ignoreErrors\` option.\nEvent: ${getEventDescription(event)}`,
75-
);
76-
return true;
77-
}
78-
if (this._isDeniedUrl(event, options)) {
79-
isDebugBuild() &&
80-
logger.warn(
81-
`Event dropped due to being matched by \`denyUrls\` option.\nEvent: ${getEventDescription(
82-
event,
83-
)}.\nUrl: ${this._getEventFilterUrl(event)}`,
84-
);
85-
return true;
86-
}
87-
if (!this._isAllowedUrl(event, options)) {
88-
isDebugBuild() &&
89-
logger.warn(
90-
`Event dropped due to not being matched by \`allowUrls\` option.\nEvent: ${getEventDescription(
91-
event,
92-
)}.\nUrl: ${this._getEventFilterUrl(event)}`,
93-
);
94-
return true;
95-
}
96-
return false;
97-
}
98-
99-
/** JSDoc */
100-
private _isSentryError(event: Event, options: Partial<InboundFiltersOptions>): boolean {
101-
if (!options.ignoreInternal) {
102-
return false;
103-
}
55+
/** JSDoc */
56+
export function _mergeOptions(
57+
internalOptions: Partial<InboundFiltersOptions> = {},
58+
clientOptions: Partial<InboundFiltersOptions> = {},
59+
): Partial<InboundFiltersOptions> {
60+
return {
61+
allowUrls: [
62+
// eslint-disable-next-line deprecation/deprecation
63+
...(internalOptions.whitelistUrls || []),
64+
...(internalOptions.allowUrls || []),
65+
// eslint-disable-next-line deprecation/deprecation
66+
...(clientOptions.whitelistUrls || []),
67+
...(clientOptions.allowUrls || []),
68+
],
69+
denyUrls: [
70+
// eslint-disable-next-line deprecation/deprecation
71+
...(internalOptions.blacklistUrls || []),
72+
...(internalOptions.denyUrls || []),
73+
// eslint-disable-next-line deprecation/deprecation
74+
...(clientOptions.blacklistUrls || []),
75+
...(clientOptions.denyUrls || []),
76+
],
77+
ignoreErrors: [
78+
...(internalOptions.ignoreErrors || []),
79+
...(clientOptions.ignoreErrors || []),
80+
...DEFAULT_IGNORE_ERRORS,
81+
],
82+
ignoreInternal: internalOptions.ignoreInternal !== undefined ? internalOptions.ignoreInternal : true,
83+
};
84+
}
10485

105-
try {
106-
// @ts-ignore can't be a sentry error if undefined
107-
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
108-
return event.exception.values[0].type === 'SentryError';
109-
} catch (e) {
110-
// ignore
111-
}
86+
/** JSDoc */
87+
export function _shouldDropEvent(event: Event, options: Partial<InboundFiltersOptions>): boolean {
88+
if (options.ignoreInternal && _isSentryError(event)) {
89+
isDebugBuild() &&
90+
logger.warn(`Event dropped due to being internal Sentry Error.\nEvent: ${getEventDescription(event)}`);
91+
return true;
92+
}
93+
if (_isIgnoredError(event, options.ignoreErrors)) {
94+
isDebugBuild() &&
95+
logger.warn(
96+
`Event dropped due to being matched by \`ignoreErrors\` option.\nEvent: ${getEventDescription(event)}`,
97+
);
98+
return true;
99+
}
100+
if (_isDeniedUrl(event, options.denyUrls)) {
101+
isDebugBuild() &&
102+
logger.warn(
103+
`Event dropped due to being matched by \`denyUrls\` option.\nEvent: ${getEventDescription(
104+
event,
105+
)}.\nUrl: ${_getEventFilterUrl(event)}`,
106+
);
107+
return true;
108+
}
109+
if (!_isAllowedUrl(event, options.allowUrls)) {
110+
isDebugBuild() &&
111+
logger.warn(
112+
`Event dropped due to not being matched by \`allowUrls\` option.\nEvent: ${getEventDescription(
113+
event,
114+
)}.\nUrl: ${_getEventFilterUrl(event)}`,
115+
);
116+
return true;
117+
}
118+
return false;
119+
}
112120

121+
function _isIgnoredError(event: Event, ignoreErrors?: Array<string | RegExp>): boolean {
122+
if (!ignoreErrors || !ignoreErrors.length) {
113123
return false;
114124
}
115125

116-
/** JSDoc */
117-
private _isIgnoredError(event: Event, options: Partial<InboundFiltersOptions>): boolean {
118-
if (!options.ignoreErrors || !options.ignoreErrors.length) {
119-
return false;
120-
}
126+
return _getPossibleEventMessages(event).some(message =>
127+
ignoreErrors.some(pattern => isMatchingPattern(message, pattern)),
128+
);
129+
}
121130

122-
return this._getPossibleEventMessages(event).some(message =>
123-
// Not sure why TypeScript complains here...
124-
(options.ignoreErrors as Array<RegExp | string>).some(pattern => isMatchingPattern(message, pattern)),
125-
);
131+
function _isDeniedUrl(event: Event, denyUrls?: Array<string | RegExp>): boolean {
132+
// TODO: Use Glob instead?
133+
if (!denyUrls || !denyUrls.length) {
134+
return false;
126135
}
136+
const url = _getEventFilterUrl(event);
137+
return !url ? false : denyUrls.some(pattern => isMatchingPattern(url, pattern));
138+
}
127139

128-
/** JSDoc */
129-
private _isDeniedUrl(event: Event, options: Partial<InboundFiltersOptions>): boolean {
130-
// TODO: Use Glob instead?
131-
if (!options.denyUrls || !options.denyUrls.length) {
132-
return false;
133-
}
134-
const url = this._getEventFilterUrl(event);
135-
return !url ? false : options.denyUrls.some(pattern => isMatchingPattern(url, pattern));
140+
function _isAllowedUrl(event: Event, allowUrls?: Array<string | RegExp>): boolean {
141+
// TODO: Use Glob instead?
142+
if (!allowUrls || !allowUrls.length) {
143+
return true;
136144
}
145+
const url = _getEventFilterUrl(event);
146+
return !url ? true : allowUrls.some(pattern => isMatchingPattern(url, pattern));
147+
}
137148

138-
/** JSDoc */
139-
private _isAllowedUrl(event: Event, options: Partial<InboundFiltersOptions>): boolean {
140-
// TODO: Use Glob instead?
141-
if (!options.allowUrls || !options.allowUrls.length) {
142-
return true;
149+
function _getPossibleEventMessages(event: Event): string[] {
150+
if (event.message) {
151+
return [event.message];
152+
}
153+
if (event.exception) {
154+
try {
155+
const { type = '', value = '' } = (event.exception.values && event.exception.values[0]) || {};
156+
return [`${value}`, `${type}: ${value}`];
157+
} catch (oO) {
158+
isDebugBuild() && logger.error(`Cannot extract message for event ${getEventDescription(event)}`);
159+
return [];
143160
}
144-
const url = this._getEventFilterUrl(event);
145-
return !url ? true : options.allowUrls.some(pattern => isMatchingPattern(url, pattern));
146161
}
162+
return [];
163+
}
147164

148-
/** JSDoc */
149-
private _mergeOptions(clientOptions: Partial<InboundFiltersOptions> = {}): Partial<InboundFiltersOptions> {
150-
return {
151-
allowUrls: [
152-
// eslint-disable-next-line deprecation/deprecation
153-
...(this._options.whitelistUrls || []),
154-
...(this._options.allowUrls || []),
155-
// eslint-disable-next-line deprecation/deprecation
156-
...(clientOptions.whitelistUrls || []),
157-
...(clientOptions.allowUrls || []),
158-
],
159-
denyUrls: [
160-
// eslint-disable-next-line deprecation/deprecation
161-
...(this._options.blacklistUrls || []),
162-
...(this._options.denyUrls || []),
163-
// eslint-disable-next-line deprecation/deprecation
164-
...(clientOptions.blacklistUrls || []),
165-
...(clientOptions.denyUrls || []),
166-
],
167-
ignoreErrors: [
168-
...(this._options.ignoreErrors || []),
169-
...(clientOptions.ignoreErrors || []),
170-
...DEFAULT_IGNORE_ERRORS,
171-
],
172-
ignoreInternal: typeof this._options.ignoreInternal !== 'undefined' ? this._options.ignoreInternal : true,
173-
};
165+
function _isSentryError(event: Event): boolean {
166+
try {
167+
// @ts-ignore can't be a sentry error if undefined
168+
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
169+
return event.exception.values[0].type === 'SentryError';
170+
} catch (e) {
171+
// ignore
174172
}
173+
return false;
174+
}
175175

176-
/** JSDoc */
177-
private _getPossibleEventMessages(event: Event): string[] {
178-
if (event.message) {
179-
return [event.message];
180-
}
181-
if (event.exception) {
182-
try {
183-
const { type = '', value = '' } = (event.exception.values && event.exception.values[0]) || {};
184-
return [`${value}`, `${type}: ${value}`];
185-
} catch (oO) {
186-
isDebugBuild() && logger.error(`Cannot extract message for event ${getEventDescription(event)}`);
187-
return [];
188-
}
176+
function _getLastValidUrl(frames: StackFrame[] = []): string | null {
177+
for (let i = frames.length - 1; i >= 0; i--) {
178+
const frame = frames[i];
179+
180+
if (frame && frame.filename !== '<anonymous>' && frame.filename !== '[native code]') {
181+
return frame.filename || null;
189182
}
190-
return [];
191183
}
192184

193-
/** JSDoc */
194-
private _getLastValidUrl(frames: StackFrame[] = []): string | null {
195-
for (let i = frames.length - 1; i >= 0; i--) {
196-
const frame = frames[i];
185+
return null;
186+
}
197187

198-
if (frame && frame.filename !== '<anonymous>' && frame.filename !== '[native code]') {
199-
return frame.filename || null;
200-
}
188+
function _getEventFilterUrl(event: Event): string | null {
189+
try {
190+
if (event.stacktrace) {
191+
return _getLastValidUrl(event.stacktrace.frames);
201192
}
202-
203-
return null;
204-
}
205-
206-
/** JSDoc */
207-
private _getEventFilterUrl(event: Event): string | null {
193+
let frames;
208194
try {
209-
if (event.stacktrace) {
210-
return this._getLastValidUrl(event.stacktrace.frames);
211-
}
212-
let frames;
213-
try {
214-
// @ts-ignore we only care about frames if the whole thing here is defined
215-
frames = event.exception.values[0].stacktrace.frames;
216-
} catch (e) {
217-
// ignore
218-
}
219-
return frames ? this._getLastValidUrl(frames) : null;
220-
} catch (oO) {
221-
isDebugBuild() && logger.error(`Cannot extract url for event ${getEventDescription(event)}`);
222-
return null;
195+
// @ts-ignore we only care about frames if the whole thing here is defined
196+
frames = event.exception.values[0].stacktrace.frames;
197+
} catch (e) {
198+
// ignore
223199
}
200+
return frames ? _getLastValidUrl(frames) : null;
201+
} catch (oO) {
202+
isDebugBuild() && logger.error(`Cannot extract url for event ${getEventDescription(event)}`);
203+
return null;
224204
}
225205
}

0 commit comments

Comments
 (0)