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

Angular Integration Issue - console.log calls cause Change detection to run #1883

Closed
4 of 8 tasks
ibedard16 opened this issue Feb 11, 2019 · 8 comments
Closed
4 of 8 tasks

Comments

@ibedard16
Copy link

ibedard16 commented Feb 11, 2019

Package + Version

  • @sentry/browser
  • @sentry/node
  • raven-js
  • raven-node (raven for node)
  • other:

Version:

4.5.4

Description

Using Sentry in an Angular project will cause Angular's change detection to fire any time console.log is called. This is because of Sentry's breadcrumb feature, which logs whatever was logged to the console, and how that interacts with zone.js.

I do not understand all of the mechanics behind it, but zone.js triggers change detection after the code to add a breadcrumb exits. An easy way to prevent the problem would be to wrap console.log statement to runOutsideAngular, which prevents zone.js from running change detection. However, the only two ways of doing would be either wrapping every call to console.log (which would be very unwieldy and not-obvious) or override the global console.log function with a different function that calls the original function outside of Angular.

I'm not entirely certain what can be done on Sentry's side to prevent the problem, as any solution would likely be very Angular-specific, in a package meant to be used with a variety of different frameworks. As such, I'm opening issues both here and against the Angular repo.

Edit: angular/angular#28647

Demo repro: https://github.com/ibedard16/sentry-recursion-demo
Instructions on how to run it are in the readme.

@ibedard16
Copy link
Author

Chrome crashing

Update: leaving the demo running for long enough crashed Chrome.
I don't mean it caused the tab to crash, I mean Chrome itself - every tab and window I had opened - crashed.
That's not a good thing.

@ibedard16
Copy link
Author

The Angular side of this issue was closed as won't fix. In the comments, the closer said

I don't think either of the two libraries [zone.js and sentry] can do much to mitigate this. ... I don't think there is a feasible fix for either side

@omares
Copy link

omares commented May 13, 2019

In version 5.2.0 this issue still exists. Just stumbled across the problem in our application and cant find any solution besides not using any console calls. Using zone.runOutsideAngular does not fix the problem entirely as it still triggers a lot of errors.

I will continue to investigate, but my hopes are not high to solve this.

Any hints or tips on how to tackle this would be really appreciated.

Edit: I have to revise the statement above, the issue we faced did not occur due to console.log and the custom wrapping. We had an issue in our application that caused angulars change detection to be retriggered in an endless loop. Applying a best practice (writing the result into a property of the component instead of calling the service directly inside the template) fixed the issue for us.

@ibedard16
Copy link
Author

@omares Your issue might have been related to this other ticket of mine: angular/angular#27804

@michelepatrassi
Copy link

same issue, does not only happens for lifecyle methods but also for getters.

There is any plan for this to be addressed?

@kamilogorek
Copy link
Contributor

kamilogorek commented Sep 13, 2019

I'll need to find some time to dig into angular zone package to see why/how it happens.

In the meantime, if console.log breadcrumbs are not crucial, they can be separately disabled with:

Sentry.init({
  // ...
  integrations: [new Sentry.Integrations.Breadcrumbs({ console: false })]
  // ...
})

@thduttonuk
Copy link

Adding this causes an infinite loop when calling console.error

const integrations = [ new CaptureConsole({ levels: ['error'], }), ];

console.error triggers CD to run so we get in a loop if a template has an error.

@kamilogorek
Copy link
Contributor

Closing the issue as a part of large repository cleanup, due to it being inactive and/or outdated. There's a large chance that this issue has been solved by 113be3a
Please do not hesitate to ping me if it is still relevant, and I will happily reopen and work on it.
Cheers!

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