Skip to content

Conversation

@philprime
Copy link
Member

This PR is derived from #5572 in an effort to make the large amount of changes easier to review for #5577.

The function sentryLevelForString is used to map a NSString to a SentryLevel and expects a non-null input value.

This is an important fix, because the implementation uses SentryLevelHelper which is a Swift file, and while the Objective-C code can deal with null values, in Swift it would actually be considered force-unwrapped and crash.

SentryLevel
sentryLevelForString(NSString *string)
{
return [SentryLevelHelper levelForName:string];
}

@objcMembers
@_spi(Private) public class SentryLevelHelper: NSObject {
public static func nameForLevel(_ level: SentryLevel) -> String {
return level.description
}
public static func levelForName(_ name: String) -> SentryLevel {
.fromName(name)
}
}

This is something we must keep in mind when bridging Objective-C files to Swift (cc @noahsmartin as you are currently most active in this field).

#skip-changelog

@codecov
Copy link

codecov bot commented Aug 5, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.439%. Comparing base (5bf2b17) to head (ab53288).
⚠️ Report is 8 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##              main     #5803       +/-   ##
=============================================
- Coverage   86.595%   86.439%   -0.156%     
=============================================
  Files          423       423               
  Lines        36360     36349       -11     
  Branches     17178     15557     -1621     
=============================================
- Hits         31486     31420       -66     
- Misses        4827      4887       +60     
+ Partials        47        42        -5     
Files with missing lines Coverage Δ
Sources/Sentry/SentryHub.m 98.086% <100.000%> (+0.006%) ⬆️
Sources/Sentry/SentryLevelMapper.m 100.000% <ø> (ø)
Sources/Swift/Core/Helper/Log/SentryLevel.swift 100.000% <100.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 5bf2b17...ab53288. Read the comment docs.

@github-actions
Copy link
Contributor

github-actions bot commented Aug 5, 2025

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1222.39 ms 1242.74 ms 20.36 ms
Size 23.75 KiB 913.13 KiB 889.39 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
162cd7f 1230.59 ms 1256.76 ms 26.16 ms
9add417 1224.33 ms 1243.06 ms 18.73 ms
a2a3bfb 1227.94 ms 1261.26 ms 33.32 ms
d38165b 1211.41 ms 1242.49 ms 31.08 ms
c63e0fe 1230.58 ms 1253.94 ms 23.35 ms
65ebbdb 1233.96 ms 1255.79 ms 21.83 ms
7c7ac56 1225.90 ms 1250.22 ms 24.33 ms
6d40fee 1217.47 ms 1245.37 ms 27.90 ms
4e3915a 1230.02 ms 1258.90 ms 28.88 ms
1936411 1231.51 ms 1253.27 ms 21.76 ms

App size

Revision Plain With Sentry Diff
162cd7f 23.75 KiB 908.39 KiB 884.64 KiB
9add417 23.75 KiB 908.40 KiB 884.65 KiB
a2a3bfb 23.75 KiB 872.67 KiB 848.92 KiB
d38165b 23.75 KiB 855.37 KiB 831.62 KiB
c63e0fe 23.74 KiB 874.08 KiB 850.33 KiB
65ebbdb 23.75 KiB 902.33 KiB 878.59 KiB
7c7ac56 23.75 KiB 902.49 KiB 878.74 KiB
6d40fee 23.75 KiB 912.37 KiB 888.63 KiB
4e3915a 23.75 KiB 858.69 KiB 834.94 KiB
1936411 23.74 KiB 913.39 KiB 889.64 KiB

Previous results on branch: philprime/strict-nullability-14

Startup times

Revision Plain With Sentry Diff
fbf9909 1221.20 ms 1239.04 ms 17.84 ms

App size

Revision Plain With Sentry Diff
fbf9909 23.74 KiB 913.70 KiB 889.96 KiB

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

@philprime philprime added the Waiting for: CI 🤖 Marks PR as ready-for-merge when CI passes. To be replaced with required checks. label Aug 6, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Aug 6, 2025

🚨 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/SentrySerialization.m

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.

Again LGTM

@philprime philprime merged commit d3e7aa6 into main Aug 7, 2025
131 of 133 checks passed
@philprime philprime deleted the philprime/strict-nullability-14 branch August 7, 2025 09:02
philipsawyerdd added a commit to justin-doordash/sentry-cocoa that referenced this pull request Sep 25, 2025
philipsawyerdd added a commit to justin-doordash/sentry-cocoa that referenced this pull request Sep 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Waiting for: CI 🤖 Marks PR as ready-for-merge when CI passes. To be replaced with required checks.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants