get_issue_report mapped every error from Client::issue_report to 503 (StatusUnavailable) unconditionally. issue_report calls issue_list_issues(org, repo, ...) first, and a Forgejo 404 there — a typo'd or deleted repo — was flattened into the same 503 a genuine forge outage produces, which tells a client to retry a request that will never succeed. Downcast the anyhow error back to forgejo_api::ForgejoError (same pattern main.rs's wanted_error_status uses for swarm_queue_client::Error) and check it structurally against the ApiErrorKind::NotFound / UnexpectedStatusCode(404) shapes forgejo's generated client produces for a 404, rather than string-matching the rendered message. Only that case answers 404, naming the org/repo; every other forge failure still answers 503. Updates the route's OpenAPI response list to document the 404. Closes #4701
220 lines
9.5 KiB
Rust
220 lines
9.5 KiB
Rust
//! The swarm-ui issue-report page's data source — repo listing plus the
|
|
//! two shapes of the report itself (all-repos default, single-repo
|
|
//! filtered). See [`crate::forge::Client::issue_report`] and
|
|
//! [`crate::forge::Client::issue_report_all`] for how a row's `blocked` /
|
|
//! `depended_on_by_count` fields are actually computed; this module is
|
|
//! just the HTTP surface over that logic, split out into its own file —
|
|
//! same shape [`crate::webhook`] already uses: business logic in
|
|
//! `crate::forge::Client`, the axum handlers next to their own routes.
|
|
|
|
use axum::{
|
|
Json,
|
|
extract::{Path, State},
|
|
};
|
|
|
|
use super::{AppState, StatusUnavailable, error_problem};
|
|
use crate::forge;
|
|
|
|
/// Repos with at least one open issue, as `owner/name` full names — the
|
|
/// data source for swarm-ui's repo-filter dropdown. Not scoped to
|
|
/// `forge::CONFIG_ORG` the way `main.rs`'s `get_config_prs` is: this
|
|
/// report deliberately covers the whole forge instance, not one org, since
|
|
/// the report itself is a general-purpose browsing tool rather than
|
|
/// something agent-config-specific.
|
|
#[utoipa::path(
|
|
get,
|
|
path = "/api/repos",
|
|
responses(
|
|
(status = 200, description = "repos with at least one open issue, as owner/name", body = Vec<String>),
|
|
(status = 503, description = "no forge is configured on this host", body = String),
|
|
),
|
|
tag = "repos"
|
|
)]
|
|
pub async fn get_repos(
|
|
State(state): State<AppState>,
|
|
) -> Result<Json<Vec<String>>, StatusUnavailable> {
|
|
let Some(client) = state.forge.as_ref() else {
|
|
return Err(StatusUnavailable(
|
|
"no forge is configured on this host".to_owned(),
|
|
));
|
|
};
|
|
match client.list_repos_with_open_issues().await {
|
|
Ok(repos) => Ok(Json(repos)),
|
|
Err(e) => {
|
|
let detail = format!("{e:#}");
|
|
tracing::warn!(error = %detail, "listing repos failed");
|
|
Err(StatusUnavailable(detail))
|
|
}
|
|
}
|
|
}
|
|
|
|
/// Every open issue across every repo with at least one — the default,
|
|
/// no-repo-filter view. See [`get_issue_report`] for the single-repo
|
|
/// counterpart (used once the dropdown's repo filter is set) and
|
|
/// [`forge::Client::issue_report_all`] for how the fan-out works.
|
|
#[utoipa::path(
|
|
get,
|
|
path = "/api/issue-report",
|
|
responses(
|
|
(status = 200, description = "every open issue across every repo with one, blocked/dependent-count pre-resolved", body = Vec<forge::IssueReportRow>),
|
|
(status = 503, description = "no forge is configured on this host, or the report could not be built", body = String),
|
|
),
|
|
tag = "repos"
|
|
)]
|
|
pub async fn get_issue_report_all(
|
|
State(state): State<AppState>,
|
|
) -> Result<Json<Vec<forge::IssueReportRow>>, StatusUnavailable> {
|
|
let Some(client) = state.forge.as_ref() else {
|
|
return Err(StatusUnavailable(
|
|
"no forge is configured on this host".to_owned(),
|
|
));
|
|
};
|
|
match client.issue_report_all().await {
|
|
Ok(rows) => Ok(Json(rows)),
|
|
Err(e) => {
|
|
let detail = format!("{e:#}");
|
|
tracing::warn!(error = %detail, "building the all-repos issue report failed");
|
|
Err(StatusUnavailable(detail))
|
|
}
|
|
}
|
|
}
|
|
|
|
/// Every open issue in `{org}/{repo}` specifically — the filtered view once
|
|
/// swarm-ui's repo dropdown (populated by [`get_repos`]) has a selection.
|
|
/// See [`forge::Client::issue_report`] for how `blocked` /
|
|
/// `depended_on_by_count` are computed and why this has to be a
|
|
/// server-side pass rather than something swarm-ui resolves itself per row.
|
|
///
|
|
/// Unlike [`get_repos`] / [`get_issue_report_all`], a failure here isn't
|
|
/// automatically 503: `issue_report` calls `issue_list_issues(org, repo,
|
|
/// ...)` first, and a Forgejo 404 there — a typo'd or deleted repo — means
|
|
/// "this will never work", not "try again later". See
|
|
/// [`issue_report_error_status`] for how that's told apart from a genuine
|
|
/// forge failure, which does stay 503.
|
|
#[utoipa::path(
|
|
get,
|
|
path = "/api/repos/{org}/{repo}/issue-report",
|
|
params(
|
|
("org" = String, Path, description = "repo owner (user or org)"),
|
|
("repo" = String, Path, description = "repo name"),
|
|
),
|
|
responses(
|
|
(status = 200, description = "every open issue in this repo, blocked/dependent-count pre-resolved", body = Vec<forge::IssueReportRow>),
|
|
(status = 404, description = "no such repo (problem+json)", body = String),
|
|
(status = 503, description = "no forge is configured on this host, or the report could not be built", body = String),
|
|
),
|
|
tag = "repos"
|
|
)]
|
|
pub async fn get_issue_report(
|
|
State(state): State<AppState>,
|
|
Path((org, repo)): Path<(String, String)>,
|
|
) -> Result<Json<Vec<forge::IssueReportRow>>, problem_details::ProblemDetails> {
|
|
let Some(client) = state.forge.as_ref() else {
|
|
return Err(error_problem(
|
|
axum::http::StatusCode::SERVICE_UNAVAILABLE,
|
|
"no forge is configured on this host",
|
|
));
|
|
};
|
|
match client.issue_report(&org, &repo).await {
|
|
Ok(rows) => Ok(Json(rows)),
|
|
Err(e) => {
|
|
let status = issue_report_error_status(&e);
|
|
if status == axum::http::StatusCode::NOT_FOUND {
|
|
tracing::info!(%org, %repo, "issue report requested for a repo that doesn't exist");
|
|
Err(error_problem(status, &format!("{org}/{repo} not found")))
|
|
} else {
|
|
let detail = format!("{e:#}");
|
|
tracing::warn!(error = %detail, %org, %repo, "building issue report failed");
|
|
Err(error_problem(status, &detail))
|
|
}
|
|
}
|
|
}
|
|
}
|
|
|
|
/// The status [`get_issue_report`]'s forge error should answer with.
|
|
///
|
|
/// `Client::issue_report` propagates through `anyhow`, so the concrete
|
|
/// [`forgejo_api::ForgejoError`] survives underneath but is not the type in
|
|
/// hand — downcasting back to it, same pattern `main.rs`'s
|
|
/// `wanted_error_status` uses for `swarm_queue_client::Error`. A 404 there
|
|
/// (`forge::is_repo_not_found`) means the repo itself doesn't exist;
|
|
/// anything else — including a 404 on some *other* call this function
|
|
/// doesn't cover, an outage, auth failure, etc. — stays 503, same as before
|
|
/// this existed.
|
|
fn issue_report_error_status(e: &anyhow::Error) -> axum::http::StatusCode {
|
|
match e.downcast_ref::<forgejo_api::ForgejoError>() {
|
|
Some(fe) if forge::is_repo_not_found(fe) => axum::http::StatusCode::NOT_FOUND,
|
|
_ => axum::http::StatusCode::SERVICE_UNAVAILABLE,
|
|
}
|
|
}
|
|
|
|
#[cfg(test)]
|
|
mod tests {
|
|
use forgejo_api::{ApiErrorKind, ForgejoError};
|
|
|
|
// ── a repo-not-found forge error maps to 404, everything else stays
|
|
// 503 ───────────────────────────────────────────────────────────
|
|
|
|
/// `issue_report_error_status` as a pure function first: build the
|
|
/// exact `ForgejoError` a 404 from `issue_list_issues` decodes to (see
|
|
/// `forge::is_repo_not_found`'s doc comment for the trace through
|
|
/// forgejo-api's generated client), wrap it the same way `?` inside
|
|
/// `Client::issue_report`'s `.with_context(...)` does, and confirm it
|
|
/// downcasts back to the status the route is meant to answer.
|
|
#[test]
|
|
fn repo_not_found_maps_to_404() {
|
|
let e: anyhow::Error =
|
|
ForgejoError::ApiError(ApiErrorKind::NotFound { errors: None }.into()).into();
|
|
let e = e.context("list open issues in some-org/some-repo");
|
|
assert_eq!(
|
|
super::issue_report_error_status(&e),
|
|
axum::http::StatusCode::NOT_FOUND
|
|
);
|
|
}
|
|
|
|
/// Belt-and-braces case: a 404 that arrives as `UnexpectedStatusCode`
|
|
/// rather than the parsed `ApiErrorKind::NotFound` shape (the body
|
|
/// didn't decode as forgejo's not-found JSON) still maps to 404 — see
|
|
/// `forge::is_repo_not_found`'s dual check.
|
|
#[test]
|
|
fn unparsed_404_still_maps_to_404() {
|
|
let e: anyhow::Error =
|
|
ForgejoError::UnexpectedStatusCode(axum::http::StatusCode::NOT_FOUND).into();
|
|
assert_eq!(
|
|
super::issue_report_error_status(&e),
|
|
axum::http::StatusCode::NOT_FOUND
|
|
);
|
|
}
|
|
|
|
/// A *different* `ForgejoError` variant — a real forge failure, not
|
|
/// "repo doesn't exist" — stays 503. Proves the match looks at which
|
|
/// kind of forge error this is, not just "was `ForgejoError` involved
|
|
/// at all".
|
|
#[test]
|
|
fn a_different_forge_error_stays_503() {
|
|
let e: anyhow::Error = ForgejoError::ApiError(ApiErrorKind::Forbidden.into()).into();
|
|
assert_eq!(
|
|
super::issue_report_error_status(&e),
|
|
axum::http::StatusCode::SERVICE_UNAVAILABLE
|
|
);
|
|
}
|
|
|
|
/// Invert-proof for the 404 cases above: an `anyhow::Error` that does
|
|
/// not wrap a `ForgejoError` at all (the shape a genuine non-forge
|
|
/// failure takes) must NOT downcast to it, and must stay 503. Without
|
|
/// this, an `issue_report_error_status` that answered 404
|
|
/// unconditionally would still pass the tests above. This is also the
|
|
/// status quo this change preserves — before this change, *every*
|
|
/// error on this route (including the 404 case above) answered 503
|
|
/// unconditionally; this test alone can't distinguish "old
|
|
/// unconditional 503" from "new, correctly-scoped 503", the 404 tests
|
|
/// above do that.
|
|
#[test]
|
|
fn a_non_forge_error_stays_503() {
|
|
let e = anyhow::anyhow!("list open issues in some-org/some-repo");
|
|
assert_eq!(
|
|
super::issue_report_error_status(&e),
|
|
axum::http::StatusCode::SERVICE_UNAVAILABLE
|
|
);
|
|
}
|
|
}
|