Surface mirror push failure details in Sentry
Assisted-by: Claude Sonnet 4.6 via Claude Code
change zvoymnvqnzxlqsxwovprktsotloustxq
commit c3dcb50a93ee0f817b42c1b6eb772952a158dae0
author Alpha Chen <alpha@kejadlen.dev>
date
parent qrzrvqpw
diff --git a/quire-core/src/telemetry.rs b/quire-core/src/telemetry.rs
index e2f9b2c..ea3f3ad 100644
--- a/quire-core/src/telemetry.rs
+++ b/quire-core/src/telemetry.rs
@@ -44,15 +44,13 @@ type RenderFn = Box<dyn (Fn(&(dyn std::error::Error + 'static)) -> Option<String
 ///
 /// # Sentry data scrubbing
 ///
-/// The rendering currently arrives in Sentry as `extra["diagnostic"] =
-/// "[Filtered]"`, so this layer's output is effectively invisible there
-/// today. The miette `NarratableReportHandler` renders the source
-/// snippet inline, and any keyword Sentry's data-scrubber recognizes
-/// (e.g. `secret` from a `(secret …)` call in a ci.fnl) trips the
-/// whole-value replacement. Fixing this — by either moving the
-/// rendering into the exception value, sending as an attachment, or
-/// configuring the Sentry project's scrubbing rules — is a prerequisite
-/// for this layer to be useful again.
+/// The rendering arrives in Sentry as `extra["diagnostic"]`. For most
+/// errors this is fine, but the miette `NarratableReportHandler` renders
+/// source snippets inline: if a CI pipeline error references a
+/// `(secret …)` call in `ci.fnl`, Sentry's data-scrubber replaces the
+/// entire value with `"[Filtered]"`. Errors without source spans (e.g.
+/// mirror push failures) are unaffected. A fuller fix would move the
+/// rendering into the exception value or send it as an attachment.
 ///
 /// # Layer ordering
 ///
diff --git a/quire-server/src/bin/quire/main.rs b/quire-server/src/bin/quire/main.rs
index cf9ed24..73e973d 100644
--- a/quire-server/src/bin/quire/main.rs
+++ b/quire-server/src/bin/quire/main.rs
@@ -114,7 +114,8 @@ async fn main() -> Result<()> {
     let miette_layer = MietteLayer::new()
         .with_type::<quire::Error>()
         .with_type::<quire::ci::Error>()
-        .with_type::<quire_core::fennel::FennelError>();
+        .with_type::<quire_core::fennel::FennelError>()
+        .with_type::<quire::mirror::MirrorErrors>();
     let _guard = telemetry::init_telemetry(
         miette_layer,
         FmtMode::AutoJson,
diff --git a/quire-server/src/bin/quire/server.rs b/quire-server/src/bin/quire/server.rs
index bb47f03..0329d3b 100644
--- a/quire-server/src/bin/quire/server.rs
+++ b/quire-server/src/bin/quire/server.rs
@@ -157,6 +157,6 @@ async fn handle_event_connection(mut stream: tokio::net::UnixStream, quire: Quir
 
     ci::trigger(&quire, &event);
     if let Err(e) = mirror::trigger(&quire, &event) {
-        tracing::error!(repo = %event.repo, error = %e, "mirror failed");
+        tracing::error!(repo = %event.repo, error = &e as &(dyn std::error::Error + 'static), "mirror failed");
     }
 }
diff --git a/quire-server/src/mirror.rs b/quire-server/src/mirror.rs
index 8ad1887..7acc021 100644
--- a/quire-server/src/mirror.rs
+++ b/quire-server/src/mirror.rs
@@ -8,13 +8,24 @@ use thiserror::Error;
 
 use crate::quire::Quire;
 
-#[derive(Debug, Error, Diagnostic)]
-#[error("mirror: {} ref(s) failed", errors.len())]
-struct MirrorErrors {
+#[derive(Debug, Diagnostic)]
+pub struct MirrorErrors {
     #[related]
     errors: Vec<MirrorError>,
 }
 
+impl std::fmt::Display for MirrorErrors {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(f, "mirror: {} ref(s) failed", self.errors.len())?;
+        for e in &self.errors {
+            write!(f, "; {e}")?;
+        }
+        Ok(())
+    }
+}
+
+impl std::error::Error for MirrorErrors {}
+
 #[derive(Debug, Error, Diagnostic)]
 enum MirrorError {
     #[error("repo not found on disk: {0}")]
@@ -36,10 +47,27 @@ enum MirrorError {
 /// ref, reads `.quire/config.fnl` at the new SHA to obtain the `github.mirror`
 /// URL. Skips repos with no mirror URL configured. If no token is configured,
 /// mirroring is skipped entirely.
-pub fn trigger(quire: &Quire, event: &PushEvent) -> miette::Result<()> {
-    let repo = quire.repo(&event.repo)?;
+pub fn trigger(quire: &Quire, event: &PushEvent) -> Result<(), MirrorErrors> {
+    let errors = collect_errors(quire, event)?;
+    if errors.is_empty() {
+        Ok(())
+    } else {
+        Err(MirrorErrors { errors })
+    }
+}
+
+fn collect_errors(
+    quire: &Quire,
+    event: &PushEvent,
+) -> Result<Vec<MirrorError>, MirrorErrors> {
+    let one = |e: MirrorError| MirrorErrors { errors: vec![e] };
+
+    let repo = quire
+        .repo(&event.repo)
+        .map_err(|e| one(MirrorError::App(e)))?;
+
     if !repo.exists() {
-        return Err(MirrorError::RepoNotFound(event.repo.clone()).into());
+        return Err(one(MirrorError::RepoNotFound(event.repo.clone())));
     }
 
     let config = &quire.config;
@@ -48,22 +76,17 @@ pub fn trigger(quire: &Quire, event: &PushEvent) -> miette::Result<()> {
         .mirror_token
         .as_ref()
         .map(|s| s.reveal().map(str::to_owned))
-        .transpose()?
+        .transpose()
+        .map_err(|e| one(MirrorError::App(crate::Error::from(e))))?
     else {
-        return Ok(());
+        return Ok(vec![]);
     };
 
-    let errors: Vec<MirrorError> = event
+    Ok(event
         .updated_refs()
         .into_iter()
         .filter_map(|push_ref| mirror_ref(&repo, push_ref, &mirror_token).err())
-        .collect();
-
-    if errors.is_empty() {
-        Ok(())
-    } else {
-        Err(MirrorErrors { errors }.into())
-    }
+        .collect())
 }
 
 fn mirror_ref(