Skip to content

Http request exceptions captured twice in Angular 9 #2532

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
lujakob opened this issue Apr 2, 2020 · 13 comments
Closed

Http request exceptions captured twice in Angular 9 #2532

lujakob opened this issue Apr 2, 2020 · 13 comments

Comments

@lujakob
Copy link

lujakob commented Apr 2, 2020

Package + Version

@sentry/[email protected]

Angular Version 9

Description

Implementing Sentry with Angular following the docs captures two event for http requests that return a 401 error.

I implemented a minimal version in Stackblitz

In app.component.ts a request to a protected API throws a 401 error. The console logs show the beforeSend logs having two events captures, also visible in the networks tab.

Replace the sentry init config dsn to use a different Sentry Account to see the logs in sentry.

If Sentry.captureException is outcommented in app.module, still there is one event captured.

@HazAT
Copy link
Member

HazAT commented Apr 3, 2020

Thank you for the 🥇 bug report.
We will take a look, what's going on.

@HazAT
Copy link
Member

HazAT commented Apr 3, 2020

Just collecting my observations so far, if you disable the
TryCatch integration it's not sent twice

integrations(integrations) {
    return integrations.filter(
      integration => integration.name !== "TryCatch"
    );
  },

@HazAT
Copy link
Member

HazAT commented Apr 3, 2020

@kamilogorek do you have a better idea? It seems that by wrapping XHR and/or all other methods this will be reported twice no matter what. We are also not marking the error so we would be able to filter it out in the Angular Error handler.

@josh-m-sharpe
Copy link

It seems like disabling the try/catch integration should be a non-option since that would obviously hide any errors that occur outside of Angular.

@josh-m-sharpe
Copy link

Can we get a sitrep here? I'm a paying customer and timely responses would be appreciated. Thanks!

@kamilogorek
Copy link
Contributor

It seems like disabling the try/catch integration should be a non-option since that would obviously hide any errors that occur outside of Angular.

That is not accurate. See: #2169 (comment)

@kamilogorek
Copy link
Contributor

Closing the issue as a part of large repository cleanup, due to it being inactive and/or outdated.
Please do not hesitate to ping me if it is still relevant, and I will happily reopen and work on it.
Cheers!

@lujakob
Copy link
Author

lujakob commented Jul 2, 2021

@kamilogorek this issue is still relevant. For error in http requests, Sentry events are sent twice. You can test it running the stackblitz example i posted in the description, verifying the console.log output. I still think this is an important issue to not pollute the Sentry errors list. Thanks.

@kamilogorek
Copy link
Contributor

@lujakob thanks for letting me know. Did some debugging around this issue and it appears that HttpClient (which is using XHR underneath) is interacting with Zone.js in a way, where request timeouts are triggered inside setTimeout loop. This means that whenever it throws, our setTimeout instrumentation will catch it, in addition to it being passed to Angular error handler.

There are 2 options. Disabling setTimeout instrumentation (not preferable):

import * as Sentry from '@sentry/angular';

Sentry.init({
  dsn: '...',
  integrations: [new Sentry.Integrations.TryCatch({
    setTimeout: false,
  })]
});

@NgModule({
  imports:      [ BrowserModule, FormsModule, HttpClientModule ],
  declarations: [ AppComponent, HelloComponent ],
  providers: [{ provide: ErrorHandler, useValue: Sentry.createErrorHandler() }],
  bootstrap:    [ AppComponent ]
})
export class AppModule { }

or filtering out HttpErrorResponse and migrating to @sentry/angular that handler deals with this type of error, extracting correct data (preferable). Keep in mind that it requires Angular update to at least v9, as we need TS3.5+ for a correct compilation - https://update.angular.io/?v=8.0-9.0 :

import * as Sentry from '@sentry/angular';

Sentry.init({
  dsn: 'https://[email protected]/1352844',
  beforeSend(event, hint) {
    // Handled by NgModule's ErrorHandler
    if (hint?.originalException instanceof HttpErrorResponse) {
      return null;
    }
    return event;
  }
});

@NgModule({
  imports:      [ BrowserModule, FormsModule, HttpClientModule ],
  declarations: [ AppComponent, HelloComponent ],
  providers: [{ provide: ErrorHandler, useValue: Sentry.createErrorHandler() }],
  bootstrap:    [ AppComponent ]
})
export class AppModule { }

Repro:
image

After update to @sentry/[email protected]:
image

After update to @sentry/[email protected] and using a correct error handler (https://github.com/getsentry/sentry-javascript/tree/master/packages/angular#errorhandler):
image

After update to @sentry/[email protected], using a correct error handler and using beforeSend filter for HttpErrorResponse:
image

Hope that helps!
Cheers

@lujakob
Copy link
Author

lujakob commented Jul 9, 2021

@kamilogorek your second proposal using @sentry/angular seems to work. That's awesome, thanks very much. So for me the issue is resolved then.
(Not sure anymore, I guess we originally implemented using a custom error handler because at the time there was no @sentry/angular available.)

@kamilogorek
Copy link
Contributor

(Not sure anymore, I guess we originally implemented using a custom error handler because at the time there was no @sentry/angular available.)

Correct, there was none back then :) Glad to hear that, thanks!

@pfei5
Copy link

pfei5 commented Mar 4, 2022

@kamilogorek @sentry/angular continues to report duplicate events for Http errors: https://github.com/pfei5/sentry-duplicate-events.git
Your workaround works for me, i.e.:

Sentry.init({
  beforeSend(event, hint) {
    if (hint?.originalException instanceof HttpErrorResponse) {
      return null;
    }
    return event;
  }
});

It is however disconcerting that I need to research for hours to solve a known yet undocumented issue that's been in the wild for years.

related: #2744

@kamilogorek
Copy link
Contributor

cc @Lms24

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

5 participants