Skip to content

fix: Async safe log for backtrace in CPPException #5098

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

Merged
merged 5 commits into from
Apr 16, 2025

Conversation

philipphofmann
Copy link
Member

📜 Description

Use async safe logging when getting the backtrace for CPP Exceptions. When an unhandled CPPExceptions occurs, the crash monitor for CPPExceptions uses SentryCrashStackCursor_SelfThread, which uses non async safe SenryLog. This is fixed now by using async safe logging in SentryCrashStackCursor_SelfThread. Only users using debug = true or setting the log level to debug were impacted by this issue.

💡 Motivation and Context

Came up while investigating #4226.

💚 How did you test it?

Unit tests are still green.

📝 Checklist

You have to check all boxes before merging:

  • I added tests to verify the changes.
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
  • I updated the docs if needed.
  • I updated the wizard if needed.
  • Review from the native team if needed.
  • No breaking change or entry added to the changelog.
  • No breaking change for hybrid SDKs or communicated to hybrid SDKs.

Sorry, something went wrong.

Use async safe logging when getting the backtrace for CPP Exceptions.
When an unhandled CPPExceptions occurs, the crash monitor for
CPPExceptions uses SentryCrashStackCursor_SelfThread, which uses non
async safe SenryLog. This is fixed now by using async safe logging in
SentryCrashStackCursor_SelfThread. Only users using debug = true or
setting the log level to debug were impacted by this issue.
@philipphofmann philipphofmann changed the title Fix/async safe logging cpp backtrace fix: Async safe log for backtrace in CPPException Apr 16, 2025
philipphofmann and others added 2 commits April 16, 2025 09:30
Copy link
Member

@philprime philprime left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Copy link
Contributor

github-actions bot commented Apr 16, 2025

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1221.59 ms 1260.29 ms 38.69 ms
Size 22.30 KiB 848.26 KiB 825.95 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
f25febb 1209.08 ms 1225.90 ms 16.81 ms
2337de8 1227.42 ms 1236.61 ms 9.19 ms
1bbcb9c 1214.25 ms 1230.04 ms 15.79 ms
e324230 1230.59 ms 1248.20 ms 17.61 ms
c0b4b71 1218.16 ms 1251.28 ms 33.12 ms
92d1dc2 1226.92 ms 1246.02 ms 19.10 ms
bb7da60 1231.80 ms 1252.65 ms 20.85 ms
7afad57 1216.52 ms 1245.59 ms 29.07 ms
5eb5b30 1227.06 ms 1243.47 ms 16.41 ms
e2484ac 1236.94 ms 1257.82 ms 20.88 ms

App size

Revision Plain With Sentry Diff
f25febb 21.58 KiB 414.92 KiB 393.34 KiB
2337de8 22.31 KiB 818.67 KiB 796.36 KiB
1bbcb9c 20.76 KiB 426.10 KiB 405.34 KiB
e324230 22.85 KiB 408.87 KiB 386.02 KiB
c0b4b71 20.76 KiB 430.98 KiB 410.22 KiB
92d1dc2 22.30 KiB 844.42 KiB 822.11 KiB
bb7da60 21.58 KiB 697.84 KiB 676.25 KiB
7afad57 22.30 KiB 832.29 KiB 809.99 KiB
5eb5b30 22.30 KiB 823.44 KiB 801.13 KiB
e2484ac 22.31 KiB 768.19 KiB 745.87 KiB

Previous results on branch: fix/async-safe-logging-cpp-backtrace

Startup times

Revision Plain With Sentry Diff
97d6703 1216.18 ms 1240.85 ms 24.67 ms

App size

Revision Plain With Sentry Diff
97d6703 22.30 KiB 847.84 KiB 825.53 KiB

@philipphofmann philipphofmann merged commit 4c5c48c into main Apr 16, 2025
56 of 57 checks passed
@philipphofmann philipphofmann deleted the fix/async-safe-logging-cpp-backtrace branch April 16, 2025 10:45
Copy link

codecov bot commented Apr 16, 2025

Codecov Report

Attention: Patch coverage is 91.66667% with 2 lines in your changes missing coverage. Please review.

Project coverage is 92.789%. Comparing base (03fd0bb) to head (3e76db2).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...ecording/Tools/SentryCrashStackCursor_SelfThread.m 60.000% 2 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##              main     #5098       +/-   ##
=============================================
- Coverage   92.789%   92.789%   -0.001%     
=============================================
  Files          675       676        +1     
  Lines        83865     83877       +12     
  Branches     30553     30531       -22     
=============================================
+ Hits         77818     77829       +11     
+ Misses        5950      5947        -3     
- Partials        97       101        +4     
Files with missing lines Coverage Δ
...yCrash/SentryCrashStackCursorSelfThreadTests.swift 100.000% <100.000%> (ø)
...ecording/Tools/SentryCrashStackCursor_SelfThread.m 88.461% <60.000%> (ø)

... and 17 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 03fd0bb...3e76db2. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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

Successfully merging this pull request may close these issues.

None yet

3 participants