Merge pull request #13918 from gitbutlerapp/metrics-soft-fail

fix(but): never fail a command due to failing to emit metrics

Author: slarse

crates/but/src/utils/metrics.rs

@@ -418,7 +418,17 @@ where
             return self;
         };
 
-        tokio::process::Command::new(binary_path::current_exe_for_but_exec()?)
+        // We can fail both in resolving the path to the but binary, and in invoking it. As metrics
+        // emissions shouldn't impact user experience, we swallow these errors.
+        let but_path = match binary_path::current_exe_for_but_exec() {
+            Err(err) => {
+                tracing::warn!(?err, "Failed to resolve binary path to `but`");
+                return self;
+            }
+            Ok(path) => path,
+        };
+
+        let _ = tokio::process::Command::new(but_path)
             .arg("metrics")
             .arg("--command-name")
             .arg(v.get_name())
@@ -428,7 +438,9 @@ where
             .stdout(std::process::Stdio::null())
             .group()
             .kill_on_drop(false)
-            .spawn()?;
+            .spawn()
+            .map_err(|err| tracing::warn!(?err, "Failed to emit metrics"));
+
         self
     }
 }

crates/but/src/utils/mod.rs

@@ -43,8 +43,13 @@ impl ResultErrorExt for anyhow::Result<()> {
     }
 }
 
-/// Utilities attached to `anyhow::Result<T>`.
+/// Metrics utilities for results
 pub trait ResultMetricsExt<T, E> {
+    /// Emit metrics for the [`Result`].
+    ///
+    /// The result must simply be propagated through this method, regardless of if emitting metrics
+    /// is successful or not. We do not want a failure to emit metrics to impact the user
+    /// experience.
     fn emit_metrics(self, ctx: Option<OneshotMetricsContext>) -> Result<T, E>;
 }