refactor(apple): Remove Client-side retry logic in favor of letting the error take effect (#4196)

Currently we retry starting the tunnel based on why it disconnected.
This has happened for example if the stored token is invalid and we
don't clear it, causing a connect -> disconnect loop that causes the UI
to glitch every second.

Instead, we just let it fail, and log the error, or maybe even crash.
This PR makes that more noisy so (1) we know about it, and (2) the app
enters a valid state (signed out) more quickly.

Fixes #4138
This commit is contained in:
Jamil
2024-03-18 12:24:59 -07:00
committed by GitHub
parent f5db34b597
commit a9dfe009b7
3 changed files with 3 additions and 51 deletions

View File

@@ -20,8 +20,6 @@ public struct TunnelShutdownEvent: Codable, CustomStringConvertible {
case connlibDisconnected
case badTunnelConfiguration
case tokenNotFound
case networkSettingsApplyFailure
case invalidAdapterState
public var description: String {
switch self {
@@ -30,8 +28,6 @@ public struct TunnelShutdownEvent: Codable, CustomStringConvertible {
case .connlibDisconnected: return "connlib disconnected"
case .badTunnelConfiguration: return "bad tunnel configuration"
case .tokenNotFound: return "token not found"
case .networkSettingsApplyFailure: return "network settings apply failure"
case .invalidAdapterState: return "invalid adapter state"
}
}
@@ -40,13 +36,9 @@ public struct TunnelShutdownEvent: Codable, CustomStringConvertible {
case .stopped(let reason):
if reason == .userInitiated {
return .signoutImmediatelySilently
} else if reason == .userLogout || reason == .userSwitch {
return .doNothing
} else {
return .retryThenSignout
return .doNothing
}
case .networkSettingsApplyFailure, .invalidAdapterState:
return .retryThenSignout
case .connlibConnectFailure, .connlibDisconnected,
.badTunnelConfiguration, .tokenNotFound:
return .signoutImmediately
@@ -58,7 +50,6 @@ public struct TunnelShutdownEvent: Codable, CustomStringConvertible {
case doNothing
case signoutImmediately
case signoutImmediatelySilently
case retryThenSignout
}
public let reason: TunnelShutdownEvent.Reason

View File

@@ -83,9 +83,6 @@ public final class AuthStore: ObservableObject {
if status == .disconnected {
self.handleTunnelDisconnectionEvent()
}
if status == .connected {
self.resetReconnectionAttemptsRemaining()
}
self.status = status
}
}
@@ -169,8 +166,6 @@ public final class AuthStore: ObservableObject {
} catch {
logger.error("\(#function): Error signing out: \(error)")
}
resetReconnectionAttemptsRemaining()
}
public func cancelSignIn() {
@@ -220,37 +215,11 @@ public final class AuthStore: ObservableObject {
Task {
await self.signOut()
}
case .retryThenSignout:
self.retryStartTunnel()
case .doNothing:
break
}
} else {
self.logger.log("\(#function): Tunnel shutdown event not found")
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 every second to reconnect (\(self.reconnectionAttemptsRemaining) attempts after this)"
)
DispatchQueue.main.asyncAfter(deadline: .now() + .seconds(self.reconnectDelaySecs)) {
self.logger.log("\(#function): Trying to reconnect")
self.startTunnel()
}
} else {
Task {
await self.signOut()
}
#if os(macOS)
SessionNotificationHelper.showSignedOutAlertmacOS(logger: self.logger, authStore: self)
#endif
}
}
@@ -278,10 +247,6 @@ public final class AuthStore: ObservableObject {
}
}
func resetReconnectionAttemptsRemaining() {
self.reconnectionAttemptsRemaining = Self.maxReconnectionAttemptCount
}
func tunnelAuthStatus(for authBaseURL: URL) async -> TunnelAuthStatus {
if let tokenRef = await keychain.searchByAuthBaseURL(authBaseURL) {
return .signedIn(authBaseURL: authBaseURL, tokenReference: tokenRef)

View File

@@ -129,9 +129,7 @@ class Adapter {
self.logger.log("Adapter.start")
guard case .stoppedTunnel = self.state else {
packetTunnelProvider?.handleTunnelShutdown(
dueTo: .invalidAdapterState,
errorMessage: "Adapter is in invalid state")
logger.error("\(#function): Invalid Adapter state")
completionHandler(.invalidState)
return
}
@@ -433,9 +431,7 @@ extension Adapter: CallbackHandlerDelegate {
networkSettings.setMatchDomains([""])
networkSettings.apply(on: packetTunnelProvider, logger: self.logger) { error in
if let error = error {
self.packetTunnelProvider?.handleTunnelShutdown(
dueTo: .networkSettingsApplyFailure,
errorMessage: error.localizedDescription)
self.logger.error("\(#function): \(error)")
onStarted?(AdapterError.setNetworkSettings(error))
self.state = .stoppedTunnel
} else {