Do retry-then-signout when startTunnel() fails as well (#2723)

Fixes #2718

It appears that #2687 missed the case where the tunnel failed to start
for some reason, so the tunnel status never goes to "Connecting" and
then back to "Disconnected" -- we were reacting to the tunnel status
becoming "Disconnected".

In this PR, we do the retry when startTunnel() throws an error as well.

swift/apple/FirezoneKit/Sources/FirezoneKit/Features/MainView.swift

@@ -64,12 +64,8 @@ import SwiftUI
     }
 
     func startTunnel() async {
-      do {
-        if case .signedIn = self.loginStatus {
-          try await appStore.tunnel.start()
-        }
-      } catch {
-        logger.error("Error starting tunnel: \(String(describing: error))")
+      if case .signedIn = self.loginStatus {
+        appStore.auth.startTunnel()
       }
     }
 

swift/apple/FirezoneKit/Sources/FirezoneKit/Stores/AppStore.swift

@@ -30,35 +30,6 @@ final class AppStore: ObservableObject {
       self?.objectWillChange.send()
     }
     .store(in: &cancellables)
-
-    auth.$loginStatus
-      .receive(on: mainQueue)
-      .sink { [weak self] loginStatus in
-        Task { [weak self] in
-          await self?.handleLoginStatusChanged(loginStatus)
-        }
-      }
-      .store(in: &cancellables)
-  }
-
-  private func handleLoginStatusChanged(_ loginStatus: AuthStore.LoginStatus) async {
-    logger.log("\(#function): login status = \(loginStatus)")
-    switch loginStatus {
-    case .signedIn:
-      do {
-        try await tunnel.start()
-      } catch {
-        logger.error("\(#function): Error starting tunnel: \(String(describing: error))")
-      }
-    case .signedOut:
-      do {
-        try await tunnel.stop()
-      } catch {
-        logger.error("\(#function): Error stopping tunnel: \(String(describing: error))")
-      }
-    case .uninitialized:
-      break
-    }
   }
 
   private func signOutAndStopTunnel() {

swift/apple/FirezoneKit/Sources/FirezoneKit/Stores/AuthStore.swift

@@ -59,7 +59,12 @@ final class AuthStore: ObservableObject {
 
   private var cancellables = Set<AnyCancellable>()
 
-  @Published private(set) var loginStatus: LoginStatus
+  @Published private(set) var loginStatus: LoginStatus {
+    didSet {
+      self.handleLoginStatusChanged()
+    }
+  }
+
   private var status: NEVPNStatus = .invalid
 
   private static let maxReconnectionAttemptCount = 3
@@ -101,19 +106,7 @@ final class AuthStore: ObservableObject {
               case .signoutImmediately:
                 self.signOut()
               case .retryThenSignout:
-                let shouldReconnect = (self.reconnectionAttemptsRemaining > 0)
-                self.reconnectionAttemptsRemaining = self.reconnectionAttemptsRemaining - 1
-                if shouldReconnect {
-                  self.logger.log(
-                    "\(#function): Will try to reconnect after 1 second (\(self.reconnectionAttemptsRemaining) attempts after this)"
-                  )
-                  DispatchQueue.main.asyncAfter(deadline: .now() + .seconds(1)) {
-                    self.logger.log("\(#function): Trying to reconnect")
-                    self.startTunnel()
-                  }
-                } else {
-                  self.signOut()
-                }
+                self.retryStartTunnel()
               }
             } else {
               self.logger.log("\(#function): Tunnel shutdown event not found")
@@ -215,10 +208,54 @@ final class AuthStore: ObservableObject {
         try await tunnelStore.start()
       } catch {
         logger.error("\(#function): Error starting tunnel: \(String(describing: error))")
+        self.retryStartTunnel()
       }
     }
   }
 
+  func retryStartTunnel() {
+    // Try to reconnect, but don't try more than 3 times at a time.
+    // If this gets called the third time, sign out.
+    let shouldReconnect = (self.reconnectionAttemptsRemaining > 0)
+    self.reconnectionAttemptsRemaining = self.reconnectionAttemptsRemaining - 1
+    if shouldReconnect {
+      self.logger.log(
+        "\(#function): Will try to reconnect after 1 second (\(self.reconnectionAttemptsRemaining) attempts after this)"
+      )
+      DispatchQueue.main.asyncAfter(deadline: .now() + .seconds(1)) {
+        self.logger.log("\(#function): Trying to reconnect")
+        self.startTunnel()
+      }
+    } else {
+      self.signOut()
+    }
+  }
+
+  private func handleLoginStatusChanged() {
+    logger.log("\(#function): Login status: \(self.loginStatus)")
+    switch self.loginStatus {
+    case .signedIn:
+      Task {
+        do {
+          try await tunnelStore.start()
+        } catch {
+          logger.error("\(#function): Error starting tunnel: \(String(describing: error))")
+          self.retryStartTunnel()
+        }
+      }
+    case .signedOut:
+      Task {
+        do {
+          try await tunnelStore.stop()
+        } catch {
+          logger.error("\(#function): Error stopping tunnel: \(String(describing: error))")
+        }
+      }
+    case .uninitialized:
+      break
+    }
+  }
+
   func resetReconnectionAttemptsRemaining() {
     self.reconnectionAttemptsRemaining = Self.maxReconnectionAttemptCount
   }

swift/apple/FirezoneKit/Sources/FirezoneKit/Stores/TunnelStore.swift

@@ -192,7 +192,13 @@ final class TunnelStore: ObservableObject {
     }
 
     if case .signedIn(let authBaseURL, let accountId, let tokenReference) = self.tunnelAuthStatus {
-      try await saveAuthStatus(.signedOut(authBaseURL: authBaseURL, accountId: accountId))
+      do {
+        try await saveAuthStatus(.signedOut(authBaseURL: authBaseURL, accountId: accountId))
+      } catch {
+        TunnelStore.logger.trace(
+          "\(#function): Error saving signed out auth status: \(error)"
+        )
+      }
       return tokenReference
     }
 
@@ -416,6 +422,7 @@ extension NETunnelProviderManager {
 
       protocolConfiguration.providerConfiguration = providerConfig
 
+      ensureTunnelConfigurationIsValid()
       try await saveToPreferences()
     }
   }
@@ -471,7 +478,30 @@ extension NETunnelProviderManager {
       protocolConfiguration.providerConfiguration = providerConfig
       protocolConfiguration.serverAddress = advancedSettings.apiURLString
 
+      ensureTunnelConfigurationIsValid()
       try await saveToPreferences()
     }
   }
+
+  private func ensureTunnelConfigurationIsValid() {
+    // Ensure the tunnel config has required values populated, because
+    // to even sign out, we need saveToPreferences() to succeed.
+    if let protocolConfiguration = protocolConfiguration as? NETunnelProviderProtocol {
+      protocolConfiguration.providerBundleIdentifier = Bundle.main.bundleIdentifier.map {
+        "\($0).network-extension"
+      }
+      if protocolConfiguration.serverAddress?.isEmpty ?? true {
+        protocolConfiguration.serverAddress = "unknown-server"
+      }
+    } else {
+      let protocolConfiguration = NETunnelProviderProtocol()
+      protocolConfiguration.providerBundleIdentifier = Bundle.main.bundleIdentifier.map {
+        "\($0).network-extension"
+      }
+      protocolConfiguration.serverAddress = "unknown-server"
+    }
+    if localizedDescription?.isEmpty ?? true {
+      localizedDescription = "Firezone"
+    }
+  }
 }

swift/apple/FirezoneKit/Sources/FirezoneKit/Views/MenuBar.swift

@@ -201,14 +201,8 @@
     }
 
     @objc private func reconnectButtonTapped() {
-      Task {
-        if case .signedIn = appStore?.auth.loginStatus {
-          do {
-            try await appStore?.tunnel.start()
-          } catch {
-            logger.error("error connecting to tunnel (reconnect): \(String(describing: error))")
-          }
-        }
+      if case .signedIn = appStore?.auth.loginStatus {
+        appStore?.auth.startTunnel()
       }
     }