-
-
Notifications
You must be signed in to change notification settings - Fork 308
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: Use proper application state before sending an event #4005
Conversation
Added CHANGELOG entry
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.
Thanks a lot, @bguidolim. Please add some tests in SentryClient for that. Then we can merge this.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4005 +/- ##
=============================================
+ Coverage 90.863% 90.889% +0.025%
=============================================
Files 593 594 +1
Lines 46506 46647 +141
Branches 16624 16688 +64
=============================================
+ Hits 42257 42397 +140
- Misses 4067 4069 +2
+ Partials 182 181 -1
... and 16 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Hey @philipphofmann Do I need to do something regarding the failed checks? |
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.
LGTM, thanks 💯
No action is required. That's out of your control. |
📜 Description
Replaced source of truth for app state from
SentryAppStateManager
instance toSentryUIApplication
.💡 Motivation and Context
This change is required since it's sending wrong value to for all events in an UIKit application.
This PR fixes #3991
💚 How did you test it?
Existing unit tests pass
📝 Checklist
You have to check all boxes before merging:
sendDefaultPII
is enabled.🔮 Next steps