Skip to content

Commit

Permalink
[fix] Correct usage of init config in inboundFilters (#1703)
Browse files Browse the repository at this point in the history
  • Loading branch information
@kamilogorek
kamilogorek committed Oct 30, 2018
1 parent 141f55f commit c69749d
Show file tree
Hide file tree
Showing 4 changed files with 313 additions and 170 deletions.
1 change: 1 addition & 0 deletions packages/browser/test/integration/init.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ Sentry.init({
// debug: true,
attachStacktrace: true,
transport: DummyTransport,
ignoreErrors: ['ignoreErrorTest'],
// integrations: function(old) {
// return [new Sentry.Integrations.Debug({ stringify: true })].concat(old);
// },
Expand Down
25 changes: 25 additions & 0 deletions packages/browser/test/integration/test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,31 @@ for (var idx in frames) {
document.body.removeChild(this.iframe);
});

describe('config', function() {
it('should allow to ignore specific errors', function(done) {
var iframe = this.iframe;

iframeExecute(
iframe,
done,
function() {
Sentry.captureException(new Error('foo'));
Sentry.captureException(new Error('ignoreErrorTest'));
Sentry.captureException(new Error('bar'));
},
function(sentryData) {
if (debounceAssertEventCount(sentryData, 2, done)) {
assert.equal(sentryData[0].exception.values[0].type, 'Error');
assert.equal(sentryData[0].exception.values[0].value, 'foo');
assert.equal(sentryData[1].exception.values[0].type, 'Error');
assert.equal(sentryData[1].exception.values[0].value, 'bar');
done();
}
}
);
});
});

describe('API', function() {
it('should capture Sentry.captureMessage', function(done) {
var iframe = this.iframe;
Expand Down
76 changes: 37 additions & 39 deletions packages/core/src/integrations/inboundfilters.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { isRegExp } from '@sentry/utils/is';
import { logger } from '@sentry/utils/logger';
import { getEventDescription } from '@sentry/utils/misc';
import { includes } from '@sentry/utils/string';
import { Client } from '../interfaces';

// "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.
Expand All @@ -18,13 +19,6 @@ interface InboundFiltersOptions {

/** Inbound filters configurable by the user */
export class InboundFilters implements Integration {
/** JSDoc */
private ignoreErrors?: Array<string | RegExp> = DEFAULT_IGNORE_ERRORS;
/** JSDoc */
private blacklistUrls?: Array<string | RegExp>;
/** JSDoc */
private whitelistUrls?: Array<string | RegExp>;

/**
* @inheritDoc
*/
Expand All @@ -34,19 +28,23 @@ export class InboundFilters implements Integration {
*/
public static id: string = 'InboundFilters';

public constructor(private readonly options: InboundFiltersOptions = {}) {
this.configureOptions();
}
public constructor(private readonly options: InboundFiltersOptions = {}) {}

/**
* @inheritDoc
*/
public setupOnce(): void {
addGlobalEventProcessor(async (event: SentryEvent) => {
const self = getCurrentHub().getIntegration(InboundFilters);
const hub = getCurrentHub();
if (!hub) {
return event;
}
const self = hub.getIntegration(InboundFilters);
if (self) {
self.configureOptions();
if (self.shouldDropEvent(event)) {
const client = hub.getClient() as Client;
const clientOptions = client ? client.getOptions() : {};
const options = self.mergeOptions(clientOptions);
if (self.shouldDropEvent(event, options)) {
return null;
}
}
Expand All @@ -55,22 +53,22 @@ export class InboundFilters implements Integration {
}

/** JSDoc */
public shouldDropEvent(event: SentryEvent): boolean {
if (this.isIgnoredError(event)) {
public shouldDropEvent(event: SentryEvent, options: InboundFiltersOptions): boolean {
if (this.isIgnoredError(event, options)) {
logger.warn(
`Event dropped due to being matched by \`ignoreErrors\` option.\nEvent: ${getEventDescription(event)}`,
);
return true;
}
if (this.isBlacklistedUrl(event)) {
if (this.isBlacklistedUrl(event, options)) {
logger.warn(
`Event dropped due to being matched by \`blacklistUrls\` option.\nEvent: ${getEventDescription(
event,
)}.\nUrl: ${this.getEventFilterUrl(event)}`,
);
return true;
}
if (!this.isWhitelistedUrl(event)) {
if (!this.isWhitelistedUrl(event, options)) {
logger.warn(
`Event dropped due to not being matched by \`whitelistUrls\` option.\nEvent: ${getEventDescription(
event,
Expand All @@ -82,35 +80,48 @@ export class InboundFilters implements Integration {
}

/** JSDoc */
public isIgnoredError(event: SentryEvent): boolean {
if (!this.ignoreErrors) {
public isIgnoredError(event: SentryEvent, options: InboundFiltersOptions = {}): boolean {
if (!options.ignoreErrors || !options.ignoreErrors.length) {
return false;
}

return this.getPossibleEventMessages(event).some(message =>
// Not sure why TypeScript complains here...
(this.ignoreErrors as Array<RegExp | string>).some(pattern => this.isMatchingPattern(message, pattern)),
(options.ignoreErrors as Array<RegExp | string>).some(pattern => this.isMatchingPattern(message, pattern)),
);
}

/** JSDoc */
public isBlacklistedUrl(event: SentryEvent): boolean {
public isBlacklistedUrl(event: SentryEvent, options: InboundFiltersOptions = {}): boolean {
// TODO: Use Glob instead?
if (!this.blacklistUrls) {
if (!options.blacklistUrls || !options.blacklistUrls.length) {
return false;
}
const url = this.getEventFilterUrl(event);
return !url ? false : this.blacklistUrls.some(pattern => this.isMatchingPattern(url, pattern));
return !url ? false : options.blacklistUrls.some(pattern => this.isMatchingPattern(url, pattern));
}

/** JSDoc */
public isWhitelistedUrl(event: SentryEvent): boolean {
public isWhitelistedUrl(event: SentryEvent, options: InboundFiltersOptions = {}): boolean {
// TODO: Use Glob instead?
if (!this.whitelistUrls) {
if (!options.whitelistUrls || !options.whitelistUrls.length) {
return true;
}
const url = this.getEventFilterUrl(event);
return !url ? true : this.whitelistUrls.some(pattern => this.isMatchingPattern(url, pattern));
return !url ? true : options.whitelistUrls.some(pattern => this.isMatchingPattern(url, pattern));
}

/** JSDoc */
public mergeOptions(clientOptions: InboundFiltersOptions = {}): InboundFiltersOptions {
return {
blacklistUrls: [...(this.options.blacklistUrls || []), ...(clientOptions.blacklistUrls || [])],
ignoreErrors: [
...(this.options.ignoreErrors || []),
...(clientOptions.ignoreErrors || []),
...DEFAULT_IGNORE_ERRORS,
],
whitelistUrls: [...(this.options.whitelistUrls || []), ...(clientOptions.whitelistUrls || [])],
};
}

/** JSDoc */
Expand All @@ -124,19 +135,6 @@ export class InboundFilters implements Integration {
}
}

/** JSDoc */
private configureOptions(): void {
if (this.options.ignoreErrors) {
this.ignoreErrors = [...DEFAULT_IGNORE_ERRORS, ...this.options.ignoreErrors];
}
if (this.options.blacklistUrls) {
this.blacklistUrls = [...this.options.blacklistUrls];
}
if (this.options.whitelistUrls) {
this.whitelistUrls = [...this.options.whitelistUrls];
}
}

/** JSDoc */
private getPossibleEventMessages(event: SentryEvent): string[] {
if (event.message) {
Expand Down
Loading

0 comments on commit c69749d

Please sign in to comment.