hive-forge: carry the HTTP status structurally instead of flattening it to a string
This commit is contained in:
parent
3b5bdcf262
commit
9065898e08
1 changed files with 60 additions and 5 deletions
|
|
@ -483,22 +483,59 @@ fn disposition_filename(value: &str) -> Option<String> {
|
||||||
(!name.is_empty()).then(|| name.to_owned())
|
(!name.is_empty()).then(|| name.to_owned())
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Surface non-2xx HTTP responses on the raw web routes as anyhow
|
/// A non-2xx HTTP response from one of the raw *web-router* calls
|
||||||
/// errors with the response body included (matches `curl
|
/// (`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::<WebStatusError>()` 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
|
/// --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<Response> {
|
fn check_status(resp: Response, op: &str) -> Result<Response> {
|
||||||
let status = resp.status();
|
let status = resp.status();
|
||||||
if status.is_success() {
|
if status.is_success() {
|
||||||
return Ok(resp);
|
return Ok(resp);
|
||||||
}
|
}
|
||||||
let body = resp.text().unwrap_or_default();
|
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)]
|
#[cfg(test)]
|
||||||
mod tests {
|
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]
|
#[test]
|
||||||
fn disposition_filename_reads_forgejos_real_header() {
|
fn disposition_filename_reads_forgejos_real_header() {
|
||||||
|
|
@ -528,6 +565,24 @@ mod tests {
|
||||||
assert_eq!(disposition_filename("attachment; filename=\"\""), None);
|
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::<WebStatusError>().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::<WebStatusError>().is_none());
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn owner_repo_from_git_url_strips_scheme_and_creds() {
|
fn owner_repo_from_git_url_strips_scheme_and_creds() {
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue