Convert SentryReachability to Swift

[itaybre]

#skip-changelog

[linear]

COCOA-533 Convert SentryReachability to Swift

[codecov]

Codecov Report

❌ Patch coverage is 97.25275% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.671%. Comparing base (2137530) to head (62f0e70).
⚠️ Report is 28 commits behind head on main.

Files with missing lines	Patch %	Lines
Sources/Swift/Networking/SentryReachability.swift	97.191%	5 Missing ⚠️

Additional details and impacted files

@@              Coverage Diff              @@
##              main     #6019       +/-   ##
=============================================
- Coverage   86.774%   86.671%   -0.104%     
=============================================
  Files          425       427        +2     
  Lines        36741     36792       +51     
  Branches     17366     17302       -64     
=============================================
+ Hits         31882     31888        +6     
- Misses        4814      4858       +44     
- Partials        45        46        +1     

Files with missing lines	Coverage Δ
Sources/Sentry/SentryBreadcrumbTracker.m	87.317% <100.000%> (ø)
Sources/Sentry/SentryDependencyContainer.m	88.532% <100.000%> (+0.106%)	⬆️
Sources/Sentry/SentryHttpTransport.m	98.214% <100.000%> (ø)
Sources/Sentry/SentrySessionReplayIntegration.m	87.061% <ø> (ø)
Sources/Swift/Networking/SentryReachability.swift	97.191% <97.191%> (ø)

... and 63 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 2137530...62f0e70. Read the comment docs.

[github-actions]

Performance metrics 🚀

	Plain	With Sentry	Diff
Startup time	1249.61 ms	1269.37 ms	19.76 ms
Size	23.75 KiB	936.82 KiB	913.07 KiB

Baseline results on branch: main

Startup times

Revision	Plain	With Sentry	Diff
ccf1278	1226.84 ms	1248.51 ms	21.67 ms
d23a1b1	1218.94 ms	1239.53 ms	20.60 ms
1b008ee	1209.57 ms	1258.67 ms	49.10 ms
c6d2354	1226.53 ms	1252.69 ms	26.16 ms
be6a4ee	1226.33 ms	1249.77 ms	23.44 ms
fac4ca3	1222.81 ms	1235.83 ms	13.02 ms
b193842	1231.36 ms	1247.69 ms	16.33 ms
2de3f92	1207.56 ms	1234.96 ms	27.40 ms
2675d3c	1218.94 ms	1239.36 ms	20.42 ms
f8029e2	1245.16 ms	1261.32 ms	16.16 ms

App size

Revision	Plain	With Sentry	Diff
ccf1278	23.75 KiB	877.15 KiB	853.40 KiB
d23a1b1	23.75 KiB	913.64 KiB	889.88 KiB
1b008ee	23.75 KiB	929.71 KiB	905.96 KiB
c6d2354	23.75 KiB	919.70 KiB	895.95 KiB
be6a4ee	23.75 KiB	913.14 KiB	889.39 KiB
fac4ca3	23.75 KiB	902.01 KiB	878.27 KiB
b193842	23.75 KiB	920.65 KiB	896.90 KiB
2de3f92	23.75 KiB	919.69 KiB	895.94 KiB
2675d3c	23.75 KiB	928.16 KiB	904.41 KiB
f8029e2	23.75 KiB	893.72 KiB	869.97 KiB

Previous results on branch: itay/cocoa-533-convert-sentryreachability-to-swift

Startup times

Revision	Plain	With Sentry	Diff
6806bfa	1221.80 ms	1247.77 ms	25.97 ms
fb615c0	1228.12 ms	1256.26 ms	28.13 ms
4ff1298	1213.61 ms	1241.84 ms	28.23 ms
96d41c3	1236.33 ms	1253.82 ms	17.49 ms

App size

Revision	Plain	With Sentry	Diff
6806bfa	23.75 KiB	935.87 KiB	912.13 KiB
fb615c0	23.75 KiB	935.53 KiB	911.78 KiB
4ff1298	23.75 KiB	935.54 KiB	911.79 KiB
96d41c3	23.75 KiB	935.54 KiB	911.79 KiB

[philipphofmann]

@itaybre we actually want to rewrite SentryReachability because it uses SCNetworkReachability see #5440. I'm unsure if it's worth converting it to Swift. If you still think we should do it, I can give this PR a review.

[itaybre]

@itaybre we actually want to rewrite SentryReachability because it uses SCNetworkReachability see #5440. I'm unsure if it's worth converting it to Swift. If you still think we should do it, I can give this PR a review.

I remember that change proposal, but while we still support iOS 11, we need to use this reachability since this will be a blocker for the swift conversion project

[philipphofmann]

I remember that change proposal, but while we still support iOS 11, we need to use this reachability since this will be a blocker for the swift conversion project

We can also consider removing support for iOS 11 for SentryReachability. We only use it for network breadcrumbs and sending envelopes when a device gets online again. The impact of the removed functionality for iOS 11 users is minimal, and the market share is close to 0%.

[philprime]

Almost LGTM, left a couple of comments to discuss.

@philipphofmann I would like us to stick to the plan and convert to Swift as much as possible, instead of rewriting parts of the SDK now.

[philprime]

m: I am seeing this sync solution for the first time in the repository. Is there an advantage of objc_sync_enter over other sync methods like locks?

[itaybre]

It is the same behavior

[philprime]

m: We could consider SentryDispatchQueueWrapper even if it is a 1:1 conversion

[itaybre]

NWPathMonitor requires a queue to start

[philprime]

m: This should be a user-visible warning, not only in the debug logs.

Suggested change

	SentrySDKLog.debug("NWPathMonitor not available. Using fallback: always connected via WiFi")
	SentrySDKLog.warn("NWPathMonitor not available. Using fallback: always connected via WiFi")

[philipphofmann]

@philipphofmann I would like us to stick to the plan and convert to Swift as much as possible, instead of rewriting parts of the SDK now.

@philprime, the conversion wasn't looking so great because the APIs are mostly ObjC low-level. As we want to ditch SentryReachability with SCNetworkReachability anyways, I raised the question if it wouldn't be better to replace the implementation with NWPathMonitor already as we would ditch the Swift SCNetworkReachability version soon.