From 151d2c39093dadfda97f2c44d78c354633390e4f Mon Sep 17 00:00:00 2001 From: Paul LeMarquand Date: Fri, 14 Nov 2025 11:15:05 -0500 Subject: [PATCH] Improve Reliability of Reachability Detection when Fetching Dependencies 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. --- Sources/SourceControl/RepositoryManager.swift | 54 +++++-------------- 1 file changed, 12 insertions(+), 42 deletions(-) diff --git a/Sources/SourceControl/RepositoryManager.swift b/Sources/SourceControl/RepositoryManager.swift index 6cbf8bde3ac..248a2be308a 100644 --- a/Sources/SourceControl/RepositoryManager.swift +++ b/Sources/SourceControl/RepositoryManager.swift @@ -46,7 +46,7 @@ public class RepositoryManager: Cancellable { // Limits how many concurrent operations can be performed at once. private let asyncOperationQueue: AsyncOperationQueue - private var emitNoConnectivityWarning = ThreadSafeBox(true) + private var emitNoConnectivityWarning = ThreadSafeBox<[String: Bool]>([:]) /// Create a new empty manager. /// @@ -343,9 +343,11 @@ public class RepositoryManager: Cancellable { // If we are offline and have a valid cached repository, use the cache anyway. if try isOffline(error) && self.provider.isValidDirectory(cachedRepositoryPath, for: handle.repository) { // For the first offline use in the lifetime of this repository manager, emit a warning. - if self.emitNoConnectivityWarning.get(default: false) { - self.emitNoConnectivityWarning.put(false) - observabilityScope.emit(warning: "no connectivity, using previously cached repository state") + var warningState = self.emitNoConnectivityWarning.get(default: [:]) + if !(warningState[handle.repository.url] ?? false) { + warningState[handle.repository.url] = true + self.emitNoConnectivityWarning.put(warningState) + observabilityScope.emit(warning: "no connectivity to \(handle.repository.url), using previously cached repository state") } observabilityScope.emit(info: "using previously cached repository state for \(package)") @@ -645,45 +647,13 @@ extension RepositorySpecifier { } } -#if canImport(SystemConfiguration) -import SystemConfiguration - -private struct Reachability { - let reachability: SCNetworkReachability - - init?() { - var emptyAddress = sockaddr() - emptyAddress.sa_len = UInt8(MemoryLayout.size) - emptyAddress.sa_family = sa_family_t(AF_INET) - - guard let reachability = withUnsafePointer(to: &emptyAddress, { - SCNetworkReachabilityCreateWithAddress(nil, UnsafePointer($0)) - }) else { - return nil - } - self.reachability = reachability - } - - var connectionRequired: Bool { - var flags = SCNetworkReachabilityFlags() - let hasFlags = withUnsafeMutablePointer(to: &flags) { - SCNetworkReachabilityGetFlags(reachability, UnsafeMutablePointer($0)) - } - guard hasFlags else { return false } - guard flags.contains(.reachable) else { - return true - } - return flags.contains(.connectionRequired) || flags.contains(.transientConnection) - } -} - -fileprivate func isOffline(_ error: Swift.Error) -> Bool { - return Reachability()?.connectionRequired == true -} -#else +/// This used to rely on the SCNetworkReachability APIs on Darwin platforms and +/// the string match elsewhere, however SCNetworkReachability has not been recommended +/// to determine online/offline status. Instead do a simple string match on the error +/// message indicating the host could not be resolved. +/// This may falsely report offline status if the host is down, but this is effectively +/// equivalent from the user's perspective. fileprivate func isOffline(_ error: Swift.Error) -> Bool { - // TODO: Find a better way to determine reachability on non-Darwin platforms. return "\(error)".contains("Could not resolve host") } -#endif