Skip to content

Commit

Permalink
ref(angular) Add error-like objects handling (#6446)
Browse files Browse the repository at this point in the history 
It is more beneficial to handle error shaped objects instead of just instances of `Error`s
  • Loading branch information
@theofidry
theofidry authored Jan 9, 2023
1 parent 7316b85 commit f052394
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 31 deletions.
39 changes: 32 additions & 7 deletions packages/angular/src/errorhandler.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { HttpErrorResponse } from '@angular/common/http';
import { ErrorHandler as AngularErrorHandler, Inject, Injectable } from '@angular/core';
import * as Sentry from '@sentry/browser';
import { captureException } from '@sentry/browser';
import { addExceptionMechanism } from '@sentry/utils';
import { addExceptionMechanism, isString } from '@sentry/utils';

import { runOutsideAngular } from './zone';

Expand Down Expand Up @@ -32,7 +32,7 @@ function tryToUnwrapZonejsError(error: unknown): unknown | Error {

function extractHttpModuleError(error: HttpErrorResponse): string | Error {
// The `error` property of http exception can be either an `Error` object, which we can use directly...
if (error.error instanceof Error) {
if (isErrorOrErrorLikeObject(error.error)) {
return error.error;
}

Expand All @@ -50,6 +50,31 @@ function extractHttpModuleError(error: HttpErrorResponse): string | Error {
return error.message;
}

type ErrorCandidate = {
name?: unknown;
message?: unknown;
stack?: unknown;
};

function isErrorOrErrorLikeObject(value: unknown): value is Error {
if (value instanceof Error) {
return true;
}

if (value === null || typeof value !== 'object') {
return false;
}

const candidate = value as ErrorCandidate;

return (
isString(candidate.name) &&
isString(candidate.name) &&
isString(candidate.message) &&
(undefined === candidate.stack || isString(candidate.stack))
);
}

/**
* Implementation of Angular's ErrorHandler provider that can be used as a drop-in replacement for the stock one.
*/
Expand Down Expand Up @@ -117,16 +142,16 @@ class SentryErrorHandler implements AngularErrorHandler {
protected _defaultExtractor(errorCandidate: unknown): unknown {
const error = tryToUnwrapZonejsError(errorCandidate);

// We can handle messages and Error objects directly.
if (typeof error === 'string' || error instanceof Error) {
return error;
}

// If it's http module error, extract as much information from it as we can.
if (error instanceof HttpErrorResponse) {
return extractHttpModuleError(error);
}

// We can handle messages and Error objects directly.
if (typeof error === 'string' || isErrorOrErrorLikeObject(error)) {
return error;
}

// Nothing was extracted, fallback to default error message.
return null;
}
Expand Down
33 changes: 9 additions & 24 deletions packages/angular/test/errorhandler.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ class CustomError extends Error {
}

class ErrorLikeShapedClass implements Partial<Error> {
constructor(public message: string) {}
constructor(public name: string, public message: string) {}
}

function createErrorEvent(message: string, innerError: any): ErrorEvent {
Expand Down Expand Up @@ -118,8 +118,7 @@ describe('SentryErrorHandler', () => {
createErrorHandler().handleError(errorLikeWithoutStack);

expect(captureExceptionSpy).toHaveBeenCalledTimes(1);
// TODO: to be changed; see https://github.com/getsentry/sentry-javascript/issues/6332
expect(captureExceptionSpy).toHaveBeenCalledWith('Handled unknown error', expect.any(Function));
expect(captureExceptionSpy).toHaveBeenCalledWith(errorLikeWithoutStack, expect.any(Function));
});

it('extracts an error-like object with a stack', () => {
Expand All @@ -132,8 +131,7 @@ describe('SentryErrorHandler', () => {
createErrorHandler().handleError(errorLikeWithStack);

expect(captureExceptionSpy).toHaveBeenCalledTimes(1);
// TODO: to be changed; see https://github.com/getsentry/sentry-javascript/issues/6332
expect(captureExceptionSpy).toHaveBeenCalledWith('Handled unknown error', expect.any(Function));
expect(captureExceptionSpy).toHaveBeenCalledWith(errorLikeWithStack, expect.any(Function));
});

it('extracts an object that could look like an error but is not (does not have a message)', () => {
Expand All @@ -150,7 +148,6 @@ describe('SentryErrorHandler', () => {

it('extracts an object that could look like an error but is not (does not have an explicit name)', () => {
const notErr: Partial<Error> = {
// missing name; but actually is always there as part of the Object prototype
message: 'something failed.',
};

Expand Down Expand Up @@ -194,12 +191,12 @@ describe('SentryErrorHandler', () => {
});

it('extracts an instance of class not extending Error but that has an error-like shape', () => {
const err = new ErrorLikeShapedClass('something happened');
const err = new ErrorLikeShapedClass('sentry-error', 'something happened');

createErrorHandler().handleError(err);

expect(captureExceptionSpy).toHaveBeenCalledTimes(1);
expect(captureExceptionSpy).toHaveBeenCalledWith('Handled unknown error', expect.any(Function));
expect(captureExceptionSpy).toHaveBeenCalledWith(err, expect.any(Function));
});

it('extracts an instance of a class that does not extend Error and does not have an error-like shape', () => {
Expand Down Expand Up @@ -304,11 +301,7 @@ describe('SentryErrorHandler', () => {
createErrorHandler().handleError(err);

expect(captureExceptionSpy).toHaveBeenCalledTimes(1);
// TODO: to be changed; see https://github.com/getsentry/sentry-javascript/issues/6332
expect(captureExceptionSpy).toHaveBeenCalledWith(
'Http failure response for (unknown url): undefined undefined',
expect.any(Function),
);
expect(captureExceptionSpy).toHaveBeenCalledWith(errorLikeWithoutStack, expect.any(Function));
});

it('extracts an `HttpErrorResponse` with error-like object with a stack', () => {
Expand All @@ -322,11 +315,7 @@ describe('SentryErrorHandler', () => {
createErrorHandler().handleError(err);

expect(captureExceptionSpy).toHaveBeenCalledTimes(1);
// TODO: to be changed; see https://github.com/getsentry/sentry-javascript/issues/6332
expect(captureExceptionSpy).toHaveBeenCalledWith(
'Http failure response for (unknown url): undefined undefined',
expect.any(Function),
);
expect(captureExceptionSpy).toHaveBeenCalledWith(errorLikeWithStack, expect.any(Function));
});

it('extracts an `HttpErrorResponse` with an object that could look like an error but is not (does not have a message)', () => {
Expand All @@ -347,7 +336,6 @@ describe('SentryErrorHandler', () => {

it('extracts an `HttpErrorResponse` with an object that could look like an error but is not (does not have an explicit name)', () => {
const notErr: Partial<Error> = {
// missing name; but actually is always there as part of the Object prototype
message: 'something failed.',
};
const err = new HttpErrorResponse({ error: notErr });
Expand Down Expand Up @@ -453,16 +441,13 @@ describe('SentryErrorHandler', () => {
});

it('extracts an `HttpErrorResponse` with an instance of class not extending Error but that has an error-like shape', () => {
const innerErr = new ErrorLikeShapedClass('something happened');
const innerErr = new ErrorLikeShapedClass('sentry-error', 'something happened');
const err = new HttpErrorResponse({ error: innerErr });

createErrorHandler().handleError(err);

expect(captureExceptionSpy).toHaveBeenCalledTimes(1);
expect(captureExceptionSpy).toHaveBeenCalledWith(
'Http failure response for (unknown url): undefined undefined',
expect.any(Function),
);
expect(captureExceptionSpy).toHaveBeenCalledWith(innerErr, expect.any(Function));
});

it('extracts an `HttpErrorResponse` with an instance of a class that does not extend Error and does not have an error-like shape', () => {
Expand Down

0 comments on commit f052394

Please sign in to comment.