Skip to content

Commit c535c23

Browse files
itaybreitaybre
committed
Remove async queue requirement
1 parent 5de0617 commit c535c23

File tree

6 files changed

+129
-859
lines changed

6 files changed

+129
-859
lines changed

‎Sentry.xcodeproj/project.pbxproj‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,6 @@
4949
0A9BF4EB28A127120068D266 /* SentryViewHierarchyIntegrationTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0A9BF4EA28A127120068D266 /* SentryViewHierarchyIntegrationTests.swift */; };
5050
0A9E917128DC7E7000FB4182 /* SentryInternalCDefines.h in Headers */ = {isa = PBXBuildFile; fileRef = 0A9E917028DC7E7000FB4182 /* SentryInternalCDefines.h */; };
5151
0ADC33F128D9BE940078D980 /* TestSentryUIDeviceWrapper.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0ADC33EF28D9BE690078D980 /* TestSentryUIDeviceWrapper.swift */; };
52-
0AE455AD28F584D2006680E5 /* SentryReachabilityTests.m in Sources */ = {isa = PBXBuildFile; fileRef = 0AE455AC28F584D2006680E5 /* SentryReachabilityTests.m */; };
5352
15360CCF2432777500112302 /* SentrySessionTracker.m in Sources */ = {isa = PBXBuildFile; fileRef = 15360CCE2432777400112302 /* SentrySessionTracker.m */; };
5453
15360CD2243277A000112302 /* SentrySessionTracker.h in Headers */ = {isa = PBXBuildFile; fileRef = 15360CD12432779F00112302 /* SentrySessionTracker.h */; };
5554
15360CD62432832400112302 /* SentryAutoSessionTrackingIntegration.m in Sources */ = {isa = PBXBuildFile; fileRef = 15360CD52432832400112302 /* SentryAutoSessionTrackingIntegration.m */; };
@@ -1286,7 +1285,6 @@
12861285
0A9BF4EA28A127120068D266 /* SentryViewHierarchyIntegrationTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentryViewHierarchyIntegrationTests.swift; sourceTree = "<group>"; };
12871286
0A9E917028DC7E7000FB4182 /* SentryInternalCDefines.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = SentryInternalCDefines.h; path = include/SentryInternalCDefines.h; sourceTree = "<group>"; };
12881287
0ADC33EF28D9BE690078D980 /* TestSentryUIDeviceWrapper.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TestSentryUIDeviceWrapper.swift; sourceTree = "<group>"; };
1289-
0AE455AC28F584D2006680E5 /* SentryReachabilityTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = SentryReachabilityTests.m; sourceTree = "<group>"; };
12901288
15360CCE2432777400112302 /* SentrySessionTracker.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = SentrySessionTracker.m; sourceTree = "<group>"; };
12911289
15360CD12432779F00112302 /* SentrySessionTracker.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = SentrySessionTracker.h; path = include/SentrySessionTracker.h; sourceTree = "<group>"; };
12921290
15360CD52432832400112302 /* SentryAutoSessionTrackingIntegration.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = SentryAutoSessionTrackingIntegration.m; sourceTree = "<group>"; };
@@ -3547,7 +3545,6 @@
35473545
7B01CE3C271993AB00B5AF31 /* SentryTransportFactoryTests.swift */,
35483546
7B5CAF7C27F5AD0600ED0DB6 /* TestNSURLRequestBuilder.h */,
35493547
7B5CAF7D27F5AD3500ED0DB6 /* TestNSURLRequestBuilder.m */,
3550-
0AE455AC28F584D2006680E5 /* SentryReachabilityTests.m */,
35513548
629690522AD3E060000185FA /* SentryReachabilitySwiftTests.swift */,
35523549
0A9415B928F96CAC006A5DD1 /* TestSentryReachability.swift */,
35533550
D4FC68192DD63465001B74FF /* SentryDispatchQueueWrapperTests.m */,
@@ -6235,7 +6232,6 @@
62356232
D808FB92281BF6EC009A2A33 /* SentryUIEventTrackingIntegrationTests.swift in Sources */,
62366233
7BC6EC04255C235F0059822A /* SentryFrameTests.swift in Sources */,
62376234
D82DD1CD2BEEB1A0001AB556 /* SentrySRDefaultBreadcrumbConverterTests.swift in Sources */,
6238-
0AE455AD28F584D2006680E5 /* SentryReachabilityTests.m in Sources */,
62396235
63FE720420DA66EC00CDBAE8 /* SentryCrashString_Tests.m in Sources */,
62406236
62872B632BA1B86100A4FA7D /* LocksTests.swift in Sources */,
62416237
849AC40029E0C1FF00889C16 /* SentryFormatterTests.swift in Sources */,

‎Sources/Swift/Networking/SentryReachability.swift‎

Lines changed: 14 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -2,23 +2,11 @@ import Foundation
22
import Network
33

44
// MARK: - SentryConectivity
5-
#if DEBUG || SENTRY_TEST || SENTRY_TEST_CI
6-
@objc @_spi(Private)
7-
public enum SentryConnectivity: Int {
8-
case cellular
9-
case wiFi
10-
case none
11-
}
12-
#else
13-
@objc
145
enum SentryConnectivity: Int {
156
case cellular
167
case wiFi
178
case none
18-
}
19-
#endif // DEBUG || SENTRY_TEST || SENTRY_TEST_CI
209

21-
extension SentryConnectivity {
2210
func toString() -> String {
2311
switch self {
2412
case .cellular:
@@ -41,9 +29,9 @@ public protocol SentryReachabilityObserver: NSObjectProtocol {
4129
public class SentryReachability: NSObject {
4230
private var reachabilityObservers = NSHashTable<SentryReachabilityObserver>.weakObjects()
4331
private var currentConnectivity: SentryConnectivity = .none
44-
private var pathMonitor: Any? // NWPathMonitor for iOS 12+
32+
private var pathMonitor: NWPathMonitor?
4533
private let reachabilityQueue: DispatchQueue = DispatchQueue(label: "io.sentry.cocoa.connectivity", qos: .background, attributes: [])
46-
private let observersLock = NSLock()
34+
private let observersLock = NSRecursiveLock()
4735

4836
#if DEBUG || SENTRY_TEST || SENTRY_TEST_CI
4937
@objc public var skipRegisteringActualCallbacks = false
@@ -77,24 +65,10 @@ public class SentryReachability: NSObject {
7765
}
7866
#endif // DEBUG || SENTRY_TEST || SENTRY_TEST_CI
7967

80-
if #available(iOS 12.0, macOS 10.14, tvOS 12.0, visionOS 1.0, watchOS 5.0, *) {
81-
// If we don't use the main queue to start the monitor, the app seems to freeze on iOS 14.8 (SauceLabs)
82-
// Also do it async to avoid blocking the main thread
83-
// Right now `SentryDispatchQueueWrapper.dispatchAsyncOnMainQueue` is not used because the SDK starts on the main thread,
84-
// thus the block is executed in the main thread, not async.
85-
DispatchQueue.main.async { [weak self] in
86-
let monitor = NWPathMonitor()
87-
self?.pathMonitor = monitor
88-
monitor.pathUpdateHandler = self?.pathUpdateHandler
89-
monitor.start(queue: self?.reachabilityQueue ?? DispatchQueue.global(qos: .background))
90-
}
91-
} else {
92-
// For iOS 11 and earlier, simulate always being connected via WiFi
93-
SentrySDKLog.warning("NWPathMonitor not available. Using fallback: always connected via WiFi")
94-
reachabilityQueue.async { [weak self] in
95-
self?.connectivityCallback(.wiFi)
96-
}
97-
}
68+
let monitor = NWPathMonitor()
69+
self.pathMonitor = monitor
70+
monitor.pathUpdateHandler = self.pathUpdateHandler
71+
monitor.start(queue: self.reachabilityQueue)
9872
}
9973

10074
@objc(removeObserver:)
@@ -134,18 +108,15 @@ public class SentryReachability: NSObject {
134108
currentConnectivity = .none
135109

136110
// Clean up NWPathMonitor
137-
if #available(iOS 12.0, macOS 10.14, tvOS 12.0, visionOS 1.0, watchOS 5.0, *) {
138-
if let monitor = pathMonitor as? NWPathMonitor {
139-
SentrySDKLog.debug("Stopping NWPathMonitor")
140-
monitor.cancel()
141-
pathMonitor = nil
142-
}
111+
if let monitor = pathMonitor {
112+
SentrySDKLog.debug("Stopping NWPathMonitor")
113+
monitor.cancel()
114+
pathMonitor = nil
143115
}
144116

145117
SentrySDKLog.debug("Cleaning up reachability queue.")
146118
}
147119

148-
@available(iOS 12.0, macOS 10.14, tvOS 12.0, visionOS 1.0, watchOS 5.0, *)
149120
private func pathUpdateHandler(_ path: NWPath) {
150121
SentrySDKLog.debug("SentryPathUpdateHandler called with path status: \(path.status)")
151122

@@ -160,7 +131,6 @@ public class SentryReachability: NSObject {
160131
connectivityCallback(connectivity)
161132
}
162133

163-
@available(iOS 12.0, macOS 10.14, tvOS 12.0, visionOS 1.0, watchOS 5.0, *)
164134
private func connectivityFromPath(_ path: NWPath) -> SentryConnectivity {
165135
guard path.status == .satisfied else {
166136
return .none
@@ -220,20 +190,19 @@ public class SentryReachability: NSObject {
220190

221191
// MARK: - Test utils
222192
#if DEBUG || SENTRY_TEST || SENTRY_TEST_CI
223-
@available(iOS 12.0, macOS 10.14, tvOS 12.0, visionOS 1.0, watchOS 5.0, *)
224193
extension SentryReachability {
225-
@objc public func setReachabilityIgnoreActualCallback(_ value: Bool) {
194+
func setReachabilityIgnoreActualCallback(_ value: Bool) {
226195
SentrySDKLog.debug("Setting ignore actual callback to \(value)")
227196
ignoreActualCallback = value
228197
}
229198

230-
@objc public func triggerConnectivityCallback(_ connectivity: SentryConnectivity) {
199+
func triggerConnectivityCallback(_ connectivity: SentryConnectivity) {
231200
connectivityCallback(connectivity)
232201
}
233202
}
234203

235-
@_spi(Private) @objc public class SentryReachabilityTestHelper: NSObject {
236-
@objc static public func stringForSentryConnectivity(_ type: SentryConnectivity) -> String {
204+
class SentryReachabilityTestHelper: NSObject {
205+
static func stringForSentryConnectivity(_ type: SentryConnectivity) -> String {
237206
type.toString()
238207
}
239208
}

‎Tests/SentryTests/Networking/SentryReachabilitySwiftTests.swift‎

Lines changed: 115 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,120 @@
11
@_spi(Private) @testable import Sentry
22
import XCTest
33

4-
final class SentryReachabilitySwiftTests: XCTestCase {
4+
class TestSentryReachabilityObserver: NSObject, SentryReachabilityObserver {
5+
var connectivityChangedInvocations: UInt = 0
6+
7+
override init() {
8+
super.init()
9+
connectivityChangedInvocations = 0
10+
}
11+
12+
func connectivityChanged(_ connected: Bool, typeDescription: String) {
13+
print("Received connectivity notification: \(connected); type: \(typeDescription)")
14+
connectivityChangedInvocations += 1
15+
}
16+
}
517

18+
final class SentryReachabilitySwiftTests: XCTestCase {
19+
20+
private var reachability: SentryReachability!
21+
22+
override func setUp() {
23+
super.setUp()
24+
// Ignore the actual reachability callbacks, cause we call the callbacks manually.
25+
// Otherwise, the actual reachability callbacks are called during later unrelated tests causing
26+
// flakes.
27+
reachability = SentryReachability()
28+
reachability.skipRegisteringActualCallbacks = true
29+
reachability.setReachabilityIgnoreActualCallback(true)
30+
}
31+
32+
override func tearDown() {
33+
reachability.removeAllObservers()
34+
reachability.setReachabilityIgnoreActualCallback(false)
35+
reachability = nil
36+
super.tearDown()
37+
}
38+
39+
func testConnectivityRepresentations() {
40+
XCTAssertEqual("none", SentryReachabilityTestHelper.stringForSentryConnectivity(.none))
41+
XCTAssertEqual("wifi", SentryReachabilityTestHelper.stringForSentryConnectivity(.wiFi))
42+
#if canImport(UIKit)
43+
XCTAssertEqual("cellular", SentryReachabilityTestHelper.stringForSentryConnectivity(.cellular))
44+
#endif
45+
}
46+
47+
func testMultipleReachabilityObservers() {
48+
print("[Sentry] [TEST] creating observer A")
49+
let observerA = TestSentryReachabilityObserver()
50+
print("[Sentry] [TEST] adding observer A as reachability observer")
51+
reachability.add(observerA)
52+
53+
print("[Sentry] [TEST] throwaway reachability callback, setting to reachable")
54+
reachability.triggerConnectivityCallback(.wiFi) // ignored, as it's the first callback
55+
print("[Sentry] [TEST] reachability callback set to unreachable")
56+
reachability.triggerConnectivityCallback(.none)
57+
58+
print("[Sentry] [TEST] creating observer B")
59+
let observerB = TestSentryReachabilityObserver()
60+
print("[Sentry] [TEST] adding observer B as reachability observer")
61+
reachability.add(observerB)
62+
63+
print("[Sentry] [TEST] reachability callback set back to reachable")
64+
reachability.triggerConnectivityCallback(.wiFi)
65+
print("[Sentry] [TEST] reachability callback set back to unreachable")
66+
reachability.triggerConnectivityCallback(.none)
67+
68+
print("[Sentry] [TEST] removing observer B as reachability observer")
69+
reachability.remove(observerB)
70+
71+
print("[Sentry] [TEST] reachability callback set back to reachable")
72+
reachability.triggerConnectivityCallback(.wiFi)
73+
74+
XCTAssertEqual(5, observerA.connectivityChangedInvocations)
75+
XCTAssertEqual(2, observerB.connectivityChangedInvocations)
76+
77+
print("[Sentry] [TEST] removing observer A as reachability observer")
78+
reachability.remove(observerA)
79+
}
80+
81+
func testNoObservers() {
82+
let observer = TestSentryReachabilityObserver()
83+
reachability.add(observer)
84+
reachability.remove(observer)
85+
86+
reachability.triggerConnectivityCallback(.wiFi)
87+
88+
XCTAssertEqual(0, observer.connectivityChangedInvocations)
89+
90+
reachability.removeAllObservers()
91+
}
92+
93+
func testReportSameObserver_OnlyCalledOnce() {
94+
let observer = TestSentryReachabilityObserver()
95+
reachability.add(observer)
96+
reachability.add(observer)
97+
98+
reachability.triggerConnectivityCallback(.wiFi)
99+
100+
XCTAssertEqual(1, observer.connectivityChangedInvocations)
101+
102+
reachability.remove(observer)
103+
}
104+
105+
/**
106+
* We only want to make sure running the actual registering and unregistering callbacks doesn't
107+
* crash.
108+
*/
109+
func testRegisteringActualCallbacks() {
110+
reachability.skipRegisteringActualCallbacks = false
111+
112+
let observer = TestSentryReachabilityObserver()
113+
114+
reachability.add(observer)
115+
reachability.remove(observer)
116+
}
117+
6118
func testAddRemoveFromMultipleThreads() throws {
7119
let sut = SentryReachability()
8120
// Calling the methods of SCNetworkReachability in a tight loop from
@@ -11,16 +123,10 @@ final class SentryReachabilitySwiftTests: XCTestCase {
11123
// observers are adequately synchronized and not if we call
12124
// SCNetworkReachability correctly.
13125
sut.skipRegisteringActualCallbacks = true
14-
testConcurrentModifications(writeWork: {_ in
15-
sut.add(TestReachabilityObserver())
126+
testConcurrentModifications(writeWork: { _ in
127+
sut.add(TestSentryReachabilityObserver())
16128
}, readWork: {
17129
sut.removeAllObservers()
18130
})
19131
}
20132
}
21-
22-
class TestReachabilityObserver: NSObject, SentryReachabilityObserver {
23-
func connectivityChanged(_ connected: Bool, typeDescription: String) {
24-
// Do nothing
25-
}
26-
}

0 commit comments

Comments
 (0)