Angular - errors are reported multiple times

[kamilchlebek]

Package + Version

• @sentry/browser
• @sentry/integrations

Version:

^5.19.1

Description

I'm having an issue with errors being reported multiple times. One error is correctly catched via angular ErrorHandler, second one is catched by captureException executed inside sentryWrapped function.

I tried proposed solution for angular, shown below:

// TryCatch has to be configured to disable XMLHttpRequest wrapping, as we are going to handle
// http module exceptions manually in Angular's ErrorHandler and we don't want it to capture the same error twice.
// Please note that TryCatch configuration requires at least @sentry/browser v5.16.0.
integrations: [new Sentry.Integrations.TryCatch({
    XMLHttpRequest: false,
})],

but it didn't help. To block additional error reports I had to set:

setTimeout: false

It's just a workaround that I'd rather not use, since errors may also occur outside angular.

I started debugging which setTimeout was the root cause of duplicated reports. I found that it's caused by hostReportError from rxjs ( https://github.com/ReactiveX/rxjs/blob/f5278aa89ea164caf5cf10e77d7bd00eff26fc0f/src/internal/util/hostReportError.ts#L6 )

Starting from rxjs v6.0.0 they changed the way of reporting the errors from sync to async mode.

error handling: Unhandled errors are no longer caught and rethrown, rather they are caught and scheduled to be thrown, which causes them to be reported to window.onerror or process.on('error'), depending on the environment.
https://github.com/ReactiveX/rxjs/blob/master/CHANGELOG.md#breaking-changes-10

As a part from investigation I tried to switch back to sync error reporting (using rxjs config.useDeprecatedSynchronousErrorHandling = true).

After this change, to stop duplicated error reports I had to set TryCatch options to eventTarget: false (setTimeout: false was no more needed).

Seems that Sentry is wrapping addEventListener for angular http client https://github.com/angular/angular/blob/master/packages/common/http/src/xhr.ts#L311. Setting XMLHttpRequest: false doesn't work.

As a temporary fix I'll probably check error report in beforeSend and skip unwanted ones, but could you fix it internally? Thanks :)

Related issues

#2533
#2532
#2169

[kamilogorek]

I'm currently working on @sentry/angular specific package. I'll make sure to take this into account, thanks!

[kamilchlebek]

Great! Thank you. Looking forward to new package.

[PhilippMeissner]

I'm currently working on @sentry/angular specific package. I'll make sure to take this into account, thanks!

Sounds great! Let us know when we can be of any help :)

[szechyjs]

I'm currently working on @sentry/angular specific package. I'll make sure to take this into account, thanks!

Looking forward to this, would love for these Non-Error exception captured with keys: error, headers, message, name, ok errors to stop.

[EdwardHinkle]

Any update on this? It's been over six months

[pranav-js]

I still get multiple events with @sentry/angular

[vladanpaunovic]

@pranav-js @kamilchlebek

We want to get to the bottom of this and we need a bit of your help.

We need a reproduction of an app where this is happening. Please, submit one and we will look into it.

[pfei5]

@pranav-js @kamilchlebek

We want to get to the bottom of this and we need a bit of your help.

We need a reproduction of an app where this is happening. Please, submit one and we will look into it.

I am able to reproduce this issue with a bare bones Angular setup: https://github.com/pfei5/sentry-duplicate-events.git

As soon as I provide the Sentry errorHandler, duplicate events are reported for HttpErrorResponses.

The workaround proposed here works, but it took me hours to figure this out. It is documented nowhere, as far as I can tell, and this issue has been out in the wild for years. Decluttering Sentry would be a good place to document this sort of behaviour, IMHO.

[github-actions]

This issue has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you label it Status: Backlog or Status: In Progress, I will leave it alone ... forever!

---

"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

[pfei5]

This is, IMHO, a serious issue and should not be closed @github-actions.

[github-actions]

This issue has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you label it Status: Backlog or Status: In Progress, I will leave it alone ... forever!

---

"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

[pfei5]

@pranav-js @kamilchlebek
We want to get to the bottom of this and we need a bit of your help.
We need a reproduction of an app where this is happening. Please, submit one and we will look into it.

I am able to reproduce this issue with a bare bones Angular setup: https://github.com/pfei5/sentry-duplicate-events.git

As soon as I provide the Sentry errorHandler, duplicate events are reported for HttpErrorResponses.

The workaround proposed here works, but it took me hours to figure this out. It is documented nowhere, as far as I can tell, and this issue has been out in the wild for years. Decluttering Sentry would be a good place to document this sort of behaviour, IMHO.

As far as I can tell this issue is as hot as ever. Do not close @github-actions.

[pfei5]

@vladanpaunovic or someone else could you please stop this issue from automatically being closed. As far as I can tell it has not been fixed. Also I did provide a setup to reproduce this issue: #2744 (comment)

[Rafik-Belkadi-malou]

Any updates on this ?

[LoganDupont]

@HazAT is it possible to update the labels for this issue? Like removing the label Status: Needs Reproduction?

[Lms24]

Hi folks, sorry for letting this slip through our radar! I recently looked at #5417 which is a slightly different report but I believe the root cause is the same in both issues: The TryCatch integration seems to step in to catch errors in timer functions before our Angular-specific ErrorHandler catches the error again.

I wrote up a workaround in the issue, which basically boils down to disabeling the TryCatch integration. As Far as I can tell, this seems to be pretty safe, considering that timer errors are caught by the Angular error handler anyway. As I wrote in the issue though, excluding TryCatch by default in the SDK has the drawback that incomplete SDK setups will definitely miss certain errors.

I'm not entirely against excluding it though, so feel free to leave some thoughts what you'd prefer.

Does this solve the issue for you all?

[LoganDupont]

Hi folks, sorry for letting this slip through our radar! I recently looked at #5417 which is a slightly different report but I believe the root cause is the same in both issues: The TryCatch integration seems to step in to catch errors in timer functions before our Angular-specific ErrorHandler catches the error again.

I wrote up a workaround in the issue, which basically boils down to disabeling the TryCatch integration. As Far as I can tell, this seems to be pretty safe, considering that timer errors are caught by the Angular error handler anyway. As I wrote in the issue though, excluding TryCatch by default in the SDK has the drawback that incomplete SDK setups will definitely miss certain errors.

I'm not entirely against excluding it though, so feel free to leave some thoughts what you'd prefer.

Does this solve the issue for you all?

Thank you for the quick response! The workaround fixed my problem.

[Lms24]

I'll close this issue for the time being, as the workaround seems to fix the problem. Feel free to tag me if you think it should be repoened.

[LoganDupont]

@Lms24 Does this mean the issue won't be fixed?

[LoganDupont]

@Lms24 I think this issue needs to be reopened. Yes there is currently a workaround but the initial issue is still not fixed until now. I don't see an other issue which will remind the Sentry team to fix this as they are all closed.

[Lms24]

Hi, sorry for taking so long. We're going to disable TryCatch by default which should fix this problem for good and no longer require the workaround. We're just swamped with work at the moment but I'll try to get this in later this week 🤞