Skip to content

feat: Fishhook for C++ Exceptions #5128

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

Draft
wants to merge 9 commits into
base: main
Choose a base branch
from
Draft

Conversation

philipphofmann
Copy link
Member

📜 Description

Early draft PR to use a fishhook to hook into __cxa_throw so we can get a proper stacktrace for unhandled C++ exceptions. This is related to #4517 and inspired by the code in https://github.com/kstenerud/KSCrash.

💡 Motivation and Context

💚 How did you test it?

📝 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.

Copy link
Contributor

github-actions bot commented Apr 23, 2025

Fails
🚫 Please consider adding a changelog entry for the next release.
Messages
📖 Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Instructions and example for changelog

Please add an entry to CHANGELOG.md to the "Unreleased" section. Make sure the entry includes this PR's number.

Example:

## Unreleased

- Fishhook for C++ Exceptions ([#5128](https://github.com/getsentry/sentry-cocoa/pull/5128))

If none of the above apply, you can opt out of this check by adding #skip-changelog to the PR description.

Generated by 🚫 dangerJS against dc92fa8

Copy link

codecov bot commented Apr 23, 2025

Codecov Report

Attention: Patch coverage is 92.95958% with 54 lines in your changes missing coverage. Please review.

Project coverage is 92.797%. Comparing base (d59dcc0) to head (dc92fa8).
Report is 5 commits behind head on main.

Files with missing lines Patch % Lines
...Crash/Recording/Tools/SentryCrashCxaThrowSwapper.c 88.990% 24 Missing ⚠️
...rding/Monitors/SentryCrashMonitor_CPPException.cpp 16.666% 20 Missing ⚠️
...ts/SentryCrash/SentryCrashCxaThrowSwapper_Tests.mm 93.617% 9 Missing ⚠️
Sources/Sentry/SentryCrashIntegration.m 80.000% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##              main     #5128       +/-   ##
=============================================
+ Coverage   92.783%   92.797%   +0.013%     
=============================================
  Files          676       681        +5     
  Lines        83881     84641      +760     
  Branches     30535     30930      +395     
=============================================
+ Hits         77828     78545      +717     
- Misses        5953      5997       +44     
+ Partials       100        99        -1     
Files with missing lines Coverage Δ
Sources/Sentry/SentryAsyncSafeLog.h 88.235% <ø> (+3.619%) ⬆️
Sources/Sentry/SentryCrashReportConverter.m 96.163% <100.000%> (+0.018%) ⬆️
Sources/Sentry/include/SentryCompiler.h 100.000% <100.000%> (ø)
...es/SentryCrash/Recording/Tools/SentryCrashMach-O.c 100.000% <100.000%> (ø)
...cording/Tools/SentryCrashPlatformSpecificDefines.h 100.000% <100.000%> (ø)
...es/Swift/Helper/SentryEnabledFeaturesBuilder.swift 100.000% <100.000%> (ø)
Sources/Swift/SentryExperimentalOptions.swift 60.000% <100.000%> (+10.000%) ⬆️
...sts/Helper/SentryEnabledFeaturesBuilderTests.swift 100.000% <100.000%> (ø)
...s/SentryTests/SentryCrash/SentryCrashMach-OTests.m 100.000% <100.000%> (ø)
...ts/SentryTests/SentryKSCrashReportConverterTests.m 97.733% <100.000%> (+0.034%) ⬆️
... and 4 more

... and 16 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 d59dcc0...dc92fa8. Read the comment docs.

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

Copy link
Contributor

github-actions bot commented Apr 24, 2025

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1228.86 ms 1247.71 ms 18.85 ms
Size 22.30 KiB 853.40 KiB 831.09 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
f25febb 1238.80 ms 1245.35 ms 6.56 ms
69d8759 1235.71 ms 1241.34 ms 5.63 ms
ce4cfaf 1203.61 ms 1218.86 ms 15.25 ms
7419285 1209.53 ms 1244.72 ms 35.19 ms
b15521e 1224.44 ms 1251.13 ms 26.68 ms
bd2afa6 1245.24 ms 1263.18 ms 17.94 ms
53b29ac 1228.81 ms 1244.96 ms 16.15 ms
443fb02 1231.06 ms 1252.60 ms 21.54 ms
888a145 1228.63 ms 1248.94 ms 20.30 ms
70c49ca 1206.44 ms 1233.07 ms 26.62 ms

App size

Revision Plain With Sentry Diff
f25febb 21.58 KiB 414.92 KiB 393.34 KiB
69d8759 20.76 KiB 393.05 KiB 372.29 KiB
ce4cfaf 20.76 KiB 423.19 KiB 402.43 KiB
7419285 20.76 KiB 432.99 KiB 412.22 KiB
b15521e 21.58 KiB 573.18 KiB 551.60 KiB
bd2afa6 20.76 KiB 420.55 KiB 399.79 KiB
53b29ac 22.32 KiB 819.55 KiB 797.24 KiB
443fb02 22.30 KiB 832.42 KiB 810.11 KiB
888a145 21.58 KiB 713.54 KiB 691.96 KiB
70c49ca 22.31 KiB 778.76 KiB 756.45 KiB

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.

1 participant