-
-
Notifications
You must be signed in to change notification settings - Fork 339
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: Crash in Tracer for idle timeout #2834
Conversation
Performance metrics 🚀
|
Revision | Plain | With Sentry | Diff |
---|---|---|---|
49819af | 1263.92 ms | 1275.66 ms | 11.74 ms |
8f397a7 | 1230.10 ms | 1253.88 ms | 23.77 ms |
fdfe96b | 1227.90 ms | 1242.56 ms | 14.66 ms |
d40512b | 1231.12 ms | 1244.54 ms | 13.42 ms |
c9724f9 | 1199.38 ms | 1229.54 ms | 30.16 ms |
28333b6 | 1247.29 ms | 1262.51 ms | 15.22 ms |
f576153 | 1210.02 ms | 1228.94 ms | 18.92 ms |
0dedab7 | 1221.26 ms | 1235.34 ms | 14.08 ms |
bd2afa6 | 1241.37 ms | 1246.20 ms | 4.83 ms |
10ee2ce | 1250.90 ms | 1258.57 ms | 7.67 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
49819af | 20.76 KiB | 427.31 KiB | 406.55 KiB |
8f397a7 | 20.76 KiB | 420.55 KiB | 399.79 KiB |
fdfe96b | 20.76 KiB | 419.70 KiB | 398.95 KiB |
d40512b | 20.76 KiB | 427.77 KiB | 407.00 KiB |
c9724f9 | 20.76 KiB | 427.66 KiB | 406.90 KiB |
28333b6 | 20.76 KiB | 424.69 KiB | 403.93 KiB |
f576153 | 20.76 KiB | 425.77 KiB | 405.01 KiB |
0dedab7 | 20.76 KiB | 420.00 KiB | 399.24 KiB |
bd2afa6 | 20.76 KiB | 420.55 KiB | 399.79 KiB |
10ee2ce | 20.76 KiB | 427.77 KiB | 407.00 KiB |
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.
Looks good.
Just one suggestion.
Sources/Sentry/SentryTracer.m
Outdated
SENTRY_LOG_WARN(@"Couln't create idle time out block. Can't schedule idle timeout. " | ||
@"Finishing transaction"); | ||
[self finishInternal]; |
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.
m: I would just add a comment here that the transaction will not be send because it does not have any child.
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.
Better now?
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #2834 +/- ##
==========================================
- Coverage 81.33% 81.33% -0.01%
==========================================
Files 258 258
Lines 24131 24142 +11
Branches 10708 10712 +4
==========================================
+ Hits 19628 19636 +8
- Misses 4005 4007 +2
- Partials 498 499 +1
... and 7 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
📜 Description
dispatch_block_create can return NULL. If it does, we finish the transaction and don't schedule the timeout.
💡 Motivation and Context
Fixes GH-2832, GH-2769
💚 How did you test it?
Unit tests, and on an iPhone to validate UI event transactions are still working properly.
📝 Checklist
You have to check all boxes before merging:
sendDefaultPII
is enabled.🔮 Next steps