Remove the reconnect/disconnect logic from the connection tester

Task/Issue URL: https://app.asana.com/0/0/1206173513538620/f

iOS PR: duckduckgo/iOS#2272
macOS PR: duckduckgo/macos-browser#1970
What kind of version bump will this require?: Patch

## Description

Removes the logic that would reconnect / disconnect NetP in case of trouble.

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))
     }
 }

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

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<Bool, Never> = Just(false).eraseToAnyPublisher()
+    public var recentValue = false
+
+    public init() {}
+}