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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: InvalidArgException for sentryErrorWithDomain #3819

Merged
merged 1 commit into from Apr 3, 2024

Conversation

philipphofmann
Copy link
Member

@philipphofmann philipphofmann commented Apr 3, 2024

馃摐 Description

Fix NSInvalidArgumentException for NSError sentryErrorWithDomain by removing the category functions because they get stripped for static libraries. This is related to GH-3764.

馃挕 Motivation and Context

Fixes GH-3818

馃挌 How did you test it?

Same problem as #3763, for which removing the category function worked. So no need for an extra test.

馃摑 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

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
Copy link

codecov bot commented Apr 3, 2024

Codecov Report

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

Project coverage is 91.013%. Comparing base (408f43e) to head (3892d0b).

Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##              main     #3819       +/-   ##
=============================================
- Coverage   91.046%   91.013%   -0.033%     
=============================================
  Files          560       560               
  Lines        44506     44455       -51     
  Branches     15770     15773        +3     
=============================================
- Hits         40521     40460       -61     
+ Misses        3915      3818       -97     
- Partials        70       177      +107     
Files Coverage 螖
...ntryCrash/Recording/Tools/SentryCrashNSErrorUtil.m 100.000% <100.000%> (酶)
...tryTests/SentryCrash/SentryCrashNSErrorUtilTests.m 100.000% <100.000%> (酶)
...yTests/SentryCrash/SentryCrashReportFilter_Tests.m 95.811% <酶> (酶)
Sources/SentryCrash/Recording/SentryCrash.m 81.938% <0.000%> (+0.359%) 猬嗭笍
...h/Reporting/Filters/SentryCrashReportFilterBasic.m 95.022% <83.333%> (+0.285%) 猬嗭笍
...ryCrash/Recording/Tools/SentryCrashJSONCodecObjC.m 92.817% <60.000%> (+1.812%) 猬嗭笍
...entryCrash/Installations/SentryCrashInstallation.m 34.210% <0.000%> (+0.877%) 猬嗭笍

... 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 408f43e...3892d0b. Read the comment docs.

Copy link
Contributor

@brustolin brustolin left a comment

Choose a reason for hiding this comment

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

Thanks!!

@philipphofmann philipphofmann merged commit 645f63f into main Apr 3, 2024
69 of 71 checks passed
@philipphofmann philipphofmann deleted the fix/sentry-error-invalid-arg-exception branch April 3, 2024 08:00
Copy link

github-actions bot commented Apr 3, 2024

Performance metrics 馃殌

Plain With Sentry Diff
Startup time 1202.36 ms 1225.41 ms 23.05 ms
Size 21.58 KiB 572.91 KiB 551.33 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
bfa3487 1194.19 ms 1224.20 ms 30.01 ms
d3edf46 1213.00 ms 1227.46 ms 14.46 ms
5616e0a 1237.00 ms 1260.43 ms 23.43 ms
7c5d161 1224.38 ms 1249.66 ms 25.28 ms
a2af9fa 1236.29 ms 1251.67 ms 15.38 ms
0776564 1224.35 ms 1247.26 ms 22.91 ms
408f43e 1229.31 ms 1247.52 ms 18.21 ms
ecd9ecd 1215.77 ms 1238.70 ms 22.93 ms
6001822 1233.31 ms 1251.64 ms 18.33 ms
9cc7e7c 1228.90 ms 1237.96 ms 9.06 ms

App size

Revision Plain With Sentry Diff
bfa3487 20.76 KiB 393.36 KiB 372.60 KiB
d3edf46 20.76 KiB 436.65 KiB 415.89 KiB
5616e0a 22.85 KiB 407.45 KiB 384.60 KiB
7c5d161 20.76 KiB 414.44 KiB 393.68 KiB
a2af9fa 20.76 KiB 432.88 KiB 412.11 KiB
0776564 20.76 KiB 399.19 KiB 378.43 KiB
408f43e 21.58 KiB 573.17 KiB 551.59 KiB
ecd9ecd 20.76 KiB 420.23 KiB 399.47 KiB
6001822 22.85 KiB 410.99 KiB 388.14 KiB
9cc7e7c 22.84 KiB 403.13 KiB 380.29 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.

[NSError sentryErrorWithDomain:code:description:] crash
2 participants