refactor(gui-client): de-duplicate logging of IPC message errors (#8157)

This commit is contained in:
Thomas Eizinger
2025-02-18 01:21:52 +11:00
committed by GitHub
parent 8c7c0a9e8e
commit 7ea17c144a
4 changed files with 48 additions and 29 deletions

23
rust/Cargo.lock generated
View File

@@ -2160,6 +2160,7 @@ dependencies = [
"serde_json",
"serde_variant",
"smbios-lib",
"strum",
"tempfile",
"thiserror 1.0.69",
"tokio",
@@ -6231,6 +6232,28 @@ dependencies = [
"syn 2.0.87",
]
[[package]]
name = "strum"
version = "0.27.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "f64def088c51c9510a8579e3c5d67c65349dcf755e5479ad3d010aa6454e2c32"
dependencies = [
"strum_macros",
]
[[package]]
name = "strum_macros"
version = "0.27.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "c77a8c5abcaf0f9ce05d62342b7d298c346515365c36b673df4ebe3ced01fde8"
dependencies = [
"heck 0.5.0",
"proc-macro2",
"quote",
"rustversion",
"syn 2.0.87",
]
[[package]]
name = "stun_codec"
version = "0.3.5"

View File

@@ -107,6 +107,7 @@ smbios-lib = "0.9.2"
smoltcp = { version = "0.12", default-features = false }
static_assertions = "1.1.0"
str0m = { version = "0.6.3", default-features = false, features = ["sha1"] }
strum = { version = "0.27.1", features = ["derive"] }
stun_codec = "0.3.4"
subprocess = "0.2.9"
subtle = "2.5.0"

View File

@@ -28,6 +28,7 @@ serde = { workspace = true, features = ["derive"] }
serde_json = { workspace = true }
serde_variant = { workspace = true }
smbios-lib = { workspace = true }
strum = { workspace = true }
thiserror = { workspace = true }
# This actually relies on many other features in Tokio, so this will probably
# fail to build outside the workspace. <https://github.com/firezone/firezone/pull/4328#discussion_r1540342142>

View File

@@ -104,7 +104,7 @@ pub enum ClientMsg {
}
/// Messages that end up in the GUI, either forwarded from connlib or from the IPC service.
#[derive(Debug, Deserialize, Serialize)]
#[derive(Debug, Deserialize, Serialize, strum::Display)]
pub enum ServerMsg {
/// The IPC service finished clearing its log dir.
ClearedLogs(Result<(), String>),
@@ -392,7 +392,7 @@ impl<'a> Handler<'a> {
"Caught SIGINT / SIGTERM / Ctrl+C while an IPC client is connected"
);
// Ignore the result here because we're terminating anyway.
let _ = self.ipc_tx.send(&ServerMsg::TerminatingGracefully).await;
let _ = self.send_ipc(ServerMsg::TerminatingGracefully).await;
break HandlerOk::ServiceTerminating;
}
}
@@ -439,13 +439,11 @@ impl<'a> Handler<'a> {
session.connlib.disconnect();
}
self.dns_controller.deactivate()?;
self.ipc_tx
.send(&ServerMsg::OnDisconnect {
error_msg,
is_authentication_error,
})
.await
.context("Error while sending IPC message `OnDisconnect`")?
self.send_ipc(ServerMsg::OnDisconnect {
error_msg,
is_authentication_error,
})
.await?
}
ConnlibMsg::OnSetInterfaceConfig {
ipv4,
@@ -462,18 +460,13 @@ impl<'a> Handler<'a> {
self.tun_device.set_routes(ipv4_routes, ipv6_routes).await?;
self.dns_controller.flush()?;
self.ipc_tx
.send(&ServerMsg::TunnelReady)
.await
.context("Error while sending IPC message `TunnelReady`")?;
self.send_ipc(ServerMsg::TunnelReady).await?;
}
ConnlibMsg::OnUpdateResources(resources) => {
// On every resources update, flush DNS to mitigate <https://github.com/firezone/firezone/issues/5052>
self.dns_controller.flush()?;
self.ipc_tx
.send(&ServerMsg::OnUpdateResources(resources))
.await
.context("Error while sending IPC message `OnUpdateResources`")?;
self.send_ipc(ServerMsg::OnUpdateResources(resources))
.await?;
}
}
Ok(())
@@ -486,20 +479,15 @@ impl<'a> Handler<'a> {
&crate::known_dirs::ipc_service_logs().context("Can't compute logs dir")?,
)
.await;
self.ipc_tx
.send(&ServerMsg::ClearedLogs(result.map_err(|e| e.to_string())))
.await
.context("Error while sending IPC message")?
self.send_ipc(ServerMsg::ClearedLogs(result.map_err(|e| e.to_string())))
.await?
}
ClientMsg::Connect { api_url, token } => {
// Warning: Connection errors don't bubble to callers of `handle_ipc_msg`.
let token = secrecy::SecretString::from(token);
let result = self.connect_to_firezone(&api_url, token);
self.ipc_tx
.send(&ServerMsg::ConnectResult(result))
.await
.context("Failed to send `ConnectResult`")?
self.send_ipc(ServerMsg::ConnectResult(result)).await?
}
ClientMsg::Disconnect => {
if let Some(session) = self.session.take() {
@@ -509,10 +497,7 @@ impl<'a> Handler<'a> {
}
// Always send `DisconnectedGracefully` even if we weren't connected,
// so this will be idempotent.
self.ipc_tx
.send(&ServerMsg::DisconnectedGracefully)
.await
.context("Failed to send `DisconnectedGracefully`")?;
self.send_ipc(ServerMsg::DisconnectedGracefully).await?;
}
ClientMsg::ApplyLogFilter { directives } => {
self.log_filter_reloader.reload(directives.clone())?;
@@ -640,6 +625,15 @@ impl<'a> Handler<'a> {
Ok(())
}
async fn send_ipc(&mut self, msg: ServerMsg) -> Result<()> {
self.ipc_tx
.send(&msg)
.await
.with_context(|| format!("Failed to send IPC message `{msg}`"))?;
Ok(())
}
}
/// Starts logging for the production IPC service