Skip to content

Commit

Permalink
fix(angular): Filter out TryCatch integration by default (#8367)
Browse files Browse the repository at this point in the history
The `TryCatch` default integration interferes with the
`SentryErrorHander` error handler of the Angular(-Ivy) SDKs by catching
certain errors too early, before the Angular SDK-specific error handler
can catch them. This caused missing data on the event in some or
duplicated errors in other cases.

This fix filters out the `TryCatch` by default, as long as users didn't
set `defaultIntegrations` in their SDK init. Therefore, it makes the
previously provided
[workaround](#5417 (comment))
obsolete.
  • Loading branch information
Lms24 authored Jun 20, 2023
1 parent 365c750 commit e5e6a6b
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 3 deletions.
14 changes: 13 additions & 1 deletion packages/angular-ivy/src/sdk.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { VERSION } from '@angular/core';
import type { BrowserOptions } from '@sentry/browser';
import { init as browserInit, SDK_VERSION, setContext } from '@sentry/browser';
import { defaultIntegrations, init as browserInit, SDK_VERSION, setContext } from '@sentry/browser';
import { logger } from '@sentry/utils';

import { IS_DEBUG_BUILD } from './flags';
Expand All @@ -21,6 +21,18 @@ export function init(options: BrowserOptions): void {
version: SDK_VERSION,
};

// Filter out TryCatch integration as it interferes with our Angular `ErrorHandler`:
// TryCatch would catch certain errors before they reach the `ErrorHandler` and thus provide a
// lower fidelity error than what `SentryErrorHandler` (see errorhandler.ts) would provide.
// see:
// - https://github.com/getsentry/sentry-javascript/issues/5417#issuecomment-1453407097
// - https://github.com/getsentry/sentry-javascript/issues/2744
if (options.defaultIntegrations === undefined) {
options.defaultIntegrations = defaultIntegrations.filter(integration => {
return integration.name !== 'TryCatch';
});
}

checkAndSetAngularVersion();
browserInit(options);
}
Expand Down
14 changes: 13 additions & 1 deletion packages/angular/src/sdk.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { VERSION } from '@angular/core';
import type { BrowserOptions } from '@sentry/browser';
import { init as browserInit, SDK_VERSION, setContext } from '@sentry/browser';
import { defaultIntegrations, init as browserInit, SDK_VERSION, setContext } from '@sentry/browser';
import { logger } from '@sentry/utils';

import { IS_DEBUG_BUILD } from './flags';
Expand All @@ -21,6 +21,18 @@ export function init(options: BrowserOptions): void {
version: SDK_VERSION,
};

// Filter out TryCatch integration as it interferes with our Angular `ErrorHandler`:
// TryCatch would catch certain errors before they reach the `ErrorHandler` and thus provide a
// lower fidelity error than what `SentryErrorHandler` (see errorhandler.ts) would provide.
// see:
// - https://github.com/getsentry/sentry-javascript/issues/5417#issuecomment-1453407097
// - https://github.com/getsentry/sentry-javascript/issues/2744
if (options.defaultIntegrations === undefined) {
options.defaultIntegrations = defaultIntegrations.filter(integration => {
return integration.name !== 'TryCatch';
});
}

checkAndSetAngularVersion();
browserInit(options);
}
Expand Down
31 changes: 30 additions & 1 deletion packages/angular/test/sdk.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import * as SentryBrowser from '@sentry/browser';

import { init } from '../src/sdk';
import { defaultIntegrations, init } from '../src/index';

describe('init', () => {
it('sets the Angular version (if available) in the global scope', () => {
Expand All @@ -13,4 +13,33 @@ describe('init', () => {
expect(setContextSpy).toHaveBeenCalledTimes(1);
expect(setContextSpy).toHaveBeenCalledWith('angular', { version: 10 });
});

describe('filtering out the `TryCatch` integration', () => {
const browserInitSpy = jest.spyOn(SentryBrowser, 'init');

beforeEach(() => {
browserInitSpy.mockClear();
});

it('filters if `defaultIntegrations` is not set', () => {
init({});

expect(browserInitSpy).toHaveBeenCalledTimes(1);

const options = browserInitSpy.mock.calls[0][0] || {};
expect(options.defaultIntegrations).not.toContainEqual(expect.objectContaining({ name: 'TryCatch' }));
});

it.each([false as const, defaultIntegrations])(
"doesn't filter if `defaultIntegrations` is set to %s",
defaultIntegrations => {
init({ defaultIntegrations });

expect(browserInitSpy).toHaveBeenCalledTimes(1);

const options = browserInitSpy.mock.calls[0][0] || {};
expect(options.defaultIntegrations).toEqual(defaultIntegrations);
},
);
});
});

0 comments on commit e5e6a6b

Please sign in to comment.