diff --git a/src/core/mod.rs b/src/core/mod.rs index 2fde72d..1f650df 100644 --- a/src/core/mod.rs +++ b/src/core/mod.rs @@ -888,6 +888,103 @@ async fn build_net_stack( }) } +/// Retry policy for a live net-stack replacement, generic over the builder so +/// it is unit-testable without binding sockets: build for `requested`; if that +/// fails, build for `live` (the posture the old stack was actually running) so +/// a bad posture change degrades to the previous posture instead of leaving no +/// stack at all. When `requested == live` the second attempt is a plain retry. +/// +/// `Ok((stack, mode, primary_err))` — a stack is up on `mode`; `primary_err` +/// is `Some` when the first attempt failed. `Err((primary, fallback))` — both +/// attempts failed and networking is gone. +async fn rebuild_with_fallback( + mut build: F, + requested: NetworkMode, + live: NetworkMode, +) -> Result<(T, NetworkMode, Option), (E, E)> +where + F: FnMut(NetworkMode) -> Fut, + Fut: std::future::Future>, +{ + match build(requested).await { + Ok(stack) => Ok((stack, requested, None)), + Err(primary) => match build(live).await { + Ok(stack) => Ok((stack, live, Some(primary))), + Err(fallback) => Err((primary, fallback)), + }, + } +} + +/// Tear down `old` and stand up a replacement stack for `requested_mode`. +/// +/// A build failure here is rare (only the local socket bind can fail; the +/// relay handshake is backgrounded), but it used to propagate straight out of +/// `run_core_loop` with no `UiEvent`, silently killing every future command — +/// the app looked alive and did nothing. Instead, fall back to `live_mode` +/// via `rebuild_with_fallback`, tell the UI when the requested change did not +/// stick, and return the mode the new stack actually runs so the caller can +/// keep its state honest. `Err` only when both builds fail: networking is +/// gone (already reported to the UI as fatal) and the caller should exit. +#[allow(clippy::too_many_arguments)] +async fn replace_net_stack( + old: NetStack, + what: &str, + secret_key: &SecretKey, + requested_mode: NetworkMode, + live_mode: NetworkMode, + friends_handler: &crate::presence_net::Handler, + publish: bool, + ui_tx: &mpsc::Sender, +) -> Result<(NetStack, NetworkMode), anyhow::Error> { + let lookup = old.memory_lookup.clone(); + old.shutdown().await; + let outcome = rebuild_with_fallback( + |mode| { + build_net_stack( + secret_key.clone(), + mode, + lookup.clone(), + friends_handler.clone(), + publish, + ) + }, + requested_mode, + live_mode, + ) + .await; + match outcome { + Ok((stack, mode, None)) => Ok((stack, mode)), + Ok((stack, mode, Some(primary))) => { + if mode == requested_mode { + // Same-posture retry succeeded — everything the user asked for + // is in effect, so log it rather than raising a UI error. + crate::log_msg(&format!( + "{what}: net stack build failed once ({primary:#}); retry succeeded" + )); + } else { + let _ = ui_tx + .send(UiEvent::Error(format!( + "{what} failed ({primary:#}); staying on the previous \ + network mode for this session" + ))) + .await; + } + Ok((stack, mode)) + } + Err((primary, fallback)) => { + let _ = ui_tx + .send(UiEvent::Error(format!( + "Networking lost: {primary:#} (recovery attempt also failed: \ + {fallback:#}). Restart PeerSpeak to reconnect." + ))) + .await; + Err(anyhow::anyhow!( + "net stack rebuild failed: {primary:#}; fallback: {fallback:#}" + )) + } + } +} + /// Maximum number of *automatic* chat-attachment fetches in flight at once. /// /// Auto-fetch (inline image preview) is triggered by an untrusted peer's chat @@ -1335,6 +1432,10 @@ async fn run_core_loop( // rebuilt on the next Leave (or before the next Join), preserving the old // "applies on next join" semantics while keeping the endpoint up while idle. let mut net_rebuild_pending = false; + // The posture the live stack was actually built with. Trails `network_mode` + // while a rebuild is pending, and is the fallback posture when a rebuild + // fails (see `replace_net_stack`). + let mut net_mode = network_mode; // When Discoverable is on, the instant it auto-reverts to Normal (W7 P6 time-box). // `None` = not Discoverable, no pending revert. Set on SetPresenceMode(Discoverable), @@ -1552,17 +1653,24 @@ async fn run_core_loop( // active, rebuild the persistent stack now — after the old session is // gone, before the new one binds — so this join uses the new posture. if net_rebuild_pending { - let lookup = net.memory_lookup.clone(); - net.shutdown().await; let publish = presence_mode.lock().unwrap().publishes_to_discovery(); - net = build_net_stack( - secret_key.clone(), + let (stack, live) = replace_net_stack( + net, + "Applying deferred network settings", + &secret_key, network_mode, - lookup, - friends_handler.clone(), + net_mode, + &friends_handler, publish, + &ui_tx, ) .await?; + net = stack; + net_mode = live; + // If the new posture failed and we fell back, keep the mode + // state honest (and re-attemptable) rather than pretending + // the change applied. The join proceeds on the live stack. + network_mode = live; net_rebuild_pending = false; } @@ -2594,17 +2702,21 @@ async fn run_core_loop( // Apply any network-mode / identity change that was deferred while we // were in the call (rebuild while idle keeps the endpoint reachable). if net_rebuild_pending { - let lookup = net.memory_lookup.clone(); - net.shutdown().await; let publish = presence_mode.lock().unwrap().publishes_to_discovery(); - net = build_net_stack( - secret_key.clone(), + let (stack, live) = replace_net_stack( + net, + "Applying deferred network settings", + &secret_key, network_mode, - lookup, - friends_handler.clone(), + net_mode, + &friends_handler, publish, + &ui_tx, ) .await?; + net = stack; + net_mode = live; + network_mode = live; net_rebuild_pending = false; } } @@ -2750,17 +2862,21 @@ async fn run_core_loop( // idle; if a call is active, defer to the next Leave/Join so the // live call isn't disrupted (preserves "applies on next join"). if active_session.is_none() { - let lookup = net.memory_lookup.clone(); - net.shutdown().await; let publish = presence_mode.lock().unwrap().publishes_to_discovery(); - net = build_net_stack( - secret_key.clone(), + let (stack, live) = replace_net_stack( + net, + "Network mode change", + &secret_key, network_mode, - lookup, - friends_handler.clone(), + net_mode, + &friends_handler, publish, + &ui_tx, ) .await?; + net = stack; + net_mode = live; + network_mode = live; } else { net_rebuild_pending = true; } @@ -2798,17 +2914,22 @@ async fn run_core_loop( // key unchanged, so a rebuild would be pointless churn). if regenerated { if active_session.is_none() { - let lookup = net.memory_lookup.clone(); - net.shutdown().await; let publish = presence_mode.lock().unwrap().publishes_to_discovery(); - net = build_net_stack( - secret_key.clone(), + // Same mode both attempts — the fallback is a plain + // retry under the (already persisted) new key. + let (stack, live) = replace_net_stack( + net, + "Endpoint restart after identity change", + &secret_key, network_mode, - lookup, - friends_handler.clone(), + net_mode, + &friends_handler, publish, + &ui_tx, ) .await?; + net = stack; + net_mode = live; } else { net_rebuild_pending = true; } @@ -3322,10 +3443,10 @@ fn replace_viewer_index(viewers: &[(String, T)], ticket: &str) -> Option(&[], "ticket-A"), None); } + // --- rebuild_with_fallback: the retry policy behind replace_net_stack --- + // The builder is injected, so these cover the policy without sockets. The + // closure does its bookkeeping synchronously and returns a ready future. + + #[tokio::test] + async fn rebuild_keeps_requested_posture_on_first_success() { + let calls = std::cell::RefCell::new(Vec::new()); + let out = rebuild_with_fallback( + |mode| { + calls.borrow_mut().push(mode); + std::future::ready(Ok::(7)) + }, + NetworkMode::DirectOnly, + NetworkMode::N0Full, + ) + .await; + assert_eq!(out, Ok((7, NetworkMode::DirectOnly, None))); + // No second build: the live posture is only a fallback. + assert_eq!(*calls.borrow(), vec![NetworkMode::DirectOnly]); + } + + #[tokio::test] + async fn rebuild_falls_back_to_the_live_posture_when_the_requested_one_fails() { + let calls = std::cell::RefCell::new(Vec::new()); + let out = rebuild_with_fallback( + |mode| { + calls.borrow_mut().push(mode); + std::future::ready(if mode == NetworkMode::DirectOnly { + Err("bind failed".to_string()) + } else { + Ok(7u8) + }) + }, + NetworkMode::DirectOnly, + NetworkMode::N0Full, + ) + .await; + // A stack is up on the OLD posture and the caller learns both that it + // fell back (mode) and why (the primary error) — no silent zombie. + assert_eq!( + out, + Ok((7, NetworkMode::N0Full, Some("bind failed".to_string()))) + ); + assert_eq!( + *calls.borrow(), + vec![NetworkMode::DirectOnly, NetworkMode::N0Full] + ); + } + + #[tokio::test] + async fn rebuild_reports_both_errors_when_networking_is_gone() { + let out = rebuild_with_fallback( + |_| std::future::ready(Err::("bind failed".to_string())), + NetworkMode::DirectOnly, + NetworkMode::N0Full, + ) + .await; + assert_eq!( + out, + Err(("bind failed".to_string(), "bind failed".to_string())) + ); + } + + #[tokio::test] + async fn rebuild_with_equal_postures_is_a_plain_retry() { + // RegenerateIdentity rebuilds under the same mode: the fallback is a + // second attempt with identical parameters, not a posture change. + let calls = std::cell::Cell::new(0u8); + let out = rebuild_with_fallback( + |mode| { + calls.set(calls.get() + 1); + assert_eq!(mode, NetworkMode::RelayNoDiscovery); + std::future::ready(if calls.get() == 1 { + Err("transient".to_string()) + } else { + Ok(7u8) + }) + }, + NetworkMode::RelayNoDiscovery, + NetworkMode::RelayNoDiscovery, + ) + .await; + // Succeeded on the requested posture, so the caller treats the change + // as applied (the Some(err) is logged, not surfaced as a UI error). + assert_eq!( + out, + Ok(( + 7, + NetworkMode::RelayNoDiscovery, + Some("transient".to_string()) + )) + ); + assert_eq!(calls.get(), 2); + } + #[test] fn admit_retained_rejects_only_new_ids_at_the_cap() { // Below the cap, a brand-new identity is retained.