diff --git a/crates/fbuild-daemon/src/handlers/websockets.rs b/crates/fbuild-daemon/src/handlers/websockets.rs index 13771b286..6d6a1a190 100644 --- a/crates/fbuild-daemon/src/handlers/websockets.rs +++ b/crates/fbuild-daemon/src/handlers/websockets.rs @@ -9,6 +9,7 @@ use axum::response::IntoResponse; use fbuild_core::channel as mpsc; use fbuild_serial::{SerialClientMessage, SerialServerMessage, SerialStreamEvent}; use futures::{SinkExt, StreamExt}; +use std::future::Future; use std::sync::Arc; use std::time::Duration; use tokio::sync::oneshot; @@ -114,6 +115,40 @@ async fn cleanup_ws_serial_session( /// for every dead connection. See FastLED/fbuild#808. const WS_ATTACH_HANDSHAKE_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10); +/// Cap on the WebSocket serial attach `open_port` await. HTTP monitor and +/// post-deploy monitor paths already bound this call at 30 s; the WebSocket +/// attach path needs the same ceiling so a wedged USB driver cannot leave a +/// pending serial attach counted forever. See FastLED/fbuild#977. +const WS_SERIAL_OPEN_PORT_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(30); + +fn format_timeout_for_error(timeout: Duration) -> String { + let millis = timeout.as_millis(); + if millis > 0 && millis < 1_000 { + format!("{millis}ms") + } else { + format!("{}s", timeout.as_secs()) + } +} + +async fn await_ws_serial_open_port( + port: &str, + open_future: F, + timeout: Duration, +) -> Result<(), String> +where + F: Future>, +{ + match tokio::time::timeout(timeout, open_future).await { + Ok(Ok(())) => Ok(()), + Ok(Err(e)) => Err(format!("failed to open port: {}", e)), + Err(_) => Err(format!( + "open_port({}) exceeded {}; serial driver may be wedged", + port, + format_timeout_for_error(timeout) + )), + } +} + async fn handle_serial_ws(mut socket: WebSocket, ctx: Arc) { // Mark this attach as pending so the self-eviction loop won't shut the // daemon down while we're waiting for `open_port` to finish (USB @@ -151,14 +186,20 @@ async fn handle_serial_ws(mut socket: WebSocket, ctx: Arc) { attach_guard.set_target(client_id.clone(), port.clone()); // Open port if needed if open_if_needed { - if let Err(e) = ctx - .serial_manager - .open_port(&port, baud_rate, &client_id, None, client_metadata.clone()) - .await - { - let err_msg = SerialServerMessage::Error { - message: format!("failed to open port: {}", e), - }; + let open_result = await_ws_serial_open_port( + &port, + ctx.serial_manager.open_port( + &port, + baud_rate, + &client_id, + None, + client_metadata.clone(), + ), + WS_SERIAL_OPEN_PORT_TIMEOUT, + ) + .await; + if let Err(message) = open_result { + let err_msg = SerialServerMessage::Error { message }; let _ = socket .send(Message::Text(serialize_or_fallback(&err_msg))) .await; diff --git a/crates/fbuild-daemon/src/handlers/websockets_tests.rs b/crates/fbuild-daemon/src/handlers/websockets_tests.rs index 20ae3b7d3..9ee6098ec 100644 --- a/crates/fbuild-daemon/src/handlers/websockets_tests.rs +++ b/crates/fbuild-daemon/src/handlers/websockets_tests.rs @@ -18,6 +18,78 @@ fn now_unix_returns_reasonable_value() { assert!(ts > 1_577_836_800.0); } +#[tokio::test] +async fn ws_open_port_timeout_returns_error_with_deadline() { + let result = tokio::time::timeout( + std::time::Duration::from_secs(1), + await_ws_serial_open_port( + "COM_HUNG", + std::future::pending::>(), + std::time::Duration::from_millis(10), + ), + ) + .await + .expect("test helper should return at the injected deadline"); + + let message = result.expect_err("hung open_port must be reported as an error"); + assert!( + message.contains("open_port(COM_HUNG) exceeded 10ms"), + "timeout error should name the port and deadline, got: {message}" + ); + assert!( + message.contains("serial driver may be wedged"), + "timeout error should explain likely serial-driver wedge, got: {message}" + ); +} + +async fn run_pending_attach_timeout_scope(ctx: std::sync::Arc) -> String { + let attach_guard = PendingAttachGuard::new(ctx.clone()); + attach_guard.set_target("client-hung".to_string(), "COM_HUNG".to_string()); + + assert_eq!( + ctx.pending_serial_attaches + .load(std::sync::atomic::Ordering::Relaxed), + 1 + ); + assert_eq!(ctx.pending_serial_attach_infos().len(), 1); + + let message = await_ws_serial_open_port( + "COM_HUNG", + std::future::pending::>(), + std::time::Duration::from_millis(10), + ) + .await + .expect_err("hung open_port must time out"); + + // `attach_guard` drops as this async function returns, matching the + // production handler's timeout/error return path. + message +} + +#[tokio::test] +async fn ws_open_port_timeout_drops_pending_attach_guard() { + let (tx, _rx) = tokio::sync::watch::channel(false); + let ctx = std::sync::Arc::new(DaemonContext::new(8765, tx, "test".to_string())); + + let message = run_pending_attach_timeout_scope(ctx.clone()).await; + assert!(message.contains("open_port(COM_HUNG) exceeded 10ms")); + + assert_eq!( + ctx.pending_serial_attaches + .load(std::sync::atomic::Ordering::Relaxed), + 0 + ); + assert!( + ctx.pending_serial_attach_infos().is_empty(), + "pending attach details should be removed after timeout" + ); + assert_eq!( + ctx.busy_reason(), + None, + "timed-out WebSocket attach must not keep the daemon busy" + ); +} + // --------------------------------------------------------------- // ReaderControl + writer-batching topology tests (#757). //