feat: add file sync daemon error handling to the UI

Author: ethanndickson

Diff for: Coder-Desktop/Coder-Desktop/Preview Content/PreviewFileSync.swift

@@ -6,11 +6,13 @@ final class PreviewFileSync: FileSyncDaemon {
 
     var state: DaemonState = .running
 
+    var recentLogs: [String] = []
+
     init() {}
 
     func refreshSessions() async {}
 
-    func start() async throws(DaemonError) {
+    func tryStart() async {
         state = .running
     }
 

Diff for: Coder-Desktop/Coder-Desktop/Views/FileSync/FileSyncConfig.swift

@@ -11,6 +11,8 @@ struct FileSyncConfig<VPN: VPNService, FS: FileSyncDaemon>: View {
 
     @State private var loading: Bool = false
     @State private var deleteError: DaemonError?
+    @State private var isVisible: Bool = false
+    @State private var dontRetry: Bool = false
 
     var body: some View {
         Group {
@@ -36,87 +38,138 @@ struct FileSyncConfig<VPN: VPNService, FS: FileSyncDaemon>: View {
             .frame(minWidth: 400, minHeight: 200)
             .padding(.bottom, 25)
             .overlay(alignment: .bottom) {
-                VStack(alignment: .leading, spacing: 0) {
-                    Divider()
-                    HStack(spacing: 0) {
-                        Button {
-                            addingNewSession = true
-                        } label: {
-                            Image(systemName: "plus")
-                                .frame(width: 24, height: 24)
-                        }.disabled(vpn.menuState.agents.isEmpty)
+                tableFooter
+            }
+            // Only the table & footer should be disabled if the daemon has crashed
+            // otherwise the alert buttons will be disabled too
+        }.disabled(fileSync.state.isFailed)
+            .sheet(isPresented: $addingNewSession) {
+                FileSyncSessionModal<VPN, FS>()
+                    .frame(width: 700)
+            }.sheet(item: $editingSession) { session in
+                FileSyncSessionModal<VPN, FS>(existingSession: session)
+                    .frame(width: 700)
+            }.alert("Error", isPresented: Binding(
+                get: { deleteError != nil },
+                set: { isPresented in
+                    if !isPresented {
+                        deleteError = nil
+                    }
+                }
+            )) {} message: {
+                Text(deleteError?.description ?? "An unknown error occurred.")
+            }.alert("Error", isPresented: Binding(
+                // We only show the alert if the file config window is open
+                // Users will see the alert symbol on the menu bar to prompt them to
+                // open it. The requirement on `!loading` prevents the alert from
+                // re-opening immediately.
+                get: { !loading && isVisible && fileSync.state.isFailed },
+                set: { isPresented in
+                    if !isPresented {
+                        if dontRetry {
+                            dontRetry = false
+                            return
+                        }
+                        loading = true
+                        Task {
+                            await fileSync.tryStart()
+                            loading = false
+                        }
+                    }
+                }
+            )) {
+                Button("Retry") {}
+                // This gives the user an out if the daemon is crashing on launch,
+                // they can cancel the alert, and it will reappear if they re-open the
+                // file sync window.
+                Button("Cancel", role: .cancel) {
+                    dontRetry = true
+                }
+            } message: {
+                // You can't have styled text in alert messages
+                Text("""
+                File sync daemon failed: \(fileSync.state.description)\n\n\(fileSync.recentLogs.joined(separator: "\n"))
+                """)
+            }.task {
+                // When the Window is visible, poll for session updates every
+                // two seconds.
+                while !Task.isCancelled {
+                    if !fileSync.state.isFailed {
+                        await fileSync.refreshSessions()
+                    }
+                    try? await Task.sleep(for: .seconds(2))
+                }
+            }.onAppear {
+                isVisible = true
+            }.onDisappear {
+                isVisible = false
+                // If the failure alert is dismissed without restarting the daemon,
+                // (by clicking cancel) this makes it clear that the daemon
+                // is still in a failed state.
+            }.navigationTitle("Coder File Sync \(fileSync.state.isFailed ? "- Failed" : "")")
+            .disabled(loading)
+    }
+
+    var tableFooter: some View {
+        VStack(alignment: .leading, spacing: 0) {
+            Divider()
+            HStack(spacing: 0) {
+                Button {
+                    addingNewSession = true
+                } label: {
+                    Image(systemName: "plus")
+                        .frame(width: 24, height: 24)
+                }.disabled(vpn.menuState.agents.isEmpty)
+                Divider()
+                Button {
+                    Task {
+                        loading = true
+                        defer { loading = false }
+                        do throws(DaemonError) {
+                            // TODO: Support selecting & deleting multiple sessions at once
+                            try await fileSync.deleteSessions(ids: [selection!])
+                            if fileSync.sessionState.isEmpty {
+                                // Last session was deleted, stop the daemon
+                                await fileSync.stop()
+                            }
+                        } catch {
+                            deleteError = error
+                        }
+                        selection = nil
+                    }
+                } label: {
+                    Image(systemName: "minus").frame(width: 24, height: 24)
+                }.disabled(selection == nil)
+                if let selection {
+                    if let selectedSession = fileSync.sessionState.first(where: { $0.id == selection }) {
                         Divider()
                         Button {
                             Task {
+                                // TODO: Support pausing & resuming multiple sessions at once
                                 loading = true
                                 defer { loading = false }
-                                do throws(DaemonError) {
-                                    // TODO: Support selecting & deleting multiple sessions at once
-                                    try await fileSync.deleteSessions(ids: [selection!])
-                                    if fileSync.sessionState.isEmpty {
-                                        // Last session was deleted, stop the daemon
-                                        await fileSync.stop()
-                                    }
-                                } catch {
-                                    deleteError = error
+                                switch selectedSession.status {
+                                case .paused:
+                                    try await fileSync.resumeSessions(ids: [selectedSession.id])
+                                default:
+                                    try await fileSync.pauseSessions(ids: [selectedSession.id])
                                 }
-                                selection = nil
                             }
                         } label: {
-                            Image(systemName: "minus").frame(width: 24, height: 24)
-                        }.disabled(selection == nil)
-                        if let selection {
-                            if let selectedSession = fileSync.sessionState.first(where: { $0.id == selection }) {
-                                Divider()
-                                Button {
-                                    Task {
-                                        // TODO: Support pausing & resuming multiple sessions at once
-                                        loading = true
-                                        defer { loading = false }
-                                        switch selectedSession.status {
-                                        case .paused:
-                                            try await fileSync.resumeSessions(ids: [selectedSession.id])
-                                        default:
-                                            try await fileSync.pauseSessions(ids: [selectedSession.id])
-                                        }
-                                    }
-                                } label: {
-                                    switch selectedSession.status {
-                                    case .paused:
-                                        Image(systemName: "play").frame(width: 24, height: 24)
-                                    default:
-                                        Image(systemName: "pause").frame(width: 24, height: 24)
-                                    }
-                                }
+                            switch selectedSession.status {
+                            case .paused:
+                                Image(systemName: "play").frame(width: 24, height: 24)
+                            default:
+                                Image(systemName: "pause").frame(width: 24, height: 24)
                             }
                         }
                     }
-                    .buttonStyle(.borderless)
                 }
-                .background(.primary.opacity(0.04))
-                .fixedSize(horizontal: false, vertical: true)
-            }
-        }.sheet(isPresented: $addingNewSession) {
-            FileSyncSessionModal<VPN, FS>()
-                .frame(width: 700)
-        }.sheet(item: $editingSession) { session in
-            FileSyncSessionModal<VPN, FS>(existingSession: session)
-                .frame(width: 700)
-        }.alert("Error", isPresented: Binding(
-            get: { deleteError != nil },
-            set: { isPresented in
-                if !isPresented {
-                    deleteError = nil
-                }
-            }
-        )) {} message: {
-            Text(deleteError?.description ?? "An unknown error occurred.")
-        }.task {
-            while !Task.isCancelled {
-                await fileSync.refreshSessions()
-                try? await Task.sleep(for: .seconds(2))
             }
-        }.disabled(loading)
+            .buttonStyle(.borderless)
+        }
+        .background(.primary.opacity(0.04))
+        .fixedSize(horizontal: false, vertical: true)
     }
 }
 

Diff for: Coder-Desktop/Coder-Desktop/Views/VPN/VPNMenu.swift

@@ -68,11 +68,12 @@ struct VPNMenu<VPN: VPNService, FS: FileSyncDaemon>: View {
                     } label: {
                         ButtonRowView {
                             HStack {
-                                // TODO: A future PR will provide users a way to recover from a daemon failure without
-                                // needing to restart the app
-                                if case .failed = fileSync.state, sessionsHaveError(fileSync.sessionState) {
+                                if fileSync.state.isFailed || sessionsHaveError(fileSync.sessionState) {
                                     Image(systemName: "exclamationmark.arrow.trianglehead.2.clockwise.rotate.90")
-                                        .frame(width: 12, height: 12).help("One or more sync sessions have errors")
+                                        .frame(width: 12, height: 12)
+                                        .help(fileSync.state.isFailed ?
+                                            "The file sync daemon encountered an error" :
+                                            "One or more file sync sessions have errors")
                                 }
                                 Text("File sync")
                             }

Diff for: Coder-Desktop/Coder-DesktopTests/Util.swift

@@ -33,13 +33,13 @@ class MockFileSyncDaemon: FileSyncDaemon {
 
     func refreshSessions() async {}
 
+    var recentLogs: [String] = []
+
     func deleteSessions(ids _: [String]) async throws(VPNLib.DaemonError) {}
 
     var state: VPNLib.DaemonState = .running
 
-    func start() async throws(VPNLib.DaemonError) {
-        return
-    }
+    func tryStart() async {}
 
     func stop() async {}
 

Diff for: Coder-Desktop/VPNLib/FileSync/FileSyncDaemon.swift

@@ -10,7 +10,8 @@ import SwiftUI
 public protocol FileSyncDaemon: ObservableObject {
     var state: DaemonState { get }
     var sessionState: [FileSyncSession] { get }
-    func start() async throws(DaemonError)
+    var recentLogs: [String] { get }
+    func tryStart() async
     func stop() async
     func refreshSessions() async
     func createSession(localPath: String, agentHost: String, remotePath: String) async throws(DaemonError)
@@ -38,6 +39,10 @@ public class MutagenDaemon: FileSyncDaemon {
 
     @Published public var sessionState: [FileSyncSession] = []
 
+    // We store the last N log lines to show in the UI if the daemon crashes
+    private var logBuffer: RingBuffer<String>
+    public var recentLogs: [String] { logBuffer.elements }
+
     private var mutagenProcess: Subprocess?
     private let mutagenPath: URL!
     private let mutagenDataDirectory: URL
@@ -50,6 +55,7 @@ public class MutagenDaemon: FileSyncDaemon {
     var client: DaemonClient?
     private var group: MultiThreadedEventLoopGroup?
     private var channel: GRPCChannel?
+    private var waitForExit: (@Sendable () async -> Void)?
 
     // Protect start & stop transitions against re-entrancy
     private let transition = AsyncSemaphore(value: 1)
@@ -58,8 +64,10 @@ public class MutagenDaemon: FileSyncDaemon {
                 mutagenDataDirectory: URL = FileManager.default.urls(
                     for: .applicationSupportDirectory,
                     in: .userDomainMask
-                ).first!.appending(path: "Coder Desktop").appending(path: "Mutagen"))
+                ).first!.appending(path: "Coder Desktop").appending(path: "Mutagen"),
+                logBufferCapacity: Int = 10)
     {
+        logBuffer = .init(capacity: logBufferCapacity)
         self.mutagenPath = mutagenPath
         self.mutagenDataDirectory = mutagenDataDirectory
         mutagenDaemonSocket = mutagenDataDirectory.appending(path: "daemon").appending(path: "daemon.sock")
@@ -87,13 +95,31 @@ public class MutagenDaemon: FileSyncDaemon {
         }
     }
 
-    public func start() async throws(DaemonError) {
+    public func tryStart() async {
+        if case .failed = state { state = .stopped }
+        do throws(DaemonError) {
+            try await start()
+        } catch {
+            state = .failed(error)
+        }
+    }
+
+    func start() async throws(DaemonError) {
         if case .unavailable = state { return }
 
         // Stop an orphaned daemon, if there is one
         try? await connect()
         await stop()
 
+        // Creating the same process twice from Swift will crash the MainActor,
+        // so we need to wait for an earlier process to die
+        if let waitForExit {
+            await waitForExit()
+            // We *need* to be sure the process is dead or the app ends up in an
+            // unrecoverable state
+            try? await Task.sleep(for: .seconds(1))
+        }
+
         await transition.wait()
         defer { transition.signal() }
         logger.info("starting mutagen daemon")
@@ -106,6 +132,7 @@ public class MutagenDaemon: FileSyncDaemon {
         } catch {
             throw .daemonStartFailure(error)
         }
+        self.waitForExit = waitForExit
 
         Task {
             await streamHandler(io: standardOutput)
@@ -259,6 +286,7 @@ public class MutagenDaemon: FileSyncDaemon {
     private func streamHandler(io: Pipe.AsyncBytes) async {
         for await line in io.lines {
             logger.info("\(line, privacy: .public)")
+            logBuffer.append(line)
         }
     }
 }
@@ -282,7 +310,7 @@ public enum DaemonState {
         case .stopped:
             "Stopped"
         case let .failed(error):
-            "Failed: \(error)"
+            "\(error.description)"
         case .unavailable:
             "Unavailable"
         }
@@ -300,6 +328,15 @@ public enum DaemonState {
             .gray
         }
     }
+
+    // `if case`s are a pain to work with: they're not bools (such as for ORing)
+    // and you can't negate them without doing `if case .. {} else`.
+    public var isFailed: Bool {
+        if case .failed = self {
+            return true
+        }
+        return false
+    }
 }
 
 public enum DaemonError: Error {

Diff for: Coder-Desktop/VPNLib/Util.swift

@@ -29,3 +29,39 @@ public func makeNSError(suffix: String, code: Int = -1, desc: String) -> NSError
         userInfo: [NSLocalizedDescriptionKey: desc]
     )
 }
+
+// Insertion-only RingBuffer for buffering the last `capacity` elements,
+// and retrieving them in insertion order.
+public struct RingBuffer<T> {
+    private var buffer: [T?]
+    private var start = 0
+    private var size = 0
+
+    public init(capacity: Int) {
+        buffer = Array(repeating: nil, count: capacity)
+    }
+
+    public mutating func append(_ element: T) {
+        let writeIndex = (start + size) % buffer.count
+        buffer[writeIndex] = element
+
+        if size < buffer.count {
+            size += 1
+        } else {
+            start = (start + 1) % buffer.count
+        }
+    }
+
+    public var elements: [T] {
+        var result = [T]()
+        result.reserveCapacity(size)
+        for i in 0 ..< size {
+            let index = (start + i) % buffer.count
+            if let element = buffer[index] {
+                result.append(element)
+            }
+        }
+
+        return result
+    }
+}

Diff for: Coder-Desktop/VPNLibTests/UtilTests.swift

@@ -0,0 +1,91 @@
+import Testing
+@testable import VPNLib
+
+@Suite()
+struct RingBufferTests {
+    @Test
+    func belowCapacity() {
+        var buffer = RingBuffer<Int>(capacity: 3)
+
+        buffer.append(1)
+        buffer.append(2)
+
+        #expect(buffer.elements.count == 2)
+        #expect(buffer.elements == [1, 2])
+    }
+
+    @Test
+    func toCapacity() {
+        var buffer = RingBuffer<Int>(capacity: 3)
+
+        buffer.append(1)
+        buffer.append(2)
+        buffer.append(3)
+
+        #expect(buffer.elements.count == 3)
+        #expect(buffer.elements == [1, 2, 3])
+    }
+
+    @Test
+    func pastCapacity() {
+        var buffer = RingBuffer<Int>(capacity: 3)
+
+        buffer.append(1)
+        buffer.append(2)
+        buffer.append(3)
+        buffer.append(4)
+        buffer.append(5)
+
+        #expect(buffer.elements.count == 3)
+        #expect(buffer.elements == [3, 4, 5])
+    }
+
+    @Test
+    func singleCapacity() {
+        var buffer = RingBuffer<Int>(capacity: 1)
+
+        buffer.append(1)
+        #expect(buffer.elements == [1])
+
+        buffer.append(2)
+        #expect(buffer.elements == [2])
+
+        buffer.append(3)
+        #expect(buffer.elements == [3])
+    }
+
+    @Test
+    func replaceAll() {
+        var buffer = RingBuffer<Int>(capacity: 3)
+
+        buffer.append(1)
+        buffer.append(2)
+        buffer.append(3)
+
+        buffer.append(4)
+        buffer.append(5)
+        buffer.append(6)
+
+        #expect(buffer.elements.count == 3)
+        #expect(buffer.elements == [4, 5, 6])
+    }
+
+    @Test
+    func replacePartial() {
+        var buffer = RingBuffer<Int>(capacity: 3)
+
+        buffer.append(1)
+        buffer.append(2)
+        buffer.append(3)
+
+        buffer.append(4)
+        buffer.append(5)
+
+        #expect(buffer.elements == [3, 4, 5])
+
+        buffer.append(6)
+        buffer.append(7)
+
+        #expect(buffer.elements == [5, 6, 7])
+    }
+}