Convert SentryReachability to Swift

[itaybre]

#skip-changelog

[linear]

COCOA-533 Convert SentryReachability to Swift

[codecov]

❌ 4 Tests Failed:

Tests completed	Failed	Passed	Skipped
3987	4	3983	17

View the top 3 failed test(s) by shortest run time

iOS_Swift_UITests.LaunchUITests::testCaptureError

Stack Traces | 0s run time

.../iOS-Swift/iOS-Swift-UITests/LaunchUITests.swift:39 - Failed to Error Domain=com.apple.dt.XCTest.XCTFuture Code=1000 "Timed out while evaluating UI query." UserInfo={NSLocalizedDescription=Timed out while evaluating UI query.}: Timed out while evaluating UI query.

iOS_Swift_UITests.LaunchUITests::testCaptureException

Stack Traces | 0s run time

.../iOS-Swift/iOS-Swift-UITests/LaunchUITests.swift:43 - Failed to Error Domain=com.apple.dt.XCTest.XCTFuture Code=1000 "Timed out while evaluating UI query." UserInfo={NSLocalizedDescription=Timed out while evaluating UI query.}: Timed out while evaluating UI query.

iOS_Swift_UITests.UIEventBreadcrumbTests::testExtractInfoFromView

Stack Traces | 0s run time

.../iOS-Swift/iOS-Swift-UITests/UIEventBreadcrumbTests.swift:22 - Failed to Error Domain=com.apple.dt.XCTest.XCTFuture Code=1000 "Timed out while evaluating UI query." UserInfo={NSLocalizedDescription=Timed out while evaluating UI query.}: Timed out while evaluating UI query.

iOS_Swift_UITests.UIEventBreadcrumbTests::testNoBreadcrumbForTextFieldEditingChanged

Stack Traces | 0s run time

.../iOS-Swift/iOS-Swift-UITests/UIEventBreadcrumbTests.swift:6 - Failed to Error Domain=com.apple.dt.XCTest.XCTFuture Code=1000 "Timed out while evaluating UI query." UserInfo={NSLocalizedDescription=Timed out while evaluating UI query.}: Timed out while evaluating UI query.

View the full list of 1 ❄️ flaky test(s)

SentryTests.SentryDispatchQueueWrapperSwiftTests::testDispatchAsyncOnMainQueueIfNotMainThreadFromBackground

Flake rate in main: 11.67% (Passed 53 times, Failed 7 times)

Stack Traces | 0s run time

.../SentryTests/Networking/SentryDispatchQueueWrapperTests.swift:59 - XCTAssertTrue failed - Block did not run asynchronously

To view more test analytics, go to the Test Analytics Dashboard
_{📋 Got 3 mins? Take this short survey to help us improve Test Analytics.}

[github-actions]

Performance metrics 🚀

	Plain	With Sentry	Diff
Startup time	1221.38 ms	1256.85 ms	35.48 ms
Size	23.75 KiB	988.71 KiB	964.96 KiB

Baseline results on branch: main

Startup times

Revision	Plain	With Sentry	Diff
0b5fd21	1237.52 ms	1251.36 ms	13.84 ms
fd5961e	1210.59 ms	1235.57 ms	24.98 ms
3067c23	1230.48 ms	1257.90 ms	27.42 ms
7fc6927	1221.21 ms	1242.36 ms	21.15 ms
8fd192f	1202.10 ms	1220.19 ms	18.09 ms
f0e2579	1224.82 ms	1245.49 ms	20.67 ms
670b474	1225.33 ms	1259.59 ms	34.26 ms
be8375a	1212.65 ms	1239.72 ms	27.08 ms
2b02431	1229.63 ms	1248.98 ms	19.35 ms
5bf2b17	1213.53 ms	1238.54 ms	25.01 ms

App size

Revision	Plain	With Sentry	Diff
0b5fd21	23.75 KiB	912.78 KiB	889.03 KiB
fd5961e	23.74 KiB	874.07 KiB	850.32 KiB
3067c23	23.75 KiB	928.15 KiB	904.40 KiB
7fc6927	23.75 KiB	908.54 KiB	884.79 KiB
8fd192f	23.74 KiB	872.75 KiB	849.01 KiB
f0e2579	23.75 KiB	969.22 KiB	945.47 KiB
670b474	23.75 KiB	974.89 KiB	951.14 KiB
be8375a	23.75 KiB	933.03 KiB	909.28 KiB
2b02431	23.75 KiB	850.73 KiB	826.98 KiB
5bf2b17	23.75 KiB	913.27 KiB	889.52 KiB

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

Startup times

Revision	Plain	With Sentry	Diff
51a71e7	1218.26 ms	1250.64 ms	32.38 ms
3f879db	1221.96 ms	1247.60 ms	25.64 ms
1a5b01e	1249.61 ms	1269.37 ms	19.76 ms
4ff1298	1213.61 ms	1241.84 ms	28.23 ms
d4ac58b	1203.88 ms	1237.92 ms	34.04 ms
f1e051c	1192.66 ms	1214.35 ms	21.69 ms
fb615c0	1228.12 ms	1256.26 ms	28.13 ms
6806bfa	1221.80 ms	1247.77 ms	25.97 ms
2733042	1223.00 ms	1246.23 ms	23.23 ms
2a9b553	1218.77 ms	1250.04 ms	31.28 ms

App size

Revision	Plain	With Sentry	Diff
51a71e7	23.75 KiB	988.71 KiB	964.96 KiB
3f879db	23.75 KiB	994.13 KiB	970.38 KiB
1a5b01e	23.75 KiB	936.82 KiB	913.07 KiB
4ff1298	23.75 KiB	935.54 KiB	911.79 KiB
d4ac58b	23.75 KiB	991.30 KiB	967.56 KiB
f1e051c	23.75 KiB	991.70 KiB	967.95 KiB
fb615c0	23.75 KiB	935.53 KiB	911.78 KiB
6806bfa	23.75 KiB	935.87 KiB	912.13 KiB
2733042	23.75 KiB	994.12 KiB	970.38 KiB
2a9b553	23.75 KiB	991.36 KiB	967.62 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.

[itaybre]

Looks like I was able to find the cause: Iterable/iterable-swift-sdk#688
NWPathMonitor crashed if the min iOS as is lower than iOS 12, but SauceLabs was not reporting any crash.

This happens on iOS 14 and iOS15.

[philipphofmann]

Looks like I was able to find the cause: Iterable/iterable-swift-sdk#688 NWPathMonitor crashed if the min iOS as is lower than iOS 12, but SauceLabs was not reporting any crash.

This happens on iOS 14 and iOS15.

Excellent, so this means we should remove the async main thread dispatch and wait until we bump the minOS versions before merging this?

[philipphofmann]

This PR deserves a changelog entry as it fixes #5440 and #5426

Almost LGTM, thank you @itaybre

[philipphofmann]

Thank you @itaybre

[cursor]

Bug: Reachability Initialization Affects Unsupported Platforms

Removing the SENTRY_HAS_REACHABILITY macro and its conditional compilation causes SentryReachability to be unconditionally initialized and its removeAllObservers method called. This affects platforms like watchOS where network reachability isn't supported, potentially leading to runtime issues.

Sources/Sentry/SentryDependencyContainer.m#L121-L122

sentry-cocoa/Sources/Sentry/SentryDependencyContainer.m

Lines 121 to 122 in ce4c2e9

	@synchronized(sentryDependencyContainerInstanceLock) {
	[instance->_reachability removeAllObservers];

Sources/Sentry/SentryDependencyContainer.m#L185-L186

sentry-cocoa/Sources/Sentry/SentryDependencyContainer.m

Lines 185 to 186 in ce4c2e9

	_reachability = [[SentryReachability alloc] init];

 

---