Stop sending user-level CI errors to Sentry
Compile failures, job failures, and quire-ci exiting non-zero are
user-pipeline issues — the user wrote a bad ci.fnl or their job code
crashed. Those surface in the run UI (state, CRI log) and don't need
to alert ops. Sentry should only see operational errors: db, git,
worker spawn, materialize, sentry init, etc.

  - pipeline::compile no longer emits tracing::error on failure; it
    just returns the typed error. The doc comment now states the
    user/ops boundary so callers don't accidentally re-add reporting.

  - quire-ci's "job run-fn failed" log drops from tracing::error to
    tracing::warn so it stays in the run's CRI log (stderr) but doesn't
    trip sentry-tracing's ERROR → Event mapping. miette still prints
    the full diagnostic on the propagated Err.

  - quire-ci's MietteLayer no longer registers user-level error types
    (JobError, pipeline::CompileError, FennelError) — they have no
    tracing::error site to render for anymore.

  - quire-server's trigger() pattern-matches on error::Error::QuireCiExit
    and downgrades that one variant to tracing::info. Real ops failures
    (CommandSpawnFailed, Io, Git, Sql, InvalidTransition, …) stay at
    tracing::error.

Drop the per-diagnostic tracing-capture unit tests added in the
previous commit; they verified a behavior we're no longer doing.

Assisted-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
change orszpuklsxynppzvvpzmmsxzylrmuotz
commit 0bbf0a42659e9e2fd1a33ccd1977f7560950a0ca
author Alpha Chen <alpha@kejadlen.dev>
date
parent tltrurkx
diff --git a/quire-ci/src/main.rs b/quire-ci/src/main.rs
index 69c1897..0eb33fe 100644
--- a/quire-ci/src/main.rs
+++ b/quire-ci/src/main.rs
@@ -211,10 +211,12 @@ fn main() -> miette::Result<()> {
             // Drop order: `_sentry` flushes first (still inside the
             // runtime), then `_enter`, then `rt`.
             let _sentry = init_sentry(sentry_handoff.as_ref(), &meta);
-            let miette_layer = MietteLayer::new()
-                .with_type::<JobError>()
-                .with_type::<pipeline::CompileError>()
-                .with_type::<quire_core::fennel::FennelError>();
+            // No type registrations: quire-ci's user-level errors
+            // (CompileError, JobError, FennelError) are no longer logged
+            // at tracing::error, so the miette renderer would never fire
+            // for them. The layer stays installed in case future ops
+            // errors want to register types.
+            let miette_layer = MietteLayer::new();
             telemetry::init_tracing(miette_layer, FmtMode::Plain)?;
 
             run_pipeline(cli.workspace, sink, log_dir, git_dir, meta, secrets)
@@ -456,11 +458,12 @@ fn run_pipeline(
     }
 
     if let Some((job_id, err)) = failed_job {
-        // Log before returning so the sentry-tracing layer captures
-        // the failure (terminal output is handled by miette via the
-        // returned `Err`). `%err` carries the full diagnostic now
-        // that error Display impls are self-contained.
-        tracing::error!(job = %job_id, error = &err as &(dyn std::error::Error + 'static), "job run-fn failed");
+        // Log at warn so it appears in stderr (and the run's CRI log
+        // viewed in the UI) without tripping sentry-tracing's ERROR →
+        // Event mapping. Job failures are user-pipeline issues, not
+        // ops, and miette prints the full diagnostic to stderr on the
+        // returned Err anyway.
+        tracing::warn!(job = %job_id, error = &err as &(dyn std::error::Error + 'static), "job run-fn failed");
         return Err(err.into());
     }
 
diff --git a/quire-core/src/ci/pipeline.rs b/quire-core/src/ci/pipeline.rs
index 889772b..e4b1167 100644
--- a/quire-core/src/ci/pipeline.rs
+++ b/quire-core/src/ci/pipeline.rs
@@ -390,43 +390,11 @@ pub type CompileResult<T> = std::result::Result<T, CompileError>;
 /// phase are wrapped in a [`PipelineError`] for miette to render with
 /// inline labels.
 ///
-/// Failures are emitted via `tracing::error!` so any caller running
-/// under a tracing subscriber (and a configured Sentry layer) gets the
-/// diagnostic reported. A [`PipelineError`] holds a vector of inner
-/// diagnostics — those are emitted one event each so the exception
-/// value in Sentry carries the actual `Display` message of each
-/// definition/structure error, not just the outer "ci.fnl has errors"
-/// summary.
+/// Compile failures are user-pipeline problems, not operational
+/// errors — callers should surface them in the run UI rather than
+/// emitting `tracing::error!` events. This function intentionally does
+/// not log; doing so would route every malformed `ci.fnl` to Sentry.
 pub fn compile(source: &str, name: &str) -> CompileResult<Pipeline> {
-    let result = compile_inner(source, name);
-    if let Err(e) = &result {
-        report_compile_error(name, e);
-    }
-    result
-}
-
-fn report_compile_error(name: &str, err: &CompileError) {
-    match err {
-        CompileError::Pipeline(pe) => {
-            for diag in &pe.diagnostics {
-                tracing::error!(
-                    ci_fnl = %name,
-                    error = diag as &(dyn std::error::Error + 'static),
-                    "ci.fnl diagnostic",
-                );
-            }
-        }
-        CompileError::Fennel(_) => {
-            tracing::error!(
-                ci_fnl = %name,
-                error = err as &(dyn std::error::Error + 'static),
-                "ci.fnl compilation failed",
-            );
-        }
-    }
-}
-
-fn compile_inner(source: &str, name: &str) -> CompileResult<Pipeline> {
     let fennel = Fennel::new()?;
     let Registrations { jobs, image } = registration::register(&fennel, source, name)?;
 
@@ -927,108 +895,4 @@ mod tests {
             "expected pipeline error: {msg}"
         );
     }
-
-    /// Captures the `Display` text of each `error` field recorded on a
-    /// tracing event. Mirrors what sentry-tracing's `record_error` does
-    /// to build the exception value, so the test asserts on the same
-    /// thing Sentry would surface as the event title.
-    mod tracing_capture {
-        use std::sync::{Arc, Mutex};
-        use tracing_subscriber::layer::SubscriberExt;
-
-        pub struct ErrorCapture {
-            messages: Arc<Mutex<Vec<String>>>,
-        }
-
-        struct CaptureVisitor<'a>(&'a Arc<Mutex<Vec<String>>>);
-
-        impl tracing::field::Visit for CaptureVisitor<'_> {
-            fn record_error(
-                &mut self,
-                field: &tracing::field::Field,
-                value: &(dyn std::error::Error + 'static),
-            ) {
-                if field.name() == "error" {
-                    self.0.lock().unwrap().push(format!("{value}"));
-                }
-            }
-
-            fn record_debug(&mut self, _: &tracing::field::Field, _: &dyn std::fmt::Debug) {}
-        }
-
-        impl<S: tracing::Subscriber> tracing_subscriber::Layer<S> for ErrorCapture {
-            fn on_event(
-                &self,
-                event: &tracing::Event<'_>,
-                _ctx: tracing_subscriber::layer::Context<'_, S>,
-            ) {
-                if *event.metadata().level() != tracing::Level::ERROR {
-                    return;
-                }
-                let mut visitor = CaptureVisitor(&self.messages);
-                event.record(&mut visitor);
-            }
-        }
-
-        /// Run `f` under a subscriber that records the `Display` of every
-        /// `error` field on an ERROR-level event, and return the captured
-        /// strings.
-        pub fn capture(f: impl FnOnce()) -> Vec<String> {
-            let messages = Arc::new(Mutex::new(Vec::new()));
-            let layer = ErrorCapture {
-                messages: messages.clone(),
-            };
-            let subscriber = tracing_subscriber::registry().with(layer);
-            tracing::subscriber::with_default(subscriber, f);
-            let out = messages.lock().unwrap().clone();
-            out
-        }
-    }
-
-    #[test]
-    fn compile_emits_one_tracing_error_per_inner_diagnostic() {
-        let source = r#"(local ci (require :quire.ci))
-(ci.image "alpine")
-(ci.image "ubuntu")
-(ci.job :a [] (fn [] nil))"#;
-
-        let errors = tracing_capture::capture(|| {
-            let _ = compile(source, "ci.fnl");
-        });
-
-        // Two definition errors expected: duplicate image, empty inputs.
-        assert_eq!(
-            errors.len(),
-            2,
-            "one event per inner diagnostic, got: {errors:?}"
-        );
-        assert!(
-            errors
-                .iter()
-                .any(|m| m.contains("image declared more than once")),
-            "expected duplicate-image diagnostic in: {errors:?}"
-        );
-        assert!(
-            errors.iter().any(|m| m.contains("empty inputs")),
-            "expected empty-inputs diagnostic in: {errors:?}"
-        );
-        // Crucially, the bare outer "ci.fnl has errors" message must NOT be
-        // what reaches Sentry — that's the bug this fixes.
-        assert!(
-            !errors.iter().any(|m| m == "ci.fnl has errors"),
-            "outer wrapper message should not be emitted as an error event: {errors:?}"
-        );
-    }
-
-    #[test]
-    fn compile_emits_one_tracing_error_for_fennel_failure() {
-        let errors = tracing_capture::capture(|| {
-            let _ = compile("{:bad {:}", "ci.fnl");
-        });
-        assert_eq!(
-            errors.len(),
-            1,
-            "fennel parse failure should produce a single event, got: {errors:?}"
-        );
-    }
 }
diff --git a/quire-server/src/ci/mod.rs b/quire-server/src/ci/mod.rs
index e91fa00..1c8fbe6 100644
--- a/quire-server/src/ci/mod.rs
+++ b/quire-server/src/ci/mod.rs
@@ -171,12 +171,27 @@ pub fn trigger(quire: &crate::Quire, event: &PushEvent) {
                     config.executor,
                     sentry_handoff.as_ref(),
                 ) {
-                    tracing::error!(
-                        repo = %event.repo,
-                        sha = %push_ref.new_sha, // cov-excl-line
-                        error = &e as &(dyn std::error::Error + 'static),
-                        "CI trigger failed"
-                    );
+                    // QuireCiExit means quire-ci itself ran and reported a
+                    // user-pipeline failure (bad ci.fnl, failing job). That
+                    // shows up in the UI via the run state and CRI log —
+                    // log it at info so it doesn't trigger a Sentry event.
+                    // Everything else (db, git, spawn, materialize) is
+                    // operational and stays at error.
+                    if matches!(e, error::Error::QuireCiExit { .. }) {
+                        tracing::info!(
+                            repo = %event.repo,
+                            sha = %push_ref.new_sha, // cov-excl-line
+                            error = %e,
+                            "ci run finished with non-zero exit",
+                        );
+                    } else {
+                        tracing::error!(
+                            repo = %event.repo,
+                            sha = %push_ref.new_sha, // cov-excl-line
+                            error = &e as &(dyn std::error::Error + 'static),
+                            "CI trigger failed",
+                        );
+                    }
                 }
             },
         );