windows: fix advanced settings loading, catch deep link error (#2811)

Based / Blocked on #2795

Fixes #2807 where I accidentally bail out of the controller task if the
settings file is missing or isn't valid in any way.

---------

Signed-off-by: Reactor Scram <ReactorScram@users.noreply.github.com>
Co-authored-by: Jamil <jamilbk@users.noreply.github.com>

rust/Cargo.lock

@@ -2013,6 +2013,7 @@ dependencies = [
  "anyhow",
  "clap",
  "connlib-client-shared",
+ "connlib-shared",
  "firezone-cli-utils",
  "http-body-util",
  "hyper 1.0.1",
@@ -2025,6 +2026,7 @@ dependencies = [
  "tauri",
  "tauri-build",
  "tauri-plugin-deep-link",
+ "thiserror",
  "tokio",
  "tracing",
  "tracing-subscriber",

rust/connlib/tunnel/src/device_channel/device_channel_win.rs

@@ -4,27 +4,36 @@ use connlib_shared::{messages::Interface, Callbacks, Result};
 use ip_network::IpNetwork;
 use std::task::{Context, Poll};
 
+// TODO: Fill all this out. These are just stubs to test the GUI.
+
 pub(crate) struct DeviceIo;
 
 pub(crate) struct IfaceConfig;
 
 impl DeviceIo {
     pub fn poll_read(&self, _: &mut [u8], _: &mut Context<'_>) -> Poll<std::io::Result<usize>> {
-        todo!()
+        // Incoming packets will never appear
+        Poll::Pending
     }
 
-    pub fn write(&self, _: Packet<'_>) -> std::io::Result<usize> {
-        todo!()
+    pub fn write(&self, packet: Packet<'_>) -> std::io::Result<usize> {
+        // All outgoing packets are successfully written to the void
+        match packet {
+            Packet::Ipv4(msg) => Ok(msg.len()),
+            Packet::Ipv6(msg) => Ok(msg.len()),
+        }
     }
 }
 
+const BOGUS_MTU: usize = 1_500;
+
 impl IfaceConfig {
     pub(crate) fn mtu(&self) -> usize {
-        todo!()
+        BOGUS_MTU
     }
 
     pub(crate) async fn refresh_mtu(&self) -> Result<usize> {
-        todo!()
+        Ok(BOGUS_MTU)
     }
 
     pub(crate) async fn add_route(
@@ -32,10 +41,15 @@ impl IfaceConfig {
         _: IpNetwork,
         _: &impl Callbacks,
     ) -> Result<Option<Device>> {
-        todo!()
+        let io = DeviceIo {};
+        let config = IfaceConfig {};
+        Ok(Some(Device { io, config }))
     }
 }
 
 pub(crate) async fn create_iface(_: &Interface, _: &impl Callbacks) -> Result<Device> {
-    todo!()
+    Ok(Device {
+        config: IfaceConfig {},
+        io: DeviceIo {},
+    })
 }

rust/windows-client/src-tauri/Cargo.toml

@@ -12,6 +12,7 @@ tauri-build = { version = "1.5", features = [] }
 anyhow = { version = "1.0" }
 clap = { version = "4.4", features = ["derive",  "env"] }
 connlib-client-shared = { workspace = true }
+connlib-shared = { workspace = true }
 firezone-cli-utils = { workspace = true }
 http-body-util = "0.1.0"
 hyper = { version = "1.0.1", features = ["http1", "server"] }
@@ -21,6 +22,7 @@ smbios-lib = "0.9"
 secrecy.workspace = true
 serde = { version = "1.0", features = ["derive"] }
 serde_json = "1.0"
+thiserror = { version = "1.0", default-features = false }
 tokio = { version = "1.33.0", features = ["time"] }
 tracing = { workspace = true }
 tracing-subscriber = { version = "0.3.17", features = ["env-filter"] }

rust/windows-client/src-tauri/src/gui.rs

@@ -13,7 +13,6 @@ use tauri::{
     SystemTraySubmenu,
 };
 use tokio::sync::{mpsc, oneshot};
-use url::Url;
 use ControllerRequest as Req;
 
 pub(crate) type CtlrTx = mpsc::Sender<ControllerRequest>;
@@ -100,12 +99,14 @@ pub(crate) fn run(params: crate::GuiParams) -> Result<()> {
 
             // From https://github.com/FabianLars/tauri-plugin-deep-link/blob/main/example/main.rs
             let handle = app.handle();
-            tauri_plugin_deep_link::register(crate::DEEP_LINK_SCHEME, move |url| {
+            if let Err(e) = tauri_plugin_deep_link::register(crate::DEEP_LINK_SCHEME, move |url| {
                 match handle_deep_link(&handle, url) {
                     Ok(()) => {}
                     Err(e) => tracing::error!("{e}"),
                 }
-            })?;
+            }) {
+                tracing::error!("couldn't register deep link scheme: {e}");
+            }
             Ok(())
         })
         .build(tauri::generate_context!())?
@@ -218,12 +219,17 @@ fn keyring_entry() -> Result<keyring::Entry> {
 #[derive(Clone)]
 struct CallbackHandler {
     ctlr_tx: CtlrTx,
-    handle: Option<file_logger::Handle>,
+    logger: Option<file_logger::Handle>,
+}
+
+#[derive(thiserror::Error, Debug)]
+enum CallbackError {
+    #[error(transparent)]
+    ControllerRequest(#[from] tokio::sync::mpsc::error::TrySendError<ControllerRequest>),
 }
 
 impl connlib_client_shared::Callbacks for CallbackHandler {
-    // TODO: add thiserror type
-    type Error = std::convert::Infallible;
+    type Error = CallbackError;
 
     fn on_disconnect(
         &self,
@@ -242,15 +248,15 @@ impl connlib_client_shared::Callbacks for CallbackHandler {
         &self,
         resources: Vec<connlib_client_shared::ResourceDescription>,
     ) -> Result<(), Self::Error> {
-        tracing::debug!("on_update_resources");
+        tracing::trace!("on_update_resources");
+        // TODO: Better error handling?
         self.ctlr_tx
-            .blocking_send(ControllerRequest::UpdateResources(resources))
-            .unwrap();
+            .try_send(ControllerRequest::UpdateResources(resources))?;
         Ok(())
     }
 
     fn roll_log_file(&self) -> Option<PathBuf> {
-        self.handle
+        self.logger
             .as_ref()?
             .roll_to_new_file()
             .unwrap_or_else(|e| {
@@ -276,7 +282,9 @@ impl Controller {
             .ok_or_else(|| anyhow::anyhow!("can't get Managed object from Tauri"))?
             .ctlr_tx
             .clone();
-        let advanced_settings = settings::load_advanced_settings(&app).await?;
+        let advanced_settings = settings::load_advanced_settings(&app)
+            .await
+            .unwrap_or_default();
 
         tracing::trace!("re-loading token");
         let token: Option<SecretString> = tokio::task::spawn_blocking(|| {
@@ -318,7 +326,7 @@ impl Controller {
         ctlr_tx: CtlrTx,
         token: &SecretString,
     ) -> Result<connlib_client_shared::Session<CallbackHandler>> {
-        let (layer, handle) = file_logger::layer(std::path::Path::new("logs"));
+        let (layer, logger) = file_logger::layer(std::path::Path::new("logs"));
         // TODO: How can I set up the tracing subscriber if the Session isn't ready yet? Check what other clients do.
         if false {
             // This helps the type inference
@@ -332,7 +340,7 @@ impl Controller {
             crate::device_id::get(),
             CallbackHandler {
                 ctlr_tx,
-                handle: Some(handle),
+                logger: Some(logger),
             },
         )?)
     }
@@ -342,7 +350,14 @@ async fn run_controller(
     app: tauri::AppHandle,
     mut rx: mpsc::Receiver<ControllerRequest>,
 ) -> Result<()> {
-    let mut controller = Controller::new(app.clone()).await?;
+    let mut controller = match Controller::new(app.clone()).await {
+        Err(e) => {
+            // TODO: There must be a shorter way to write these?
+            tracing::error!("couldn't create controller: {e}");
+            return Err(e);
+        }
+        Ok(x) => x,
+    };
 
     tracing::debug!("GUI controller main loop start");
 
@@ -378,7 +393,12 @@ async fn run_controller(
                 )?;
             }
             Req::UpdateResources(resources) => {
-                tracing::debug!("got {} resources", resources.len());
+                tracing::debug!("controller got UpdateResources");
+                let resources: Vec<_> = resources.into_iter().map(ResourceDisplay::from).collect();
+
+                // TODO: Save the user name between runs of the app
+                app.tray_handle()
+                    .set_menu(signed_in_menu("placeholder", &resources))?;
             }
         }
     }
@@ -424,29 +444,45 @@ fn parse_auth_callback(input: &SecretString) -> Result<AuthCallback> {
 }
 
 /// The information needed for the GUI to display a resource inside the Firezone VPN
-struct _ResourceDisplay {
-    /// UUIDv4 (Fully random)
-    /// This should be stable over time even if the DNS / IP / name change, so we can use it for callbacks from the tray menu
-    id: String,
+struct ResourceDisplay {
+    id: connlib_shared::messages::ResourceId,
     /// User-friendly name, e.g. "GitLab"
     name: String,
     /// What will be copied to the clipboard to paste into a web browser
-    url: Url,
+    pastable: String,
 }
 
-fn _signed_in_menu(user_email: &str, resources: &[_ResourceDisplay]) -> SystemTrayMenu {
+impl From<connlib_client_shared::ResourceDescription> for ResourceDisplay {
+    fn from(x: connlib_client_shared::ResourceDescription) -> Self {
+        match x {
+            connlib_client_shared::ResourceDescription::Dns(x) => Self {
+                id: x.id,
+                name: x.name,
+                pastable: x.address,
+            },
+            connlib_client_shared::ResourceDescription::Cidr(x) => Self {
+                id: x.id,
+                name: x.name,
+                // TODO: CIDRs aren't URLs right?
+                pastable: x.address.to_string(),
+            },
+        }
+    }
+}
+
+fn signed_in_menu(user_name: &str, resources: &[ResourceDisplay]) -> SystemTrayMenu {
     let mut menu = SystemTrayMenu::new()
         .add_item(
-            CustomMenuItem::new("".to_string(), format!("Signed in as {user_email}")).disabled(),
+            CustomMenuItem::new("".to_string(), format!("Signed in as {user_name}")).disabled(),
         )
         .add_item(CustomMenuItem::new("/sign_out".to_string(), "Sign out"))
         .add_native_item(SystemTrayMenuItem::Separator)
         .add_item(CustomMenuItem::new("".to_string(), "Resources").disabled());
 
-    for _ResourceDisplay { id, name, url } in resources {
+    for ResourceDisplay { id, name, pastable } in resources {
         let submenu = SystemTrayMenu::new().add_item(CustomMenuItem::new(
             format!("/resource/{id}"),
-            url.to_string(),
+            pastable.to_string(),
         ));
         menu = menu.add_submenu(SystemTraySubmenu::new(name, submenu));
     }

rust/windows-client/src-tauri/src/settings.rs

@@ -64,11 +64,13 @@ pub(crate) async fn get_advanced_settings(
     managed: tauri::State<'_, Managed>,
 ) -> StdResult<AdvancedSettings, String> {
     let (tx, rx) = oneshot::channel();
-    managed
+    if let Err(e) = managed
         .ctlr_tx
         .send(ControllerRequest::GetAdvancedSettings(tx))
         .await
-        .unwrap();
+    {
+        tracing::error!("couldn't request advanced settings from controller task: {e}");
+    }
     Ok(rx.await.unwrap())
 }