From aaac75435640bff06b19780d79ff59a9443f2f4d Mon Sep 17 00:00:00 2001 From: atlas Date: Thu, 15 Oct 2026 12:56:15 +0200 Subject: [PATCH] swarm-controller: bound the issue-report forge calls with the forge deadline The three issue-report handlers awaited the forge bare, so a forge that accepts a connection and never answers hung the request. Wrap each in within_forge_deadline, as pulls.rs does, and answer 503 on overrun. --- swarm-controller/src/issue_report.rs | 76 +++++++++++++++++++++++++--- 1 file changed, 70 insertions(+), 6 deletions(-) diff --git a/swarm-controller/src/issue_report.rs b/swarm-controller/src/issue_report.rs index 4b391640..da25f509 100644 --- a/swarm-controller/src/issue_report.rs +++ b/swarm-controller/src/issue_report.rs @@ -15,6 +15,10 @@ use axum::{ use super::{AppState, StatusUnavailable, error_problem}; use crate::forge; +/// The 503 detail when the forge did not answer within +/// [`forge::FORGE_DEADLINE`]. +const DEADLINE_DETAIL: &str = "the forge did not answer within the deadline"; + /// 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 @@ -26,7 +30,7 @@ use crate::forge; path = "/api/repos", responses( (status = 200, description = "repos with at least one open issue, as owner/name", body = Vec), - (status = 503, description = "no forge is configured on this host", body = String), + (status = 503, description = "no forge is configured on this host, or the forge did not answer within two minutes", body = String), ), tag = "repos" )] @@ -38,7 +42,12 @@ pub async fn get_repos( "no forge is configured on this host".to_owned(), )); }; - match client.list_repos_with_open_issues().await { + let Some(listed) = + forge::within_forge_deadline("issue repos", client.list_repos_with_open_issues()).await + else { + return Err(StatusUnavailable(DEADLINE_DETAIL.to_owned())); + }; + match listed { Ok(repos) => Ok(Json(repos)), Err(e) => { let detail = format!("{e:#}"); @@ -57,7 +66,7 @@ pub async fn get_repos( path = "/api/issue-report", responses( (status = 200, description = "every open issue across every repo with one, blocked/dependent-count pre-resolved", body = Vec), - (status = 503, description = "no forge is configured on this host, or the report could not be built", body = String), + (status = 503, description = "no forge is configured on this host, the report could not be built, or the forge did not answer within two minutes", body = String), ), tag = "repos" )] @@ -69,7 +78,11 @@ pub async fn get_issue_report_all( "no forge is configured on this host".to_owned(), )); }; - match client.issue_report_all().await { + let Some(built) = forge::within_forge_deadline("issue report", client.issue_report_all()).await + else { + return Err(StatusUnavailable(DEADLINE_DETAIL.to_owned())); + }; + match built { Ok(rows) => Ok(Json(rows)), Err(e) => { let detail = format!("{e:#}"); @@ -101,7 +114,7 @@ pub async fn get_issue_report_all( responses( (status = 200, description = "every open issue in this repo, blocked/dependent-count pre-resolved", body = Vec), (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), + (status = 503, description = "no forge is configured on this host, the report could not be built, or the forge did not answer within two minutes", body = String), ), tag = "repos" )] @@ -115,7 +128,15 @@ pub async fn get_issue_report( "no forge is configured on this host", )); }; - match client.issue_report(&org, &repo).await { + let Some(built) = + forge::within_forge_deadline("repo issue report", client.issue_report(&org, &repo)).await + else { + return Err(error_problem( + axum::http::StatusCode::SERVICE_UNAVAILABLE, + DEADLINE_DETAIL, + )); + }; + match built { Ok(rows) => Ok(Json(rows)), Err(e) => { let status = issue_report_error_status(&e); @@ -150,6 +171,7 @@ pub(crate) fn issue_report_error_status(e: &anyhow::Error) -> axum::http::Status #[cfg(test)] mod tests { + use crate::forge; use forgejo_api::{ApiErrorKind, ForgejoError}; // ── a repo-not-found forge error maps to 404, everything else stays @@ -217,4 +239,46 @@ mod tests { axum::http::StatusCode::SERVICE_UNAVAILABLE ); } + + /// Each route against a forge that accepts the connection and never + /// answers: 503 at the deadline. The paused clock jumps to the deadline + /// once nothing else can run, so this takes no wall time. + #[tokio::test(start_paused = true)] + async fn every_route_is_503_when_the_forge_never_answers() { + use axum::body::Body; + use axum::http::{Request, StatusCode}; + use tower::ServiceExt as _; + + // Bound but never accepted: the handshake completes, no response comes. + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let url = url::Url::parse(&format!("http://{}/", listener.local_addr().unwrap())).unwrap(); + for uri in [ + "/api/repos", + "/api/issue-report", + "/api/repos/o/r/issue-report", + ] { + let (state, _) = crate::tests::state_with_roster(); + assert!( + state + .forge + .set(std::sync::Arc::new(forge::Client::for_test_forge( + url.clone() + ))) + .is_ok() + ); + let start = tokio::time::Instant::now(); + let req = Request::get(uri).body(Body::empty()).expect("a request"); + let res = crate::build_app(state) + .oneshot(req) + .await + .expect("infallible"); + assert_eq!(res.status(), StatusCode::SERVICE_UNAVAILABLE, "{uri}"); + let bytes = axum::body::to_bytes(res.into_body(), usize::MAX) + .await + .unwrap(); + let body = String::from_utf8_lossy(&bytes); + assert!(body.contains(super::DEADLINE_DETAIL), "{uri}: {body}"); + assert_eq!(start.elapsed(), forge::FORGE_DEADLINE, "{uri}"); + } + } }