diff --git a/Cargo.lock b/Cargo.lock index 041b08a9..edbb6a33 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4591,6 +4591,7 @@ dependencies = [ "hive-jobq", "hive-jobq-wire", "hive-types", + "problem_details", "reqwest 0.13.1", "serde", "serde_json", diff --git a/docs/conventions.md b/docs/conventions.md index bbc97f2f..c19fedfe 100644 --- a/docs/conventions.md +++ b/docs/conventions.md @@ -318,6 +318,28 @@ refactor's concern. The dashboard frontend parses via `util.js::epochSec` wherever it needs arithmetic and feeds the string straight to `new Date(s)` for display. +### HTTP error bodies + +Every HTTP API in this repo answers failures with **RFC 9457 +`application/problem+json`** (`{ type, title, status, detail }`), with the +human-readable cause in `detail`. An endpoint of ours returning a bare string +or a bespoke error shape is a **bug to file against the backend**, not +something for the caller to work around. + +Use the `problem_details` crate (`features = ["axum"]`), which the daemons +already depend on: type a handler `Result<_, ProblemDetails>` and hand +`ProblemDetails::from_status_code(...).with_detail(...)` to `Err`. + +The reason is the consumer, not tidiness. The UIs show errors through one +shared component with a copy button, so a caller has to know **which part of +the body is the message**. A bare string forces it to treat the whole payload +as prose, which is the difference between offering "copy the cause" and +dumping a response — and the cause is frequently the entire diagnosis (a +JetStream permission refusal, a TLS chain failure) rather than a summary. + +Not in scope: the `hivectl` host-admin and in-agent unix sockets. Those are a +JSON-line protocol with their own result types; RFC 9457 is an HTTP format. + ## Tool groups The MCP tool surface an agent receives is derived from a set of named diff --git a/swarm-controller/Cargo.toml b/swarm-controller/Cargo.toml index aad95ae4..6ac33c60 100644 --- a/swarm-controller/Cargo.toml +++ b/swarm-controller/Cargo.toml @@ -27,6 +27,10 @@ forgejo-api.workspace = true # raw bytes. base64.workspace = true futures-util.workspace = true +# RFC 9457 `application/problem+json` error bodies. Same version + `axum` +# feature as hive-c0re: the two daemons answer the same operator UIs, so a +# reader that handles one's failures has to handle the other's. +problem_details = { version = "0.9.0", features = ["axum"] } # The graph itself, held directly rather than behind a c0re-style wrapper # module — that layering (`hive-c0re::job_queue`) is partially legacy (predates # `hive-jobq`'s extraction into its own crate) and this daemon does not need it diff --git a/swarm-controller/src/main.rs b/swarm-controller/src/main.rs index 26aa08fd..f0a9841d 100644 --- a/swarm-controller/src/main.rs +++ b/swarm-controller/src/main.rs @@ -428,14 +428,29 @@ async fn get_links(State(state): State) -> Json> { /// body because a bare 503 on an operator-facing diagnostic is how a /// misconfiguration costs an afternoon; it is a queue/JetStream error /// string, and this surface is already behind the swarm's SSO. +/// +/// It travels in `detail` of an RFC 9457 `application/problem+json` body +/// rather than as a bare string, so the cause is an addressable field +/// instead of the whole payload — see `error_problem` below. struct StatusUnavailable(String); impl axum::response::IntoResponse for StatusUnavailable { fn into_response(self) -> axum::response::Response { - (axum::http::StatusCode::SERVICE_UNAVAILABLE, self.0).into_response() + error_problem(axum::http::StatusCode::SERVICE_UNAVAILABLE, &self.0).into_response() } } +/// Every error this daemon returns, in one shape. +/// +/// RFC 9457 `application/problem+json` is the hive-wide contract for HTTP +/// error bodies (`docs/conventions.md`), and the operator UIs read `detail` +/// for display. A bare string forces the reader to treat the entire body as +/// the message, which is the difference between a UI that can offer "copy the +/// cause" and one that can only dump a response. +fn error_problem(status: axum::http::StatusCode, detail: &str) -> problem_details::ProblemDetails { + problem_details::ProblemDetails::from_status_code(status).with_detail(detail) +} + /// What each hive last said about itself, read from the swarm queue at /// request time. /// @@ -522,17 +537,17 @@ struct CreateAgentResponse { request_body = CreateAgentRequest, responses( (status = 200, description = "job chain queued", body = CreateAgentResponse), - (status = 400, description = "`name` is not a valid identifier", body = String), - (status = 500, description = "the job chain could not be queued", body = String), + (status = 400, description = "`name` is not a valid identifier (problem+json)", body = String), + (status = 500, description = "the job chain could not be queued (problem+json)", body = String), ), tag = "agents" )] async fn create_agent( State(state): State, Json(req): Json, -) -> Result, (axum::http::StatusCode, String)> { +) -> Result, problem_details::ProblemDetails> { let agent = hive_types::Ident::parse(&req.name) - .map_err(|reason| (axum::http::StatusCode::BAD_REQUEST, reason.to_owned()))? + .map_err(|reason| error_problem(axum::http::StatusCode::BAD_REQUEST, reason))? .into_string(); let repo = agent.clone(); @@ -562,7 +577,12 @@ async fn create_agent( .after_ok(create_repo); vec![create_identity.guid()] }) - .map_err(|e| (axum::http::StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?; + .map_err(|e| { + error_problem( + axum::http::StatusCode::INTERNAL_SERVER_ERROR, + &e.to_string(), + ) + })?; let [id] = ids[..] else { unreachable!("exactly one handle was asked for"); }; @@ -770,11 +790,46 @@ async fn main() -> Result<()> { #[cfg(test)] mod tests { use super::{ - DEFAULT_SOCKET, HIVES_ENV, HiveEntry, LINKS_ENV, ServiceLink, SwarmNodeKind, WorkerDeps, - load_hives, load_links, run_swarm_node, + DEFAULT_SOCKET, HIVES_ENV, HiveEntry, LINKS_ENV, ServiceLink, StatusUnavailable, + SwarmNodeKind, WorkerDeps, load_hives, load_links, run_swarm_node, }; use std::path::Path; + /// The 503 this route returns is what the operator UIs' shared error + /// component renders, so assert the RENDERED response rather than the + /// `problem_details` crate: the contract a UI depends on is the content + /// type plus a `detail` it can address, and a handler that built the + /// value and returned it as a bare string would satisfy any test + /// written against the type alone. + #[tokio::test] + async fn status_unavailable_renders_problem_json_with_the_cause_in_detail() { + use axum::response::IntoResponse as _; + + let cause = "listing status bucket keys: timed out"; + let resp = StatusUnavailable(cause.to_owned()).into_response(); + + assert_eq!(resp.status(), axum::http::StatusCode::SERVICE_UNAVAILABLE); + let ct = resp + .headers() + .get(axum::http::header::CONTENT_TYPE) + .and_then(|v| v.to_str().ok()) + .unwrap_or_default() + .to_owned(); + assert!( + ct.starts_with("application/problem+json"), + "RFC 9457 media type, got {ct:?}" + ); + + let bytes = axum::body::to_bytes(resp.into_body(), 64 * 1024) + .await + .expect("body reads"); + let v: serde_json::Value = serde_json::from_slice(&bytes).expect("problem+json parses"); + assert_eq!(v["status"], 503); + // The cause is an addressable field, not the entire payload — that + // distinction is the point of the change, so it is what is asserted. + assert_eq!(v["detail"], cause); + } + /// Drives `SwarmNodeKind::CreateRepo` through the real /// `hive_jobq::scheduler::Scheduler` claim → run → complete path, /// rather than only through `create_agent`'s endpoint test (there