Skip to content
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: Missing mach info for crash reports #4230

Merged
merged 4 commits into from
Aug 5, 2024
Merged

Conversation

philipphofmann
Copy link
Member

📜 Description

The SentryInternalCDefines didn't define the SENTRY_HOST_APPLE, which led to missing mach info for crash reports, as the SentryCrashReport relies on the SENTRY_HOST_APPLE to write that info. For reference, the SentryCrashCRASH_HOST_APPLE was changed to SENTRY_HOST_APPLE in GH-4101. The problem is fixed now by defining SENTRY_HOST_APPLE in SentryInternalCDefines.

💡 Motivation and Context

Fixes GH-4227

💚 How did you test it?

Unit tests and checking crash reports of a simulator.

Before After
CleanShot 2024-08-05 at 09 42 52@2x CleanShot 2024-08-05 at 09 43 23@2x
CleanShot 2024-08-05 at 09 43 44@2x CleanShot 2024-08-05 at 09 44 08@2x

📝 Checklist

You have to check all boxes before merging:

  • I reviewed the submitted code.
  • 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.
  • 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.

🔮 Next steps

The SentryInternalCDefines didn't define the SENTRY_HOST_APPLE, which
led to missing mach info for crash reports, as the SentryCrashReport
relies on the SENTRY_HOST_APPLE to write that info. For reference, the
SentryCrashCRASH_HOST_APPLE was changed to SENTRY_HOST_APPLE in GH-4101.
The problem is fixed now by defining SENTRY_HOST_APPLE in
SentryInternalCDefines.

Fixes GH-4227
Copy link

codecov bot commented Aug 5, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.431%. Comparing base (649d940) to head (861f7a8).

Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##              main     #4230       +/-   ##
=============================================
+ Coverage   91.424%   91.431%   +0.006%     
=============================================
  Files          611       611               
  Lines        49163     49213       +50     
  Branches     17757     17785       +28     
=============================================
+ Hits         44947     44996       +49     
- Misses        4123      4124        +1     
  Partials        93        93               
Files Coverage Δ
...egrations/SentryCrash/SentryCrashReportTests.swift 96.428% <100.000%> (+1.079%) ⬆️
...ts/SentryTests/SentryKSCrashReportConverterTests.m 97.254% <100.000%> (+0.021%) ⬆️

... and 9 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 649d940...861f7a8. Read the comment docs.

Copy link

github-actions bot commented Aug 5, 2024

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1239.17 ms 1255.04 ms 15.88 ms
Size 21.58 KiB 695.48 KiB 673.90 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
2dd2e3a 1218.26 ms 1229.69 ms 11.44 ms
257c2a9 1231.45 ms 1252.12 ms 20.67 ms
78d5983 1229.98 ms 1245.60 ms 15.62 ms
28333b6 1186.29 ms 1225.18 ms 38.89 ms
1731a1c 1209.14 ms 1227.92 ms 18.78 ms
7e8d5fd 1208.69 ms 1228.14 ms 19.45 ms
ca507ec 1224.43 ms 1246.04 ms 21.61 ms
01a28a9 1200.78 ms 1227.90 ms 27.12 ms
06548c0 1225.58 ms 1244.70 ms 19.12 ms
4259afd 1222.12 ms 1249.74 ms 27.62 ms

App size

Revision Plain With Sentry Diff
2dd2e3a 21.58 KiB 540.04 KiB 518.46 KiB
257c2a9 20.76 KiB 401.36 KiB 380.60 KiB
78d5983 20.76 KiB 427.80 KiB 407.04 KiB
28333b6 20.76 KiB 424.69 KiB 403.93 KiB
1731a1c 21.58 KiB 542.28 KiB 520.69 KiB
7e8d5fd 20.76 KiB 435.50 KiB 414.74 KiB
ca507ec 21.58 KiB 616.76 KiB 595.17 KiB
01a28a9 22.85 KiB 405.39 KiB 382.54 KiB
06548c0 20.76 KiB 427.35 KiB 406.59 KiB
4259afd 20.76 KiB 419.70 KiB 398.94 KiB

@philipphofmann philipphofmann merged commit 2feccaf into main Aug 5, 2024
64 of 67 checks passed
@philipphofmann philipphofmann deleted the fix/missing-mach-info branch August 5, 2024 10:42
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.

Crashes miss handled property
2 participants