diff --git a/CHANGELOG.md b/CHANGELOG.md index 33ec120e3..f36937ac2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -100,6 +100,12 @@ The newest come first, and a press on When turns the order round. A search box a Started by buttons narrow the events already loaded, and the count says how many of them match. A row about a Bot (a message routed to it, say) now names it under Bot. +### The desktop setup page does not freeze when it loads during an install + +If the setup page loaded again while the desktop app downloaded local software, the window showed a +spinning cursor until the download finished. Its checks for a running OpenBot and for the last +failure now wait in the background, so the window keeps answering. + ### Each Bot on a shared computer has its own files A computer that serves many Bots kept one folder of files for all of them, so a note one Bot saved diff --git a/desktop/src-tauri/src/main.rs b/desktop/src-tauri/src/main.rs index dff5e434b..bcafefee8 100644 --- a/desktop/src-tauri/src/main.rs +++ b/desktop/src-tauri/src/main.rs @@ -2770,9 +2770,17 @@ where server_capabilities_answer(port) && owns_server(root, port).unwrap_or(false) } +/// Off the event-loop thread: installation holds startup for the whole download, and waiting for it +/// there froze the window until the download finished. #[tauri::command] -fn already_running(app: tauri::AppHandle, root: String) -> bool { - let root = stack::root_from(&root); +async fn already_running(app: tauri::AppHandle, root: String) -> bool { + tauri::async_runtime::spawn_blocking(move || already_running_now(&app, &root)) + .await + .unwrap_or(false) +} + +fn already_running_now(app: &tauri::AppHandle, root: &str) -> bool { + let root = stack::root_from(root); let shell = app.state::(); let _startup = shell.startup.lock().unwrap(); !recovery_required_or_pending_quit_notice(&shell, &root) @@ -2914,9 +2922,23 @@ fn recorder_host_environment() -> recorder_host::HostEnvironment { /// What stopped the stack, if anything did, and forget it once it has been read. /// /// Cleared on reading so a failure from an hour ago does not greet somebody who has since fixed it. +/// Off the event-loop thread for the same reason as `already_running`. #[tauri::command] -fn last_failure( +async fn last_failure( app: tauri::AppHandle, +) -> Option { + tauri::async_runtime::spawn_blocking(move || last_failure_now(&app)) + .await + .unwrap_or_else(|error| { + Some(Problem::with( + "OpenBot could not read what stopped it.", + error.to_string(), + )) + }) +} + +fn last_failure_now( + app: &tauri::AppHandle, ) -> Option { let shell = app.state::(); if let Some(problem) = shell.last_failure.lock().unwrap().take() { @@ -4950,7 +4972,7 @@ mod tests { "http://tauri.localhost/recovery" ); assert_eq!( - last_failure(app.handle().clone()).unwrap().connection, + last_failure_now(app.handle()).unwrap().connection, Some(openbot_desktop_lib::problem::Connection::Model) ); assert!(publish_connection_failure( @@ -4961,7 +4983,7 @@ mod tests { ) .unwrap()); assert!( - last_failure(app.handle().clone()).is_none(), + last_failure_now(app.handle()).is_none(), "one refresh per run; a second signal cannot replace the pending screen" ); } @@ -9772,15 +9794,15 @@ fn main() { mark_recovery_required(&shell, &f.owned, 0); } *shell.last_failure.lock().unwrap() = Some(Problem::plain("worker exhausted")); - assert!(last_failure(app.handle().clone()).is_some()); - assert!(last_failure(app.handle().clone()).is_none()); + assert!(last_failure_now(app.handle()).is_some()); + assert!(last_failure_now(app.handle()).is_none()); assert!(recovery_required(&shell, &f.owned)); write_quit_cleanup_notice( &f.owned, &["[exit] cleanup failed: Compose down failed: synthetic".into()], ) .unwrap(); - let notice = last_failure(app.handle().clone()).expect("persisted Quit notice"); + let notice = last_failure_now(app.handle()).expect("persisted Quit notice"); assert_eq!(notice.said, "OpenBot had trouble shutting down last time."); assert!(notice .detail @@ -9792,7 +9814,7 @@ fn main() { recovery_required(&shell, &f.owned), "consuming the persisted notice cannot make survivors adoptable" ); - assert!(last_failure(app.handle().clone()).is_none()); + assert!(last_failure_now(app.handle()).is_none()); let window = app.get_webview_window("main").unwrap(); let problem = tauri::test::get_ipc_response(&window, tauri::webview::InvokeRequest { cmd: "start_stack".into(), callback: tauri::ipc::CallbackFn(0), error: tauri::ipc::CallbackFn(1), @@ -9856,7 +9878,7 @@ fn main() { quit_cleanup_notice_path(&f.owned).exists(), "restore must not consume the persisted notice before React asks for it" ); - let notice = last_failure(app.handle().clone()).expect("persisted Quit notice"); + let notice = last_failure_now(app.handle()).expect("persisted Quit notice"); assert_eq!(notice.said, "OpenBot had trouble shutting down last time."); assert!(notice .detail @@ -9887,7 +9909,7 @@ fn main() { publish_quit_notice_failure(app.handle().clone(), "notice path is unavailable".into()); assert!(recovery_required(&shell, &f.owned)); - let failure = last_failure(app.handle().clone()).expect("volatile sink failure"); + let failure = last_failure_now(app.handle()).expect("volatile sink failure"); assert_eq!(failure.said, "OpenBot could not record a shutdown problem."); assert!(failure .detail @@ -10816,7 +10838,11 @@ fn main() { *shell.setup_url.lock().unwrap() = Some(setup.into()); let app = tauri::test::mock_builder() .manage(shell) - .invoke_handler(tauri::generate_handler![start_stack]) + .invoke_handler(tauri::generate_handler![ + start_stack, + already_running, + last_failure + ]) .build(tauri::test::mock_context(tauri::test::noop_assets())) .unwrap(); let window = tauri::WebviewWindowBuilder::new(&app, "main", Default::default()) @@ -11065,10 +11091,10 @@ fn main() { } let app = f.app(&f.owned, "tauri://localhost/"); assert_eq!(ports_for_shell(app.handle()).unwrap(), f.ports); - assert!(already_running( + assert!(tauri::async_runtime::block_on(already_running( app.handle().clone(), f.owned.to_string_lossy().into_owned() - )); + ))); tauri::async_runtime::block_on(show_openbot(app.handle().clone())).unwrap(); assert_eq!( app.get_webview_window("main") @@ -11079,13 +11105,87 @@ fn main() { Some(f.ports.app) ); let other = f.app(&f.selected, "tauri://localhost/"); - assert!(!already_running( + assert!(!tauri::async_runtime::block_on(already_running( other.handle().clone(), f.selected.to_string_lossy().into_owned() - )); + ))); assert!(tauri::async_runtime::block_on(show_openbot(other.handle().clone())).is_err()); } + #[test] + fn setup_page_probes_do_not_wait_on_the_event_loop_for_startup_held_by_installation() { + let f = RestoreFixture::new(); + openbot_env::write( + &f.owned.join(".env"), + &f.ports.settings(), + &std::collections::BTreeMap::new(), + ) + .unwrap(); + let app = f.app(&f.owned, "tauri://localhost/"); + let window = app.get_webview_window("main").unwrap(); + let shell = app.state::(); + let held = shell.startup.lock().unwrap(); + let (answered, answers) = std::sync::mpsc::channel(); + let (dispatched, dispatches) = std::sync::mpsc::channel(); + let webview = window.as_ref().clone(); + let root = f.owned.clone(); + // `on_message` is where the window's invoke lands. A synchronous command runs inside it, + // which is the event-loop thread in the real app. + std::thread::spawn(move || { + for (command, body) in [ + ("already_running", serde_json::json!({ "root": root })), + ("last_failure", serde_json::json!({})), + ] { + let answered = answered.clone(); + webview.clone().on_message( + tauri::webview::InvokeRequest { + cmd: command.into(), + callback: tauri::ipc::CallbackFn(0), + error: tauri::ipc::CallbackFn(1), + url: if cfg!(any(windows, target_os = "android")) { + "http://tauri.localhost" + } else { + "tauri://localhost" + } + .parse() + .unwrap(), + body: tauri::ipc::InvokeBody::Json(body), + headers: Default::default(), + invoke_key: tauri::test::INVOKE_KEY.into(), + }, + Box::new(move |_, _, response, _, _| { + let _ = answered.send((command, response)); + }), + ); + } + let _ = dispatched.send(()); + }); + let returned = dispatches.recv_timeout(std::time::Duration::from_secs(5)); + let early = answers.try_recv(); + drop(held); + returned.expect("the window's invoke waited on the startup lock installation was holding"); + assert!(early.is_err(), "a probe answered before startup was free"); + let mut answered: Vec<_> = (0..2) + .map(|_| { + let (command, response) = answers + .recv_timeout(std::time::Duration::from_secs(5)) + .unwrap(); + let tauri::ipc::InvokeResponse::Ok(body) = response else { + panic!("{command} failed"); + }; + (command, body.deserialize::().unwrap()) + }) + .collect(); + answered.sort_by_key(|(command, _)| *command); + assert_eq!( + answered, + [ + ("already_running", serde_json::json!(true)), + ("last_failure", serde_json::Value::Null), + ] + ); + } + #[test] fn restore_window_refuses_answering_other_deployment_and_shows_recorded_setup() { let f = RestoreFixture::new(); diff --git a/desktop/src-tauri/src/stop_ipc_tests.rs b/desktop/src-tauri/src/stop_ipc_tests.rs index aa5cd15d4..e66a74764 100644 --- a/desktop/src-tauri/src/stop_ipc_tests.rs +++ b/desktop/src-tauri/src/stop_ipc_tests.rs @@ -312,7 +312,7 @@ mod stop_ipc { let shell = fixture.app.state::(); wait_until(|| fixture.window.url().unwrap().as_str() == setup); assert_compose_down_ran_under(&fixture.root.join("commands.log"), &fixture.root); - let problem = last_failure(fixture.app.handle().clone()) + let problem = last_failure_now(fixture.app.handle()) .expect("menu Stop failure should be retained for setup"); assert_eq!( problem.said, @@ -326,7 +326,7 @@ mod stop_ipc { && detail.contains("synthetic Compose refusal")), "{problem:?}" ); - assert!(last_failure(fixture.app.handle().clone()).is_none()); + assert!(last_failure_now(fixture.app.handle()).is_none()); assert!( recovery_required(&shell, &fixture.root), "reading the retained notice must not clear recovery-required state" @@ -363,7 +363,7 @@ mod stop_ipc { wait_until(|| fixture.window.url().unwrap().as_str() == setup); assert_compose_down_ran_under(&fixture.root.join("commands.log"), &fixture.root); let shell = fixture.app.state::(); - assert!(last_failure(fixture.app.handle().clone()).is_none()); + assert!(last_failure_now(fixture.app.handle()).is_none()); assert!(!recovery_required(&shell, &fixture.root)); let stopped = configured(); assert!(stopped.launch.is_some(), "Stop retains the installed setup");