Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ref(angular) Add error-like objects handling #6446

Merged
merged 3 commits into from Jan 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
39 changes: 32 additions & 7 deletions packages/angular/src/errorhandler.ts
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
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