From 6f0b471652eb902e8528e2a402c9f0180759fe3c Mon Sep 17 00:00:00 2001
From: Thomas Eizinger <thomas@eizinger.io>
Date: Mon, 23 Dec 2024 12:44:09 +0100
Subject: [PATCH] fix(gui-client): ensure IPC errors are mapped correctly
 (#7554)

In order to make sure that the correct error message is displayed to
users, we need to preserve error information as much as possible.
---
 rust/gui-client/src-common/src/errors.rs      | 19 ++++++++++++-------
 rust/gui-client/src-common/src/ipc.rs         |  2 +-
 rust/headless-client/src/ipc_service/ipc.rs   |  2 --
 .../src/ipc_service/ipc/linux.rs              |  1 -
 4 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/rust/gui-client/src-common/src/errors.rs b/rust/gui-client/src-common/src/errors.rs
index 41fb4a188..8bc9ca249 100644
--- a/rust/gui-client/src-common/src/errors.rs
+++ b/rust/gui-client/src-common/src/errors.rs
@@ -11,10 +11,8 @@ pub enum Error {
     DeepLink(#[from] deep_link::Error),
     #[error("Logging module error: {0}")]
     Logging(#[from] common::logging::Error),
-
-    // `client.rs` provides a more user-friendly message when showing the error dialog box for certain variants
-    #[error("IPC")]
-    Ipc(#[from] ipc::Error),
+    #[error("IPC service not found")]
+    IpcNotFound,
     #[error("IPC closed")]
     IpcClosed,
     #[error("IPC read failed")]
@@ -29,6 +27,15 @@ pub enum Error {
     Other(#[from] anyhow::Error),
 }
 
+impl From<ipc::Error> for Error {
+    fn from(value: ipc::Error) -> Self {
+        match value {
+            ipc::Error::NotFound(_) => Self::IpcNotFound,
+            ipc::Error::Other(error) => Self::Other(error),
+        }
+    }
+}
+
 impl Error {
     // Decision to put the error strings here: <https://github.com/firezone/firezone/pull/3464#discussion_r1473608415>
     // This message gets shown to users in the GUI and could be localized, unlike
@@ -39,9 +46,7 @@ impl Error {
             Error::WebViewNotInstalled => "Firezone cannot start because WebView2 is not installed. Follow the instructions at <https://www.firezone.dev/kb/client-apps/windows-client>.".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(),
-            Error::Ipc(ipc::Error::NotFound(_)) => "Couldn't find Firezone IPC service. Is the service running?".to_string(),
-            Error::Ipc(ipc::Error::PermissionDenied) => "Permission denied for Firezone IPC service. This should only happen on dev systems.".to_string(),
-            Error::Ipc(ipc::Error::Other(error)) => error.to_string(),
+            Error::IpcNotFound => "Couldn't find Firezone IPC service. Is the service running?".to_string(),
             Error::IpcClosed => "IPC connection closed".to_string(),
             Error::IpcRead => "IPC read failure".to_string(),
             Error::IpcServiceTerminating => "The Firezone IPC service is terminating. Please restart the GUI Client.".to_string(),
diff --git a/rust/gui-client/src-common/src/ipc.rs b/rust/gui-client/src-common/src/ipc.rs
index 799414dd6..4b0a8a60b 100644
--- a/rust/gui-client/src-common/src/ipc.rs
+++ b/rust/gui-client/src-common/src/ipc.rs
@@ -27,7 +27,7 @@ impl Drop for Client {
 }
 
 impl Client {
-    pub async fn new(ctlr_tx: tokio::sync::mpsc::Sender<Event>) -> Result<Self> {
+    pub async fn new(ctlr_tx: tokio::sync::mpsc::Sender<Event>) -> Result<Self, ipc::Error> {
         tracing::debug!(
             client_pid = std::process::id(),
             "Connecting to IPC service..."
diff --git a/rust/headless-client/src/ipc_service/ipc.rs b/rust/headless-client/src/ipc_service/ipc.rs
index 7f13d138a..7b3f9785d 100644
--- a/rust/headless-client/src/ipc_service/ipc.rs
+++ b/rust/headless-client/src/ipc_service/ipc.rs
@@ -29,8 +29,6 @@ pub(crate) type ServerWrite = FramedWrite<WriteHalf<ServerStream>, Encoder<IpcSe
 pub enum Error {
     #[error("Couldn't find IPC service `{0}`")]
     NotFound(String),
-    #[error("Permission denied")]
-    PermissionDenied,
 
     #[error("{0:#}")] // Use alternate display here to log entire chain of errors.
     Other(anyhow::Error),
diff --git a/rust/headless-client/src/ipc_service/ipc/linux.rs b/rust/headless-client/src/ipc_service/ipc/linux.rs
index 3153dd1fd..ecc5c3700 100644
--- a/rust/headless-client/src/ipc_service/ipc/linux.rs
+++ b/rust/headless-client/src/ipc_service/ipc/linux.rs
@@ -27,7 +27,6 @@ pub async fn connect_to_service(id: ServiceId) -> Result<ClientStream, Error> {
                 Error::Other(anyhow!("ConnectionRefused by Unix domain socket"))
             }
             ErrorKind::NotFound => Error::NotFound(path.display().to_string()),
-            ErrorKind::PermissionDenied => Error::PermissionDenied,
             _ => Error::Other(
                 anyhow!(error.to_string()).context("Couldn't connect to Unix domain socket"),
             ),