From 438e74f6add4fb713a980d3f5e1540990e8502b4 Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Tue, 27 Jul 2021 13:17:16 +0000 Subject: [PATCH 1/7] feat(browser): Allow mechanism to be provided as a hint. - Provide 'angular' as the mechanism for handled angular errors. --- packages/angular/src/errorhandler.ts | 5 ++++- packages/browser/src/eventbuilder.ts | 22 ++++++++++++++++++++-- 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/packages/angular/src/errorhandler.ts b/packages/angular/src/errorhandler.ts index 54449071ee89..76044e5e2013 100644 --- a/packages/angular/src/errorhandler.ts +++ b/packages/angular/src/errorhandler.ts @@ -1,6 +1,7 @@ import { HttpErrorResponse } from '@angular/common/http'; import { ErrorHandler as AngularErrorHandler, Inject, Injectable } from '@angular/core'; import * as Sentry from '@sentry/browser'; +import { getCurrentHub } from '@sentry/browser'; import { runOutsideAngular } from './zone'; @@ -40,7 +41,9 @@ class SentryErrorHandler implements AngularErrorHandler { const extractedError = this._extractError(error) || 'Handled unknown error'; // Capture handled exception and send it to Sentry. - const eventId = runOutsideAngular(() => Sentry.captureException(extractedError)); + const eventId = runOutsideAngular(() => + getCurrentHub().captureException(extractedError, { data: { mechanism: { type: 'angular', handled: false } } }), + ); // When in development mode, log the error to console for immediate feedback. if (this._options.logErrors) { diff --git a/packages/browser/src/eventbuilder.ts b/packages/browser/src/eventbuilder.ts index 2324fcd8b296..18b24edc1e40 100644 --- a/packages/browser/src/eventbuilder.ts +++ b/packages/browser/src/eventbuilder.ts @@ -1,4 +1,13 @@ -import { Event, EventHint, Exception, Severity, SeverityLevel, StackFrame, StackParser } from '@sentry/types'; +import { + Event, + EventHint, + Exception, + Mechanism, + Severity, + SeverityLevel, + StackFrame, + StackParser, +} from '@sentry/types'; import { addExceptionMechanism, addExceptionTypeValue, @@ -149,8 +158,17 @@ export function eventFromException( ): PromiseLike { const syntheticException = (hint && hint.syntheticException) || undefined; const event = eventFromUnknownInput(stackParser, exception, syntheticException, attachStacktrace); - addExceptionMechanism(event); // defaults to { type: 'generic', handled: true } + const providedMechanism: Mechanism | undefined = + hint && hint.data && (hint.data as { mechanism: Mechanism }).mechanism; + const mechanism: Mechanism = providedMechanism || { + handled: true, + type: 'generic', + }; + + addExceptionMechanism(event, mechanism); + event.level = 'error'; + if (hint && hint.event_id) { event.event_id = hint.event_id; } From 755a5193a04bff18a1e522c7b59f2fd147eae7ca Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Wed, 20 Jul 2022 16:22:40 +0100 Subject: [PATCH 2/7] Use event processor to pass mechanism. --- packages/angular/src/errorhandler.ts | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/packages/angular/src/errorhandler.ts b/packages/angular/src/errorhandler.ts index 76044e5e2013..9c091f2a2b16 100644 --- a/packages/angular/src/errorhandler.ts +++ b/packages/angular/src/errorhandler.ts @@ -1,7 +1,8 @@ import { HttpErrorResponse } from '@angular/common/http'; import { ErrorHandler as AngularErrorHandler, Inject, Injectable } from '@angular/core'; import * as Sentry from '@sentry/browser'; -import { getCurrentHub } from '@sentry/browser'; +import { captureException } from '@sentry/browser'; +import { addExceptionMechanism } from '@sentry/utils'; import { runOutsideAngular } from './zone'; @@ -42,7 +43,18 @@ class SentryErrorHandler implements AngularErrorHandler { // Capture handled exception and send it to Sentry. const eventId = runOutsideAngular(() => - getCurrentHub().captureException(extractedError, { data: { mechanism: { type: 'angular', handled: false } } }), + captureException(extractedError, scope => { + scope.addEventProcessor(event => { + addExceptionMechanism(event, { + type: 'angular', + handled: false, + }); + + return event; + }); + + return scope; + }), ); // When in development mode, log the error to console for immediate feedback. From 5d94e2abd1b8c03d03102d3eaeb27871c4f5e55f Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Wed, 20 Jul 2022 16:22:55 +0100 Subject: [PATCH 3/7] Add `errorhandler` tests. --- packages/angular/test/errorhandler.test.ts | 124 +++++++++++++++++++++ 1 file changed, 124 insertions(+) create mode 100644 packages/angular/test/errorhandler.test.ts diff --git a/packages/angular/test/errorhandler.test.ts b/packages/angular/test/errorhandler.test.ts new file mode 100644 index 000000000000..062de5525793 --- /dev/null +++ b/packages/angular/test/errorhandler.test.ts @@ -0,0 +1,124 @@ +import { createErrorHandler, SentryErrorHandler } from '../src/errorhandler'; + +import * as SentryBrowser from '@sentry/browser'; +import * as SentryUtils from '@sentry/utils'; +import { HttpErrorResponse } from '@angular/common/http'; +import { Scope } from '@sentry/browser'; + +const FakeScope = new Scope(); + +jest.mock('@sentry/browser', () => { + const original = jest.requireActual('@sentry/browser'); + return { + ...original, + captureException: (err: unknown, cb: (arg0?: unknown) => unknown) => { + cb(FakeScope); + return original.captureException(err, cb); + }, + }; +}); + +const captureExceptionSpy = jest.spyOn(SentryBrowser, 'captureException'); + +jest.spyOn(console, 'error').mockImplementation(); + +describe('SentryErrorHandler', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('`createErrorHandler `creates a SentryErrorHandler with an empty config', () => { + const errorHandler = createErrorHandler(); + + expect(errorHandler).toBeInstanceOf(SentryErrorHandler); + }); + + it('handleError method assigns the correct mechanism', () => { + const addEventProcessorSpy = jest.spyOn(FakeScope, 'addEventProcessor').mockImplementationOnce(callback => { + callback({}, { event_id: 'fake-event-id' }); + return FakeScope; + }); + + const addExceptionMechanismSpy = jest.spyOn(SentryUtils, 'addExceptionMechanism'); + + const errorHandler = createErrorHandler(); + errorHandler.handleError(new Error('test')); + + expect(addEventProcessorSpy).toBeCalledTimes(1); + expect(addExceptionMechanismSpy).toBeCalledTimes(1); + expect(addExceptionMechanismSpy).toBeCalledWith({}, { handled: false, type: 'angular' }); + }); + + it('handleError method extracts `null` error', () => { + createErrorHandler().handleError(null); + + expect(captureExceptionSpy).toHaveBeenCalledTimes(1); + expect(captureExceptionSpy).toHaveBeenCalledWith('Handled unknown error', expect.any(Function)); + }); + + it('handleError method extracts `undefined` error', () => { + createErrorHandler().handleError(undefined); + + expect(captureExceptionSpy).toHaveBeenCalledTimes(1); + expect(captureExceptionSpy).toHaveBeenCalledWith('Handled unknown error', expect.any(Function)); + }); + + it('handleError method extracts a string', () => { + const str = 'sentry-test'; + createErrorHandler().handleError(str); + + expect(captureExceptionSpy).toHaveBeenCalledTimes(1); + expect(captureExceptionSpy).toHaveBeenCalledWith(str, expect.any(Function)); + }); + + it('handleError method extracts an empty Error', () => { + const err = new Error(); + createErrorHandler().handleError(err); + + expect(captureExceptionSpy).toHaveBeenCalledTimes(1); + expect(captureExceptionSpy).toHaveBeenCalledWith(err, expect.any(Function)); + }); + + it('handleError method extracts an Error with `ngOriginalError`', () => { + const ngErr = new Error('sentry-ng-test'); + const err = { + ngOriginalError: ngErr, + }; + + createErrorHandler().handleError(err); + + expect(captureExceptionSpy).toHaveBeenCalledTimes(1); + expect(captureExceptionSpy).toHaveBeenCalledWith(ngErr, expect.any(Function)); + }); + + it('handleError method extracts an `HttpErrorResponse` with `Error`', () => { + const httpErr = new Error('sentry-http-test'); + const err = new HttpErrorResponse({ error: httpErr }); + + createErrorHandler().handleError(err); + + expect(captureExceptionSpy).toHaveBeenCalledTimes(1); + expect(captureExceptionSpy).toHaveBeenCalledWith(httpErr, expect.any(Function)); + }); + + it('handleError method extracts an `HttpErrorResponse` with `ErrorEvent`', () => { + const httpErr = new ErrorEvent('http', { message: 'sentry-http-test' }); + const err = new HttpErrorResponse({ error: httpErr }); + + createErrorHandler().handleError(err); + + expect(captureExceptionSpy).toHaveBeenCalledTimes(1); + expect(captureExceptionSpy).toHaveBeenCalledWith('sentry-http-test', expect.any(Function)); + }); + + it('handleError method extracts an `HttpErrorResponse` with string', () => { + const err = new HttpErrorResponse({ error: 'sentry-http-test' }); + createErrorHandler().handleError(err); + + expect(captureExceptionSpy).toHaveBeenCalledTimes(1); + expect(captureExceptionSpy).toHaveBeenCalledWith( + 'Server returned code 0 with body "sentry-http-test"', + expect.any(Function), + ); + }); +}); From 914aa5630d9e64888500fdf22843fc0dc6bfaf20 Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Wed, 20 Jul 2022 17:13:37 +0100 Subject: [PATCH 4/7] Fix linter. --- packages/angular/test/errorhandler.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/angular/test/errorhandler.test.ts b/packages/angular/test/errorhandler.test.ts index 062de5525793..228b1e916514 100644 --- a/packages/angular/test/errorhandler.test.ts +++ b/packages/angular/test/errorhandler.test.ts @@ -35,7 +35,7 @@ describe('SentryErrorHandler', () => { it('handleError method assigns the correct mechanism', () => { const addEventProcessorSpy = jest.spyOn(FakeScope, 'addEventProcessor').mockImplementationOnce(callback => { - callback({}, { event_id: 'fake-event-id' }); + void callback({}, { event_id: 'fake-event-id' }); return FakeScope; }); From bd1944d606b55f78384f4f5cb67f45c88cd5da96 Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Wed, 20 Jul 2022 17:14:18 +0100 Subject: [PATCH 5/7] Remove unneeded default fallback mechanism. --- packages/browser/src/eventbuilder.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/packages/browser/src/eventbuilder.ts b/packages/browser/src/eventbuilder.ts index 18b24edc1e40..5cd8e3418cd8 100644 --- a/packages/browser/src/eventbuilder.ts +++ b/packages/browser/src/eventbuilder.ts @@ -160,12 +160,8 @@ export function eventFromException( const event = eventFromUnknownInput(stackParser, exception, syntheticException, attachStacktrace); const providedMechanism: Mechanism | undefined = hint && hint.data && (hint.data as { mechanism: Mechanism }).mechanism; - const mechanism: Mechanism = providedMechanism || { - handled: true, - type: 'generic', - }; - addExceptionMechanism(event, mechanism); + addExceptionMechanism(event, providedMechanism); event.level = 'error'; From e8b9a862365e43e148f162719f9e8458936374ef Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Wed, 20 Jul 2022 19:49:01 +0100 Subject: [PATCH 6/7] Fix linter --- packages/angular/test/errorhandler.test.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/angular/test/errorhandler.test.ts b/packages/angular/test/errorhandler.test.ts index 228b1e916514..8c0059ae892b 100644 --- a/packages/angular/test/errorhandler.test.ts +++ b/packages/angular/test/errorhandler.test.ts @@ -1,9 +1,9 @@ -import { createErrorHandler, SentryErrorHandler } from '../src/errorhandler'; - -import * as SentryBrowser from '@sentry/browser'; -import * as SentryUtils from '@sentry/utils'; import { HttpErrorResponse } from '@angular/common/http'; +import * as SentryBrowser from '@sentry/browser'; import { Scope } from '@sentry/browser'; +import * as SentryUtils from '@sentry/utils'; + +import { createErrorHandler, SentryErrorHandler } from '../src/errorhandler'; const FakeScope = new Scope(); From 53a1d8b0bf41407eabb715726f5fe965944cfb4f Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Thu, 21 Jul 2022 15:08:01 +0100 Subject: [PATCH 7/7] Remove changes on `eventbuilder`. --- packages/browser/src/eventbuilder.ts | 18 ++---------------- 1 file changed, 2 insertions(+), 16 deletions(-) diff --git a/packages/browser/src/eventbuilder.ts b/packages/browser/src/eventbuilder.ts index 5cd8e3418cd8..2324fcd8b296 100644 --- a/packages/browser/src/eventbuilder.ts +++ b/packages/browser/src/eventbuilder.ts @@ -1,13 +1,4 @@ -import { - Event, - EventHint, - Exception, - Mechanism, - Severity, - SeverityLevel, - StackFrame, - StackParser, -} from '@sentry/types'; +import { Event, EventHint, Exception, Severity, SeverityLevel, StackFrame, StackParser } from '@sentry/types'; import { addExceptionMechanism, addExceptionTypeValue, @@ -158,13 +149,8 @@ export function eventFromException( ): PromiseLike { const syntheticException = (hint && hint.syntheticException) || undefined; const event = eventFromUnknownInput(stackParser, exception, syntheticException, attachStacktrace); - const providedMechanism: Mechanism | undefined = - hint && hint.data && (hint.data as { mechanism: Mechanism }).mechanism; - - addExceptionMechanism(event, providedMechanism); - + addExceptionMechanism(event); // defaults to { type: 'generic', handled: true } event.level = 'error'; - if (hint && hint.event_id) { event.event_id = hint.event_id; }