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: Remove categories functions that are striped of static library #3764

Merged
merged 2 commits into from
Mar 19, 2024

Conversation

brustolin
Copy link
Contributor

Part of a fixes for #3763

#skip-changelog

Copy link

github-actions bot commented Mar 19, 2024

🚨 Detected changes in high risk code 🚨

High-risk code can easily blow up and is hard to test. We had severe bugs in the past. Be extra careful when changing these files, and have an extra careful look at these:

  • Sources/Sentry/SentryUIViewControllerSwizzling.m

Copy link

codecov bot commented Mar 19, 2024

Codecov Report

Attention: Patch coverage is 95.68345% with 6 lines in your changes are missing coverage. Please review.

Project coverage is 89.300%. Comparing base (6bc31df) to head (e70b839).

Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##              main     #3764       +/-   ##
=============================================
+ Coverage   89.297%   89.300%   +0.003%     
=============================================
  Files          545       544        -1     
  Lines        59322     59323        +1     
  Branches     21322     21318        -4     
=============================================
+ Hits         52973     52976        +3     
- Misses        5312      5421      +109     
+ Partials      1037       926      -111     
Files Coverage Δ
Sources/Sentry/NSArray+SentrySanitize.m 100.000% <100.000%> (ø)
Sources/Sentry/NSLocale+Sentry.m 90.000% <ø> (ø)
Sources/Sentry/NSMutableDictionary+Sentry.m 100.000% <100.000%> (ø)
Sources/Sentry/SentryAppStartMeasurement.m 100.000% <ø> (ø)
Sources/Sentry/SentryAppState.m 97.777% <100.000%> (ø)
Sources/Sentry/SentryBreadcrumb.m 97.142% <100.000%> (ø)
Sources/Sentry/SentryClient.m 96.021% <100.000%> (ø)
Sources/Sentry/SentryCrashReportConverter.m 93.924% <100.000%> (-0.016%) ⬇️
Sources/Sentry/SentryCrashScopeObserver.m 93.457% <100.000%> (ø)
Sources/Sentry/SentryDateUtils.m 100.000% <100.000%> (ø)
... and 38 more

... and 28 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 6bc31df...e70b839. Read the comment docs.

Copy link
Member

@philipphofmann philipphofmann left a comment

Choose a reason for hiding this comment

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

LGTM, but we need to merge #3765 before merging this PR.

Copy link

github-actions bot commented Mar 19, 2024

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1227.48 ms 1255.06 ms 27.58 ms
Size 21.58 KiB 544.72 KiB 523.14 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
1c96132 1229.29 ms 1248.14 ms 18.86 ms
e7b566f 1197.49 ms 1229.46 ms 31.97 ms
62c15d4 1231.80 ms 1248.86 ms 17.06 ms
cb6ab62 1213.47 ms 1248.46 ms 34.99 ms
6129be5 1215.65 ms 1247.18 ms 31.54 ms
253bb71 1221.62 ms 1250.82 ms 29.20 ms
c319795 1256.18 ms 1266.87 ms 10.69 ms
8f397a7 1230.10 ms 1253.88 ms 23.77 ms
d40512b 1231.12 ms 1244.54 ms 13.42 ms
7ce3cf6 1217.98 ms 1246.41 ms 28.43 ms

App size

Revision Plain With Sentry Diff
1c96132 21.58 KiB 418.51 KiB 396.93 KiB
e7b566f 22.85 KiB 414.80 KiB 391.95 KiB
62c15d4 22.85 KiB 411.14 KiB 388.29 KiB
cb6ab62 22.85 KiB 413.42 KiB 390.57 KiB
6129be5 21.58 KiB 418.00 KiB 396.42 KiB
253bb71 20.76 KiB 393.37 KiB 372.60 KiB
c319795 20.76 KiB 431.99 KiB 411.22 KiB
8f397a7 20.76 KiB 420.55 KiB 399.79 KiB
d40512b 20.76 KiB 427.77 KiB 407.00 KiB
7ce3cf6 22.85 KiB 407.63 KiB 384.78 KiB

Previous results on branch: ref/remove-category-functions

Startup times

Revision Plain With Sentry Diff
571d868 1232.63 ms 1246.20 ms 13.58 ms

App size

Revision Plain With Sentry Diff
571d868 21.58 KiB 545.26 KiB 523.68 KiB

Copy link

github-actions bot commented Mar 19, 2024

🚨 Detected changes in high risk code 🚨

High-risk code can easily blow up and is hard to test. We had severe bugs in the past. Be extra careful when changing these files, and have an extra careful look at these:

  • Sources/Sentry/SentryUIViewControllerSwizzling.m

@brustolin brustolin merged commit f79ec31 into main Mar 19, 2024
70 checks passed
@brustolin brustolin deleted the ref/remove-category-functions branch March 19, 2024 12:02
philipphofmann added a commit that referenced this pull request Apr 3, 2024
Fix NSInvalidArgumentException for NSError sentryErrorWithDomain by
removing the category functions because they get stripped for static
libraries. This is related to GH-3764.

Fixes GH-3818
philipphofmann added a commit that referenced this pull request Apr 3, 2024
Fix NSInvalidArgumentException for NSError sentryErrorWithDomain by
removing the category functions because they get stripped for static
libraries. This is related to GH-3764.

Fixes GH-3818
dKasabwala pushed a commit to dKasabwala/sentry-cocoa that referenced this pull request May 6, 2024
Fix NSInvalidArgumentException for NSError sentryErrorWithDomain by
removing the category functions because they get stripped for static
libraries. This is related to getsentryGH-3764.

Fixes getsentryGH-3818
threema-matteo pushed a commit to threema-ch/sentry-cocoa that referenced this pull request May 21, 2024
…etsentry#3764)

Remove categories functions that are striped of static library

Co-authored-by: Philipp Hofmann <[email protected]>
threema-matteo pushed a commit to threema-ch/sentry-cocoa that referenced this pull request May 21, 2024
Fix NSInvalidArgumentException for NSError sentryErrorWithDomain by
removing the category functions because they get stripped for static
libraries. This is related to getsentryGH-3764.

Fixes getsentryGH-3818
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.

SDK start crashes when including via SPM or Carthage on 8.22.1 Remove category functions
2 participants