refactor(windows): don't block connlib callbacks (#2960)

The code is not pretty but if I land this then I can make it pretty later. --------- Signed-off-by: Reactor Scram <ReactorScram@users.noreply.github.com> Co-authored-by: Gabi <gabrielalejandro7@gmail.com>
2026-01-27 10:18:54 +00:00 · 2023-12-21 15:03:24 -06:00
parent 6e77978da7
commit 09bfb72d3b
5 changed files with 239 additions and 179 deletions
--- a/rust/Cargo.lock
+++ b/rust/Cargo.lock
@@ -2026,6 +2026,7 @@ version = "1.0.0"
 dependencies = [
 "anyhow",
 "arboard",
+ "arc-swap",
 "chrono",
 "clap",
 "connlib-client-shared",
@@ -4338,9 +4339,9 @@ dependencies = [

 [[package]]
 name = "pkg-config"
-version = "0.3.27"
+version = "0.3.28"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "26072860ba924cbfa98ea39c8c19b4dd6a4a25423dbdf219c1eca91aa0cf6964"
+checksum = "69d3587f8a9e599cc7ec2c00e331f71c4e69a5f9a4b8a6efd5b07466b9736f9a"

 [[package]]
 name = "platforms"
@@ -5281,9 +5282,9 @@ dependencies = [

 [[package]]
 name = "serde_spanned"
-version = "0.6.4"
+version = "0.6.5"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "12022b835073e5b11e90a14f86838ceb1c8fb0325b72416845c487ac0fa95e80"
+checksum = "eb3622f419d1296904700073ea6cc23ad690adbd66f13ea683df73298736f0c1"
 dependencies = [
 "serde",
 ]
@@ -5864,9 +5865,9 @@ checksum = "14c39fd04924ca3a864207c66fc2cd7d22d7c016007f9ce846cbb9326331930a"

 [[package]]
 name = "tauri"
-version = "1.5.3"
+version = "1.5.4"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "32d563b672acde8d0cc4c1b1f5b855976923f67e8d6fe1eba51df0211e197be2"
+checksum = "fd27c04b9543776a972c86ccf70660b517ecabbeced9fb58d8b961a13ad129af"
 dependencies = [
 "anyhow",
 "cocoa",
@@ -5912,9 +5913,9 @@ dependencies = [

 [[package]]
 name = "tauri-build"
-version = "1.5.0"
+version = "1.5.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "defbfc551bd38ab997e5f8e458f87396d2559d05ce32095076ad6c30f7fc5f9c"
+checksum = "e9914a4715e0b75d9f387a285c7e26b5bbfeb1249ad9f842675a82481565c532"
 dependencies = [
 "anyhow",
 "cargo_toml",
@@ -5931,9 +5932,9 @@ dependencies = [

 [[package]]
 name = "tauri-codegen"
-version = "1.4.1"
+version = "1.4.2"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "7b3475e55acec0b4a50fb96435f19631fb58cbcd31923e1a213de5c382536bbb"
+checksum = "a1554c5857f65dbc377cefb6b97c8ac77b1cb2a90d30d3448114d5d6b48a77fc"
 dependencies = [
 "base64 0.21.5",
 "brotli",
@@ -5957,9 +5958,9 @@ dependencies = [

 [[package]]
 name = "tauri-macros"
-version = "1.4.2"
+version = "1.4.3"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "acea6445eececebd72ed7720cfcca46eee3b5bad8eb408be8f7ef2e3f7411500"
+checksum = "277abf361a3a6993ec16bcbb179de0d6518009b851090a01adfea12ac89fa875"
 dependencies = [
 "heck 0.4.1",
 "proc-macro2",
@@ -5971,9 +5972,9 @@ dependencies = [

 [[package]]
 name = "tauri-runtime"
-version = "0.14.1"
+version = "0.14.2"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "07f8e9e53e00e9f41212c115749e87d5cd2a9eebccafca77a19722eeecd56d43"
+checksum = "cf2d0652aa2891ff3e9caa2401405257ea29ab8372cce01f186a5825f1bd0e76"
 dependencies = [
 "gtk",
 "http",
@@ -5992,9 +5993,9 @@ dependencies = [

 [[package]]
 name = "tauri-runtime-wry"
-version = "0.14.2"
+version = "0.14.3"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "803a01101bc611ba03e13329951a1bde44287a54234189b9024b78619c1bc206"
+checksum = "6cae61fbc731f690a4899681c9052dde6d05b159b44563ace8186fc1bfb7d158"
 dependencies = [
 "cocoa",
 "gtk",
@@ -6012,9 +6013,9 @@ dependencies = [

 [[package]]
 name = "tauri-utils"
-version = "1.5.1"
+version = "1.5.2"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "a52165bb340e6f6a75f1f5eeeab1bb49f861c12abe3a176067d53642b5454986"
+checksum = "ece74810b1d3d44f29f732a7ae09a63183d63949bbdd59c61f8ed2a1b70150db"
 dependencies = [
 "brotli",
 "ctor",
--- a/rust/connlib/tunnel/src/device_channel/tun_windows.rs
+++ b/rust/connlib/tunnel/src/device_channel/tun_windows.rs
@@ -113,6 +113,7 @@ impl Tun {
        })
    }

+    // It's okay if this blocks until the route is added in the OS.
    pub fn add_route(&self, route: IpNetwork) -> Result<()> {
        tracing::debug!("add_route {route}");
        let iface_idx = self.iface_idx;
--- a/rust/windows-client/docs/manual_testing.md
+++ b/rust/windows-client/docs/manual_testing.md
@@ -49,6 +49,7 @@ If the client stops running while signed in, then the token may be stored in Win
 - [ ] Given the client is signed in, when you click "Sign Out", then the GUI will change to signed-out state, and the token will be wiped from the disk, and the client will continue running ([#2809](https://github.com/firezone/firezone/issues/2809))
 - [ ] Given the client is signed in, when you click "Disconnect and Quit", then the client will stop running, and the token will stay on disk in Window's credential manager. ([#2809](https://github.com/firezone/firezone/issues/2809))
 - [ ] Given the client was signed in when it stopped, when you start the client again, then the GUI will be in the signed-in state, and the user's name will be shown in the tray menu. ([#2712](https://github.com/firezone/firezone/issues/2712))
+- [ ] Given the client is signed out, when you sign in, then sign out, then sign in again, then the 2nd sign-in will work

 # Advanced settings

--- a/rust/windows-client/src-tauri/Cargo.toml
+++ b/rust/windows-client/src-tauri/Cargo.toml
@@ -12,6 +12,7 @@ tauri-build = { version = "1.5", features = [] }
 [dependencies]
 arboard = { version = "3.3.0", default-features = false }
 anyhow = { version = "1.0" }
+arc-swap = "1.6.0"
 chrono = { workspace = true }
 clap = { version = "4.4", features = ["derive",  "env"] }
 connlib-client-shared = { workspace = true }
--- a/rust/windows-client/src-tauri/src/client/gui.rs
+++ b/rust/windows-client/src-tauri/src/client/gui.rs
@@ -4,18 +4,22 @@
 // TODO: `git grep` for unwraps before 1.0, especially this gui module

 use crate::client::{self, deep_link, AppLocalDataDir};
-use anyhow::{anyhow, Context, Result};
+use anyhow::{anyhow, bail, Context, Result};
+use arc_swap::ArcSwap;
 use client::{
    logging,
    settings::{self, AdvancedSettings},
 };
-use connlib_client_shared::file_logger;
+use connlib_client_shared::{file_logger, ResourceDescription};
 use connlib_shared::messages::ResourceId;
 use secrecy::{ExposeSecret, SecretString};
-use std::{net::IpAddr, path::PathBuf, str::FromStr, time::Duration};
+use std::{net::IpAddr, path::PathBuf, str::FromStr, sync::Arc, time::Duration};
 use system_tray_menu::{Event as TrayMenuEvent, Resource as ResourceDisplay};
 use tauri::{Manager, SystemTray, SystemTrayEvent};
-use tokio::sync::{mpsc, oneshot};
+use tokio::{
+    sync::{mpsc, oneshot, Notify},
+    task::spawn_blocking,
+};
 use ControllerRequest as Req;

 mod system_tray_menu;
@@ -60,11 +64,12 @@ pub(crate) fn run(params: client::GuiParams) -> Result<()> {
    deep_link::register(TAURI_ID)?;

    let (ctlr_tx, ctlr_rx) = mpsc::channel(5);
+    let notify_controller = Arc::new(Notify::new());

    tokio::spawn(accept_deep_links(server, ctlr_tx.clone()));

    let managed = Managed {
-        ctlr_tx,
+        ctlr_tx: ctlr_tx.clone(),
        inject_faults,
    };

@@ -123,10 +128,17 @@ pub(crate) fn run(params: client::GuiParams) -> Result<()> {

            let app_handle = app.handle();
            let _ctlr_task = tokio::spawn(async move {
-                if let Err(e) =
-                    run_controller(app_handle, ctlr_rx, logging_handles, advanced_settings).await
+                if let Err(e) = run_controller(
+                    app_handle,
+                    ctlr_tx,
+                    ctlr_rx,
+                    logging_handles,
+                    advanced_settings,
+                    notify_controller,
+                )
+                .await
                {
-                    tracing::error!("run_controller returned an error: {e}");
+                    tracing::error!("run_controller returned an error: {e:#?}");
                }
            });

@@ -147,10 +159,7 @@ pub(crate) fn run(params: client::GuiParams) -> Result<()> {
 /// Worker task to accept deep links from a named pipe forever
 ///
 /// * `server` An initial named pipe server to consume before making new servers. This lets us also use the named pipe to enforce single-instance
-async fn accept_deep_links(
-    mut server: deep_link::Server,
-    ctlr_tx: mpsc::Sender<ControllerRequest>,
-) -> Result<()> {
+async fn accept_deep_links(mut server: deep_link::Server, ctlr_tx: CtlrTx) -> Result<()> {
    loop {
        if let Ok(url) = server.accept().await {
            ctlr_tx
@@ -205,13 +214,13 @@ fn handle_system_tray_event(app: &tauri::AppHandle, event: TrayMenuEvent) -> Res

 pub(crate) enum ControllerRequest {
    CopyResource(String),
+    Disconnected,
    ExportLogs(PathBuf),
    GetAdvancedSettings(oneshot::Sender<AdvancedSettings>),
    SchemeRequest(url::Url),
    SignIn,
    StartStopLogCounting(bool),
    SignOut,
-    UpdateResources(Vec<connlib_client_shared::ResourceDescription>),
 }

 // TODO: Should these be keyed to the Google ID or email or something?
@@ -226,18 +235,21 @@ fn keyring_entry() -> Result<keyring::Entry> {

 #[derive(Clone)]
 struct CallbackHandler {
+    logger: file_logger::Handle,
+    notify_controller: Arc<Notify>,
    ctlr_tx: CtlrTx,
-    logger: Option<file_logger::Handle>,
+    resources: Arc<ArcSwap<Vec<ResourceDescription>>>,
 }

 #[derive(thiserror::Error, Debug)]
 enum CallbackError {
-    #[error(transparent)]
-    ControllerRequest(#[from] tokio::sync::mpsc::error::TrySendError<ControllerRequest>),
    #[error("system DNS resolver problem: {0}")]
    Resolvers(#[from] client::resolvers::Error),
+    #[error("can't send to controller task: {0}")]
+    SendError(#[from] mpsc::error::TrySendError<ControllerRequest>),
 }

+// Callbacks must all be non-blocking
 impl connlib_client_shared::Callbacks for CallbackHandler {
    type Error = CallbackError;

@@ -245,8 +257,8 @@ impl connlib_client_shared::Callbacks for CallbackHandler {
        &self,
        error: Option<&connlib_client_shared::Error>,
    ) -> Result<(), Self::Error> {
-        // TODO: implement
-        tracing::error!("on_disconnect {error:?}");
+        tracing::debug!("on_disconnect {error:?}");
+        self.ctlr_tx.try_send(ControllerRequest::Disconnected)?;
        Ok(())
    }

@@ -261,14 +273,9 @@ impl connlib_client_shared::Callbacks for CallbackHandler {
        Ok(())
    }

-    fn on_update_resources(
-        &self,
-        resources: Vec<connlib_client_shared::ResourceDescription>,
-    ) -> Result<(), Self::Error> {
-        tracing::trace!("on_update_resources");
-        // TODO: Better error handling?
-        self.ctlr_tx
-            .try_send(ControllerRequest::UpdateResources(resources))?;
+    fn on_update_resources(&self, resources: Vec<ResourceDescription>) -> Result<(), Self::Error> {
+        self.resources.store(resources.into());
+        self.notify_controller.notify_one();
        Ok(())
    }

@@ -277,64 +284,77 @@ impl connlib_client_shared::Callbacks for CallbackHandler {
    }

    fn roll_log_file(&self) -> Option<PathBuf> {
-        self.logger
-            .as_ref()?
-            .roll_to_new_file()
-            .unwrap_or_else(|e| {
-                tracing::debug!("Failed to roll over to new file: {e}");
-                let _ = self.on_error(&connlib_client_shared::Error::LogFileRollError(e));
+        self.logger.roll_to_new_file().unwrap_or_else(|e| {
+            tracing::debug!("Failed to roll over to new file: {e}");
+            let _ = self.on_error(&connlib_client_shared::Error::LogFileRollError(e));

-                None
-            })
+            None
+        })
    }
 }

 struct Controller {
    /// Debugging-only settings like API URL, auth URL, log filter
    advanced_settings: AdvancedSettings,
-    /// mpsc sender to send things to the controller task
    ctlr_tx: CtlrTx,
-    /// connlib / tunnel session
-    connlib_session: Option<connlib_client_shared::Session<CallbackHandler>>,
+    /// Session for the currently signed-in user, if there is one
+    session: Option<Session>,
    /// The UUIDv4 device ID persisted to disk
    /// Sent verbatim to Session::connect
    device_id: String,
    logging_handles: client::logging::Handles,
-    /// Info about currently signed-in user, if there is one
-    session: Option<Session>,
+    /// Tells us when to wake up and look for a new resource list. Tokio docs say that memory reads and writes are synchronized when notifying, so we don't need an extra mutex on the resources.
+    notify_controller: Arc<Notify>,
 }

-/// Information for a signed-in user session
+/// Everything related to a signed-in user session
 struct Session {
+    auth_info: AuthInfo,
+    callback_handler: CallbackHandler,
+    connlib: connlib_client_shared::Session<CallbackHandler>,
+}
+
+/// Auth info that's persisted to disk if a session outlives an app instance
+struct AuthInfo {
    /// User name, e.g. "John Doe", from the sign-in deep link
    actor_name: String,
+    /// Secret token to authenticate with the portal
    token: SecretString,
 }

 impl Controller {
    async fn new(
        app: tauri::AppHandle,
+        ctlr_tx: CtlrTx,
        logging_handles: client::logging::Handles,
        advanced_settings: AdvancedSettings,
+        notify_controller: Arc<Notify>,
    ) -> Result<Self> {
-        let ctlr_tx = app
-            .try_state::<Managed>()
-            .ok_or_else(|| anyhow::anyhow!("can't get Managed object from Tauri"))?
-            .ctlr_tx
-            .clone();
+        let device_id = client::device_id::device_id(&app_local_data_dir(&app)?).await?;
+
+        let mut this = Self {
+            advanced_settings,
+            ctlr_tx,
+            session: None,
+            device_id,
+            logging_handles,
+            notify_controller,
+        };

        tracing::trace!("re-loading token");
-        let session: Option<Session> = tokio::task::spawn_blocking(|| {
+        // spawn_blocking because accessing the keyring is I/O
+        if let Some(auth_info) = spawn_blocking(|| {
            let entry = keyring_entry()?;
            match entry.get_password() {
                Ok(token) => {
                    let token = SecretString::new(token);
                    tracing::debug!("re-loaded token from Windows credential manager");
-                    let session = Session {
+                    let auth_info = AuthInfo {
+                        // TODO: Reload actor name from disk here
                        actor_name: "TODO".to_string(),
                        token,
                    };
-                    Ok(Some(session))
+                    Ok(Some(auth_info))
                }
                Err(keyring::Error::NoEntry) => {
                    tracing::debug!("no token in Windows credential manager");
@@ -343,145 +363,181 @@ impl Controller {
                Err(e) => Err(anyhow::Error::from(e)),
            }
        })
-        .await??;
+        .await??
+        {
+            // Connect immediately if we reloaded the token
+            if let Err(e) = this.start_session(auth_info) {
+                tracing::error!("couldn't restart session on app start: {e:#?}");
+            }
+        }

-        let device_id = client::device_id::device_id(&app_local_data_dir(&app)?).await?;
-
-        // Connect immediately if we reloaded the token
-        let connlib_session = if let Some(session) = session.as_ref() {
-            Some(Self::start_session(
-                &advanced_settings,
-                ctlr_tx.clone(),
-                device_id.clone(),
-                &session.token,
-                logging_handles.logger.clone(),
-            )?)
-        } else {
-            None
-        };
-
-        Ok(Self {
-            advanced_settings,
-            ctlr_tx,
-            connlib_session,
-            device_id,
-            logging_handles,
-            session,
-        })
+        Ok(this)
    }

-    fn start_session(
-        advanced_settings: &settings::AdvancedSettings,
-        ctlr_tx: CtlrTx,
-        device_id: String,
-        token: &SecretString,
-        logger: file_logger::Handle,
-    ) -> Result<connlib_client_shared::Session<CallbackHandler>> {
-        tracing::info!("Session::connect");
-        Ok(connlib_client_shared::Session::connect(
-            advanced_settings.api_url.clone(),
-            token.clone(),
-            device_id,
-            CallbackHandler {
-                ctlr_tx,
-                logger: Some(logger),
-            },
+    // TODO: Figure out how re-starting sessions automatically will work
+    fn start_session(&mut self, auth_info: AuthInfo) -> Result<()> {
+        if self.session.is_some() {
+            bail!("can't start session, we're already in a session");
+        }
+
+        let callback_handler = CallbackHandler {
+            ctlr_tx: self.ctlr_tx.clone(),
+            logger: self.logging_handles.logger.clone(),
+            notify_controller: Arc::clone(&self.notify_controller),
+            resources: Default::default(),
+        };
+
+        let connlib = connlib_client_shared::Session::connect(
+            self.advanced_settings.api_url.clone(),
+            auth_info.token.clone(),
+            self.device_id.clone(),
+            callback_handler.clone(),
            Duration::from_secs(5 * 60),
-        )?)
+        )?;
+
+        self.session = Some(Session {
+            auth_info,
+            callback_handler,
+            connlib,
+        });
+
+        Ok(())
    }
 }

+// TODO: After PR #2960 lands, move some of this into `impl Controller`
 async fn run_controller(
    app: tauri::AppHandle,
+    ctlr_tx: CtlrTx,
    mut rx: mpsc::Receiver<ControllerRequest>,
    logging_handles: client::logging::Handles,
    advanced_settings: AdvancedSettings,
+    notify_controller: Arc<Notify>,
 ) -> Result<()> {
-    let mut controller = Controller::new(app.clone(), logging_handles, advanced_settings)
-        .await
-        .context("couldn't create Controller")?;
+    let mut controller = Controller::new(
+        app.clone(),
+        ctlr_tx,
+        logging_handles,
+        advanced_settings,
+        notify_controller,
+    )
+    .await
+    .context("couldn't create Controller")?;

    let mut log_counting_task = None;
-    let mut resources: Vec<ResourceDisplay> = vec![];

-    tracing::debug!("GUI controller main loop start");
-
-    while let Some(req) = rx.recv().await {
-        match req {
-            Req::CopyResource(id) => {
-                let id = ResourceId::from_str(&id)?;
-                let Some(res) = resources.iter().find(|r| r.id == id) else {
+    loop {
+        tokio::select! {
+            () = controller.notify_controller.notified() => {
+                let Some(session) = &controller.session else {
+                    tracing::warn!("got notified to update resources but there is no session");
                    continue;
                };
-                let mut clipboard = arboard::Clipboard::new()?;
-                clipboard.set_text(&res.pastable)?;
-            }
-            Req::ExportLogs(file_path) => logging::export_logs_to(file_path).await?,
-            Req::GetAdvancedSettings(tx) => {
-                tx.send(controller.advanced_settings.clone()).ok();
-            }
-            Req::SchemeRequest(url) => {
-                if let Some(auth) = client::deep_link::parse_auth_callback(&url) {
-                    tracing::debug!("setting new token");
-                    let entry = keyring_entry()?;
-                    entry.set_password(auth.token.expose_secret())?;
-                    controller.connlib_session = Some(Controller::start_session(
-                        &controller.advanced_settings,
-                        controller.ctlr_tx.clone(),
-                        controller.device_id.clone(),
-                        &auth.token,
-                        controller.logging_handles.logger.clone(),
-                    )?);
-                    controller.session = Some(Session {
-                        actor_name: auth.actor_name,
-                        token: auth.token,
-                    });
-                } else {
-                    tracing::warn!("couldn't handle scheme request");
-                }
-            }
-            Req::SignIn => {
-                // TODO: Put the platform and local server callback in here
-                tauri::api::shell::open(
-                    &app.shell_scope(),
-                    &controller.advanced_settings.auth_base_url,
-                    None,
-                )?;
-            }
-            Req::StartStopLogCounting(enable) => {
-                if enable {
-                    if log_counting_task.is_none() {
-                        let app = app.clone();
-                        log_counting_task = Some(tokio::spawn(logging::count_logs(app)));
-                        tracing::debug!("started log counting");
-                    }
-                } else if let Some(t) = log_counting_task {
-                    t.abort();
-                    log_counting_task = None;
-                    tracing::debug!("cancelled log counting");
-                }
-            }
-            Req::SignOut => {
-                keyring_entry()?.delete_password()?;
-                if let Some(mut session) = controller.connlib_session.take() {
-                    // TODO: Needs testing
-                    session.disconnect(None);
-                }
-                app.tray_handle().set_menu(system_tray_menu::signed_out())?;
-            }
-            Req::UpdateResources(r) => {
-                tracing::debug!("controller got UpdateResources");
-                resources = r.into_iter().map(ResourceDisplay::from).collect();
-
+                let resources = session.callback_handler.resources.load().as_ref().clone();
+                let resources: Vec<_> = resources.into_iter().map(ResourceDisplay::from).collect();
                // TODO: Save the user name between runs of the app
                let actor_name = controller
                    .session
                    .as_ref()
-                    .map(|x| x.actor_name.as_str())
+                    .map(|x| x.auth_info.actor_name.as_str())
                    .unwrap_or("TODO");
                app.tray_handle()
                    .set_menu(system_tray_menu::signed_in(actor_name, &resources))?;
            }
+            req = rx.recv() => {
+                let Some(req) = req else {
+                    break;
+                };
+                match req {
+                    Req::CopyResource(id) => {
+                        let Some(session) = &controller.session else {
+                            tracing::warn!("got notified to update resources but there is no session");
+                            continue;
+                        };
+                        let resources = session.callback_handler.resources.load();
+                        let id = ResourceId::from_str(&id)?;
+                        let Some(res) = resources.iter().find(|r| r.id() == id) else {
+                            continue;
+                        };
+                        let mut clipboard = arboard::Clipboard::new()?;
+                        // TODO: Make this a method on `ResourceDescription`
+                        match res {
+                            ResourceDescription::Dns(x) => clipboard.set_text(&x.address)?,
+                            ResourceDescription::Cidr(x) => clipboard.set_text(&x.address.to_string())?,
+                        }
+                    }
+                    Req::Disconnected => {
+                        tracing::debug!("connlib disconnected, tearing down Session");
+                        if let Some(mut session) = controller.session.take() {
+                            tracing::debug!("disconnecting connlib");
+                            session.connlib.disconnect(None);
+                        }
+                    },
+                    Req::ExportLogs(file_path) => logging::export_logs_to(file_path).await?,
+                    Req::GetAdvancedSettings(tx) => {
+                        tx.send(controller.advanced_settings.clone()).ok();
+                    }
+                    Req::SchemeRequest(url) => {
+                        let Some(auth) = client::deep_link::parse_auth_callback(&url) else {
+                            tracing::error!("couldn't parse scheme request");
+                            // TODO: Move `run_controller` inside `Controller` and replace these `continue`s with `?`
+                            continue;
+                        };
+
+                        let token = auth.token.clone();
+                        // spawn_blocking because keyring access is I/O
+                        if let Err(e) = spawn_blocking(move || {
+                            let entry = keyring_entry()?;
+                            entry.set_password(token.expose_secret())?;
+                            Ok::<_, anyhow::Error>(())
+                        }).await? {
+                            tracing::warn!("couldn't save token to keyring: {e:#?}");
+                        }
+
+                        let auth_info = AuthInfo {
+                            actor_name: auth.actor_name,
+                            token: auth.token,
+                        };
+                        if let Err(e) = controller.start_session(auth_info) {
+                            tracing::error!("couldn't start session: {e:#?}");
+                            continue;
+                        }
+                    }
+                    Req::SignIn => {
+                        // TODO: Put the platform and local server callback in here
+                        tauri::api::shell::open(
+                            &app.shell_scope(),
+                            &controller.advanced_settings.auth_base_url,
+                            None,
+                        )?;
+                    }
+                    Req::SignOut => {
+                        // TODO: After we store the actor name on disk, clear the actor name here too.
+                        keyring_entry()?.delete_password()?;
+                        if let Some(mut session) = controller.session.take() {
+                            tracing::debug!("disconnecting connlib");
+                            session.connlib.disconnect(None);
+                        }
+                        else {
+                            tracing::error!("tried to sign out but there's no session");
+                        }
+                        app.tray_handle().set_menu(system_tray_menu::signed_out())?;
+                    }
+                    Req::StartStopLogCounting(enable) => {
+                        if enable {
+                            if log_counting_task.is_none() {
+                                let app = app.clone();
+                                log_counting_task = Some(tokio::spawn(logging::count_logs(app)));
+                                tracing::debug!("started log counting");
+                            }
+                        } else if let Some(t) = log_counting_task {
+                            t.abort();
+                            log_counting_task = None;
+                            tracing::debug!("cancelled log counting");
+                        }
+                    }
+                }
+            }
        }
    }
    tracing::debug!("GUI controller task exiting cleanly");