-
Notifications
You must be signed in to change notification settings - Fork 81
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
fix(sentry): remove problematic Sentry integration #488
base: master
Are you sure you want to change the base?
Conversation
* Use Sentry Breadcrumbs mechanism to log information about occured errors. | ||
* It can override global objects like console, error etc. Defaults to `true`. | ||
*/ | ||
useSentryBreadcrumbs?: boolean; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm unsure how this is used.
This module also needs to be formatted with Prettier, but I minimized the diff for readability.
5a0401f
to
75950ff
Compare
@sebastian-wec, @pcholuj – do you have any thoughts on this change? This is currently blocking our ability to upgrade to Sentry's latest major version. |
This seems to be causing us issues too when upgrading to the latest version of sentry. |
@johnkoehn Hey, I see this is approved. How do we go about getting this merged? |
No clue @ryan-gray-db, I just approved it for the heck of it. All the engineers at filestack left after the company was acquired. It is a dead product. |
Haha, that's good to know... We had no idea. Thank you! |
@sethk4783, @Yaminim07, @prem-celestial, @hemanth-3, @RatGabi (with recent activity in this repository) – are you able to help get this change shipped? @johnkoehn and I have been repeatedly requesting that this pull request be merged for nearly a year. Unfortunately, the delay in addressing this matter has significantly hindered our ability to upgrade Sentry to its latest version across our applications. |
@namoscato I have tried to use your branch in the package.json, but it was throwing some errors, so tried to create my own fork and make it work. in case anybody needs - |
Resolves #467
Resolves #482
What kind of change does this PR introduce?
Bug fix and arguably breaking change
What is the current behavior? (You can also link to an open issue here)
The current Sentry integration:
What is the new behavior (if this is a feature change)?
This removes the Sentry integration in favor of users implementing this at their application layer, which seems simpler and more appropriate.