-
-
Notifications
You must be signed in to change notification settings - Fork 368
fix: Crash in setMeasurement when name is nil #5064
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
Conversation
Although the name for setMeasurement is nonnull, we saw occurrences when it was nil, which led to crashes. Now, when the name is nil, the SDK discards the measurement and logs an error.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5064 +/- ##
=============================================
- Coverage 92.731% 92.729% -0.002%
=============================================
Files 674 675 +1
Lines 83552 83629 +77
Branches 30451 30480 +29
=============================================
+ Hits 77479 77549 +70
- Misses 5976 5980 +4
- Partials 97 100 +3
... and 11 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
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, left some comments
Performance metrics 🚀
|
Revision | Plain | With Sentry | Diff |
---|---|---|---|
6164c67 | 1219.02 ms | 1231.38 ms | 12.35 ms |
596ccc1 | 1230.85 ms | 1244.24 ms | 13.39 ms |
253bb71 | 1221.62 ms | 1250.82 ms | 29.20 ms |
c59914b | 1342.08 ms | 1375.73 ms | 33.65 ms |
ebfe678 | 1234.63 ms | 1254.52 ms | 19.89 ms |
9ce54cc | 1230.31 ms | 1237.20 ms | 6.90 ms |
0a12181 | 1231.37 ms | 1248.13 ms | 16.75 ms |
7bbb7c3 | 1232.40 ms | 1249.78 ms | 17.38 ms |
3cba0e8 | 1243.04 ms | 1257.46 ms | 14.42 ms |
c0ea6a1 | 1236.42 ms | 1250.94 ms | 14.52 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
6164c67 | 21.58 KiB | 681.72 KiB | 660.14 KiB |
596ccc1 | 22.84 KiB | 401.44 KiB | 378.60 KiB |
253bb71 | 20.76 KiB | 393.37 KiB | 372.60 KiB |
c59914b | 21.58 KiB | 671.90 KiB | 650.32 KiB |
ebfe678 | 22.31 KiB | 775.27 KiB | 752.95 KiB |
9ce54cc | 21.58 KiB | 573.71 KiB | 552.13 KiB |
0a12181 | 21.58 KiB | 419.20 KiB | 397.61 KiB |
7bbb7c3 | 21.58 KiB | 418.78 KiB | 397.20 KiB |
3cba0e8 | 22.84 KiB | 403.19 KiB | 380.34 KiB |
c0ea6a1 | 21.58 KiB | 671.94 KiB | 650.35 KiB |
Calling setMeasurement on transactions ans spans could lead to crashes. This is fixed now by making it thread safe.
#5061) Bumps [actions/create-github-app-token](https://github.com/actions/create-github-app-token) from 1.12.0 to 2.0.2. - [Release notes](https://github.com/actions/create-github-app-token/releases) - [Commits](actions/create-github-app-token@d72941d...3ff1caa) --- updated-dependencies: - dependency-name: actions/create-github-app-token dependency-version: 2.0.2 dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [fastlane](https://github.com/fastlane/fastlane) from 2.227.0 to 2.227.1. - [Release notes](https://github.com/fastlane/fastlane/releases) - [Changelog](https://github.com/fastlane/fastlane/blob/master/CHANGELOG.latest.md) - [Commits](fastlane/fastlane@fastlane/2.227.0...fastlane/2.227.1) --- updated-dependencies: - dependency-name: fastlane dependency-version: 2.227.1 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
fix duplicate note in continuous profiling GA entry
Sorry about the commit messup, I accidentially merged #5067 into this one before merging this PR. |
📜 Description
Although the name for setMeasurement is nonnull, we saw occurrences when it was nil, which led to crashes. Now, when the name is nil, the SDK discards the measurement and logs an error.
💡 Motivation and Context
A customer reported these crashes.
💚 How did you test it?
Unit tests
📝 Checklist
You have to check all boxes before merging:
sendDefaultPII
is enabled.