From ef042ddf82a034a2edaead556590186afac22e7f Mon Sep 17 00:00:00 2001 From: wonfen Date: Tue, 1 Sep 2026 23:38:51 +0800 Subject: [PATCH] fix(sysproxy): stop turning a missing network service into a hard failure --- src-tauri/src/cmd/network.rs | 13 ++- src-tauri/src/core/manager/lifecycle.rs | 30 +++--- src-tauri/src/core/notification.rs | 33 ++++--- src-tauri/src/core/proxy_control.rs | 123 ++++++++++++++++++------ src-tauri/src/core/sysopt.rs | 101 +++++++++++++------ src-tauri/src/feat/config.rs | 81 +--------------- src/hooks/use-system-proxy-state.ts | 45 +++++---- 7 files changed, 238 insertions(+), 188 deletions(-) diff --git a/src-tauri/src/cmd/network.rs b/src-tauri/src/cmd/network.rs index faef3a080..b8925f649 100644 --- a/src-tauri/src/cmd/network.rs +++ b/src-tauri/src/cmd/network.rs @@ -1,6 +1,6 @@ use super::CmdResult; use crate::cmd::StringifyErr as _; -use crate::core::sysopt::Sysopt; +use crate::core::{proxy_control, sysopt::Sysopt}; use clash_verge_logging::{Type, logging}; use gethostname::gethostname; use network_interface::NetworkInterface; @@ -13,7 +13,11 @@ pub async fn get_sys_proxy() -> CmdResult { logging!(debug, Type::Network, "异步获取系统代理配置"); Sysopt::global().wait_idle().await; - let sys_proxy = Sysproxy::get_system_proxy().stringify_err()?; + // With no network service there is no proxy configured anywhere, which reads as disabled. + let sys_proxy = match Sysproxy::get_system_proxy() { + Err(error) if proxy_control::is_missing_network_service(&error) => Sysproxy::default(), + other => other.stringify_err()?, + }; let Sysproxy { ref host, ref bypass, @@ -40,7 +44,10 @@ pub async fn get_sys_proxy() -> CmdResult { #[tauri::command] pub async fn get_auto_proxy() -> CmdResult { Sysopt::global().wait_idle().await; - let auto_proxy = Autoproxy::get_auto_proxy().stringify_err()?; + let auto_proxy = match Autoproxy::get_auto_proxy() { + Err(error) if proxy_control::is_missing_network_service(&error) => Autoproxy::default(), + other => other.stringify_err()?, + }; let Autoproxy { ref enable, ref url } = auto_proxy; let mut map = Mapping::new(); diff --git a/src-tauri/src/core/manager/lifecycle.rs b/src-tauri/src/core/manager/lifecycle.rs index 24c453a48..ed463f97d 100644 --- a/src-tauri/src/core/manager/lifecycle.rs +++ b/src-tauri/src/core/manager/lifecycle.rs @@ -4,7 +4,7 @@ use crate::core::handle::Handle; use crate::core::manager::CLASH_LOGGER; use crate::core::proxy_control::{self, SysproxyFailure}; use crate::core::service::{SERVICE_MANAGER, ServiceStatus}; -use anyhow::{Result, ensure}; +use anyhow::Result; use clash_verge_logging::{Type, logging}; use scopeguard::defer; use smartstring::alias::String; @@ -129,20 +129,24 @@ where { match (running_mode, proxy_intent) { (RunningMode::NotRunning, _) => {} - // Do not hand the OS to another owner while the old guard may still write. - (RunningMode::Sidecar, ProxyStopIntent::HandOverToService) => ensure!( - stop_guard().await, - "the system proxy guard did not stop in time; not handing the proxy to the service" - ), - (RunningMode::Service, _) if is_macos => ensure!( - stop_guard().await, - "the system proxy guard did not stop in time; not stopping the service-owned core" - ), + // A guard that will not stop must not block the core from stopping. + (RunningMode::Sidecar, ProxyStopIntent::HandOverToService) => warn_if_guard_lingers(stop_guard().await), + (RunningMode::Service, _) if is_macos => warn_if_guard_lingers(stop_guard().await), (RunningMode::Service | RunningMode::Sidecar, _) => clear_proxy().await?, } stop_core().await } +fn warn_if_guard_lingers(stopped: bool) { + if !stopped { + logging!( + warn, + Type::Core, + "the system proxy guard did not stop in time; stopping the core anyway" + ); + } +} + async fn run_core_start_transition( start_core: Start, core_is_ready: Ready, @@ -951,7 +955,7 @@ mod tests { } #[tokio::test] - async fn a_guard_that_would_not_stop_keeps_the_core_where_it_is() { + async fn a_guard_that_would_not_stop_does_not_block_the_core_from_stopping() { for (mode, intent) in [ (RunningMode::Sidecar, ProxyStopIntent::HandOverToService), (RunningMode::Service, ProxyStopIntent::Clear), @@ -968,8 +972,8 @@ mod tests { ) .await; - assert!(result.is_err(), "{mode:?} {intent:?}"); - assert!(calls.lock().is_empty(), "{mode:?} {intent:?}: {:?}", *calls.lock()); + assert!(result.is_ok(), "{mode:?} {intent:?}"); + assert_eq!(&*calls.lock(), &["core_stop"], "{mode:?} {intent:?}"); } } diff --git a/src-tauri/src/core/notification.rs b/src-tauri/src/core/notification.rs index bcc200105..9f8be9797 100644 --- a/src-tauri/src/core/notification.rs +++ b/src-tauri/src/core/notification.rs @@ -59,14 +59,16 @@ impl FailedOperation { matches!(self, Self::SystemProxyEnable | Self::SystemProxyDisable) && matches!(other, Self::SystemProxyRestore) } - /// Whether reaching `enabled` resolves this operation's failure. - const fn retired_by_system_proxy_success(self, enabled: bool) -> bool { - match self { - Self::SystemProxyEnable => enabled, - Self::SystemProxyDisable => !enabled, - Self::SystemProxyRestore => true, + /// Whether a successful apply of `asked` resolves this operation's failure. + const fn retired_by_success_of(self, asked: Self) -> bool { + match (self, asked) { // Only replacing or stopping the guard resolves its failure. - Self::SystemProxyGuard => false, + (Self::SystemProxyGuard, _) | (_, Self::SystemProxyGuard) => false, + // The user asked and got an answer, so nothing earlier is still owed to them. + (_, Self::SystemProxyEnable | Self::SystemProxyDisable) => true, + // An incidental restore says nothing about a request they may not have seen fail. + (Self::SystemProxyRestore, Self::SystemProxyRestore) => true, + (_, Self::SystemProxyRestore) => false, } } } @@ -124,10 +126,10 @@ impl FailureTable { } /// Return whether any proxy failure was retired. - fn retire_system_proxy(&self, enabled: bool) -> bool { + fn retire_system_proxy(&self, asked: FailedOperation) -> bool { let mut entries = self.entries.lock(); let before = entries.len(); - entries.retain(|_, failure| !failure.operation.retired_by_system_proxy_success(enabled)); + entries.retain(|_, failure| !failure.operation.retired_by_success_of(asked)); before != entries.len() } } @@ -167,8 +169,8 @@ mod failure_table_tests { assert_eq!(entries[0].operation, FailedOperation::SystemProxyEnable); assert_eq!(entries[0].detail, "and a restart hit it too"); - assert!(!table.retire_system_proxy(false)); - assert!(table.retire_system_proxy(true)); + assert!(!table.retire_system_proxy(FailedOperation::SystemProxyRestore)); + assert!(table.retire_system_proxy(FailedOperation::SystemProxyEnable)); } #[test] @@ -187,8 +189,7 @@ mod failure_table_tests { let entries = table.snapshot(); assert_eq!(entries[0].operation, FailedOperation::SystemProxyDisable); - assert!(!table.retire_system_proxy(true)); - assert!(table.retire_system_proxy(false)); + assert!(table.retire_system_proxy(FailedOperation::SystemProxyDisable)); } #[test] @@ -251,9 +252,9 @@ pub fn retire_guard_failures() { } } -/// Retire failures resolved by the resulting proxy state. -pub fn retire_system_proxy_failures(enabled: bool) { - if PENDING_FAILURES.retire_system_proxy(enabled) { +/// Retire failures resolved by a successful apply of what was asked. +pub fn retire_system_proxy_failures(asked: FailedOperation) { + if PENDING_FAILURES.retire_system_proxy(asked) { notify_pending_failures_changed(); } } diff --git a/src-tauri/src/core/proxy_control.rs b/src-tauri/src/core/proxy_control.rs index 3be5d3873..37e0f8d5c 100644 --- a/src-tauri/src/core/proxy_control.rs +++ b/src-tauri/src/core/proxy_control.rs @@ -162,6 +162,24 @@ fn was_refused_locally(error: &anyhow::Error) -> bool { }) } +/// Whether macOS has no network service to write the proxy on. +/// +/// Offline, mid network switch, or the primary interface is not a configured service. A read +/// reports "no proxy" and a disable has nothing left to do; an enable still fails, since a proxy +/// the OS never received must not be recorded as applied. +#[cfg(target_os = "macos")] +pub const fn is_missing_network_service(error: &sysproxy::Error) -> bool { + matches!( + error, + sysproxy::Error::NoActiveNetworkService | sysproxy::Error::NetworkInterface + ) +} + +#[cfg(not(target_os = "macos"))] +pub const fn is_missing_network_service(_error: &sysproxy::Error) -> bool { + false +} + /// Distinguish a misplaced sidecar from a missing privileged service. async fn classify_local_apply_failure(error: anyhow::Error) -> anyhow::Error { if SysproxyFailure::from_chain(&error).is_some() || !was_refused_locally(&error) { @@ -340,6 +358,10 @@ fn service_proxy_config(verge: &IVerge, mixed_port: u16, pac_port: u16) -> Resul } } +/// Judge the outcome the service reported. +/// +/// A write it could not roll back arrives as a plain error whose cause is the rollback's, not +/// the write's, so it stays a failure. fn service_apply_result(requested: &MacosProxyConfig, outcome: ProxyApplyOutcome) -> Result<()> { match outcome { ProxyApplyOutcome::Applied | ProxyApplyOutcome::NotRequested => Ok(()), @@ -389,16 +411,21 @@ where Write: FnOnce() -> WriteFuture, WriteFuture: Future>, { - ensure!( - drain_local_guard().await, - "the system proxy guard did not stop in time; not writing the system proxy through the service" - ); + if !drain_local_guard().await { + // Its `networksetup` calls are separate from the service's one privileged transaction, + // so a stale guard write may still land after ours. + logging!( + warn, + Type::Core, + "the system proxy guard did not stop in time; writing through the service anyway" + ); + } write().await } #[derive(Debug)] enum TableEffect<'a> { - Retire { enabled: bool }, + Retire(FailedOperation), File(FailedOperation, &'a anyhow::Error), Nothing, } @@ -415,19 +442,19 @@ fn clear_table_effect(result: &Result<()>) -> TableEffect<'_> { fn settle_table(effect: TableEffect<'_>, retire: Retire, file: File) where - Retire: FnOnce(bool), + Retire: FnOnce(FailedOperation), File: FnOnce(FailedOperation, &anyhow::Error), { match effect { - TableEffect::Retire { enabled } => retire(enabled), + TableEffect::Retire(asked) => retire(asked), TableEffect::File(operation, error) => file(operation, error), TableEffect::Nothing => {} } } -fn table_effect(result: &Result<()>, enabled: bool) -> TableEffect<'_> { +fn table_effect(result: &Result<()>) -> TableEffect<'_> { match result { - Ok(()) => TableEffect::Retire { enabled }, + Ok(()) => TableEffect::Retire(notification::what_was_asked()), Err(error) if SysproxyFailure::from_chain(error).is_some() => { TableEffect::File(notification::what_was_asked(), error) } @@ -437,9 +464,7 @@ fn table_effect(result: &Result<()>, enabled: bool) -> TableEffect<'_> { pub async fn apply() -> Result<()> { let running_mode = CoreManager::global().get_running_mode(); - // Use the pre-apply snapshot when retiring matching failures. let verge = Config::verge().await.latest_arc(); - let enabled = verge.enable_system_proxy.unwrap_or_default(); let result = match proxy_backend_route(cfg!(target_os = "macos"), &running_mode) { ProxyBackendRoute::Local => match Sysopt::global().update_sysproxy().await { Ok(()) => Ok(()), @@ -462,7 +487,7 @@ pub async fn apply() -> Result<()> { }; // Settle failures where every proxy-apply path converges. settle_table( - table_effect(&result, enabled), + table_effect(&result), notification::retire_system_proxy_failures, |operation, error| { report_failure(operation, error); @@ -906,8 +931,8 @@ mod tests { } #[tokio::test] - async fn a_local_guard_that_would_not_stop_keeps_root_from_writing() { - assert_eq!(record_service_apply(false, true).await, (vec!["drain"], false)); + async fn a_local_guard_that_would_not_stop_does_not_block_the_service_write() { + assert_eq!(record_service_apply(false, true).await, (vec!["drain", "write"], true)); } #[test] @@ -1044,7 +1069,45 @@ mod tests { assert_eq!(refusal_classification(None), SysproxyFailure::PrivilegeRequired); } - use super::{SystemProxyStateUnknown, is_reportable_given, rollback_failure}; + use super::{SystemProxyStateUnknown, is_reportable_given, rollback_failure, service_apply_result}; + + #[cfg(target_os = "macos")] + #[test] + fn an_enable_the_os_never_received_is_never_reported_as_applied() { + let requested = MacosProxyConfig::Global { + host: "127.0.0.1".to_owned(), + port: 7890, + bypass: String::new(), + }; + + let error = service_apply_result( + &requested, + ProxyApplyOutcome::DirectFallback { + message: sysproxy::Error::NoActiveNetworkService.to_string(), + }, + ) + .err() + .unwrap_or_else(|| anyhow::anyhow!("an enable that never reached the OS must fail")); + + assert_eq!( + SysproxyFailure::from_chain(&error).map(SysproxyFailure::code), + Some("SYSPROXY_DIRECT_FALLBACK") + ); + } + + #[cfg(target_os = "macos")] + #[test] + fn a_disable_is_satisfied_by_the_os_ending_up_direct() { + assert!( + service_apply_result( + &MacosProxyConfig::Disabled, + ProxyApplyOutcome::DirectFallback { + message: sysproxy::Error::NoActiveNetworkService.to_string(), + }, + ) + .is_ok() + ); + } fn mappable() -> anyhow::Error { anyhow::anyhow!("networksetup refused").context(SysproxyFailure::PrivilegeRequired) @@ -1054,7 +1117,7 @@ mod tests { let events = Mutex::new(Vec::new()); super::settle_table( effect, - |enabled| events.lock().push(format!("retire:{enabled}")), + |asked| events.lock().push(format!("retire:{asked:?}")), |operation, _| events.lock().push(format!("file:{operation:?}")), ); events.into_inner() @@ -1063,8 +1126,8 @@ mod tests { #[test] fn each_effect_reaches_the_table_it_names() { assert_eq!( - record_settlement(super::TableEffect::Retire { enabled: true }), - vec!["retire:true"] + record_settlement(super::TableEffect::Retire(FailedOperation::SystemProxyEnable)), + vec!["retire:SystemProxyEnable"] ); assert_eq!( record_settlement(super::TableEffect::File( @@ -1092,16 +1155,20 @@ mod tests { )); } - #[test] - fn a_clean_apply_retires_in_the_direction_it_reached() { + #[tokio::test] + async fn a_clean_apply_retires_in_the_direction_that_was_asked_for() { assert!(matches!( - super::table_effect(&Ok(()), true), - super::TableEffect::Retire { enabled: true } - )); - assert!(matches!( - super::table_effect(&Ok(()), false), - super::TableEffect::Retire { enabled: false } + super::table_effect(&Ok(())), + super::TableEffect::Retire(FailedOperation::SystemProxyRestore) )); + let asked = super::notification::asking_for(FailedOperation::SystemProxyEnable, async { + matches!( + super::table_effect(&Ok(())), + super::TableEffect::Retire(FailedOperation::SystemProxyEnable) + ) + }) + .await; + assert!(asked); } #[tokio::test] @@ -1109,7 +1176,7 @@ mod tests { let (applied, cleared) = super::notification::asking_for(FailedOperation::SystemProxyEnable, async { let (apply_error, clear_error) = (mappable(), mappable()); ( - format!("{:?}", super::table_effect(&Err(apply_error), true)), + format!("{:?}", super::table_effect(&Err(apply_error))), format!("{:?}", super::clear_table_effect(&Err(clear_error))), ) }) @@ -1129,7 +1196,7 @@ mod tests { #[test] fn a_failure_with_nothing_to_say_is_not_filed() { assert!(matches!( - super::table_effect(&Err(anyhow::anyhow!("the core exited immediately")), true), + super::table_effect(&Err(anyhow::anyhow!("the core exited immediately"))), super::TableEffect::Nothing )); } diff --git a/src-tauri/src/core/sysopt.rs b/src-tauri/src/core/sysopt.rs index 7309101ab..3c0eb2073 100644 --- a/src-tauri/src/core/sysopt.rs +++ b/src-tauri/src/core/sysopt.rs @@ -1,6 +1,6 @@ use crate::{ config::{Config, MixedPort}, - core::proxy_control::SystemProxyStateUnknown, + core::proxy_control::{self, SystemProxyStateUnknown}, singleton, utils::server, }; @@ -137,10 +137,13 @@ where return Ok(()); } - // The final disable must happen after the last in-flight guard write. + // The final disable should follow the last in-flight guard write, but a guard that will not + // stop must not keep the proxy on: disable again either way. if !drain_again().await { - anyhow::bail!( - "the system proxy guard did not finish within {GUARD_DRAIN_TIMEOUT:?}; the OS proxy state is unknown" + logging!( + warn, + Type::Core, + "the system proxy guard did not finish within {GUARD_DRAIN_TIMEOUT:?}; disabling anyway" ); } disable().await @@ -171,15 +174,19 @@ where } /// Treats a missing network service as a completed disable: with nothing to write to, the -/// requested "no proxy" state already holds. Turning a proxy on must still fail. -fn tolerate_missing_network_service(_enabling: bool, outcome: sysproxy::Result<()>) -> Result<()> { +/// requested "no proxy" state already holds. +/// +/// Enabling still fails: the guard is opt-in and nothing else re-applies the proxy when a +/// service appears, so a skipped enable would claim a proxy the OS never received. +/// +/// Reports whether the write reached the OS, so recovery does not mistake a skip for evidence. +fn skip_without_network_service(enabling: bool, outcome: sysproxy::Result<()>) -> Result { match outcome { - #[cfg(target_os = "macos")] - Err(sysproxy::Error::NoActiveNetworkService) if !_enabling => { + Err(error) if !enabling && proxy_control::is_missing_network_service(&error) => { logging!(warn, Type::Core, "no active network service, nothing to turn off"); - Ok(()) + Ok(false) } - other => other.map_err(anyhow::Error::from), + other => other.map(|()| true).map_err(anyhow::Error::from), } } @@ -187,8 +194,8 @@ fn tolerate_missing_network_service(_enabling: bool, outcome: sysproxy::Result<( async fn disable_all_proxies(sys: Sysproxy, auto: Autoproxy) -> Result<()> { tokio::task::spawn_blocking(move || { disable_both( - || tolerate_missing_network_service(sys.enable, sys.set_system_proxy()), - || tolerate_missing_network_service(auto.enable, auto.set_auto_proxy()), + || skip_without_network_service(sys.enable, sys.set_system_proxy()).map(|_reached_os| ()), + || skip_without_network_service(auto.enable, auto.set_auto_proxy()).map(|_reached_os| ()), ) }) .await? @@ -463,16 +470,10 @@ impl Sysopt { let guard_was_running = !self.access_guard().read().get_state().is_stopped(); let idle = self.access_guard().read().shutdown(); let drained = idle.wait_timeout(GUARD_DRAIN_TIMEOUT).await; - if !drained { - // A pending guard write makes OS state unknown. - self.access_guard().write().set_guard_type(GuardType::None); - anyhow::bail!( - "the system proxy guard did not finish within {GUARD_DRAIN_TIMEOUT:?}; the OS proxy state is unknown" - ); - } - // Skip writes only when macOS exactly matches the target. + // A slow guard does not stop the write, but it does make the OS read untrustworthy. if cfg!(target_os = "macos") + && drained && current_os_proxy_state(sys.clone(), auto.clone()).await == OsProxyState::AlreadyApplied { self.access_guard().write().set_guard_type(guard_type); @@ -489,15 +490,17 @@ impl Sysopt { (off_sys, off_auto) }; - // Track whether an earlier setter already changed the OS. + // Only a step that actually wrote is evidence the OS changed; a skipped one is not. let applied = tokio::task::spawn_blocking(move || { - for (index, step) in apply_steps.into_iter().enumerate() { + let mut earlier_step_reached_os = false; + for step in apply_steps { let written = match step { - ProxyApplyStep::Autoproxy => tolerate_missing_network_service(auto.enable, auto.set_auto_proxy()), - ProxyApplyStep::Sysproxy => tolerate_missing_network_service(sys.enable, sys.set_system_proxy()), + ProxyApplyStep::Autoproxy => skip_without_network_service(auto.enable, auto.set_auto_proxy()), + ProxyApplyStep::Sysproxy => skip_without_network_service(sys.enable, sys.set_system_proxy()), }; - if let Err(error) = written { - return Err((index > 0, error)); + match written { + Ok(reached_os) => earlier_step_reached_os |= reached_os, + Err(error) => return Err((earlier_step_reached_os, error)), } } Ok(()) @@ -571,12 +574,50 @@ mod tests { AuthoritativeState, BYPASS_SEPARATOR, DEFAULT_BYPASS, OsProxyState, ProxyApplyStep, SystemProxyStateUnknown, authoritative_state, authoritative_state_from, classify_os_proxy_state, disable_both, disable_until_the_last_write_is_ours, first_failure, format_bypass, proxy_apply_steps, - recover_from_failed_write, target_is_already_in_place, + recover_from_failed_write, skip_without_network_service, target_is_already_in_place, }; use parking_lot::Mutex; use std::collections::VecDeque; use sysproxy::{Autoproxy, Sysproxy}; + #[cfg(target_os = "macos")] + #[test] + fn a_machine_with_no_network_service_has_nothing_to_turn_off() { + for missing in [ + sysproxy::Error::NoActiveNetworkService, + sysproxy::Error::NetworkInterface, + ] { + assert_eq!(skip_without_network_service(false, Err(missing)).ok(), Some(false)); + } + assert_eq!(skip_without_network_service(false, Ok(())).ok(), Some(true)); + assert!(skip_without_network_service(false, Err(sysproxy::Error::RequiresAdminPrivileges)).is_err()); + } + + /// Enabling the global proxy disables PAC first; that skipped step must not look like a write. + #[cfg(target_os = "macos")] + #[test] + fn a_skipped_step_is_not_evidence_that_the_os_was_touched() { + let skipped = skip_without_network_service(false, Err(sysproxy::Error::NoActiveNetworkService)); + let earlier_step_reached_os = skipped.unwrap_or(true); + assert!(!earlier_step_reached_os); + + let failed = skip_without_network_service(true, Err(sysproxy::Error::NoActiveNetworkService)) + .err() + .unwrap_or_else(|| anyhow::anyhow!("enabling without a network service must fail")); + + assert_eq!( + authoritative_state(&failed, earlier_step_reached_os), + AuthoritativeState::Unchanged + ); + } + + #[cfg(target_os = "macos")] + #[test] + fn turning_the_proxy_on_without_a_network_service_still_fails() { + assert!(skip_without_network_service(true, Err(sysproxy::Error::NoActiveNetworkService)).is_err()); + assert!(skip_without_network_service(true, Err(sysproxy::Error::NetworkInterface)).is_err()); + } + #[test] fn empty_custom_bypass_uses_defaults() { assert_eq!(format_bypass(false, ""), DEFAULT_BYPASS); @@ -796,10 +837,10 @@ mod tests { } #[tokio::test] - async fn a_guard_that_never_finishes_is_reported_rather_than_written_over() { + async fn a_guard_that_never_finishes_still_gets_the_proxy_disabled() { assert_eq!( - record_disable_sequence(false, false, &[true]).await, - (vec!["disable", "drain"], false) + record_disable_sequence(false, false, &[true, true]).await, + (vec!["disable", "drain", "disable"], true) ); } diff --git a/src-tauri/src/feat/config.rs b/src-tauri/src/feat/config.rs index c8fc630af..db7077387 100644 --- a/src-tauri/src/feat/config.rs +++ b/src-tauri/src/feat/config.rs @@ -1,9 +1,6 @@ use crate::{ config::{Config, IVerge}, - core::{ - CoreManager, autostart, handle, hotkey, logger::Logger, proxy_control, proxy_control::SystemProxyStateUnknown, - tray, - }, + core::{CoreManager, autostart, handle, hotkey, logger::Logger, proxy_control, tray}, module::{auto_backup::AutoBackupManager, lightweight}, }; use anyhow::Result; @@ -311,23 +308,8 @@ pub(super) async fn apply_verge_patch_locked( let update_flags = determine_update_flags(patch); logging!(debug, Type::Setup, "Determined update flags: {:?}", update_flags); - if let Err(error) = process_terminated_flags(update_flags, patch).await { - if !may_commit_system_proxy_disabled(patch, &error) { - return Err(error); - } - // Commit the safe state after a user-requested proxy patch partly lands. - verge.edit_draft(|d| d.enable_system_proxy = Some(false)); - transaction.commit(); - announce_verge_change(); - // Persist the forced-off value even when the original patch was pre-saved. - let unsaved = verge.data_arc().save_file().await.err(); - return Err(match unsaved { - Some(save) => error.context(format!( - "the system proxy state became unknown and the disabled safety value could not be saved: {save:#}" - )), - None => error, - }); - } + // A failed patch rolls back to what the user already had; it never invents a value for them. + process_terminated_flags(update_flags, patch).await?; transaction.commit(); announce_verge_change(); @@ -345,65 +327,8 @@ fn announce_verge_change() { handle::Handle::refresh_verge(); } -/// Whether a user proxy patch may commit disabled after a partial write. -fn may_commit_system_proxy_disabled(patch: &IVerge, error: &anyhow::Error) -> bool { - patch.enable_system_proxy.is_some() && SystemProxyStateUnknown::is_in(error) -} - pub async fn fetch_verge_config() -> Result> { let draft = Config::verge().await; let data = draft.data_arc(); Ok(data) } - -#[cfg(test)] -mod tests { - use super::{IVerge, SystemProxyStateUnknown, may_commit_system_proxy_disabled}; - - fn state_unknown() -> anyhow::Error { - anyhow::anyhow!("networksetup refused") - .context(SystemProxyStateUnknown) - .context("failed to apply the system proxy") - } - - #[test] - fn the_user_turning_the_proxy_on_gets_it_committed_as_off_when_the_os_ended_up_unknown() { - let patch = IVerge { - enable_system_proxy: Some(true), - ..IVerge::default() - }; - - assert!(may_commit_system_proxy_disabled(&patch, &state_unknown())); - } - - #[test] - fn a_patch_about_something_else_never_rewrites_the_proxy_setting() { - let patch = IVerge { - enable_tun_mode: Some(true), - ..IVerge::default() - }; - - assert!(!may_commit_system_proxy_disabled(&patch, &state_unknown())); - } - - #[test] - fn nothing_is_committed_when_nothing_is_known_to_have_been_written() { - let patch = IVerge { - enable_system_proxy: Some(true), - ..IVerge::default() - }; - let refused = anyhow::anyhow!("networksetup refused").context("failed to apply the system proxy"); - - assert!(!may_commit_system_proxy_disabled(&patch, &refused)); - } - - #[test] - fn turning_the_proxy_off_and_failing_that_way_still_commits_off() { - let patch = IVerge { - enable_system_proxy: Some(false), - ..IVerge::default() - }; - - assert!(may_commit_system_proxy_disabled(&patch, &state_unknown())); - } -} diff --git a/src/hooks/use-system-proxy-state.ts b/src/hooks/use-system-proxy-state.ts index 9958fa381..c58cd5d70 100644 --- a/src/hooks/use-system-proxy-state.ts +++ b/src/hooks/use-system-proxy-state.ts @@ -11,6 +11,7 @@ import { } from '@/services/cmds' import { getCacheData, + removeCacheData, revalidateQueries, useQuery, } from '@/services/query-client' @@ -62,12 +63,10 @@ export const useSystemProxyState = () => { if (busyRef.current) return busyRef.current = true - let failed = false try { while (pendingRef.current !== null) { const target = pendingRef.current pendingRef.current = null - // Revalidate once below so a refetch failure cannot look like a patch failure. await patchVergeConfig({ enable_system_proxy: target }) confirmed = target if (!target && verge?.auto_close_connection) { @@ -75,7 +74,6 @@ export const useSystemProxyState = () => { } } } catch (error) { - failed = true mutateVerge( (prev) => (prev ? { ...prev, enable_system_proxy: confirmed } : prev), false, @@ -85,23 +83,30 @@ export const useSystemProxyState = () => { throw error } finally { busyRef.current = false - const revalidated = revalidateQueries([ - ['getVergeConfig'], - ['getSystemProxy'], - ['getAutotemProxy'], - ]) - if (failed) { - // Preserve the classified toggle failure. - try { - await revalidated - } catch (error) { - console.warn( - '[system-proxy] revalidating after a failed toggle failed too:', - error, - ) - } - } else { - await revalidated + // Refreshing cached state is not part of the toggle's result: a failed read must not + // turn a write that landed into a reported failure. + try { + await revalidateQueries([['getVergeConfig']]) + } catch (error) { + console.warn( + '[system-proxy] rereading the config after a toggle failed:', + error, + ) + } + // Kept separate so an unreadable config cannot discard OS state that did read. + try { + await revalidateQueries([['getSystemProxy'], ['getAutotemProxy']]) + } catch (error) { + console.warn( + '[system-proxy] rereading the OS proxy after a toggle failed:', + error, + ) + // The indicator reports observed OS state, so an unreadable one must read as inactive + // rather than stay live from a stale cache. + await Promise.all([ + removeCacheData(['getSystemProxy']), + removeCacheData(['getAutotemProxy']), + ]) } } }