diff --git a/hive-forge/src/client.rs b/hive-forge/src/client.rs index 9b91aae5..a33f87e1 100644 --- a/hive-forge/src/client.rs +++ b/hive-forge/src/client.rs @@ -483,22 +483,59 @@ fn disposition_filename(value: &str) -> Option { (!name.is_empty()).then(|| name.to_owned()) } -/// Surface non-2xx HTTP responses on the raw web routes as anyhow -/// errors with the response body included (matches `curl +/// A non-2xx HTTP response from one of the raw *web-router* calls +/// (`post_json_web` / `get_bytes_raw` / `get_api_json`), carrying the +/// status code and body as data rather than baking them into a +/// formatted message. Without this, a caller that needs to tell "gone" +/// (404, or a 500 wrapping "resource does not exist") apart from "not +/// permitted" (401/403) has no honest way to do it — the status is read +/// once by [`check_status`] and then discarded into prose. A caller +/// that needs the code back gets it with +/// `err.downcast_ref::()` on the `anyhow::Error` +/// `post_json_web` et al return; `None` means the failure wasn't a +/// non-2xx response at all (transport, JSON decode, ...), so still +/// fall through to a generic message rather than assuming success. +#[derive(Debug)] +pub struct WebStatusError { + pub op: String, + pub status: reqwest::StatusCode, + pub body: String, +} + +impl std::fmt::Display for WebStatusError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!( + f, + "hive-forge: {} failed ({}): {}", + self.op, self.status, self.body + ) + } +} + +impl std::error::Error for WebStatusError {} + +/// Surface non-2xx HTTP responses on the raw web routes as a +/// [`WebStatusError`] with the response body included (matches `curl /// --fail-with-body`) — turns silent failures into errors with a -/// clear message. +/// clear message, while keeping the status code available structurally +/// instead of only as formatted text. fn check_status(resp: Response, op: &str) -> Result { let status = resp.status(); if status.is_success() { return Ok(resp); } let body = resp.text().unwrap_or_default(); - bail!("hive-forge: {op} failed ({status}): {body}"); + Err(WebStatusError { + op: op.to_owned(), + status, + body, + } + .into()) } #[cfg(test)] mod tests { - use super::{disposition_filename, index, owner_repo_from_git_url, split_repo}; + use super::{WebStatusError, disposition_filename, index, owner_repo_from_git_url, split_repo}; #[test] fn disposition_filename_reads_forgejos_real_header() { @@ -528,6 +565,24 @@ mod tests { assert_eq!(disposition_filename("attachment; filename=\"\""), None); } + #[test] + fn web_status_error_downcasts_to_recover_the_status_code() { + let err: anyhow::Error = WebStatusError { + op: "GET https://example.invalid/x".to_owned(), + status: reqwest::StatusCode::NOT_FOUND, + body: "resource does not exist".to_owned(), + } + .into(); + let status = err.downcast_ref::().map(|e| e.status); + assert_eq!(status, Some(reqwest::StatusCode::NOT_FOUND)); + } + + #[test] + fn unrelated_errors_do_not_downcast_to_web_status_error() { + let err = anyhow::anyhow!("connection refused"); + assert!(err.downcast_ref::().is_none()); + } + #[test] fn owner_repo_from_git_url_strips_scheme_and_creds() { assert_eq!(