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.
This commit is contained in:
parent
04676bfb2d
commit
aaac754356
1 changed files with 70 additions and 6 deletions
|
|
@ -15,6 +15,10 @@ use axum::{
|
||||||
use super::{AppState, StatusUnavailable, error_problem};
|
use super::{AppState, StatusUnavailable, error_problem};
|
||||||
use crate::forge;
|
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
|
/// 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
|
/// 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
|
/// `forge::CONFIG_ORG` the way `main.rs`'s `get_config_prs` is: this
|
||||||
|
|
@ -26,7 +30,7 @@ use crate::forge;
|
||||||
path = "/api/repos",
|
path = "/api/repos",
|
||||||
responses(
|
responses(
|
||||||
(status = 200, description = "repos with at least one open issue, as owner/name", body = Vec<String>),
|
(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),
|
(status = 503, description = "no forge is configured on this host, or the forge did not answer within two minutes", body = String),
|
||||||
),
|
),
|
||||||
tag = "repos"
|
tag = "repos"
|
||||||
)]
|
)]
|
||||||
|
|
@ -38,7 +42,12 @@ pub async fn get_repos(
|
||||||
"no forge is configured on this host".to_owned(),
|
"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)),
|
Ok(repos) => Ok(Json(repos)),
|
||||||
Err(e) => {
|
Err(e) => {
|
||||||
let detail = format!("{e:#}");
|
let detail = format!("{e:#}");
|
||||||
|
|
@ -57,7 +66,7 @@ pub async fn get_repos(
|
||||||
path = "/api/issue-report",
|
path = "/api/issue-report",
|
||||||
responses(
|
responses(
|
||||||
(status = 200, description = "every open issue across every repo with one, blocked/dependent-count pre-resolved", body = Vec<forge::IssueReportRow>),
|
(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),
|
(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"
|
tag = "repos"
|
||||||
)]
|
)]
|
||||||
|
|
@ -69,7 +78,11 @@ pub async fn get_issue_report_all(
|
||||||
"no forge is configured on this host".to_owned(),
|
"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)),
|
Ok(rows) => Ok(Json(rows)),
|
||||||
Err(e) => {
|
Err(e) => {
|
||||||
let detail = format!("{e:#}");
|
let detail = format!("{e:#}");
|
||||||
|
|
@ -101,7 +114,7 @@ pub async fn get_issue_report_all(
|
||||||
responses(
|
responses(
|
||||||
(status = 200, description = "every open issue in this repo, blocked/dependent-count pre-resolved", body = Vec<forge::IssueReportRow>),
|
(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 = 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"
|
tag = "repos"
|
||||||
)]
|
)]
|
||||||
|
|
@ -115,7 +128,15 @@ pub async fn get_issue_report(
|
||||||
"no forge is configured on this host",
|
"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)),
|
Ok(rows) => Ok(Json(rows)),
|
||||||
Err(e) => {
|
Err(e) => {
|
||||||
let status = issue_report_error_status(&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)]
|
#[cfg(test)]
|
||||||
mod tests {
|
mod tests {
|
||||||
|
use crate::forge;
|
||||||
use forgejo_api::{ApiErrorKind, ForgejoError};
|
use forgejo_api::{ApiErrorKind, ForgejoError};
|
||||||
|
|
||||||
// ── a repo-not-found forge error maps to 404, everything else stays
|
// ── a repo-not-found forge error maps to 404, everything else stays
|
||||||
|
|
@ -217,4 +239,46 @@ mod tests {
|
||||||
axum::http::StatusCode::SERVICE_UNAVAILABLE
|
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}");
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue