Skip to content

Angular: Sentry.init sends errors without captureException() #2169

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

Closed
anikets43 opened this issue Jul 18, 2019 · 13 comments
Closed

Angular: Sentry.init sends errors without captureException() #2169

anikets43 opened this issue Jul 18, 2019 · 13 comments

Comments

@anikets43
Copy link

anikets43 commented Jul 18, 2019

I have a project where I followed the Sentry angular documentation at:
https://docs.sentry.io/platforms/javascript/angular/
And it worked fine and we were getting errors.

Seemed to work find for 7+ months. Soon as we started getting more users, we noticed that the number of errors in Sentry had increased a lot - a few 10s of thousands more than what we expected.

On debugging, I tried just disabling Sentry and was surprised to see quite a lot of errors still being reported!

I am using Angular 7 with "@sentry/browser": "^5.5.0",

The error is from a network call that is giving me a unauthorized because some users try to access something they are not supposed to.
We catch these and show them a "Unauthorized" page appropriately. So, I don't want Sentry to report these!

The way we set it up is:

import * as Sentry from '@sentry/browser';
import { ErrorHandler, Injectable } from '@angular/core';


@Injectable()
export class SentryErrorHandler implements ErrorHandler {
    constructor() {
        Sentry.init({dsn: '.....'});
    }
    handleError(error: Error) {
        if (error instanceof Error) {
            console.error(error);
            // Sentry.captureException(error); // <--- Even after commenting this out, Errors are reported.
        }
    }
}

I'm trying to figure out what is reporting these errors.
I see the stacktrace to be from Angular's zone modules. but I haven't configured it to report anything there to the best of my knowledge.

Screen Shot 2019-07-18 at 3 12 38 PM

@AbdealiLoKo
Copy link

AbdealiLoKo commented Jul 18, 2019

Dug into this a bit.

The Stack Trace seems to indicate that sentryWrapped is the one at:
https://github.com/getsentry/sentry-javascript/blob/5.5.0/packages/browser/src/helpers.ts#L36

Where there is a captureException at:
https://github.com/getsentry/sentry-javascript/blob/5.5.0/packages/browser/src/helpers.ts#L106

I am assuming these are coming from the Breadcrumbs and TryCatch default integrations ?

@kamilogorek
Copy link
Contributor

Only from TryCatch, Breadcrumbs don't send any events itself.
I can help you debug if you link me to the event you posted above.
It'd be also great if you configure your release and upload source maps for the files listed in the stacktrace.

@AbdealiLoKo
Copy link

https://sentry.io/organizations/corridor-dev/issues/1118900329/
is an example, although not the same as the example described in the original issue description above.

@kamilogorek
Copy link
Contributor

@AbdealiJK, unfortunately, you use external VPN and source maps are not available. Is there a chance you could upload them? https://docs.sentry.io/platforms/javascript/sourcemaps/

@michelepatrassi
Copy link

same problem here, I reproduced the problem with stackblitz: https://angular-tb2kgw.stackblitz.io.

As you open the app there are two buttons: one throw a classical error while the other one throw an error because of a failed http request. The former is sent one, the latter twice! Go into the app.component and update the SENTRY_DSN to reproduce it.

This is what I get in the sentry dashboard

image

looks like a captureException is called internally by some helpers, that causes the double error in sentry. I would like to have in sentry only exceptions that I explicitly send, because in my app I have the same/similar event twice

@kamilogorek
Copy link
Contributor

@Iar0 this is caused by Angular http client using setTimeout to detect http call progress. And when the call fails, it throws an exception inside that timeout that bubbles up higher to the main Angular's error handler.

You can either disable this integration completely or if you still want to make sure that your click handlers and other timing APIs are wrapped correctly, filter this specific type of errors.

Sentry.init({
  integrations(integrations) {
    return integrations.filter(i => i.name !== "TryCatch");
  }
});

or

Sentry.init({
  beforeSend(event) {
    try {
      if (event.extra.__serialized__.error === "ProgressEvent") {
        return null;
      }
    } catch (o_O) {}

    return event;
  }
});

(this try/catch is only so you don't have to write event.extra && event.extra.__serialized__ && event.extra.__serialized__.error === "ProgressEvent" as null coercion is still not available in JS :P)

@michelepatrassi
Copy link

@kamilogorek thx! what is exactly the TryCatch integration that you're filtering there? Just to understand what I'm doing if I use that code

@kamilogorek
Copy link
Contributor

It wraps setTimeout, setInterval, requestAnimationFrame and add/removeEventListener in try/catch clause and extracts some metadata before calling the function (as this behavior is not unified in all the browsers).
This way we can tell that the error indeed happened for example during rAF iteration, provide main function name or dom target node if applied to an event listener.

Whole code can be found here: https://github.com/getsentry/sentry-javascript/blob/master/packages/browser/src/integrations/trycatch.ts

@MEChasle
Copy link

MEChasle commented Mar 3, 2020

Hi,
@kamilogorek, thank you for your answers #2169 (comment) and #2169 (comment).

I need precision on :

this is caused by Angular http client using setTimeout

If I well understand, for all Angular project, this integration setTimeout is useless.
Is it the only one integration which could be useless ?
Must Angular have responsibilities to handle error or is Sentry responsability ?
Is it possible to update Angular integration in https://docs.sentry.io/platforms/javascript/angular to remove by default useless integration ?

Thanks for your answer

@kamilogorek
Copy link
Contributor

It's not useless. It'll just point to one of the frames as it'd come from the SDK, as it wraps the native API. It'll still function just fine.

@s5no5t
Copy link

s5no5t commented Apr 1, 2020

I am running into this issue, too. However, the second (more targeted) solution proposed by @kamilogorek doesn't seem to work for me. The event passed into the given callback function doesn't have the error property set to ProgressEvent. Instead, it is a plain js object with message set to whatever error message string the http call returned.
The first solution (disabling the TryCatch integration) does work however.
Could you describe what kind of events can be missed when the integration is disabled and all errors are caught through an Angular SentryErrorHandler?

@kamilogorek
Copy link
Contributor

Could you describe what kind of events can be missed when the integration is disabled and all errors are caught through an Angular SentryErrorHandler?

You won't miss any errors per-se, but you won't have all the metadata that could be possibly extracted (function name for event handlers, event object itself etc.).

@s5no5t
Copy link

s5no5t commented Apr 2, 2020

@kamilogorek thank you for the explanation, this is helpful to understand the tradeoffs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants