Skip to content

Commit 111b30c

Browse files
fix: start coder connect on launch after SE is installed (#113)
This is a fix for #108. Previously we would start the VPN immediately on launch if the config option was true. This starting of the VPN raced with any concurrent upgrades of the network extension, causing the VPN to be started with a VPN config belonging to the older network extension, and producing a consistent error message: ![image](https://github.com/user-attachments/assets/a69932cb-4c86-4d45-8ab5-5843e255f395) Instead, we should only start the VPN once we know that the system extension and VPN configuration are installed.
1 parent 0550ad1 commit 111b30c

File tree

3 files changed

+31
-8
lines changed

3 files changed

+31
-8
lines changed

Coder-Desktop/Coder-Desktop/Coder_DesktopApp.swift

+8-3
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,10 @@ class AppDelegate: NSObject, NSApplicationDelegate {
3737
vpn = CoderVPNService()
3838
state = AppState(onChange: vpn.configureTunnelProviderProtocol)
3939
fileSyncDaemon = MutagenDaemon()
40+
if state.startVPNOnLaunch {
41+
vpn.startWhenReady = true
42+
}
43+
vpn.installSystemExtension()
4044
}
4145

4246
func applicationDidFinishLaunching(_: Notification) {
@@ -68,16 +72,17 @@ class AppDelegate: NSObject, NSApplicationDelegate {
6872
if await !vpn.loadNetworkExtensionConfig() {
6973
state.reconfigure()
7074
}
71-
if state.startVPNOnLaunch {
72-
await vpn.start()
73-
}
7475
}
7576
// TODO: Start the daemon only once a file sync is configured
7677
Task {
7778
await fileSyncDaemon.start()
7879
}
7980
}
8081

82+
deinit {
83+
NotificationCenter.default.removeObserver(self)
84+
}
85+
8186
// This function MUST eventually call `NSApp.reply(toApplicationShouldTerminate: true)`
8287
// or return `.terminateNow`
8388
func applicationShouldTerminate(_: NSApplication) -> NSApplication.TerminateReply {

Coder-Desktop/Coder-Desktop/VPN/VPNService.swift

+17-5
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,16 @@ enum VPNServiceState: Equatable {
1818
case disconnecting
1919
case connected
2020
case failed(VPNServiceError)
21+
22+
var canBeStarted: Bool {
23+
switch self {
24+
// A tunnel failure should not prevent a reconnect attempt
25+
case .disabled, .failed:
26+
true
27+
default:
28+
false
29+
}
30+
}
2131
}
2232

2333
enum VPNServiceError: Error, Equatable {
@@ -54,11 +64,18 @@ final class CoderVPNService: NSObject, VPNService {
5464
guard neState == .enabled || neState == .disabled else {
5565
return .failed(.networkExtensionError(neState))
5666
}
67+
if startWhenReady, tunnelState.canBeStarted {
68+
startWhenReady = false
69+
Task { await start() }
70+
}
5771
return tunnelState
5872
}
5973

6074
@Published var menuState: VPNMenuState = .init()
6175

76+
// Whether the VPN should start as soon as possible
77+
var startWhenReady: Bool = false
78+
6279
// systemExtnDelegate holds a reference to the SystemExtensionDelegate so that it doesn't get
6380
// garbage collected while the OSSystemExtensionRequest is in flight, since the OS framework
6481
// only stores a weak reference to the delegate.
@@ -68,11 +85,6 @@ final class CoderVPNService: NSObject, VPNService {
6885

6986
override init() {
7087
super.init()
71-
installSystemExtension()
72-
}
73-
74-
deinit {
75-
NotificationCenter.default.removeObserver(self)
7688
}
7789

7890
func start() async {

Coder-Desktop/Coder-DesktopTests/LoginFormTests.swift

+6
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,12 @@ struct LoginTests {
107107
data: [.get: Client.encoder.encode(buildInfo)]
108108
).register()
109109

110+
try Mock(
111+
url: url.appendingPathComponent("/api/v2/users/me"),
112+
statusCode: 200,
113+
data: [.get: Client.encoder.encode(User(id: UUID(), username: "username"))]
114+
).register()
115+
110116
try await ViewHosting.host(view) {
111117
try await sut.inspection.inspect { view in
112118
try view.find(ViewType.TextField.self).setInput(url.absoluteString)

0 commit comments

Comments
 (0)