From d71222c206d597e96ab4c4b9b6737fbd16682352 Mon Sep 17 00:00:00 2001 From: atlas Date: Sat, 15 Aug 2026 23:04:36 +0200 Subject: [PATCH] refactor(swarm-queue-client): a library's errors are an enum, not anyhow MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Operator ruling: libs should not use anyhow. The queue connect was moved here verbatim from swarm-controller, which is a binary, so it arrived still wearing a binary's error handling — the move changed what the code is without changing how it reports. Callers get variants they can match on, split by what an operator does about them: a half-configured environment is a deployment bug, a refused token is an identity-provider config problem, an unreachable queue is a network one. The binaries that consume this keep anyhow and `?` converts, so nothing downstream is more verbose for it. Same split hive-claude uses. One thing anyhow was doing unpaid: the auth callback hands async-nats a plain string, and a Display that stops at the top message drops the cause — the half that says why the mint failed. `chain()` walks the source chain, which is what `{:#}` was doing before. --- Cargo.lock | 2 +- swarm-queue-client/Cargo.toml | 4 +- swarm-queue-client/src/lib.rs | 119 ++++++++++++++++++++++++++-------- 3 files changed, 96 insertions(+), 29 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 98d416df..58117b75 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4595,11 +4595,11 @@ dependencies = [ name = "swarm-queue-client" version = "0.1.0" dependencies = [ - "anyhow", "async-nats", "reqwest 0.13.1", "serde", "serde_json", + "thiserror 2.0.18", "tokio", "tracing", ] diff --git a/swarm-queue-client/Cargo.toml b/swarm-queue-client/Cargo.toml index 1cdfff73..32a2d0e5 100644 --- a/swarm-queue-client/Cargo.toml +++ b/swarm-queue-client/Cargo.toml @@ -5,7 +5,6 @@ readme = "README.md" edition.workspace = true [dependencies] -anyhow.workspace = true # No `kv`/`jetstream` feature here on purpose: this crate's job ends at a # connected client. What a consumer does with it - KV for the controller and # the hive, plain messaging for anything later - is the consumer's business, @@ -14,6 +13,9 @@ async-nats.workspace = true reqwest.workspace = true serde.workspace = true serde_json.workspace = true +# A library, so its errors are a matchable enum rather than an opaque +# `anyhow::Error`. The binaries that consume this keep anyhow; `?` converts. +thiserror.workspace = true tokio.workspace = true tracing.workspace = true diff --git a/swarm-queue-client/src/lib.rs b/swarm-queue-client/src/lib.rs index d1c8ad56..29def273 100644 --- a/swarm-queue-client/src/lib.rs +++ b/swarm-queue-client/src/lib.rs @@ -31,7 +31,78 @@ use std::path::PathBuf; -use anyhow::{Context, Result, bail}; +/// Everything that can go wrong reaching the swarm queue. +/// +/// `thiserror` and not `anyhow` because this is a library: a caller gets a +/// type it can match on, and the binaries that consume it keep using +/// `anyhow` — `?` converts for free, so nothing downstream is more verbose +/// for it. The same split `hive-claude` uses. +/// +/// The variants are the failures an operator acts on differently: a +/// half-configured environment is a deployment bug, a refused token is an +/// identity-provider config problem, an unreachable queue is a network +/// one. Collapsing them into one string would make that distinction a +/// matter of reading prose. +#[derive(Debug, thiserror::Error)] +pub enum Error { + #[error( + "swarm queue is half-configured: {prefix}_NATS_URL, \ + {prefix}_OIDC_TOKEN_ENDPOINT, {prefix}_OIDC_CLIENT_ID and \ + {prefix}_OIDC_CLIENT_SECRET_FILE must be set together or not at all" + )] + PartialConfig { prefix: String }, + + #[error("reading the queue client secret from {path}")] + ClientSecret { + path: String, + #[source] + source: std::io::Error, + }, + + #[error("building the token-endpoint HTTP client")] + HttpClient(#[source] reqwest::Error), + + #[error("requesting an access token from authelia")] + TokenRequest(#[source] reqwest::Error), + + /// The body carries authelia's own `error_description`, and it is far + /// more useful than the status alone: a wrong grant says + /// `unauthorized_client`, a wrong secret says `invalid_client`, and + /// those point at different config. + #[error("authelia refused the token request ({status}): {body}")] + TokenRefused { + status: reqwest::StatusCode, + body: String, + }, + + #[error("parsing authelia's token response")] + TokenResponse(#[source] serde_json::Error), + + #[error("connecting to the swarm queue at {url}")] + Connect { + url: String, + #[source] + source: async_nats::ConnectError, + }, +} + +/// Render an error and its source chain on one line. +/// +/// The auth callback below hands `async-nats` a *string*, so a `Display` +/// that stopped at the top message would drop the cause — which is the +/// half that says why the mint failed. `anyhow`'s `{:#}` did this for +/// free; a library owes its callers the same detail without owing them +/// anyhow. +fn chain(error: &dyn std::error::Error) -> String { + let mut rendered = error.to_string(); + let mut source = error.source(); + while let Some(cause) = source { + rendered.push_str(": "); + rendered.push_str(&cause.to_string()); + source = cause.source(); + } + rendered +} /// Only the one field this needs; authelia returns several. #[derive(serde::Deserialize)] @@ -76,7 +147,7 @@ impl QueueConfig { /// there would trade a missing feature for a dead process. What must NOT /// happen is a *half* configuration silently behaving like an absent one — /// hence the explicit partial check below. - pub fn from_env(prefix: &str) -> Result> { + pub fn from_env(prefix: &str) -> Result, Error> { let url = std::env::var(format!("{prefix}_NATS_URL")).ok(); let token_endpoint = std::env::var(format!("{prefix}_OIDC_TOKEN_ENDPOINT")).ok(); let client_id = std::env::var(format!("{prefix}_OIDC_CLIENT_ID")).ok(); @@ -94,12 +165,9 @@ impl QueueConfig { // it would otherwise produce is the expensive kind: the process // comes up "fine", never connects, and the data it was supposed to // move silently stops moving. Naming the variables costs one line. - _ => bail!( - "swarm queue is half-configured: {prefix}_NATS_URL, \ - {prefix}_OIDC_TOKEN_ENDPOINT, {prefix}_OIDC_CLIENT_ID and \ - {prefix}_OIDC_CLIENT_SECRET_FILE must be set together or not \ - at all" - ), + _ => Err(Error::PartialConfig { + prefix: prefix.to_owned(), + }), } } } @@ -110,16 +178,14 @@ impl QueueConfig { /// authenticates as itself. Authelia refuses the `openid` scope for this grant /// (a machine client receives an access token and never an id-token), so no /// scope is requested. -async fn mint_token(http: &reqwest::Client, cfg: &QueueConfig) -> Result { +async fn mint_token(http: &reqwest::Client, cfg: &QueueConfig) -> Result { // Read per call rather than caching: the file is small, and a cached // secret would survive a rotation that the operator believes took effect. let secret = tokio::fs::read_to_string(&cfg.client_secret_file) .await - .with_context(|| { - format!( - "reading the queue client secret from {}", - cfg.client_secret_file.display() - ) + .map_err(|source| Error::ClientSecret { + path: cfg.client_secret_file.display().to_string(), + source, })?; let response = http @@ -131,25 +197,21 @@ async fn mint_token(http: &reqwest::Client, cfg: &QueueConfig) -> Result ]) .send() .await - .context("requesting an access token from authelia")?; + .map_err(Error::TokenRequest)?; - // The body carries authelia's own error description, and it is far more - // useful than the status alone: a wrong grant says `unauthorized_client`, - // a wrong secret says `invalid_client`, and those point at different - // config. + // Why the body and not just the status: see `Error::TokenRefused`. let status = response.status(); let body = response.text().await.unwrap_or_default(); if !status.is_success() { - bail!("authelia refused the controller's token request ({status}): {body}"); + return Err(Error::TokenRefused { status, body }); } - let parsed: TokenResponse = - serde_json::from_str(&body).context("parsing authelia's token response")?; + let parsed: TokenResponse = serde_json::from_str(&body).map_err(Error::TokenResponse)?; Ok(parsed.access_token) } /// Connect to the swarm queue, minting a token for each connection attempt. -pub async fn connect(cfg: QueueConfig) -> Result { +pub async fn connect(cfg: QueueConfig) -> Result { // A timeout, because this client runs INSIDE the auth callback: a token // endpoint that accepts the connection and then never answers would hang // the callback, and with it the connection attempt that invoked it, with @@ -159,7 +221,7 @@ pub async fn connect(cfg: QueueConfig) -> Result { let http = reqwest::Client::builder() .timeout(std::time::Duration::from_secs(10)) .build() - .context("building the token-endpoint HTTP client")?; + .map_err(Error::HttpClient)?; let url = cfg.url.clone(); let client = async_nats::ConnectOptions::with_auth_callback(move |_nonce| { @@ -168,9 +230,9 @@ pub async fn connect(cfg: QueueConfig) -> Result { async move { let token = mint_token(&http, &cfg) .await - // The callback's error type carries a string, so the context + // The callback's error type carries a string, so the source // chain would be lost; flatten it rather than dropping it. - .map_err(|e| async_nats::AuthError::new(format!("{e:#}")))?; + .map_err(|e| async_nats::AuthError::new(chain(&e)))?; let mut auth = async_nats::Auth::new(); auth.token = Some(token); Ok(auth) @@ -188,7 +250,10 @@ pub async fn connect(cfg: QueueConfig) -> Result { .retry_on_initial_connect() .connect(&url) .await - .with_context(|| format!("connecting to the swarm queue at {url}"))?; + .map_err(|source| Error::Connect { + url: url.clone(), + source, + })?; Ok(client) }