Skip to content

Commit 2096cff

Browse files
authored
Improve reliability of online/offline detection when fetching dependencies (swiftlang#9378)
When fetching dependencies on Darwin we relied on SCNetworkReachability to determine if the user is offline when a dependency fetch fails. This check considered `SCNetworkReachabilityFlags.transientConnection` to be considered "offline", however this state still has connectivity. If the user had the dependency cached, this would result in the cached local dependency being used instead of a fetch of the latest dependency. Ultimately this meant very occasionally the user may end up with a stale dependency when they had all the requirements met to ge the latest one. Move off of the SCNetworkReachability APIs as they're not really recommended for determining online status, and simply check if the fetch failed with a "Could not resolve host" error. This is what we actually want to check for anyway. Even if the host is down but the connection is fine the host being down is what is relevant to whether or not we should try and use the cached dependency.
1 parent 84b3e6e commit 2096cff

File tree

1 file changed

+12
-42
lines changed

1 file changed

+12
-42
lines changed

Sources/SourceControl/RepositoryManager.swift

Lines changed: 12 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ public class RepositoryManager: Cancellable {
4646
// Limits how many concurrent operations can be performed at once.
4747
private let asyncOperationQueue: AsyncOperationQueue
4848

49-
private var emitNoConnectivityWarning = ThreadSafeBox<Bool>(true)
49+
private var emitNoConnectivityWarning = ThreadSafeBox<[String: Bool]>([:])
5050

5151
/// Create a new empty manager.
5252
///
@@ -343,9 +343,11 @@ public class RepositoryManager: Cancellable {
343343
// If we are offline and have a valid cached repository, use the cache anyway.
344344
if try isOffline(error) && self.provider.isValidDirectory(cachedRepositoryPath, for: handle.repository) {
345345
// For the first offline use in the lifetime of this repository manager, emit a warning.
346-
if self.emitNoConnectivityWarning.get(default: false) {
347-
self.emitNoConnectivityWarning.put(false)
348-
observabilityScope.emit(warning: "no connectivity, using previously cached repository state")
346+
var warningState = self.emitNoConnectivityWarning.get(default: [:])
347+
if !(warningState[handle.repository.url] ?? false) {
348+
warningState[handle.repository.url] = true
349+
self.emitNoConnectivityWarning.put(warningState)
350+
observabilityScope.emit(warning: "no connectivity to \(handle.repository.url), using previously cached repository state")
349351
}
350352
observabilityScope.emit(info: "using previously cached repository state for \(package)")
351353

@@ -645,45 +647,13 @@ extension RepositorySpecifier {
645647
}
646648
}
647649

648-
#if canImport(SystemConfiguration)
649-
import SystemConfiguration
650-
651-
private struct Reachability {
652-
let reachability: SCNetworkReachability
653-
654-
init?() {
655-
var emptyAddress = sockaddr()
656-
emptyAddress.sa_len = UInt8(MemoryLayout<sockaddr>.size)
657-
emptyAddress.sa_family = sa_family_t(AF_INET)
658-
659-
guard let reachability = withUnsafePointer(to: &emptyAddress, {
660-
SCNetworkReachabilityCreateWithAddress(nil, UnsafePointer($0))
661-
}) else {
662-
return nil
663-
}
664-
self.reachability = reachability
665-
}
666-
667-
var connectionRequired: Bool {
668-
var flags = SCNetworkReachabilityFlags()
669-
let hasFlags = withUnsafeMutablePointer(to: &flags) {
670-
SCNetworkReachabilityGetFlags(reachability, UnsafeMutablePointer($0))
671-
}
672-
guard hasFlags else { return false }
673-
guard flags.contains(.reachable) else {
674-
return true
675-
}
676-
return flags.contains(.connectionRequired) || flags.contains(.transientConnection)
677-
}
678-
}
679-
680-
fileprivate func isOffline(_ error: Swift.Error) -> Bool {
681-
return Reachability()?.connectionRequired == true
682-
}
683-
#else
650+
/// This used to rely on the SCNetworkReachability APIs on Darwin platforms and
651+
/// the string match elsewhere, however SCNetworkReachability has not been recommended
652+
/// to determine online/offline status. Instead do a simple string match on the error
653+
/// message indicating the host could not be resolved.
654+
/// This may falsely report offline status if the host is down, but this is effectively
655+
/// equivalent from the user's perspective.
684656
fileprivate func isOffline(_ error: Swift.Error) -> Bool {
685-
// TODO: Find a better way to determine reachability on non-Darwin platforms.
686657
return "\(error)".contains("Could not resolve host")
687658
}
688-
#endif
689659

0 commit comments

Comments
 (0)