From 78ebad13ab2f6fe8c3f6cc964ac0459e7e5b784d Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Wed, 6 Nov 2024 01:36:47 +1100 Subject: [PATCH] chore(rust): log more errors as `tracing::Value`s (#7208) Logging these as structured values gives us a better stacktrace in Sentry (assuming the errors themselves make proper use of defining an error-chain). --- .../bin-shared/src/network_changes/windows.rs | 15 +++++++--- .../src/tun_device_manager/windows.rs | 6 ++-- rust/connlib/clients/android/src/lib.rs | 3 +- rust/gui-client/src-common/src/auth.rs | 8 ++++-- rust/gui-client/src-common/src/controller.rs | 25 +++++++++-------- rust/gui-client/src-common/src/errors.rs | 2 +- rust/gui-client/src-common/src/logging.rs | 3 +- rust/gui-client/src-common/src/settings.rs | 3 +- rust/gui-client/src-common/src/updates.rs | 6 +++- rust/gui-client/src-tauri/src/client.rs | 7 +++-- rust/gui-client/src-tauri/src/client/gui.rs | 21 +++++++++----- .../src-tauri/src/client/gui/os_windows.rs | 3 +- .../src-tauri/src/client/gui/system_tray.rs | 6 +++- .../src-tauri/src/client/logging.rs | 11 ++++++-- .../src-tauri/src/client/settings.rs | 3 +- .../src-tauri/src/client/welcome.rs | 6 +++- rust/headless-client/src/dns_control.rs | 6 +++- .../src/dns_control/windows.rs | 8 ++++-- rust/headless-client/src/ipc_service.rs | 28 +++++++++++++++---- rust/headless-client/src/ipc_service/ipc.rs | 8 ++++-- .../src/ipc_service/windows.rs | 8 ++++-- rust/headless-client/src/main.rs | 5 ++-- 22 files changed, 134 insertions(+), 57 deletions(-) diff --git a/rust/bin-shared/src/network_changes/windows.rs b/rust/bin-shared/src/network_changes/windows.rs index 69390917d..a60aa7d53 100644 --- a/rust/bin-shared/src/network_changes/windows.rs +++ b/rust/bin-shared/src/network_changes/windows.rs @@ -374,6 +374,7 @@ impl Drop for Callback { mod async_dns { use anyhow::{Context as _, Result}; + use firezone_logging::anyhow_dyn_err; use futures::FutureExt as _; use std::{ffi::c_void, ops::Deref, path::Path, pin::pin}; use tokio::{ @@ -445,11 +446,17 @@ mod async_dns { } } - if let Err(error) = listener_4.close() { - tracing::error!(?error, "Error while closing IPv4 DNS listener"); + if let Err(e) = listener_4.close() { + tracing::error!( + error = anyhow_dyn_err(&e), + "Error while closing IPv4 DNS listener" + ); } - if let Err(error) = listener_6.close() { - tracing::error!(?error, "Error while closing IPv6 DNS listener"); + if let Err(e) = listener_6.close() { + tracing::error!( + error = anyhow_dyn_err(&e), + "Error while closing IPv6 DNS listener" + ); } Ok(()) diff --git a/rust/bin-shared/src/tun_device_manager/windows.rs b/rust/bin-shared/src/tun_device_manager/windows.rs index 25734ec5c..2f632bfad 100644 --- a/rust/bin-shared/src/tun_device_manager/windows.rs +++ b/rust/bin-shared/src/tun_device_manager/windows.rs @@ -204,7 +204,7 @@ impl Drop for Tun { ); self.packet_rx.close(); // This avoids a deadlock when we join the worker thread, see PR 5571 if let Err(error) = self.session.shutdown() { - tracing::error!(?error, "wintun::Session::shutdown"); + tracing::error!(error = std_dyn_err(&error), "wintun::Session::shutdown"); } if let Err(error) = self .recv_thread @@ -231,7 +231,7 @@ impl Tun { let adapter = match Adapter::create(&wintun, TUNNEL_NAME, TUNNEL_NAME, Some(uuid)) { Ok(x) => x, Err(error) => { - tracing::error!(?error, "Failed in `Adapter::create`"); + tracing::error!(error = std_dyn_err(&error), "Failed in `Adapter::create`"); return Err(error)?; } }; @@ -385,7 +385,7 @@ fn try_set_mtu(luid: NET_LUID_LH, family: ADDRESS_FAMILY, mtu: u32) -> Result<() if family == AF_INET6 && error.code() == windows_core::HRESULT::from_win32(0x80070490) { tracing::debug!(?family, "Couldn't set MTU, maybe IPv6 is disabled."); } else { - tracing::warn!(?family, ?error, "Couldn't set MTU"); + tracing::warn!(?family, error = std_dyn_err(&error), "Couldn't set MTU"); } return Ok(()); } diff --git a/rust/connlib/clients/android/src/lib.rs b/rust/connlib/clients/android/src/lib.rs index 6a762b552..8ea46f808 100644 --- a/rust/connlib/clients/android/src/lib.rs +++ b/rust/connlib/clients/android/src/lib.rs @@ -7,6 +7,7 @@ use crate::tun::Tun; use backoff::ExponentialBackoffBuilder; use connlib_client_shared::{Callbacks, DisconnectError, Session, V4RouteList, V6RouteList}; use connlib_model::{ResourceId, ResourceView}; +use firezone_logging::std_dyn_err; use firezone_telemetry::{Telemetry, ANDROID_DSN}; use ip_network::{Ipv4Network, Ipv6Network}; use jni::{ @@ -270,7 +271,7 @@ impl Callbacks for CallbackHandler { fn throw(env: &mut JNIEnv, class: &str, msg: impl Into) { if let Err(err) = env.throw_new(class, msg) { // We can't panic, since unwinding across the FFI boundary is UB... - tracing::error!(?err, "failed to throw Java exception"); + tracing::error!(error = std_dyn_err(&err), "failed to throw Java exception"); } } diff --git a/rust/gui-client/src-common/src/auth.rs b/rust/gui-client/src-common/src/auth.rs index df386fe77..fcbf05dda 100644 --- a/rust/gui-client/src-common/src/auth.rs +++ b/rust/gui-client/src-common/src/auth.rs @@ -2,6 +2,7 @@ use anyhow::Result; use firezone_headless_client::known_dirs; +use firezone_logging::std_dyn_err; use rand::{thread_rng, RngCore}; use secrecy::{ExposeSecret, SecretString}; use serde::{Deserialize, Serialize}; @@ -103,7 +104,7 @@ impl Auth { match this.get_token_from_disk() { Err(error) => tracing::error!( - ?error, + error = std_dyn_err(&error), "Failed to load token from disk. Will start in signed-out state" ), Ok(Some(SessionAndToken { session, token: _ })) => { @@ -129,7 +130,10 @@ impl Auth { /// Performs I/O. pub fn sign_out(&mut self) -> Result<(), Error> { if let Err(error) = self.token_store.delete_credential() { - tracing::warn!(?error, "Couldn't delete token while signing out"); + tracing::warn!( + error = std_dyn_err(&error), + "Couldn't delete token while signing out" + ); } delete_if_exists(&actor_name_path()?)?; delete_if_exists(&session_data_path()?)?; diff --git a/rust/gui-client/src-common/src/controller.rs b/rust/gui-client/src-common/src/controller.rs index e52f871d3..9f0206f0d 100644 --- a/rust/gui-client/src-common/src/controller.rs +++ b/rust/gui-client/src-common/src/controller.rs @@ -13,6 +13,7 @@ use firezone_headless_client::{ IpcClientMsg::{self, SetDisabledResources}, IpcServerMsg, IpcServiceError, LogFilterReloader, }; +use firezone_logging::{anyhow_dyn_err, std_dyn_err}; use firezone_telemetry::Telemetry; use secrecy::{ExposeSecret as _, SecretString}; use std::{collections::BTreeSet, ops::ControlFlow, path::PathBuf, time::Instant}; @@ -300,14 +301,14 @@ impl Controller { tracing::debug!("Closing..."); if let Err(error) = dns_notifier.close() { - tracing::error!(?error, "dns_notifier"); + tracing::error!(error = anyhow_dyn_err(&error), "dns_notifier"); } if let Err(error) = network_notifier.close() { - tracing::error!(?error, "network_notifier"); + tracing::error!(error = anyhow_dyn_err(&error), "network_notifier"); } if let Err(error) = self.ipc_client.disconnect_from_ipc().await { - tracing::error!(?error, "ipc_client"); + tracing::error!(error = anyhow_dyn_err(&error), "ipc_client"); } // Don't close telemetry here, `run` will close it. @@ -379,7 +380,7 @@ impl Controller { tracing::error!("Can't clear logs, we're already waiting on another log-clearing operation"); } if let Err(error) = logging::clear_gui_logs().await { - tracing::error!(?error, "Failed to clear GUI logs"); + tracing::error!(error = anyhow_dyn_err(&error), "Failed to clear GUI logs"); } self.ipc_client.send_msg(&IpcClientMsg::ClearLogs).await?; self.clear_logs_callback = Some(completion_tx); @@ -395,7 +396,7 @@ impl Controller { } Req::SchemeRequest(url) => { if let Err(error) = self.handle_deep_link(&url).await { - tracing::error!(?error, "`handle_deep_link` failed"); + tracing::error!(error = std_dyn_err(&error), "`handle_deep_link` failed"); } } Req::SignIn | Req::SystemTrayMenu(TrayMenuEvent::SignIn) => { @@ -490,7 +491,7 @@ impl Controller { Ok(flow) => Ok(flow), // 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"); + tracing::error!(error = std_dyn_err(&error), "Failed to connect to Firezone"); self.sign_out().await?; Ok(ControlFlow::Continue(())) } @@ -498,7 +499,7 @@ impl Controller { }, ipc::Event::ReadFailed(error) => { // IPC errors are always fatal - tracing::error!(?error, "IPC read failure"); + tracing::error!(error = anyhow_dyn_err(&error), "IPC read failure"); Err(Error::IpcRead)? } ipc::Event::Closed => Err(Error::IpcClosed)?, @@ -554,7 +555,7 @@ impl Controller { tracing::debug!(len = resources.len(), "Got new Resources"); self.status = Status::TunnelReady { resources }; if let Err(error) = self.refresh_system_tray_menu() { - tracing::error!(?error, "Failed to refresh menu"); + tracing::error!(error = anyhow_dyn_err(&error), "Failed to refresh menu"); } self.update_disabled_resources().await?; @@ -583,7 +584,7 @@ impl Controller { )?; } if let Err(error) = self.refresh_system_tray_menu() { - tracing::error!(?error, "Failed to refresh menu"); + tracing::error!(error = anyhow_dyn_err(&error), "Failed to refresh menu"); } } } @@ -617,7 +618,7 @@ impl Controller { ran_before::set().await?; self.status = Status::WaitingForTunnel { start_instant }; if let Err(error) = self.refresh_system_tray_menu() { - tracing::error!(?error, "Failed to refresh menu"); + tracing::error!(error = anyhow_dyn_err(&error), "Failed to refresh menu"); } Ok(()) } @@ -625,12 +626,12 @@ impl Controller { // This is typically something like, we don't have Internet access so we can't // open the PhoenixChannel's WebSocket. tracing::warn!( - ?error, + error, "Failed to connect to Firezone Portal, will try again when the network changes" ); self.status = Status::RetryingConnection { token }; if let Err(error) = self.refresh_system_tray_menu() { - tracing::error!(?error, "Failed to refresh menu"); + tracing::error!(error = anyhow_dyn_err(&error), "Failed to refresh menu"); } Ok(()) } diff --git a/rust/gui-client/src-common/src/errors.rs b/rust/gui-client/src-common/src/errors.rs index 30a307f76..1d5aced71 100644 --- a/rust/gui-client/src-common/src/errors.rs +++ b/rust/gui-client/src-common/src/errors.rs @@ -55,7 +55,7 @@ pub fn show_error_dialog(error: &Error) -> Result<()> { Error::IpcServiceTerminating => "The Firezone IPC service is terminating. Please restart the GUI Client.".to_string(), Error::Logging(_) => "Logging error".to_string(), Error::PortalConnection(error) => { - tracing::error!(?error, "Couldn't connect to the Portal"); + tracing::error!(%error, "Couldn't connect to the Portal"); "Couldn't connect to the Firezone Portal. Are you connected to the Internet?".to_string() } Error::UserNotInFirezoneGroup => format!("You are not a member of the group `{FIREZONE_GROUP}`. Try `sudo usermod -aG {FIREZONE_GROUP} $USER` and then reboot"), diff --git a/rust/gui-client/src-common/src/logging.rs b/rust/gui-client/src-common/src/logging.rs index 3ac8a1dbf..e70e14e90 100644 --- a/rust/gui-client/src-common/src/logging.rs +++ b/rust/gui-client/src-common/src/logging.rs @@ -2,6 +2,7 @@ use anyhow::{bail, Context as _, Result}; use firezone_headless_client::{known_dirs, LogFilterReloader}; +use firezone_logging::std_dyn_err; use serde::Serialize; use std::{ fs, @@ -62,7 +63,7 @@ pub fn setup(directives: &str) -> Result { set_global_default(subscriber)?; if let Err(error) = output_vt100::try_init() { tracing::debug!( - ?error, + error = std_dyn_err(&error), "Failed to init vt100 terminal colors (expected in release builds and in CI)" ); } diff --git a/rust/gui-client/src-common/src/settings.rs b/rust/gui-client/src-common/src/settings.rs index 9172d3304..1bf13972e 100644 --- a/rust/gui-client/src-common/src/settings.rs +++ b/rust/gui-client/src-common/src/settings.rs @@ -5,6 +5,7 @@ use anyhow::{Context as _, Result}; use atomicwrites::{AtomicFile, OverwriteBehavior}; use connlib_model::ResourceId; use firezone_headless_client::known_dirs; +use firezone_logging::std_dyn_err; use serde::{Deserialize, Serialize}; use std::{collections::HashSet, io::Write, path::PathBuf}; use url::Url; @@ -73,7 +74,7 @@ pub async fn save(settings: &AdvancedSettings) -> Result<()> { // Note: Blocking file write in async function if let Err(error) = f.write(|f| f.write_all(settings.log_filter.as_bytes())) { tracing::error!( - ?error, + error = std_dyn_err(&error), ?log_filter_path, "Couldn't write log filter file for IPC service" ); diff --git a/rust/gui-client/src-common/src/updates.rs b/rust/gui-client/src-common/src/updates.rs index 46395d346..e1cd9ecbb 100644 --- a/rust/gui-client/src-common/src/updates.rs +++ b/rust/gui-client/src-common/src/updates.rs @@ -1,6 +1,7 @@ //! Module to check the Github repo for new releases use anyhow::{Context, Result}; +use firezone_logging::anyhow_dyn_err; use rand::{thread_rng, Rng as _}; use semver::Version; use serde::{Deserialize, Serialize}; @@ -46,7 +47,10 @@ pub async fn checker_task( tracing::debug!("CheckNetwork"); match check().await { Ok(release) => fsm.handle_check(release), - Err(error) => tracing::error!(?error, "Couldn't check website for update"), + Err(error) => tracing::error!( + error = anyhow_dyn_err(&error), + "Couldn't check website for update" + ), } } Event::WaitInterval => { diff --git a/rust/gui-client/src-tauri/src/client.rs b/rust/gui-client/src-tauri/src/client.rs index 054e5da33..e0ca3d25b 100644 --- a/rust/gui-client/src-tauri/src/client.rs +++ b/rust/gui-client/src-tauri/src/client.rs @@ -3,6 +3,7 @@ use clap::{Args, Parser}; use firezone_gui_client_common::{ self as common, controller::Failure, deep_link, settings::AdvancedSettings, }; +use firezone_logging::{anyhow_dyn_err, std_dyn_err}; use firezone_telemetry as telemetry; use std::collections::BTreeMap; use tracing::instrument; @@ -53,7 +54,7 @@ pub(crate) fn run() -> Result<()> { Some(Cmd::OpenDeepLink(deep_link)) => { let rt = tokio::runtime::Runtime::new()?; if let Err(error) = rt.block_on(deep_link::open(&deep_link.url)) { - tracing::error!(?error, "Error in `OpenDeepLink`"); + tracing::error!(error = anyhow_dyn_err(&error), "Error in `OpenDeepLink`"); } Ok(()) } @@ -78,7 +79,7 @@ pub(crate) fn run() -> Result<()> { // Because of , // errors returned from `gui::run` may not be logged correctly - tracing::error!(?error); + tracing::error!(error = std_dyn_err(error)); } Ok(result?) } @@ -126,7 +127,7 @@ fn run_gui(cli: Cli) -> Result<()> { // Make sure errors get logged, at least to stderr if let Err(error) = &result { - tracing::error!(?error, error_msg = %error); + tracing::error!(error = std_dyn_err(error), error_msg = %error); common::errors::show_error_dialog(error)?; } diff --git a/rust/gui-client/src-tauri/src/client/gui.rs b/rust/gui-client/src-tauri/src/client/gui.rs index aae8eb9dc..e495697c1 100644 --- a/rust/gui-client/src-tauri/src/client/gui.rs +++ b/rust/gui-client/src-tauri/src/client/gui.rs @@ -18,6 +18,7 @@ use firezone_gui_client_common::{ updates, }; use firezone_headless_client::LogFilterReloader; +use firezone_logging::{anyhow_dyn_err, std_dyn_err}; use firezone_telemetry as telemetry; use secrecy::{ExposeSecret as _, SecretString}; use std::{str::FromStr, time::Duration}; @@ -172,7 +173,7 @@ pub(crate) fn run( tokio::spawn(async move { if let Err(error) = updates::checker_task(updates_tx, cli.debug_update_check).await { - tracing::error!(?error, "Error in updates::checker_task"); + tracing::error!(error = anyhow_dyn_err(&error), "Error in updates::checker_task"); } }); @@ -180,7 +181,7 @@ pub(crate) fn run( let ctlr_tx = ctlr_tx.clone(); tokio::spawn(async move { if let Err(error) = smoke_test(ctlr_tx).await { - tracing::error!(?error, "Error during smoke test, crashing on purpose so a dev can see our stacktraces"); + tracing::error!(error = anyhow_dyn_err(&error), "Error during smoke test, crashing on purpose so a dev can see our stacktraces"); unsafe { sadness_generator::raise_segfault() } } }); @@ -257,12 +258,12 @@ pub(crate) fn run( let exit_code = match task.await { Err(error) => { - tracing::error!(?error, "run_controller panicked"); + tracing::error!(error = std_dyn_err(&error), "run_controller panicked"); telemetry::end_session_with_status(telemetry::SessionStatus::Crashed); 1 } Ok(Err(error)) => { - tracing::error!(?error, "run_controller returned an error"); + tracing::error!(error = std_dyn_err(&error), "run_controller returned an error"); errors::show_error_dialog(&error).unwrap(); telemetry::end_session_with_status(telemetry::SessionStatus::Crashed); 1 @@ -289,7 +290,7 @@ pub(crate) fn run( let result = setup_inner(); if let Err(error) = &result { - tracing::error!(?error, "Tauri setup failed"); + tracing::error!(error, "Tauri setup failed"); } result @@ -299,7 +300,10 @@ pub(crate) fn run( let app = match app { Ok(x) => x, Err(error) => { - tracing::error!(?error, "Failed to build Tauri app instance"); + tracing::error!( + error = std_dyn_err(&error), + "Failed to build Tauri app instance" + ); #[expect(clippy::wildcard_enum_match_arm)] match error { tauri::Error::Runtime(tauri_runtime::Error::CreateWebview(_)) => { @@ -445,7 +449,10 @@ async fn accept_deep_links(mut server: deep_link::Server, ctlr_tx: CtlrTx) -> Re .await .ok(); } - Err(error) => tracing::error!(?error, "error while accepting deep link"), + Err(error) => tracing::error!( + error = anyhow_dyn_err(&error), + "error while accepting deep link" + ), } // We re-create the named pipe server every time we get a link, because of an oddity in the Windows API. server = deep_link::Server::new().await?; diff --git a/rust/gui-client/src-tauri/src/client/gui/os_windows.rs b/rust/gui-client/src-tauri/src/client/gui/os_windows.rs index 44f2ab376..6059a80da 100644 --- a/rust/gui-client/src-tauri/src/client/gui/os_windows.rs +++ b/rust/gui-client/src-tauri/src/client/gui/os_windows.rs @@ -1,6 +1,7 @@ use super::{ControllerRequest, CtlrTx}; use anyhow::{Context, Result}; use firezone_bin_shared::BUNDLE_ID; +use firezone_logging::std_dyn_err; use tauri::AppHandle; pub(crate) async fn set_autostart(_enabled: bool) -> Result<()> { @@ -76,7 +77,7 @@ pub(crate) fn show_clickable_notification( if let Some(req) = req.take() { if let Err(error) = tx.blocking_send(req) { tracing::error!( - ?error, + error = std_dyn_err(&error), "User clicked on notification, but we couldn't tell `Controller`" ); } diff --git a/rust/gui-client/src-tauri/src/client/gui/system_tray.rs b/rust/gui-client/src-tauri/src/client/gui/system_tray.rs index 35414ee05..0f1dabe1c 100644 --- a/rust/gui-client/src-tauri/src/client/gui/system_tray.rs +++ b/rust/gui-client/src-tauri/src/client/gui/system_tray.rs @@ -10,6 +10,7 @@ use firezone_gui_client_common::{ compositor::{self, Image}, system_tray::{AppState, ConnlibState, Entry, Icon, IconBase, Item, Menu}, }; +use firezone_logging::anyhow_dyn_err; use tauri::AppHandle; type IsMenuItem = dyn tauri::menu::IsMenuItem; @@ -88,7 +89,10 @@ impl Tray { self.app .run_on_main_thread(move || { if let Err(error) = update(handle, &app, &menu) { - tracing::error!(?error, "Error while updating tray icon"); + tracing::error!( + error = anyhow_dyn_err(&error), + "Error while updating tray icon" + ); } }) .unwrap(); diff --git a/rust/gui-client/src-tauri/src/client/logging.rs b/rust/gui-client/src-tauri/src/client/logging.rs index 9a444b907..5134a622b 100644 --- a/rust/gui-client/src-tauri/src/client/logging.rs +++ b/rust/gui-client/src-tauri/src/client/logging.rs @@ -4,6 +4,7 @@ use firezone_gui_client_common::{ controller::{ControllerRequest, CtlrTx}, logging as common, }; +use firezone_logging::std_dyn_err; use std::path::PathBuf; use tauri_plugin_dialog::DialogExt as _; @@ -12,11 +13,17 @@ pub(crate) async fn clear_logs(managed: tauri::State<'_, Managed>) -> Result<(), let (tx, rx) = tokio::sync::oneshot::channel(); if let Err(error) = managed.ctlr_tx.send(ControllerRequest::ClearLogs(tx)).await { // Tauri will only log errors to the JS console for us, so log this ourselves. - tracing::error!(?error, "Error while asking `Controller` to clear logs"); + tracing::error!( + error = std_dyn_err(&error), + "Error while asking `Controller` to clear logs" + ); return Err(error.to_string()); } if let Err(error) = rx.await { - tracing::error!(?error, "Error while awaiting log-clearing operation"); + tracing::error!( + error = std_dyn_err(&error), + "Error while awaiting log-clearing operation" + ); return Err(error.to_string()); } Ok(()) diff --git a/rust/gui-client/src-tauri/src/client/settings.rs b/rust/gui-client/src-tauri/src/client/settings.rs index af2df5c4b..0f1d466ec 100644 --- a/rust/gui-client/src-tauri/src/client/settings.rs +++ b/rust/gui-client/src-tauri/src/client/settings.rs @@ -7,6 +7,7 @@ use firezone_gui_client_common::{ controller::{ControllerRequest, CtlrTx}, settings::{save, AdvancedSettings}, }; +use firezone_logging::std_dyn_err; use std::time::Duration; use tokio::sync::oneshot; @@ -57,7 +58,7 @@ pub(crate) async fn get_advanced_settings( .await { tracing::error!( - ?error, + error = std_dyn_err(&error), "couldn't request advanced settings from controller task" ); } diff --git a/rust/gui-client/src-tauri/src/client/welcome.rs b/rust/gui-client/src-tauri/src/client/welcome.rs index dd04f860f..34a7c2593 100644 --- a/rust/gui-client/src-tauri/src/client/welcome.rs +++ b/rust/gui-client/src-tauri/src/client/welcome.rs @@ -2,12 +2,16 @@ use crate::client::gui::Managed; use firezone_gui_client_common::controller::ControllerRequest; +use firezone_logging::std_dyn_err; // Tauri requires a `Result` here, maybe in case the managed state can't be retrieved #[tauri::command] pub(crate) async fn sign_in(managed: tauri::State<'_, Managed>) -> anyhow::Result<(), String> { if let Err(error) = managed.ctlr_tx.send(ControllerRequest::SignIn).await { - tracing::error!(?error, "Couldn't request `Controller` to begin signing in"); + tracing::error!( + error = std_dyn_err(&error), + "Couldn't request `Controller` to begin signing in" + ); } Ok(()) } diff --git a/rust/headless-client/src/dns_control.rs b/rust/headless-client/src/dns_control.rs index 1b0a7a703..735b15fbd 100644 --- a/rust/headless-client/src/dns_control.rs +++ b/rust/headless-client/src/dns_control.rs @@ -7,6 +7,7 @@ use anyhow::Result; use firezone_bin_shared::platform::DnsControlMethod; +use firezone_logging::anyhow_dyn_err; use std::net::IpAddr; #[cfg(target_os = "linux")] @@ -33,7 +34,10 @@ pub struct DnsController { impl Drop for DnsController { fn drop(&mut self) { if let Err(error) = self.deactivate() { - tracing::error!(?error, "Failed to deactivate DNS control"); + tracing::error!( + error = anyhow_dyn_err(&error), + "Failed to deactivate DNS control" + ); } } } diff --git a/rust/headless-client/src/dns_control/windows.rs b/rust/headless-client/src/dns_control/windows.rs index 93f2aa5f8..87c6ee18a 100644 --- a/rust/headless-client/src/dns_control/windows.rs +++ b/rust/headless-client/src/dns_control/windows.rs @@ -16,6 +16,7 @@ use super::DnsController; use anyhow::{Context as _, Result}; use firezone_bin_shared::platform::{DnsControlMethod, CREATE_NO_WINDOW, TUNNEL_UUID}; +use firezone_logging::std_dyn_err; use std::{ io::ErrorKind, net::IpAddr, os::windows::process::CommandExt, path::Path, process::Command, }; @@ -33,12 +34,15 @@ impl DnsController { let hklm = winreg::RegKey::predef(winreg::enums::HKEY_LOCAL_MACHINE); if let Err(error) = hklm.delete_subkey(local_nrpt_path().join(NRPT_REG_KEY)) { if error.kind() != ErrorKind::NotFound { - tracing::error!(?error, "Couldn't delete local NRPT"); + tracing::error!(error = std_dyn_err(&error), "Couldn't delete local NRPT"); } } if let Err(error) = hklm.delete_subkey(group_nrpt_path().join(NRPT_REG_KEY)) { if error.kind() != ErrorKind::NotFound { - tracing::error!(?error, "Couldn't delete Group Policy NRPT"); + tracing::error!( + error = std_dyn_err(&error), + "Couldn't delete Group Policy NRPT" + ); } } refresh_group_policy()?; diff --git a/rust/headless-client/src/ipc_service.rs b/rust/headless-client/src/ipc_service.rs index 72abef0a2..17f3695d3 100644 --- a/rust/headless-client/src/ipc_service.rs +++ b/rust/headless-client/src/ipc_service.rs @@ -9,6 +9,7 @@ use firezone_bin_shared::{ platform::{tcp_socket_factory, udp_socket_factory, DnsControlMethod}, TunDeviceManager, TOKEN_ENV_KEY, }; +use firezone_logging::{anyhow_dyn_err, std_dyn_err}; use firezone_telemetry::Telemetry; use futures::{ future::poll_fn, @@ -116,12 +117,17 @@ pub enum ServerMsg { } // All variants are `String` because almost no error type implements `Serialize` -#[derive(Debug, Deserialize, Serialize)] +#[derive(Debug, Deserialize, Serialize, thiserror::Error)] pub enum Error { + #[error("{0}")] DeviceId(String), + #[error("{0}")] LoginUrl(String), + #[error("{0}")] PortalConnection(String), + #[error("{0}")] TunnelDevice(String), + #[error("{0}")] UrlParse(String), } @@ -336,7 +342,10 @@ impl<'a> Handler<'a> { match poll_fn(|cx| self.next_event(cx, signals)).await { Event::Callback(x) => { if let Err(error) = self.handle_connlib_cb(x).await { - tracing::error!(?error, "Error while handling connlib callback"); + tracing::error!( + error = anyhow_dyn_err(&error), + "Error while handling connlib callback" + ); continue; } } @@ -350,7 +359,10 @@ impl<'a> Handler<'a> { let _entered = tracing::error_span!("handle_ipc_msg", msg = %msg_variant).entered(); if let Err(error) = self.handle_ipc_msg(msg).await { - tracing::error!(?error, "Error while handling IPC message from client"); + tracing::error!( + error = anyhow_dyn_err(&error), + "Error while handling IPC message from client" + ); continue; } } @@ -359,7 +371,10 @@ impl<'a> Handler<'a> { break HandlerOk::ClientDisconnected; } Event::IpcError(error) => { - tracing::error!(?error, "Error while deserializing IPC message"); + tracing::error!( + error = anyhow_dyn_err(&error), + "Error while deserializing IPC message" + ); continue; } Event::Terminate => { @@ -468,7 +483,10 @@ impl<'a> Handler<'a> { let token = secrecy::SecretString::from(token); let result = self.connect_to_firezone(&api_url, token); if let Err(error) = &result { - tracing::error!(?error, "Failed to connect connlib session"); + tracing::error!( + error = std_dyn_err(error), + "Failed to connect connlib session" + ); } self.ipc_tx .send(&ServerMsg::ConnectResult(result)) diff --git a/rust/headless-client/src/ipc_service/ipc.rs b/rust/headless-client/src/ipc_service/ipc.rs index 2ee1f923e..2f08c2305 100644 --- a/rust/headless-client/src/ipc_service/ipc.rs +++ b/rust/headless-client/src/ipc_service/ipc.rs @@ -1,5 +1,6 @@ use crate::{IpcClientMsg, IpcServerMsg}; use anyhow::{Context as _, Result}; +use firezone_logging::std_dyn_err; use tokio::io::{ReadHalf, WriteHalf}; use tokio_util::{ bytes::BytesMut, @@ -145,7 +146,7 @@ pub async fn connect_to_service(id: ServiceId) -> Result<(ClientRead, ClientWrit } Err(error) => { tracing::warn!( - ?error, + error = std_dyn_err(&error), "Couldn't connect to IPC service, will sleep and try again" ); last_err = Some(error); @@ -171,6 +172,7 @@ impl platform::Server { mod tests { use super::{platform::Server, *}; use anyhow::{bail, ensure, Result}; + use firezone_logging::anyhow_dyn_err; use futures::{SinkExt, StreamExt}; use std::time::Duration; use tokio::{task::JoinHandle, time::timeout}; @@ -241,14 +243,14 @@ mod tests { if let Err(panic) = &client_result { tracing::error!(?panic, "Client panic"); } else if let Ok(Err(error)) = &client_result { - tracing::error!(?error, "Client error"); + tracing::error!(error = anyhow_dyn_err(error), "Client error"); } let server_result = server_task.await; if let Err(panic) = &server_result { tracing::error!(?panic, "Server panic"); } else if let Ok(Err(error)) = &server_result { - tracing::error!(?error, "Server error"); + tracing::error!(error = anyhow_dyn_err(error), "Server error"); } if client_result.is_err() || server_result.is_err() { diff --git a/rust/headless-client/src/ipc_service/windows.rs b/rust/headless-client/src/ipc_service/windows.rs index c6f1b10ce..3667ca676 100644 --- a/rust/headless-client/src/ipc_service/windows.rs +++ b/rust/headless-client/src/ipc_service/windows.rs @@ -1,6 +1,7 @@ use crate::CliCommon; use anyhow::{bail, Context as _, Result}; use firezone_bin_shared::platform::DnsControlMethod; +use firezone_logging::anyhow_dyn_err; use futures::future::{self, Either}; use std::{ ffi::{c_void, OsString}, @@ -132,7 +133,10 @@ fn service_run(arguments: Vec) { let (handle, log_filter_reloader) = super::setup_logging(None).expect("Should be able to set up logging"); if let Err(error) = fallible_service_run(arguments, handle, log_filter_reloader) { - tracing::error!(?error, "`fallible_windows_service_run` returned an error"); + tracing::error!( + error = anyhow_dyn_err(&error), + "`fallible_windows_service_run` returned an error" + ); } } @@ -208,7 +212,7 @@ fn fallible_service_run( // Windows that we're shutting down. let result = rt.block_on(service_run_async(&log_filter_reloader, shutdown_rx)); if let Err(error) = &result { - tracing::error!(?error); + tracing::error!(error = anyhow_dyn_err(error)); } // Drop the logging handle so it flushes the logs before we let Windows kill our process. diff --git a/rust/headless-client/src/main.rs b/rust/headless-client/src/main.rs index 89a373f4f..cc59bc689 100644 --- a/rust/headless-client/src/main.rs +++ b/rust/headless-client/src/main.rs @@ -12,6 +12,7 @@ use firezone_bin_shared::{ use firezone_headless_client::{ device_id, signals, CallbackHandler, CliCommon, ConnlibMsg, DnsController, }; +use firezone_logging::anyhow_dyn_err; use firezone_telemetry::Telemetry; use futures::{FutureExt as _, StreamExt as _}; use phoenix_channel::get_user_agent; @@ -321,10 +322,10 @@ fn main() -> Result<()> { }; if let Err(error) = dns_notifier.close() { - tracing::error!(?error, "DNS notifier") + tracing::error!(error = anyhow_dyn_err(&error), "DNS notifier") } if let Err(error) = network_notifier.close() { - tracing::error!(?error, "network notifier"); + tracing::error!(error = anyhow_dyn_err(&error), "network notifier"); } telemetry.stop().await; // Stop telemetry before dropping session. `connlib` needs to be active for this, otherwise we won't be able to resolve the DNS name for sentry.