diff --git a/Sources/NetworkProtection/Diagnostics/NetworkProtectionConnectionTester.swift b/Sources/NetworkProtection/Diagnostics/NetworkProtectionConnectionTester.swift index 2a059b5b6..c765177e8 100644 --- a/Sources/NetworkProtection/Diagnostics/NetworkProtectionConnectionTester.swift +++ b/Sources/NetworkProtection/Diagnostics/NetworkProtectionConnectionTester.swift @@ -94,13 +94,13 @@ final class NetworkProtectionConnectionTester { // MARK: - Test result handling private var failureCount = 0 - private let resultHandler: @MainActor (Result, Bool) -> Void + private let resultHandler: @MainActor (Result) -> Void private var simulateFailure = false // MARK: - Init & deinit - init(timerQueue: DispatchQueue, log: OSLog, resultHandler: @escaping @MainActor (Result, Bool) -> Void) { + init(timerQueue: DispatchQueue, log: OSLog, resultHandler: @escaping @MainActor (Result) -> Void) { self.timerQueue = timerQueue self.log = log self.resultHandler = resultHandler @@ -182,7 +182,7 @@ final class NetworkProtectionConnectionTester { if testImmediately { do { - try await testConnection(isStartupTest: true) + try await testConnection() } catch { os_log("Rethrowing exception", log: log) throw error @@ -194,17 +194,13 @@ final class NetworkProtectionConnectionTester { timer.schedule(deadline: .now() + self.intervalBetweenTests, repeating: self.intervalBetweenTests) timer.setEventHandler { [weak self] in - guard let testConnection = self?.testConnection(isStartupTest:) else { - return - } - - Task { + Task { [self] in // During regular connection tests we don't care about the error thrown // by this method, as it'll be handled through the result handler callback. // The error we're ignoring here is only used when this class is initialized // with an immediate test, to know whether the connection is up while the user // still sees "Connecting..." - try? await testConnection(false) + try? await self?.testConnection() } } @@ -235,7 +231,7 @@ final class NetworkProtectionConnectionTester { // MARK: - Testing the connection - func testConnection(isStartupTest: Bool) async throws { + func testConnection() async throws { guard let tunnelInterface = tunnelInterface else { os_log("No interface to test!", log: log, type: .error) return @@ -267,11 +263,11 @@ final class NetworkProtectionConnectionTester { if onlyVPNIsDown { os_log("👎 VPN is DOWN", log: log) - await handleDisconnected(isStartupTest: isStartupTest) + await handleDisconnected() } else { os_log("👍 VPN: \(vpnIsConnected ? "UP" : "DOWN") local: \(localIsConnected ? "UP" : "DOWN")", log: log) - await handleConnected(isStartupTest: isStartupTest) + await handleConnected() } } @@ -303,19 +299,19 @@ final class NetworkProtectionConnectionTester { // MARK: - Result handling @MainActor - private func handleConnected(isStartupTest: Bool) { + private func handleConnected() { if failureCount == 0 { - resultHandler(.connected, isStartupTest) + resultHandler(.connected) } else if failureCount > 0 { failureCount = 0 - resultHandler(.reconnected, isStartupTest) + resultHandler(.reconnected) } } @MainActor - private func handleDisconnected(isStartupTest: Bool) { + private func handleDisconnected() { failureCount += 1 - resultHandler(.disconnected(failureCount: failureCount), isStartupTest) + resultHandler(.disconnected(failureCount: failureCount)) } } diff --git a/Sources/NetworkProtection/PacketTunnelProvider.swift b/Sources/NetworkProtection/PacketTunnelProvider.swift index 5748e1587..f2634d486 100644 --- a/Sources/NetworkProtection/PacketTunnelProvider.swift +++ b/Sources/NetworkProtection/PacketTunnelProvider.swift @@ -47,7 +47,6 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { enum TunnelError: LocalizedError { case startingTunnelWithoutAuthToken case couldNotGenerateTunnelConfiguration(internalError: Error) - case couldNotFixConnection case simulateTunnelFailureError var errorDescription: String? { @@ -58,11 +57,6 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { return "Failed to generate a tunnel configuration: \(internalError.localizedDescription)" case .simulateTunnelFailureError: return "Simulated a tunnel error as requested" - default: - // This is probably not the most elegant error to show to a user but - // it's a great way to get detailed reports for those cases we haven't - // provided good descriptions for yet. - return "Tunnel error: \(String(describing: self))" } } } @@ -242,7 +236,7 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { private var isConnectionTesterEnabled: Bool = true private lazy var connectionTester: NetworkProtectionConnectionTester = { - NetworkProtectionConnectionTester(timerQueue: timerQueue, log: .networkProtectionConnectionTesterLog) { @MainActor [weak self] (result, isStartupTest) in + NetworkProtectionConnectionTester(timerQueue: timerQueue, log: .networkProtectionConnectionTesterLog) { @MainActor [weak self] result in guard let self else { return } switch result { @@ -257,17 +251,6 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { case .disconnected(let failureCount): self.tunnelHealth.isHavingConnectivityIssues = true self.bandwidthAnalyzer.reset() - - if failureCount == 1 { - self.notificationsPresenter.showReconnectingNotification() - - // Only do these things if this is not a connection startup test. - if !isStartupTest { - self.fixTunnel() - } - } else if failureCount == 2 { - self.stopTunnel(with: TunnelError.couldNotFixConnection) - } } } }() @@ -686,28 +669,6 @@ open class PacketTunnelProvider: NEPacketTunnelProvider { controllerErrorStore.lastErrorMessage = nil } - /// Intentionally not async, so that we won't lock whoever called this method. This method will race against the tester - /// to see if it can fix the connection before the next failure. - /// - private func fixTunnel() { - Task { - let serverSelectionMethod: NetworkProtectionServerSelectionMethod - - if let lastServerName = lastSelectedServerInfo?.name { - serverSelectionMethod = .avoidServer(serverName: lastServerName) - } else { - assertionFailure("We should not have a situation where the VPN is trying to fix the tunnel and there's no previous server info") - serverSelectionMethod = .automatic - } - - do { - try await updateTunnelConfiguration(environment: settings.selectedEnvironment, serverSelectionMethod: serverSelectionMethod) - } catch { - return - } - } - } - // MARK: - Tunnel Configuration @MainActor diff --git a/Sources/NetworkProtection/Status/ConnectivityIssueObserver/DisabledConnectivityIssueObserver.swift b/Sources/NetworkProtection/Status/ConnectivityIssueObserver/DisabledConnectivityIssueObserver.swift new file mode 100644 index 000000000..a263b3558 --- /dev/null +++ b/Sources/NetworkProtection/Status/ConnectivityIssueObserver/DisabledConnectivityIssueObserver.swift @@ -0,0 +1,32 @@ +// +// DisabledConnectivityIssueObserver.swift +// +// Copyright © 2023 DuckDuckGo. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import Combine +import Foundation + +/// This is a convenience temporary disabler for the connectivity issues observer. Will always report no issues. +/// +/// This is useful since we decided to momentarily disable connection issue reporting in the UI: +/// ref: https://app.asana.com/0/0/1206071632962016/1206144227620065/f +/// +public final class DisabledConnectivityIssueObserver: ConnectivityIssueObserver { + public var publisher: AnyPublisher = Just(false).eraseToAnyPublisher() + public var recentValue = false + + public init() {} +}