refactor(apple): Make IPC calls async, bubbling errors (#8062)

`fetchResources` is an IPC call, and we can use
`withCheckedThrowingContinuation` like the others to yield while we wait
for the provider to respond.

The particular sentry issue related to this isn't because we are
necessarily blocking the task thread, rather, I suspect it's when
applying the fetched Resources to the UI that we're slow. There isn't
much we can do about this, but this PR will only help.

Because we're using a timer that fires off a closure to do this, we
still use a `callback` inside the timer to actually set the Resources on
the main `Store`, which updates the UI.

Unfortunately refactoring these IPC calls lead to somewhat of a ball of
yarn, so the best way to summarize the spirit of this PR is:

- Ensure IPC calls use `withCheckedThrowingContinuation` where possible
- Thusly, marking these functions `async throws`
- Bubble these errors up the view where we can ultimately decide what to
do with them
- Keep VPN state management and conditional logic based on `NEVPNStatus`
in the vpnConfigurationManager

swift/apple/FirezoneKit/Sources/FirezoneKit/Managers/VPNConfigurationManager.swift

@@ -18,8 +18,9 @@ enum VPNConfigurationManagerError: Error {
   case managerNotInitialized
   case cannotLoad
   case decodeIPCDataFailed
-  case invalidStatusChange
+  case invalidNotification
   case noIPCData
+  case invalidStatus(NEVPNStatus)
 
   var localizedDescription: String {
     switch self {
@@ -27,12 +28,14 @@ enum VPNConfigurationManagerError: Error {
       return "Manager doesn't seem initialized."
     case .decodeIPCDataFailed:
       return "Decoding IPC data failed."
-    case .invalidStatusChange:
+    case .invalidNotification:
       return "NEVPNStatusDidChange notification doesn't seem to be valid."
     case .cannotLoad:
       return "Could not load VPN configurations!"
     case .noIPCData:
       return "No IPC data returned from the XPC connection!"
+    case .invalidStatus(let status):
+      return "The IPC operation couldn't complete because the VPN status is \(status)."
     }
   }
 }
@@ -292,72 +295,71 @@ public class VPNConfigurationManager {
       options.merge(["id": id as NSObject]) { _, new in new }
     }
 
-    try session()?.startTunnel(options: options)
+    try session().startTunnel(options: options)
   }
 
-  func stop(clearToken: Bool = false) {
-    if clearToken {
-      do {
-        try session()?.sendProviderMessage(encoder.encode(TunnelMessage.signOut)) { _ in
-          self.session()?.stopTunnel()
-        }
-      } catch {
-        Log.error(error)
-      }
-    } else {
-      session()?.stopTunnel()
-    }
+  func signOut() throws {
+    try session([.connected, .connecting, .reasserting]).stopTunnel()
+    try session().sendProviderMessage(encoder.encode(TunnelMessage.signOut))
   }
 
-  func updateInternetResourceState() {
-    guard session()?.status == .connected else { return }
-
-    try? session()?.sendProviderMessage(encoder.encode(TunnelMessage.internetResourceEnabled(internetResourceEnabled)))
+  func stop() throws {
+    try session([.connected, .connecting, .reasserting]).stopTunnel()
   }
 
-  func toggleInternetResource(enabled: Bool) {
+  func updateInternetResourceState() throws {
+    try session([.connected]).sendProviderMessage(
+      encoder.encode(TunnelMessage.internetResourceEnabled(internetResourceEnabled)))
+  }
+
+  func toggleInternetResource(enabled: Bool) throws {
     internetResourceEnabled = enabled
-    updateInternetResourceState()
+    try updateInternetResourceState()
   }
 
-  func fetchResources(callback: @escaping @MainActor (ResourceList) -> Void) {
-    guard session()?.status == .connected else { return }
+  func fetchResources() async throws -> ResourceList {
+    return try await withCheckedThrowingContinuation { continuation in
+      do {
+        // Request list of resources from the provider. We send the hash of the resource list we already have.
+        // If it differs, we'll get the full list in the callback. If not, we'll get nil.
+        try session([.connected]).sendProviderMessage(
+          encoder.encode(TunnelMessage.getResourceList(resourceListHash))) { data in
 
-    do {
-      try session()?.sendProviderMessage(encoder.encode(TunnelMessage.getResourceList(resourceListHash))) { data in
-        if let data = data {
+          guard let data = data
+          else {
+            // No data returned; Resources haven't changed
+            continuation.resume(returning: self.resourcesListCache)
+
+            return
+          }
+
+          // Save hash to compare against
           self.resourceListHash = Data(SHA256.hash(data: data))
+
           let decoder = JSONDecoder()
           decoder.keyDecodingStrategy = .convertFromSnakeCase
 
-          guard let decoded = try? decoder.decode([Resource].self, from: data)
-          else {
-            fatalError("Should be able to decode ResourceList")
+          do {
+            let decoded = try decoder.decode([Resource].self, from: data)
+            self.resourcesListCache = ResourceList.loaded(decoded)
+
+            continuation.resume(returning: self.resourcesListCache)
+          } catch {
+            continuation.resume(throwing: error)
           }
-
-          self.resourcesListCache = ResourceList.loaded(decoded)
         }
-
-        Task { await MainActor.run { callback(self.resourcesListCache) } }
+      } catch {
+        continuation.resume(throwing: error)
       }
-    } catch {
-      Log.error(error)
     }
   }
 
   func clearLogs() async throws {
     return try await withCheckedThrowingContinuation { continuation in
-      guard let session = session()
-      else {
-        continuation.resume(throwing: VPNConfigurationManagerError.managerNotInitialized)
-
-        return
-      }
-
       do {
-        try session.sendProviderMessage(
-          encoder.encode(TunnelMessage.clearLogs)
-        ) { _ in continuation.resume() }
+        try session().sendProviderMessage(encoder.encode(TunnelMessage.clearLogs)) { _ in
+          continuation.resume()
+        }
       } catch {
         continuation.resume(throwing: error)
       }
@@ -366,15 +368,9 @@ public class VPNConfigurationManager {
 
   func getLogFolderSize() async throws -> Int64 {
     return try await withCheckedThrowingContinuation { continuation in
-      guard let session = session()
-      else {
-        continuation.resume(throwing: VPNConfigurationManagerError.managerNotInitialized)
-
-        return
-      }
 
       do {
-        try session.sendProviderMessage(
+        try session().sendProviderMessage(
           encoder.encode(TunnelMessage.getLogFolderSize)
         ) { data in
 
@@ -406,7 +402,7 @@ public class VPNConfigurationManager {
 
     func loop() {
       do {
-        try session()?.sendProviderMessage(
+        try session().sendProviderMessage(
           encoder.encode(TunnelMessage.exportLogs)
         ) { data in
           guard let data = data
@@ -443,15 +439,8 @@ public class VPNConfigurationManager {
 
   func consumeStopReason() async throws -> NEProviderStopReason? {
     return try await withCheckedThrowingContinuation { continuation in
-      guard let session = session()
-      else {
-        continuation.resume(throwing: VPNConfigurationManagerError.managerNotInitialized)
-
-        return
-      }
-
       do {
-        try session.sendProviderMessage(
+        try session().sendProviderMessage(
           encoder.encode(TunnelMessage.consumeStopReason)
         ) { data in
 
@@ -472,8 +461,17 @@ public class VPNConfigurationManager {
     }
   }
 
-  private func session() -> NETunnelProviderSession? {
-    return manager?.connection as? NETunnelProviderSession
+  private func session(_ requiredStatuses: Set<NEVPNStatus> = []) throws -> NETunnelProviderSession {
+    guard let session = manager?.connection as? NETunnelProviderSession
+    else {
+      throw VPNConfigurationManagerError.managerNotInitialized
+    }
+
+    if requiredStatuses.isEmpty || requiredStatuses.contains(session.status) {
+      return session
+    }
+
+    throw VPNConfigurationManagerError.invalidStatus(session.status)
   }
 
   // Subscribe to system notifications about our VPN status changing
@@ -495,7 +493,7 @@ public class VPNConfigurationManager {
         ) {
           guard let session = notification.object as? NETunnelProviderSession
           else {
-            Log.error(VPNConfigurationManagerError.invalidStatusChange)
+            Log.error(VPNConfigurationManagerError.invalidNotification)
             return
           }
 

swift/apple/FirezoneKit/Sources/FirezoneKit/Stores/Store.swift

@@ -126,25 +126,16 @@ public final class Store: ObservableObject {
   }
 
   private func start(token: String? = nil) throws {
-    guard status == .disconnected
-    else {
-      Log.log("\(#function): Already connected")
-      return
-    }
-
     try self.vpnConfigurationManager.start(token: token)
   }
 
-  func stop(clearToken: Bool = false) {
-    guard [.connected, .connecting, .reasserting].contains(status)
-    else { return }
-
-    self.vpnConfigurationManager.stop(clearToken: clearToken)
+  func stop() throws {
+    try self.vpnConfigurationManager.stop()
   }
 
   func signIn(authResponse: AuthResponse) async throws {
     // Save actorName
-    await MainActor.run { self.actorName = authResponse.actorName }
+    self.actorName = authResponse.actorName
 
     try await self.vpnConfigurationManager.saveSettings(settings)
     try await self.vpnConfigurationManager.saveAuthResponse(authResponse)
@@ -153,9 +144,8 @@ public final class Store: ObservableObject {
     try self.vpnConfigurationManager.start(token: authResponse.token)
   }
 
-  func signOut() async throws {
-    // Stop tunnel and clear token
-    stop(clearToken: true)
+  func signOut() throws {
+    try self.vpnConfigurationManager.signOut()
   }
 
   // Network Extensions don't have a 2-way binding up to the GUI process,
@@ -171,8 +161,13 @@ public final class Store: ObservableObject {
 
     // Define the Timer's closure
     let updateResources: @Sendable (Timer) -> Void = { _ in
-      Task.detached { [weak self] in
-        await self?.vpnConfigurationManager.fetchResources(callback: callback)
+      Task {
+        do {
+          let resources = try await self.vpnConfigurationManager.fetchResources()
+          await callback(resources)
+        } catch {
+          Log.error(error)
+        }
       }
     }
 
@@ -193,23 +188,15 @@ public final class Store: ObservableObject {
     resourcesTimer = nil
   }
 
-  func save(_ newSettings: Settings) {
-    Task.detached { [weak self] in
-      guard let self else { return }
-
-      do {
-        try await self.vpnConfigurationManager.saveSettings(newSettings)
-        await MainActor.run { self.settings = newSettings }
-      } catch {
-        Log.error(error)
-      }
-    }
+  func save(_ newSettings: Settings) async throws {
+    try await self.vpnConfigurationManager.saveSettings(newSettings)
+    self.settings = newSettings
   }
 
-  func toggleInternetResource(enabled: Bool) {
-    self.vpnConfigurationManager.toggleInternetResource(enabled: enabled)
+  func toggleInternetResource(enabled: Bool) async throws {
+    try self.vpnConfigurationManager.toggleInternetResource(enabled: enabled)
     var newSettings = settings
     newSettings.internetResourceEnabled = self.vpnConfigurationManager.internetResourceEnabled
-    save(newSettings)
+    try await save(newSettings)
   }
 }

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

@@ -276,9 +276,7 @@ public final class MenuBar: NSObject, ObservableObject {
   }
 
   @objc private func signOutButtonTapped() {
-    Task.detached { [weak self] in
-      try await self?.model.store.signOut()
-    }
+    do { try self.model.store.signOut() } catch { Log.error(error) }
   }
 
   @objc private func grantPermissionMenuItemTapped() {
@@ -340,11 +338,9 @@ public final class MenuBar: NSObject, ObservableObject {
   }
 
   @objc private func quitButtonTapped() {
-    Task.detached { [weak self] in
-      guard let self else { return }
-
-      await self.model.store.stop()
-      await NSApp.terminate(self)
+    Task {
+      do { try self.model.store.stop() } catch { Log.error(error) }
+      NSApp.terminate(self)
     }
   }
 
@@ -809,8 +805,15 @@ public final class MenuBar: NSObject, ObservableObject {
   }
 
   @objc private func internetResourceToggle(_ sender: NSMenuItem) {
-    self.model.store.toggleInternetResource(enabled: !model.store.internetResourceEnabled())
-    sender.title = internetResourceToggleTitle()
+    Task {
+      do {
+        try await self.model.store.toggleInternetResource(enabled: !model.store.internetResourceEnabled())
+      } catch {
+        Log.error(error)
+      }
+
+      sender.title = internetResourceToggleTitle()
+    }
   }
 
   @objc private func resourceURLTapped(_ sender: AnyObject?) {

swift/apple/FirezoneKit/Sources/FirezoneKit/Views/ResourceView.swift

@@ -245,7 +245,13 @@ struct ToggleInternetResourceButton: View {
   var body: some View {
     Button(
       action: {
-        model.store.toggleInternetResource(enabled: !model.isInternetResourceEnabled())
+        Task {
+          do {
+            try await model.store.toggleInternetResource(enabled: !model.isInternetResourceEnabled())
+          } catch {
+            Log.error(error)
+          }
+        }
       },
       label: {
         HStack {

swift/apple/FirezoneKit/Sources/FirezoneKit/Views/SessionView.swift

@@ -49,7 +49,7 @@ public final class SessionViewModel: ObservableObject {
 
         if status == .connected {
           store.beginUpdatingResources { resources in
-            Task { await MainActor.run { self.resources = resources } }
+            self.resources = resources
           }
         } else {
           store.endUpdatingResources()

swift/apple/FirezoneKit/Sources/FirezoneKit/Views/SettingsView.swift

@@ -50,13 +50,17 @@ public final class SettingsViewModel: ObservableObject {
   }
 
   func saveSettings() {
-      if [.connected, .connecting, .reasserting].contains(store.status) {
-        Task.detached { [weak self] in
-          do { try await self?.store.signOut() } catch { Log.error(error) }
+    Task {
+      do {
+        if [.connected, .connecting, .reasserting].contains(store.status) {
+          try self.store.signOut()
         }
-      }
 
-      store.save(settings)
+        try await store.save(settings)
+      } catch {
+        Log.error(error)
+      }
+    }
   }
 
   // Calculates the total size of our logs by summing the size of the

swift/apple/FirezoneKit/Sources/FirezoneKit/Views/iOSNavigationView.swift

@@ -118,9 +118,7 @@ struct iOSNavigationView<Content: View>: View { // swiftlint:disable:this type_n
   }
 
   private func signOutButtonTapped() {
-    Task {
-      try await model.store.signOut()
-    }
+    do { try model.store.signOut() } catch { Log.error(error) }
   }
 }
 #endif