ref:780d25cc78786c63418fdbd42138113e6f0f4417

fix(client): summarize HTML error bodies instead of dumping the page

When the server returned a non-2xx with an HTML body (e.g. a 406 from the CI job-logs endpoint), every error path in `Client` was pasting the entire HTML page into `ApiError::Api.message`, producing an unreadable wall of markup in the user's terminal. Centralize non-success handling into `api_error_from`, which inspects Content-Type and substitutes a short summary for `text/html` / `application/xhtml` bodies while leaving JSON/text bodies untouched. Adds wiremock-backed unit tests covering happy + unhappy paths for both JSON requests and SSE streams. Closes #7
SHA: 780d25cc78786c63418fdbd42138113e6f0f4417
Author: CI <ci@anvil.test>
Date: 2026-05-07 13:59
Parents: 1d3e677
3 files changed +392 -50
Type
Cargo.lock +139 −0
@@ -3,6 +3,15 @@
version = 4
[[package]]
name = "aho-corasick"
version = "1.1.4"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "ddd31a130427c27518df266943a5308ed92d4b226cc639f5a8f1002816174301"
dependencies = [
"memchr",
]
[[package]]
name = "android_system_properties"
version = "0.1.5"
source = "registry+https://github.com/rust-lang/crates.io-index"
@@ -83,6 +92,7 @@
"tokio",
"tokio-util",
"url",
"wiremock",
]
[[package]]
@@ -92,6 +102,16 @@
checksum = "7f202df86484c868dbad7eaa557ef785d5c66295e41b460ef922eca0723b842c"
[[package]]
name = "assert-json-diff"
version = "2.0.2"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "47e4f2b81832e72834d7518d8487a0396a28cc408186a2e8854c0f98011faf12"
dependencies = [
"serde",
"serde_json",
]
[[package]]
name = "atomic-waker"
version = "1.1.2"
source = "registry+https://github.com/rust-lang/crates.io-index"
@@ -272,6 +292,24 @@
]
[[package]]
name = "deadpool"
version = "0.12.3"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "0be2b1d1d6ec8d846f05e137292d0b89133caf95ef33695424c09568bdd39b1b"
dependencies = [
"deadpool-runtime",
"lazy_static",
"num_cpus",
"tokio",
]
[[package]]
name = "deadpool-runtime"
version = "0.1.4"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "092966b41edc516079bdf31ec78a2e0588d1d0c08f78b91d8307215928642b2b"
[[package]]
name = "dialoguer"
version = "0.11.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
@@ -526,6 +564,25 @@
checksum = "0cc23270f6e1808e30a928bdc84dea0b9b4136a8bc82338574f23baf47bbd280"
[[package]]
name = "h2"
version = "0.4.14"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "171fefbc92fe4a4de27e0698d6a5b392d6a0e333506bc49133760b3bcf948733"
dependencies = [
"atomic-waker",
"bytes",
"fnv",
"futures-core",
"futures-sink",
"http",
"indexmap",
"slab",
"tokio",
"tokio-util",
"tracing",
]
[[package]]
name = "hashbrown"
version = "0.15.5"
source = "registry+https://github.com/rust-lang/crates.io-index"
@@ -553,6 +610,12 @@
checksum = "2304e00983f87ffb38b55b444b5e3b60a884b5d30c0fca7d82fe33449bbe55ea"
[[package]]
name = "hermit-abi"
version = "0.5.2"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "fc0fef456e4baa96da950455cd02c081ca953b141298e41db3fc7e36b1da849c"
[[package]]
name = "hostname"
version = "0.4.2"
source = "registry+https://github.com/rust-lang/crates.io-index"
@@ -603,6 +666,12 @@
checksum = "6dbf3de79e51f3d586ab4cb9d5c3e2c14aa28ed23d180cf89b4df0454a69cc87"
[[package]]
name = "httpdate"
version = "1.0.3"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "df3b46402a9d5adb4c86a0cf463f42e19994e3ee891101b1841f30a545cb49a9"
[[package]]
name = "hyper"
version = "1.8.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
@@ -612,9 +681,11 @@
"bytes",
"futures-channel",
"futures-core",
"h2",
"http",
"http-body",
"httparse",
"httpdate",
"itoa",
"pin-project-lite",
"pin-utils",
@@ -846,6 +917,12 @@
]
[[package]]
name = "lazy_static"
version = "1.5.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "bbd2bcb4c963f2ddae06a2efc7e9f3591312473c50c6685e1f298068316e66fe"
[[package]]
name = "leb128fmt"
version = "0.1.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
@@ -942,6 +1019,16 @@
]
[[package]]
name = "num_cpus"
version = "1.17.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "91df4bbde75afed763b708b7eee1e8e7651e02d97f6d5dd763e89367e957b23b"
dependencies = [
"hermit-abi",
"libc",
]
[[package]]
name = "once_cell"
version = "1.21.3"
source = "registry+https://github.com/rust-lang/crates.io-index"
@@ -1196,6 +1283,35 @@
]
[[package]]
name = "regex"
version = "1.12.3"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "e10754a14b9137dd7b1e3e5b0493cc9171fdd105e0ab477f51b72e7f3ac0e276"
dependencies = [
"aho-corasick",
"memchr",
"regex-automata",
"regex-syntax",
]
[[package]]
name = "regex-automata"
version = "0.4.14"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "6e1dd4122fc1595e8162618945476892eefca7b88c52820e74af6262213cae8f"
dependencies = [
"aho-corasick",
"memchr",
"regex-syntax",
]
[[package]]
name = "regex-syntax"
version = "0.8.10"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "dc897dd8d9e8bd1ed8cdad82b5966c3e0ecae09fb1907d58efaa013543185d0a"
[[package]]
name = "reqwest"
version = "0.12.28"
source = "registry+https://github.com/rust-lang/crates.io-index"
@@ -2178,6 +2294,29 @@
version = "0.53.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "d6bbff5f0aada427a1e5a6da5f1f98158182f26556f345ac9e04d36d0ebed650"
[[package]]
name = "wiremock"
version = "0.6.5"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "08db1edfb05d9b3c1542e521aea074442088292f00b5f28e435c714a98f85031"
dependencies = [
"assert-json-diff",
"base64",
"deadpool",
"futures",
"http",
"http-body-util",
"hyper",
"hyper-util",
"log",
"once_cell",
"regex",
"serde",
"serde_json",
"tokio",
"url",
]
[[package]]
name = "wit-bindgen"
Cargo.toml +3 −0
@@ -29,3 +29,6 @@
glob = "0.3"
sha2 = "0.10"
[dev-dependencies]
wiremock = "0.6"
src/client.rs +250 −50
@@ -13,6 +13,28 @@
Api { status: u16, message: String },
}
/// Build an `ApiError::Api` from a non-success response, summarizing HTML
/// bodies so we don't paste an entire error page into the user's terminal.
async fn api_error_from(resp: reqwest::Response) -> ApiError {
let status = resp.status().as_u16();
let content_type = resp
.headers()
.get(reqwest::header::CONTENT_TYPE)
.and_then(|v| v.to_str().ok())
.unwrap_or("")
.to_lowercase();
let is_html =
content_type.starts_with("text/html") || content_type.starts_with("application/xhtml");
let body = resp.text().await.unwrap_or_default();
let message = if is_html {
"server returned an HTML error page (expected JSON)".to_string()
} else {
body
};
ApiError::Api { status, message }
}
#[allow(dead_code)]
pub struct Client {
http: reqwest::Client,
@@ -45,6 +67,29 @@
})
}
/// Construct a client for tests, pointing at an arbitrary base URL.
/// The base URL should be the bare server origin (e.g. `http://127.0.0.1:NNNN`);
/// the `/api/v1` prefix is added internally, mirroring `from_config`.
#[cfg(test)]
pub fn for_test(base_url: impl Into<String>, token: impl Into<String>) -> Self {
let token = token.into();
let mut headers = HeaderMap::new();
headers.insert(
AUTHORIZATION,
HeaderValue::from_str(&format!("Bearer {token}")).unwrap(),
);
headers.insert(CONTENT_TYPE, HeaderValue::from_static("application/json"));
let http = reqwest::Client::builder()
.default_headers(headers)
.build()
.expect("build test client");
Self {
http,
base_url: base_url.into(),
token,
}
}
fn url(&self, path: &str) -> String {
let base = self.base_url.trim_end_matches('/');
format!("{base}/api/v1{path}")
@@ -93,14 +138,9 @@
pub async fn delete_empty(&self, path: &str) -> Result<(), ApiError> {
let resp = self.http.delete(self.url(path)).send().await?;
if resp.status().is_success() {
let status = resp.status();
if status.is_success() {
Ok(())
} else {
let text = resp.text().await.unwrap_or_default();
Err(ApiError::Api {
status: status.as_u16(),
message: text,
Err(api_error_from(resp).await)
})
}
}
@@ -112,29 +152,19 @@
.json(&serde_json::json!({}))
.send()
.await?;
let status = resp.status();
if status.is_success() {
if resp.status().is_success() {
Ok(())
} else {
let text = resp.text().await.unwrap_or_default();
Err(ApiError::Api {
status: status.as_u16(),
message: text,
Err(api_error_from(resp).await)
})
}
}
/// Raw GET that returns the response body as a string (for non-JSON endpoints).
pub async fn get_raw(&self, path: &str) -> Result<String, ApiError> {
let resp = self.http.get(self.url(path)).send().await?;
let status = resp.status();
let text = resp.text().await.unwrap_or_default();
if status.is_success() {
Ok(text)
if resp.status().is_success() {
Ok(resp.text().await.unwrap_or_default())
} else {
Err(ApiError::Api {
Err(api_error_from(resp).await)
status: status.as_u16(),
message: text,
})
}
}
@@ -143,14 +173,9 @@
&self,
resp: reqwest::Response,
) -> Result<T, ApiError> {
let status = resp.status();
if status.is_success() {
if resp.status().is_success() {
Ok(resp.json().await?)
} else {
let text = resp.text().await.unwrap_or_default();
Err(ApiError::Api {
status: status.as_u16(),
message: text,
})
Err(api_error_from(resp).await)
}
}
@@ -163,14 +188,9 @@
.header("Accept", "text/event-stream")
.send()
.await?;
let status = resp.status();
if resp.status().is_success() {
if status.is_success() {
Ok(resp)
} else {
Err(api_error_from(resp).await)
let text = resp.text().await.unwrap_or_default();
Err(ApiError::Api {
status: status.as_u16(),
message: text,
})
}
}
@@ -209,14 +229,9 @@
let url = self.url(path);
let resp = self.http.post(&url).multipart(form).send().await?;
let status = resp.status();
if status.is_success() {
if resp.status().is_success() {
Ok(resp.json().await?)
} else {
let text = resp.text().await.unwrap_or_default();
Err(ApiError::Api {
status: status.as_u16(),
message: text,
})
Err(api_error_from(resp).await)
}
}
@@ -228,13 +243,8 @@
output: &std::path::Path,
) -> Result<(), ApiError> {
let resp = self.http.get(self.url(path)).send().await?;
if !resp.status().is_success() {
let status = resp.status();
return Err(api_error_from(resp).await);
if !status.is_success() {
let text = resp.text().await.unwrap_or_default();
return Err(ApiError::Api {
status: status.as_u16(),
message: text,
});
}
let bytes = resp.bytes().await?;
@@ -253,5 +263,195 @@
pub fn token(&self) -> &str {
&self.token
}
}
#[cfg(test)]
mod tests {
use super::*;
use serde::Deserialize;
use serde_json::json;
use wiremock::matchers::{method, path};
use wiremock::{Mock, MockServer, ResponseTemplate};
#[derive(Debug, Deserialize, PartialEq)]
struct Job {
id: String,
name: String,
}
fn html_error_page(status: u16) -> String {
format!(
"<!DOCTYPE html><html><head><title>{status} - Anvil</title></head>\
<body><h1>{status}</h1><p>Lorem ipsum dolor sit amet, way too long \
to paste into a terminal error message.</p></body></html>"
)
}
#[tokio::test]
async fn get_happy_path_parses_json() {
let server = MockServer::start().await;
Mock::given(method("GET"))
.and(path("/api/v1/foo/bar/ci/jobs/abc"))
.respond_with(
ResponseTemplate::new(200).set_body_json(json!({"id": "abc", "name": "clippy"})),
)
.mount(&server)
.await;
let client = Client::for_test(server.uri(), "tok");
let job: Job = client.get("/foo/bar/ci/jobs/abc").await.unwrap();
assert_eq!(
job,
Job {
id: "abc".into(),
name: "clippy".into()
}
);
}
#[tokio::test]
async fn get_unhappy_json_body_is_surfaced_verbatim() {
let server = MockServer::start().await;
let body = r#"{"error":"not_found","message":"job missing"}"#;
Mock::given(method("GET"))
.and(path("/api/v1/foo/bar/ci/jobs/missing"))
.respond_with(
ResponseTemplate::new(404)
.insert_header("content-type", "application/json")
.set_body_string(body),
)
.mount(&server)
.await;
let client = Client::for_test(server.uri(), "tok");
let err = client
.get::<serde_json::Value>("/foo/bar/ci/jobs/missing")
.await
.unwrap_err();
match err {
ApiError::Api { status, message } => {
assert_eq!(status, 404);
assert_eq!(message, body);
}
other => panic!("expected ApiError::Api, got {other:?}"),
}
}
#[tokio::test]
async fn get_unhappy_html_body_is_summarized() {
let server = MockServer::start().await;
let html = html_error_page(406);
Mock::given(method("GET"))
.and(path("/api/v1/foo/bar/ci/jobs/x/logs"))
.respond_with(
ResponseTemplate::new(406)
.set_body_raw(html.into_bytes(), "text/html; charset=utf-8"),
)
.mount(&server)
.await;
let client = Client::for_test(server.uri(), "tok");
let err = client
.get::<serde_json::Value>("/foo/bar/ci/jobs/x/logs")
.await
.unwrap_err();
match err {
ApiError::Api { status, message } => {
assert_eq!(status, 406);
assert!(
!message.contains("<!DOCTYPE"),
"HTML body leaked into error message: {message}"
);
assert!(
!message.contains("<html"),
"HTML body leaked into error message: {message}"
);
assert!(
message.to_lowercase().contains("html"),
"expected message to mention HTML; got: {message}"
);
}
other => panic!("expected ApiError::Api, got {other:?}"),
}
}
#[tokio::test]
async fn sse_stream_happy_path_returns_response() {
let server = MockServer::start().await;
Mock::given(method("GET"))
.and(path("/api/v1/foo/bar/ci/jobs/x/logs"))
.respond_with(
ResponseTemplate::new(200)
.insert_header("content-type", "text/event-stream")
.set_body_string("event: done\ndata: {}\n\n"),
)
.mount(&server)
.await;
let client = Client::for_test(server.uri(), "tok");
let resp = client
.get_sse_stream("/foo/bar/ci/jobs/x/logs")
.await
.unwrap();
assert_eq!(resp.status(), 200);
}
#[tokio::test]
async fn sse_stream_unhappy_html_body_is_summarized() {
let server = MockServer::start().await;
let html = html_error_page(406);
Mock::given(method("GET"))
.and(path("/api/v1/foo/bar/ci/jobs/x/logs"))
.respond_with(
ResponseTemplate::new(406)
.set_body_raw(html.into_bytes(), "text/html; charset=utf-8"),
)
.mount(&server)
.await;
let client = Client::for_test(server.uri(), "tok");
let err = client
.get_sse_stream("/foo/bar/ci/jobs/x/logs")
.await
.unwrap_err();
match err {
ApiError::Api { status, message } => {
assert_eq!(status, 406);
assert!(
!message.contains("<!DOCTYPE") && !message.contains("<html"),
"HTML body leaked into SSE error message: {message}"
);
}
other => panic!("expected ApiError::Api, got {other:?}"),
}
}
#[tokio::test]
async fn sse_stream_unhappy_json_body_is_surfaced_verbatim() {
let server = MockServer::start().await;
let body = r#"{"error":"not_found"}"#;
Mock::given(method("GET"))
.and(path("/api/v1/foo/bar/ci/jobs/x/logs"))
.respond_with(
ResponseTemplate::new(404)
.insert_header("content-type", "application/json")
.set_body_string(body),
)
.mount(&server)
.await;
let client = Client::for_test(server.uri(), "tok");
let err = client
.get_sse_stream("/foo/bar/ci/jobs/x/logs")
.await
.unwrap_err();
match err {
ApiError::Api { status, message } => {
assert_eq!(status, 404);
assert_eq!(message, body);
}
other => panic!("expected ApiError::Api, got {other:?}"),
}
}
}