From 112be91cae3bdffbe2204f6b6d51642ba6a4ddbd Mon Sep 17 00:00:00 2001 From: Reactor Scram Date: Wed, 4 Sep 2024 14:00:34 -0500 Subject: [PATCH] fix(rust/gui-client): if we can't raise the tunnel, sign out (#6548) Refs #6547 Given a valid token is on disk, when the Client tries to auto-sign-in to Firezone and something breaks during sign-in, then... **Old behavior:** ...then the GUI silently implodes and we can't export logs. When you restart the GUI, it reloads the token and immediately implodes again. **New behavior:** ...then the GUI doesn't silently implode, it silently signs out. Which is still weird, but exporting logs will work. This addresses an issue where a customer couldn't export logs while helping us debug #6547. --- rust/gui-client/src-tauri/src/client/gui.rs | 19 ++++++++++++------- .../src-tauri/src/client/gui/errors.rs | 6 ++++-- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/rust/gui-client/src-tauri/src/client/gui.rs b/rust/gui-client/src-tauri/src/client/gui.rs index 0e44a23f5..6155c5e7c 100644 --- a/rust/gui-client/src-tauri/src/client/gui.rs +++ b/rust/gui-client/src-tauri/src/client/gui.rs @@ -538,7 +538,15 @@ impl Controller { Req::GetAdvancedSettings(tx) => { tx.send(self.advanced_settings.clone()).ok(); } - Req::Ipc(msg) => self.handle_ipc(msg).await?, + Req::Ipc(msg) => match self.handle_ipc(msg).await { + Ok(()) => {} + // Handles more gracefully so we can still export logs even if we crashed right after sign-in + Err(Error::ConnectToFirezoneFailed(error)) => { + tracing::error!(?error, "Failed to connect to Firezone"); + self.sign_out().await?; + } + Err(error) => Err(error)?, + } Req::IpcReadFailed(error) => { // IPC errors are always fatal tracing::error!(?error, "IPC read failure"); @@ -765,6 +773,8 @@ impl Controller { Ok(()) } Err(IpcServiceError::PortalConnection(error)) => { + // This is typically something like, we don't have Internet access so we can't + // open the PhoenixChannel's WebSocket. tracing::warn!( ?error, "Failed to connect to Firezone Portal, will try again when the network changes" @@ -775,12 +785,7 @@ impl Controller { } Ok(()) } - Err(error) => { - tracing::error!(?error, "Failed to connect to Firezone"); - Err(Error::Other(anyhow!( - "Failed to connect to Firezone for non-Portal-related reason" - ))) - } + Err(msg) => Err(Error::ConnectToFirezoneFailed(msg)), } } diff --git a/rust/gui-client/src-tauri/src/client/gui/errors.rs b/rust/gui-client/src-tauri/src/client/gui/errors.rs index c5325ac69..7a94d803f 100644 --- a/rust/gui-client/src-tauri/src/client/gui/errors.rs +++ b/rust/gui-client/src-tauri/src/client/gui/errors.rs @@ -1,11 +1,13 @@ use super::{deep_link, logging}; use anyhow::Result; -use firezone_headless_client::{ipc, FIREZONE_GROUP}; +use firezone_headless_client::{ipc, IpcServiceError, FIREZONE_GROUP}; // TODO: Replace with `anyhow` gradually per #[allow(dead_code)] #[derive(Debug, thiserror::Error)] pub(crate) enum Error { + #[error("Failed to connect to Firezone for non-Portal-related reason")] + ConnectToFirezoneFailed(IpcServiceError), #[error("Deep-link module error: {0}")] DeepLink(#[from] deep_link::Error), #[error("Logging module error: {0}")] @@ -39,7 +41,7 @@ pub(crate) fn show_error_dialog(error: &Error) -> Result<()> { // This message gets shown to users in the GUI and could be localized, unlike // messages in the log which only need to be used for `git grep`. let user_friendly_error_msg = match error { - // TODO: Update this URL + Error::ConnectToFirezoneFailed(_) => error.to_string(), Error::WebViewNotInstalled => "Firezone cannot start because WebView2 is not installed. Follow the instructions at .".to_string(), Error::DeepLink(deep_link::Error::CantListen) => "Firezone is already running. If it's not responding, force-stop it.".to_string(), Error::DeepLink(deep_link::Error::Other(error)) => error.to_string(),