web ui: bulk mark-done shares one deadline and takes at most 200 ids

hive-agent/src/web_ui/actions.rs

@@ -9,7 +9,7 @@ use axum::{
 use serde::{Deserialize, Serialize};
 use std::future::Future;
 
-use super::{AppState, error_response};
+use super::{AppState, SOCKET_FETCH_TIMEOUT, error_response};
 
 pub(super) async fn post_cancel_turn(State(state): State<AppState>) -> Response {
     super::cancel_turn(&state.bus, &state.interrupted, "operator: /cancel");
@@ -52,6 +52,9 @@ pub(super) async fn post_new_session(State(state): State<AppState>) -> Response
     (axum::http::StatusCode::OK, "ok").into_response()
 }
 
+/// The most ids one mark-done request may carry.
+const MAX_MARK_DONE_IDS: usize = 200;
+
 #[derive(Deserialize)]
 pub(super) struct MarkTodosDoneForm {
     /// Comma-separated todo ids. Same "one field, JS joins the checked
@@ -71,7 +74,10 @@ pub(super) struct MarkTodosDoneForm {
 /// single-id path); a request with no ids or where every id fails to parse
 /// is rejected as a client error rather than silently acking nothing. An id
 /// whose store call errors or gets no answer fails the request with a 500
-/// naming how many failed.
+/// naming how many failed. More than [`MAX_MARK_DONE_IDS`] ids is a client
+/// error, and the calls share one [`SOCKET_FETCH_TIMEOUT`]: ids the store has
+/// not answered by then count as unanswered, so a stalled store gets its 500
+/// after about 3 seconds like the other handlers.
 pub(super) async fn post_mark_todos_done(Form(form): Form<MarkTodosDoneForm>) -> Response {
     let ids: Vec<i64> = form
         .ids
@@ -81,7 +87,13 @@ pub(super) async fn post_mark_todos_done(Form(form): Form<MarkTodosDoneForm>) ->
     if ids.is_empty() {
         return error_response(StatusCode::BAD_REQUEST, "mark-done: no todo ids selected");
     }
-    mark_ids_done(ids, |id| async move {
+    if ids.len() > MAX_MARK_DONE_IDS {
+        return error_response(
+            StatusCode::BAD_REQUEST,
+            &format!("mark-done: at most {MAX_MARK_DONE_IDS} todo ids per request"),
+        );
+    }
+    mark_ids_done(ids, SOCKET_FETCH_TIMEOUT, |id| async move {
         crate::todo_server::dial(&hive_agent_sock::Request::MarkTodoDone { id }).await
     })
     .await
@@ -90,14 +102,20 @@ pub(super) async fn post_mark_todos_done(Form(form): Form<MarkTodosDoneForm>) ->
 /// The handler body behind `dial`, which answers one id's `MarkTodoDone`
 /// (`None` when the store doesn't answer). Tests inject it so they never
 /// reach the live in-agent socket.
-async fn mark_ids_done<F, Fut>(ids: Vec<i64>, dial: F) -> Response
+async fn mark_ids_done<F, Fut>(ids: Vec<i64>, within: std::time::Duration, dial: F) -> Response
 where
     F: Fn(i64) -> Fut,
     Fut: Future<Output = Option<hive_agent_sock::Response>>,
 {
+    let deadline = tokio::time::Instant::now() + within;
     let mut answers = Vec::with_capacity(ids.len());
     for id in ids {
-        answers.push(dial(id).await);
+        // Past the deadline every remaining id is an unanswered one.
+        answers.push(
+            tokio::time::timeout_at(deadline, dial(id))
+                .await
+                .unwrap_or(None),
+        );
     }
     match settle_mark_done(answers) {
         Ok(acked) => axum::Json(MarkTodosDoneBody { acked }).into_response(),
@@ -156,7 +174,10 @@ mod tests {
 
     #[tokio::test]
     async fn mark_done_handler_answers_500_with_the_message_when_the_store_does_not_answer() {
-        let response = mark_ids_done(vec![1, 2], |_| async { None }).await;
+        let response = mark_ids_done(vec![1, 2], std::time::Duration::from_secs(3), |_| async {
+            None
+        })
+        .await;
         assert_eq!(
             response.status(),
             axum::http::StatusCode::INTERNAL_SERVER_ERROR
@@ -168,7 +189,7 @@ mod tests {
 
     #[tokio::test]
     async fn mark_done_handler_answers_500_with_the_store_error() {
-        let response = mark_ids_done(vec![7], |_| async {
+        let response = mark_ids_done(vec![7], std::time::Duration::from_secs(3), |_| async {
             Some(hive_agent_sock::Response::Err {
                 message: "database is locked".to_owned(),
             })
@@ -183,7 +204,7 @@ mod tests {
 
     #[tokio::test]
     async fn mark_done_handler_answers_200_with_the_acked_total() {
-        let response = mark_ids_done(vec![1, 2], |_| async {
+        let response = mark_ids_done(vec![1, 2], std::time::Duration::from_secs(3), |_| async {
             Some(hive_agent_sock::Response::Acked { count: 1 })
         })
         .await;
@@ -191,6 +212,38 @@ mod tests {
         assert_eq!(body_text(response).await, r#"{"acked":2}"#);
     }
 
+    #[tokio::test(start_paused = true)]
+    async fn a_stalled_store_gets_one_deadline_for_all_ids_not_one_per_id() {
+        let started = tokio::time::Instant::now();
+        let response = mark_ids_done(
+            (1..=50).collect(),
+            std::time::Duration::from_secs(3),
+            |_| async {
+                tokio::time::sleep(std::time::Duration::from_hours(1)).await;
+                None
+            },
+        )
+        .await;
+        assert_eq!(
+            response.status(),
+            axum::http::StatusCode::INTERNAL_SERVER_ERROR
+        );
+        assert!(started.elapsed() < std::time::Duration::from_secs(4));
+        assert!(body_text(response).await.contains("50 of 50 failed"));
+    }
+
+    #[tokio::test]
+    async fn more_ids_than_the_cap_are_a_client_error() {
+        let ids = (0..=super::MAX_MARK_DONE_IDS)
+            .map(|id| id.to_string())
+            .collect::<Vec<_>>()
+            .join(",");
+        let response =
+            super::post_mark_todos_done(axum::Form(super::MarkTodosDoneForm { ids })).await;
+        assert_eq!(response.status(), axum::http::StatusCode::BAD_REQUEST);
+        assert!(body_text(response).await.contains("at most 200"));
+    }
+
     #[test]
     fn mark_done_with_no_answer_is_an_error() {
         let err = settle_mark_done(vec![None]).unwrap_err();