fix(apple/macOS): Show signed out directly on MainActor (#7993)

`NSAlert`'s `runModal` method blocks the current thread until the user
takes action. When we do this via `await MainActor.run { ... }` from
within a `Task`, Sentry correctly detects this as blocking the executor
thread.

See https://github.com/getsentry/sentry-cocoa/issues/2643

As such, to show `NSAlert`s from within `Tasks` in Swift, we need to
wrap them with `withCheckedContinuation` to signal to the Task executor
we are yielding. Sentry won't complain if these are only blocking the UI
thread, which is unavoidable (and by design with `NSAlert`).

Since handling VPN status change events already occurs within a `Task`
in `VPNConfigurationManager` and we can't get around that, we mark the
handler called from there as `async` and add a fourth parameter, an
`NEProviderStopReason` which needs to be read from the tunnel process
via IPC when we encounter a `disconnected` state. This is needed because
the status change events we get from the system only include the status
and not the reason.

If the reason is `authenticationCanceled`, we run the modal.
This commit is contained in:
Jamil
2025-02-03 05:48:21 +00:00
committed by GitHub
parent 6842e044b7
commit 2a9f39b1d6
4 changed files with 53 additions and 94 deletions

View File

@@ -180,7 +180,7 @@ public class VPNConfigurationManager {
}
func loadFromPreferences(
vpnStateUpdateHandler: @escaping @MainActor (NEVPNStatus, Settings?, String?) -> Void
vpnStateUpdateHandler: @escaping @MainActor (NEVPNStatus, Settings?, String?, NEProviderStopReason?) async -> Void
) async throws {
// loadAllFromPreferences() returns list of VPN configurations created by our main app's bundle ID.
// Since our bundle ID can change (by us), find the one that's current and ignore the others.
@@ -213,7 +213,7 @@ public class VPNConfigurationManager {
Telemetry.accountSlug = providerConfiguration[VPNConfigurationManagerKeys.accountSlug]
// Share what we found with our caller
await vpnStateUpdateHandler(status, settings, actorName)
await vpnStateUpdateHandler(status, settings, actorName, nil)
// Stop looking for our tunnel
break
@@ -222,7 +222,7 @@ public class VPNConfigurationManager {
// If no tunnel configuration was found, update state to
// prompt user to create one.
if manager == nil {
await vpnStateUpdateHandler(.invalid, nil, nil)
await vpnStateUpdateHandler(.invalid, nil, nil, nil)
}
// Hook up status updates
@@ -441,7 +441,7 @@ public class VPNConfigurationManager {
loop()
}
func consumeStopReason() async throws -> String? {
func consumeStopReason() async throws -> NEProviderStopReason? {
return try await withCheckedThrowingContinuation { continuation in
guard let session = session()
else {
@@ -455,22 +455,16 @@ public class VPNConfigurationManager {
encoder.encode(TunnelMessage.consumeStopReason)
) { data in
guard let data = data
guard let data = data,
let reason = String(data: data, encoding: .utf8),
let rawValue = Int(reason)
else {
continuation.resume(returning: nil)
return
}
guard let reason = String(data: data, encoding: .utf8)
else {
continuation
.resume(throwing: VPNConfigurationManagerError.decodeIPCDataFailed)
return
}
continuation.resume(returning: reason)
continuation.resume(returning: NEProviderStopReason(rawValue: rawValue))
}
} catch {
continuation.resume(throwing: error)
@@ -484,7 +478,9 @@ public class VPNConfigurationManager {
// Subscribe to system notifications about our VPN status changing
// and let our handler know about them.
private func subscribeToVPNStatusUpdates(handler: @escaping @MainActor (NEVPNStatus, Settings?, String?) -> Void) {
private func subscribeToVPNStatusUpdates(
handler: @escaping @MainActor (NEVPNStatus, Settings?, String?, NEProviderStopReason?
) async -> Void) {
Log.log("\(#function)")
for task in tunnelObservingTasks {
@@ -503,13 +499,18 @@ public class VPNConfigurationManager {
return
}
var reason: NEProviderStopReason?
if session.status == .disconnected {
// Reset resource list on disconnect
// Reset resource list
resourceListHash = Data()
resourcesListCache = ResourceList.loading
// Attempt to consume the last stopped reason
do { reason = try await consumeStopReason() } catch { Log.error(error) }
}
await handler(session.status, nil, nil)
await handler(session.status, nil, nil, reason)
}
}
)

View File

@@ -96,17 +96,21 @@ public class SessionNotification: NSObject {
// In macOS, use a Cocoa alert.
// This gets called from the app side.
@MainActor
func showSignedOutAlertmacOS() {
func showSignedOutAlertmacOS() async {
let alert = NSAlert()
alert.messageText = "Your Firezone session has ended"
alert.informativeText = "Please sign in again to reconnect"
alert.addButton(withTitle: "Sign In")
alert.addButton(withTitle: "Cancel")
NSApp.activate(ignoringOtherApps: true)
let response = alert.runModal()
if response == NSApplication.ModalResponse.alertFirstButtonReturn {
Log.log("\(#function): 'Sign In' clicked in notification")
signInHandler()
await withCheckedContinuation { continuation in
let response = alert.runModal()
if response == NSApplication.ModalResponse.alertFirstButtonReturn {
Log.log("\(#function): 'Sign In' clicked in notification")
signInHandler()
}
continuation.resume()
}
}
#endif

View File

@@ -48,7 +48,7 @@ public final class Store: ObservableObject {
func bindToVPNConfigurationUpdates() async throws {
// Load our existing VPN configuration and set an update handler
try await self.vpnConfigurationManager.loadFromPreferences(
vpnStateUpdateHandler: { @MainActor [weak self] status, settings, actorName in
vpnStateUpdateHandler: { @MainActor [weak self] status, settings, actorName, stopReason in
guard let self else { return }
self.status = status
@@ -61,37 +61,18 @@ public final class Store: ObservableObject {
self.actorName = actorName
}
if status == .disconnected {
maybeShowSignedOutAlert()
#if os(macOS)
// On macOS we must show notifications from the UI process. On iOS, we've already initiated the notification
// from the tunnel process, because the UI process is not guaranteed to be alive.
if status == .disconnected,
stopReason == .authenticationCanceled {
await self.sessionNotification.showSignedOutAlertmacOS()
}
#endif
}
)
}
/// On iOS, we can initiate notifications directly from the tunnel process.
/// On macOS, however, the system extension runs as root which doesn't
/// support showing User notifications. Instead, we read the last stopped
/// reason and alert the user if it was due to receiving a 401 from the
/// portal.
private func maybeShowSignedOutAlert() {
Task.detached { [weak self] in
guard let self else { return }
do {
if let savedValue = try await self.vpnConfigurationManager.consumeStopReason(),
let rawValue = Int(savedValue),
let reason = NEProviderStopReason(rawValue: rawValue),
case .authenticationCanceled = reason {
#if os(macOS)
await self.sessionNotification.showSignedOutAlertmacOS()
#endif
}
} catch {
Log.error(error)
}
}
}
#if os(macOS)
func checkedSystemExtensionStatus() async throws -> SystemExtensionStatus {
let checker = SystemExtensionManager()

View File

@@ -131,26 +131,25 @@ class PacketTunnelProvider: NEPacketTunnelProvider {
) {
Log.log("stopTunnel: Reason: \(reason)")
if case .authenticationCanceled = reason {
Task {
do {
// This was triggered from onDisconnect, so clear our token
try Token.delete()
do {
// There's no good way to send data like this from the
// Network Extension to the GUI, so save it to a file for the GUI to read upon
// either status change or the next launch.
try String(reason.rawValue).write(
to: SharedAccess.providerStopReasonURL, atomically: true, encoding: .utf8)
} catch {
Log.error(
SharedAccess.Error.unableToWriteToFile(
SharedAccess.providerStopReasonURL,
error
)
)
}
if case .authenticationCanceled = reason {
// This was triggered from onDisconnect, so try to clear our token
do { try Token.delete() } catch { Log.error(error) }
// There's no good way to send data like this from the
// Network Extension to the GUI, so save it to a file for the GUI to read upon
// either status change or the next launch.
try String(reason.rawValue).write(
to: SharedAccess.providerStopReasonURL, atomically: true, encoding: .utf8)
} catch {
Log.error(
SharedAccess.Error.unableToWriteToFile(
SharedAccess.providerStopReasonURL,
error
)
)
}
}
#if os(iOS)
// iOS notifications should be shown from the tunnel process
SessionNotification.showSignedOutNotificationiOS()
@@ -305,29 +304,3 @@ class PacketTunnelProvider: NEPacketTunnelProvider {
completionHandler(data)
}
}
extension NEProviderStopReason: @retroactive CustomStringConvertible {
public var description: String {
switch self {
case .none: return "None"
case .userInitiated: return "User-initiated"
case .providerFailed: return "Provider failed"
case .noNetworkAvailable: return "No network available"
case .unrecoverableNetworkChange: return "Unrecoverable network change"
case .providerDisabled: return "Provider disabled"
case .authenticationCanceled: return "Authentication canceled"
case .configurationFailed: return "Configuration failed"
case .idleTimeout: return "Idle timeout"
case .configurationDisabled: return "Configuration disabled"
case .configurationRemoved: return "Configuration removed"
case .superceded: return "Superceded"
case .userLogout: return "User logged out"
case .userSwitch: return "User switched"
case .connectionFailed: return "Connection failed"
case .sleep: return "Sleep"
case .appUpdate: return "App update"
case .internalError: return "Internal error"
@unknown default: return "Unknown"
}
}
}