Make error Display self-contained; delete display_chain workaround
FennelError::Eval/TypeMismatch and a handful of RuntimeError/CiError variants buried the underlying cause in `#[source]` and left the top-level Display as just a filename or layer label — so plain `%err` (tracing/Sentry) saw nothing useful, hence the `display_chain` helper walking the source chain at every call site. Fold the cause into the message string at construction time and keep `#[source]` for structural access. `%err` now stands alone for tracing/Sentry; miette's `╰─▶` line still renders the chain in the terminal. Also pipe Fennel source through Pipeline so quire-ci can re-wrap runtime Lua errors via FennelError::from_lua, producing the same source-code-annotated diagnostic that compile errors get — and log via tracing::error! before returning so the sentry-tracing bridge captures job failures. https://claude.ai/code/session_016ieFpQg1FfFhs6meg1aRom
diff --git a/Cargo.lock b/Cargo.lock
index fdfac15..502d1c5 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -2138,6 +2138,7 @@ dependencies = [
"serde",
"serde_json",
"tempfile",
+ "thiserror",
"tokio",
"tracing",
"tracing-subscriber",
diff --git a/quire-ci/Cargo.toml b/quire-ci/Cargo.toml
index 530cd3c..a607d29 100644
--- a/quire-ci/Cargo.toml
+++ b/quire-ci/Cargo.toml
@@ -15,6 +15,7 @@ sentry-tracing = { workspace = true }
serde = { workspace = true }
serde_json = { workspace = true }
tempfile = { workspace = true }
+thiserror = { workspace = true }
tokio = { workspace = true, features = ["rt-multi-thread"] }
tracing = { workspace = true }
tracing-subscriber = { workspace = true }
diff --git a/quire-ci/src/main.rs b/quire-ci/src/main.rs
index bfb64eb..52b06fb 100644
--- a/quire-ci/src/main.rs
+++ b/quire-ci/src/main.rs
@@ -12,8 +12,24 @@ use quire_core::ci::event::{Event, EventKind, JobOutcome};
use quire_core::ci::pipeline::{self, Pipeline, RunFn};
use quire_core::ci::run::RunMeta;
use quire_core::ci::runtime::{Runtime, RuntimeError, RuntimeEvent, RuntimeHandle};
+use quire_core::fennel::FennelError;
use tracing_subscriber::{EnvFilter, fmt, layer::SubscriberExt, util::SubscriberInitExt};
+/// Errors from running a job's `run_fn`. Lua errors are re-wrapped
+/// via [`FennelError::from_lua`] so they carry the same source-code
+/// annotation that compile-time errors get — both miette (terminal)
+/// and `%err` (tracing/Sentry) see the full diagnostic.
+#[derive(Debug, thiserror::Error, miette::Diagnostic)]
+enum JobError {
+ #[error(transparent)]
+ #[diagnostic(transparent)]
+ Fennel(#[from] Box<FennelError>),
+
+ #[error(transparent)]
+ #[diagnostic(transparent)]
+ Runtime(#[from] RuntimeError),
+}
+
use crate::sink::{EventSink, JsonlSink, NullSink};
const VERSION: &str = env!("QUIRE_VERSION");
@@ -348,6 +364,12 @@ fn run_pipeline(
return Ok(());
}
+ // Keep the source around so a Lua failure inside a run-fn can be
+ // wrapped via `FennelError::from_lua` — same source-code-annotated
+ // diagnostic that compile-time errors get.
+ let source = pipeline.source().to_string();
+ let source_name = pipeline.source_name().to_string();
+
let sink: Rc<RefCell<Box<dyn EventSink>>> = Rc::new(RefCell::new(sink));
let runtime = Rc::new(Runtime::new(
@@ -393,7 +415,7 @@ fn run_pipeline(
let _runtime_guard =
RuntimeHandle::install(runtime.clone(), runtime.lua()).expect("install runtime on Lua VM");
- let mut failed_job: Option<(String, RuntimeError)> = None;
+ let mut failed_job: Option<(String, JobError)> = None;
for job_id in &job_ids {
*current_job.borrow_mut() = Some(job_id.clone());
@@ -413,12 +435,15 @@ fn run_pipeline(
.clone();
runtime.enter_job(job_id);
- let result: Result<(), RuntimeError> = match run_fn {
- RunFn::Lua(f) => f
- .call::<mlua::Value>(())
- .map(|_| ())
- .map_err(RuntimeError::from),
- RunFn::Rust(f) => f(&runtime),
+ let result: Result<(), JobError> = match run_fn {
+ RunFn::Lua(f) => f.call::<mlua::Value>(()).map(|_| ()).map_err(|lua_err| {
+ JobError::Fennel(Box::new(FennelError::from_lua(
+ &source,
+ &source_name,
+ lua_err,
+ )))
+ }),
+ RunFn::Rust(f) => f(&runtime).map_err(JobError::from),
};
runtime.leave_job();
@@ -445,7 +470,12 @@ fn run_pipeline(
}
}
- if let Some((_, err)) = failed_job {
+ 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, "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 140fb12..3651c0a 100644
--- a/quire-core/src/ci/pipeline.rs
+++ b/quire-core/src/ci/pipeline.rs
@@ -203,6 +203,16 @@ pub struct Pipeline {
fennel: Fennel,
/// Container image declared via `(ci.image "...")`, if any.
image: Option<String>,
+ /// The original Fennel source — kept so runtime Lua errors raised
+ /// during job execution can be re-wrapped via
+ /// [`FennelError::from_lua`] with the same source-code annotation
+ /// that compile-time errors get.
+ ///
+ /// [`FennelError::from_lua`]: crate::fennel::FennelError::from_lua
+ source: String,
+ /// The source's display name (typically the .fnl path or a
+ /// synthetic label like `HEAD:.quire/ci.fnl`).
+ source_name: String,
}
impl Pipeline {
@@ -229,6 +239,20 @@ impl Pipeline {
self.image.as_deref()
}
+ /// The original Fennel source. Held so the executor can attach
+ /// source context to runtime Lua errors via
+ /// [`FennelError::from_lua`].
+ ///
+ /// [`FennelError::from_lua`]: crate::fennel::FennelError::from_lua
+ pub fn source(&self) -> &str {
+ &self.source
+ }
+
+ /// The source's display name (path or synthetic label).
+ pub fn source_name(&self) -> &str {
+ &self.source_name
+ }
+
/// Return job IDs in topological order — dependencies before
/// dependents. The pipeline is already validated as acyclic, so
/// this never fails.
@@ -401,6 +425,8 @@ pub fn compile(source: &str, name: &str) -> CompileResult<Pipeline> {
node_index,
fennel,
image,
+ source: source.to_string(),
+ source_name: name.to_string(),
})
}
diff --git a/quire-core/src/ci/runtime.rs b/quire-core/src/ci/runtime.rs
index 3053338..4528ffc 100644
--- a/quire-core/src/ci/runtime.rs
+++ b/quire-core/src/ci/runtime.rs
@@ -29,7 +29,7 @@ pub enum RuntimeError {
#[error(transparent)]
Lua(Box<mlua::Error>),
- #[error("command spawn failed: {program} in {cwd}")]
+ #[error("command spawn failed: {program} in {}: {source}", cwd.display())]
CommandSpawnFailed {
program: String,
cwd: std::path::PathBuf,
@@ -40,7 +40,7 @@ pub enum RuntimeError {
#[error("git error: {0}")]
Git(String),
- #[error("failed to write CRI log at {path}")]
+ #[error("failed to write CRI log at {}: {source}", path.display())]
LogWriteFailed {
path: std::path::PathBuf,
#[source]
diff --git a/quire-core/src/fennel.rs b/quire-core/src/fennel.rs
index 4511675..2d37189 100644
--- a/quire-core/src/fennel.rs
+++ b/quire-core/src/fennel.rs
@@ -29,11 +29,11 @@ pub enum FennelError {
#[diagnostic(code(fennel::internal))]
Internal(#[from] mlua::Error),
- /// Fennel/Lua evaluation failed. `message` is just the source
- /// name so miette renders `× <name>`; the actual Lua error text
- /// is reachable via the `#[source]` chain. Plain `Display` will
- /// only show the name — walk the chain (e.g. via
- /// `display_chain`) to surface the underlying error.
+ /// Fennel/Lua evaluation failed. `message` is pre-formatted as
+ /// `"{name}: {lua_error}"` so plain `Display` carries the full
+ /// diagnostic (suitable for tracing/Sentry). The `#[source]`
+ /// chain is kept so structured tools (and miette's `╰─▶` line)
+ /// can still walk to the underlying `mlua::Error`.
#[error("{message}")]
#[diagnostic(code(fennel::eval))]
Eval {
@@ -47,8 +47,8 @@ pub enum FennelError {
},
/// Result couldn't be deserialized into the requested type.
- /// Same display caveat as `Eval`: `message` is the source name,
- /// the deser error is in the `#[source]` chain.
+ /// `message` is pre-formatted as `"{name}: {deser_error}"`; see
+ /// `Eval` for the rationale.
#[error("{message}")]
#[diagnostic(code(fennel::type_mismatch))]
TypeMismatch {
@@ -187,12 +187,13 @@ impl Fennel {
{
let result = self.eval_raw(source, name, |_| Ok(()))?;
- self.lua
- .from_value(result)
- .map_err(|e| FennelError::TypeMismatch {
- message: name.to_string(),
+ self.lua.from_value(result).map_err(|e| {
+ let message = format!("{name}: {e}");
+ FennelError::TypeMismatch {
+ message,
source: Box::new(e),
- })
+ }
+ })
}
/// Load and evaluate a Fennel file from disk, deserializing the result
@@ -209,12 +210,11 @@ impl Fennel {
impl FennelError {
/// Construct an `Eval` error from an mlua error, extracting line
/// information when available.
- pub(crate) fn from_lua(source: &str, name: &str, err: mlua::Error) -> Self {
- // Use only the filename/location as the message. The source chain
- // carries the full error details, so including them here would
- // duplicate the output in miette's × and ╰─▶ sections.
- let message = name.to_string();
-
+ /// Wrap an `mlua::Error` in an `Eval` variant with source-code
+ /// context. Used at compile time by the Fennel loader and at run
+ /// time by the executor so a failing run-fn surfaces with the
+ /// same inline source annotation as a compile error.
+ pub fn from_lua(source: &str, name: &str, err: mlua::Error) -> Self {
// Try to extract a line (and optional column) from the Lua
// error for a label. None when the error message doesn't carry
// a line — miette renders the source block without an inline
@@ -222,6 +222,12 @@ impl FennelError {
let label = extract_line_col(&err.to_string())
.and_then(|(line, col)| line_col_offset(source, line, col));
+ // Fold the source name + lua error into the top-level message
+ // so plain `Display` (tracing, Sentry, logs) carries the full
+ // diagnostic. `#[source]` retains the mlua error for the
+ // structural chain.
+ let message = format!("{name}: {err}");
+
FennelError::Eval {
message,
source_code: source.to_string(),
@@ -379,7 +385,14 @@ mod tests {
else {
panic!("expected Eval, got {err:?}");
};
- assert_eq!(message, "bad.fnl");
+ assert!(
+ message.starts_with("bad.fnl: "),
+ "message should fold name + lua error: {message}"
+ );
+ assert!(
+ message.len() > "bad.fnl: ".len(),
+ "message should include the lua error detail, got {message:?}"
+ );
assert_eq!(source_code, source);
assert!(
label.is_some(),
@@ -392,9 +405,16 @@ mod tests {
let f = fennel();
let result: Result<MirrorConfig, _> = f.load_string("{:mirror {:url 42}}", "types.fnl");
let err = result.unwrap_err();
+ let FennelError::TypeMismatch { message, .. } = &err else {
+ panic!("expected TypeMismatch, got {err:?}");
+ };
+ assert!(
+ message.starts_with("types.fnl: "),
+ "message should fold name + deser error: {message}"
+ );
assert!(
- matches!(&err, FennelError::TypeMismatch { message, .. } if message == "types.fnl"),
- "expected TypeMismatch, got {err:?}",
+ message.len() > "types.fnl: ".len(),
+ "message should include the deser error detail, got {message:?}"
);
}
diff --git a/quire-server/src/bin/quire/main.rs b/quire-server/src/bin/quire/main.rs
index 25e0c83..cc6d474 100644
--- a/quire-server/src/bin/quire/main.rs
+++ b/quire-server/src/bin/quire/main.rs
@@ -6,7 +6,6 @@ use clap_complete::Shell;
use miette::IntoDiagnostic;
use miette::Result;
use quire::Quire;
-use quire::display_chain;
use sentry::ClientInitGuard;
use std::io::IsTerminal;
use tracing_subscriber::EnvFilter;
@@ -114,7 +113,7 @@ fn init_sentry(quire: &Quire) -> Option<ClientInitGuard> {
let config = match quire.global_config() {
Ok(config) => config,
Err(e) => {
- tracing::warn!(error = %display_chain(&e), "failed to load global config, skipping Sentry init");
+ tracing::warn!(error = %e, "failed to load global config, skipping Sentry init");
return None;
}
};
@@ -123,7 +122,7 @@ fn init_sentry(quire: &Quire) -> Option<ClientInitGuard> {
let dsn = match sentry_config.dsn.reveal() {
Ok(dsn) => dsn,
Err(e) => {
- tracing::warn!(error = %display_chain(&e), "failed to resolve Sentry DSN, skipping Sentry init");
+ tracing::warn!(error = %e, "failed to resolve Sentry DSN, skipping Sentry init");
return None;
}
};
diff --git a/quire-server/src/bin/quire/server.rs b/quire-server/src/bin/quire/server.rs
index ba18bf9..c0b823d 100644
--- a/quire-server/src/bin/quire/server.rs
+++ b/quire-server/src/bin/quire/server.rs
@@ -6,7 +6,6 @@ use axum::routing::get;
use miette::{Context, IntoDiagnostic, Result};
use quire::Quire;
use quire::ci;
-use quire::display_chain;
use quire::event::PushEvent;
async fn health() -> &'static str {
@@ -79,7 +78,7 @@ async fn event_listener(listener: tokio::net::UnixListener, quire: Quire) {
tokio::spawn(handle_event_connection(stream, quire));
}
Err(e) => {
- tracing::error!(error = %display_chain(&e), "failed to accept event connection");
+ tracing::error!(error = %e, "failed to accept event connection");
}
}
}
@@ -96,7 +95,7 @@ async fn handle_event_connection(mut stream: tokio::net::UnixStream, quire: Quir
Ok(0) => return, // empty connection, ignore
Ok(_) => {}
Err(e) => {
- tracing::error!(error = %display_chain(&e), "failed to read event from socket");
+ tracing::error!(error = %e, "failed to read event from socket");
return;
}
}
@@ -104,7 +103,7 @@ async fn handle_event_connection(mut stream: tokio::net::UnixStream, quire: Quir
let event: PushEvent = match serde_json::from_str(&line) {
Ok(e) => e,
Err(e) => {
- tracing::error!(error = %display_chain(&e), "failed to parse push event");
+ tracing::error!(error = %e, "failed to parse push event");
return;
}
};
diff --git a/quire-server/src/ci/error.rs b/quire-server/src/ci/error.rs
index 338a589..ca3238c 100644
--- a/quire-server/src/ci/error.rs
+++ b/quire-server/src/ci/error.rs
@@ -11,7 +11,7 @@ use quire_core::secret;
/// Errors produced by CI operations.
#[derive(Debug, thiserror::Error, Diagnostic)]
pub enum Error {
- #[error("io error")]
+ #[error("io error: {0}")]
Io(#[from] std::io::Error),
#[error(transparent)]
@@ -28,7 +28,7 @@ pub enum Error {
#[error(transparent)]
Lua(Box<mlua::Error>),
- #[error("workspace materialization failed")]
+ #[error("workspace materialization failed: {source}")]
WorkspaceMaterializationFailed {
#[source]
source: std::io::Error,
@@ -49,7 +49,7 @@ pub enum Error {
#[error(transparent)]
Secret(#[from] secret::Error),
- #[error("command spawn failed: {program} in {cwd}")]
+ #[error("command spawn failed: {program} in {}: {source}", cwd.display())]
CommandSpawnFailed {
program: String,
cwd: std::path::PathBuf,
@@ -60,7 +60,7 @@ pub enum Error {
#[error("quire-ci exited with status {exit:?}")]
QuireCiExit { exit: Option<i32> },
- #[error("failed to parse quire-ci event stream at {path}")]
+ #[error("failed to parse quire-ci event stream at {}: {source}", path.display())]
EventStreamParse {
path: std::path::PathBuf,
#[source]
diff --git a/quire-server/src/ci/mod.rs b/quire-server/src/ci/mod.rs
index b083802..943e822 100644
--- a/quire-server/src/ci/mod.rs
+++ b/quire-server/src/ci/mod.rs
@@ -27,7 +27,6 @@ pub struct CommitRef {
use std::path::{Path, PathBuf};
-use crate::display_chain;
use crate::event::{PushEvent, PushRef};
use crate::quire::Repo;
@@ -123,7 +122,7 @@ pub fn trigger(quire: &crate::Quire, event: &PushEvent) {
let config = match quire.global_config() {
Ok(config) => config,
Err(e) => {
- tracing::error!(repo = %event.repo, error = %display_chain(&e), "failed to load global config");
+ tracing::error!(repo = %event.repo, error = %e, "failed to load global config");
return;
}
};
@@ -183,7 +182,7 @@ pub fn trigger(quire: &crate::Quire, event: &PushEvent) {
tracing::error!(
repo = %event.repo,
sha = %push_ref.new_sha, // cov-excl-line
- error = %display_chain(&e),
+ error = %e,
"CI trigger failed"
);
}
diff --git a/quire-server/src/error.rs b/quire-server/src/error.rs
index a581a9b..58f1798 100644
--- a/quire-server/src/error.rs
+++ b/quire-server/src/error.rs
@@ -35,39 +35,6 @@ pub enum Error {
pub type Result<T> = std::result::Result<T, Error>;
-/// Display wrapper that walks an error's `source()` chain.
-///
-/// `tracing`'s `%field` formatter only calls `Display` on the top
-/// error, which discards information for layered errors (e.g.,
-/// `FennelError::Eval` whose top message is just the filename, with
-/// the real diagnostic carried by its `source`). This wrapper joins
-/// each layer with `": "` so structured logs carry the whole chain.
-///
-/// Construct via [`display_chain`] and use as a tracing field:
-///
-/// ```ignore
-/// tracing::error!(error = %display_chain(&e), "operation failed");
-/// ```
-pub struct DisplayChain<'a>(&'a (dyn std::error::Error + 'static));
-
-impl std::fmt::Display for DisplayChain<'_> {
- fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
- write!(f, "{}", self.0)?;
- let mut cur = self.0.source();
- while let Some(err) = cur {
- write!(f, ": {err}")?;
- cur = err.source();
- }
- Ok(())
- }
-}
-
-/// Wrap an error reference for chained `Display` rendering. See
-/// [`DisplayChain`].
-pub fn display_chain<E: std::error::Error + 'static>(err: &E) -> DisplayChain<'_> {
- DisplayChain(err)
-}
-
impl From<FennelError> for Error {
fn from(err: FennelError) -> Self {
Error::Fennel(Box::new(err))
@@ -90,34 +57,28 @@ mod tests {
}
#[test]
- fn display_chain_walks_source_chain() {
- // FennelError::Eval has a top-level message of just the
- // filename and an mlua::Error in its source — the exact case
- // the helper is meant to fix.
+ fn fennel_eval_display_is_self_contained() {
+ // FennelError::Eval's Display should carry both the source
+ // name and the underlying lua error text — without needing to
+ // walk the `#[source]` chain — so plain `%err` is enough for
+ // tracing/Sentry. The chain is still preserved structurally.
let f = quire_core::fennel::Fennel::new().expect("Fennel::new");
let result: std::result::Result<i32, _> = f.load_string("(this is not valid", "bad.fnl");
let fennel_err = result.unwrap_err();
let plain = fennel_err.to_string();
- let chained = display_chain(&fennel_err).to_string();
-
assert!(
- chained.starts_with(&plain),
- "chained output should begin with the top message"
+ plain.starts_with("bad.fnl: "),
+ "Display should start with the source name: {plain:?}"
);
assert!(
- chained.len() > plain.len(),
- "chained output should add source info: top={plain:?} chained={chained:?}"
+ plain.len() > "bad.fnl: ".len(),
+ "Display should include the underlying error detail: {plain:?}"
);
- }
- #[test]
- fn display_chain_handles_no_source() {
- let err = Error::ConfigNotFound("missing.yml".to_string());
- assert_eq!(
- display_chain(&err).to_string(),
- "config not found: missing.yml"
- );
+ // The `#[source]` chain is still walkable.
+ let err: &dyn std::error::Error = &fennel_err;
+ assert!(err.source().is_some(), "source chain should be preserved");
}
#[test]
diff --git a/quire-server/src/lib.rs b/quire-server/src/lib.rs
index 1b72414..2f268e2 100644
--- a/quire-server/src/lib.rs
+++ b/quire-server/src/lib.rs
@@ -6,5 +6,4 @@ pub mod quire;
pub use error::Error;
pub use error::Result;
-pub use error::display_chain;
pub use quire::Quire;
diff --git a/quire-server/src/quire/web/handlers.rs b/quire-server/src/quire/web/handlers.rs
index 2fca595..3ac318f 100644
--- a/quire-server/src/quire/web/handlers.rs
+++ b/quire-server/src/quire/web/handlers.rs
@@ -8,7 +8,6 @@ use axum::response::{Html, IntoResponse, Redirect, Response};
use super::db;
use super::templates::*;
use crate::Quire;
-use crate::error::display_chain;
pub async fn repo_redirect(
State(quire): State<Quire>,
@@ -93,12 +92,12 @@ pub async fn run_list(State(quire): State<Quire>, AxumPath(repo): AxumPath<Strin
let runs = match tokio::task::spawn_blocking(move || db::load_runs(&q, &repo_name)).await {
Ok(Ok(r)) => r,
Ok(Err(e)) => {
- tracing::error!(repo = %repo, error = %display_chain(&e), "failed to load runs");
+ tracing::error!(repo = %repo, error = %e, "failed to load runs");
return render_error(
repo_display,
StatusCode::INTERNAL_SERVER_ERROR,
"Failed to load runs",
- display_chain(&e).to_string(),
+ e.to_string(),
);
}
Err(_) => {
@@ -150,12 +149,12 @@ pub async fn run_detail(
Ok(Ok(d)) => d,
Ok(Err(ref e)) if is_no_rows(e) => return StatusCode::NOT_FOUND.into_response(),
Ok(Err(e)) => {
- tracing::error!(repo = %repo, run_id = %run_id, error = %display_chain(&e), "failed to load run detail");
+ tracing::error!(repo = %repo, run_id = %run_id, error = %e, "failed to load run detail");
return render_error(
repo_display,
StatusCode::INTERNAL_SERVER_ERROR,
"Failed to load run",
- display_chain(&e).to_string(),
+ e.to_string(),
);
}
Err(_) => {