From 3e2da64f2327ea7dc16ae6662f006a407270da5a Mon Sep 17 00:00:00 2001 From: Aria Desires Date: Thu, 23 Oct 2025 10:44:04 -0400 Subject: [PATCH] Make the default settings try harder to avoid crashes This introduces a safe_mode to our settings type that tells our initialization code to more aggressively believe Settings Failure Is Not An Error. This allows the full settings initialization logic to be used as much as possible for the default database while trying to keep any discarded results as narrow as possible for graceful degradation. Finally, Python has SFINAE. --- crates/ty_project/src/metadata/options.rs | 56 +++++++++++-- .../src/module_resolver/resolver.rs | 81 +++++++++++++++---- crates/ty_python_semantic/src/program.rs | 7 ++ crates/ty_server/src/session.rs | 2 +- crates/ty_test/src/lib.rs | 1 + 5 files changed, 123 insertions(+), 24 deletions(-) diff --git a/crates/ty_project/src/metadata/options.rs b/crates/ty_project/src/metadata/options.rs index bcd821c53d141..de9f70352de45 100644 --- a/crates/ty_project/src/metadata/options.rs +++ b/crates/ty_project/src/metadata/options.rs @@ -94,9 +94,23 @@ pub struct Options { #[serde(skip_serializing_if = "Option::is_none")] #[option_group] pub overrides: Option, + + /// Settings Failure Is Not An Error. + /// + /// This is used by the default database, which we are incentivized to make infallible, + /// while still trying to "do our best" to set things up properly where we can. + #[serde(default, skip)] + pub safe_mode: bool, } impl Options { + pub fn safe() -> Self { + Self { + safe_mode: true, + ..Self::default() + } + } + pub fn from_toml_str(content: &str, source: ValueSource) -> Result { let _guard = ValueSourceGuard::new(source, true); let options = toml::from_str(content)?; @@ -156,20 +170,44 @@ impl Options { ValueSource::PythonVSCodeExtension => SysPrefixPathOrigin::PythonVSCodeExtension, }; - Some(PythonEnvironment::new( - python_path.absolute(project_root, system), - origin, - system, - )?) + PythonEnvironment::new(python_path.absolute(project_root, system), origin, system) + .map_err(anyhow::Error::from) + .map(Some) } else { PythonEnvironment::discover(project_root, system) - .context("Failed to discover local Python environment")? + .context("Failed to discover local Python environment") + }; + + // If in safe-mode, fallback to None if this fails instead of erroring. + let python_environment = match python_environment { + Ok(python_environment) => python_environment, + Err(err) => { + if self.safe_mode { + tracing::debug!("Default settings failed to discover local Python environment"); + None + } else { + return Err(err); + } + } }; let site_packages_paths = if let Some(python_environment) = python_environment.as_ref() { - python_environment + let site_packages_paths = python_environment .site_packages_paths(system) - .context("Failed to discover the site-packages directory")? + .context("Failed to discover the site-packages directory"); + match site_packages_paths { + Ok(paths) => paths, + Err(err) => { + if self.safe_mode { + tracing::debug!( + "Default settings failed to discover site-packages directory" + ); + SitePackagesPaths::default() + } else { + return Err(err); + } + } + } } else { tracing::debug!("No virtual environment found"); @@ -193,6 +231,7 @@ impl Options { .or_else(|| site_packages_paths.python_version_from_layout()) .unwrap_or_default(); + // Safe mode is handled inside this function, so we just assume this can't fail let search_paths = self.to_search_paths( project_root, project_name, @@ -352,6 +391,7 @@ impl Options { .map(|path| path.absolute(project_root, system)), site_packages_paths: site_packages_paths.into_vec(), real_stdlib_path, + safe_mode: self.safe_mode, }; settings.to_search_paths(system, vendored) diff --git a/crates/ty_python_semantic/src/module_resolver/resolver.rs b/crates/ty_python_semantic/src/module_resolver/resolver.rs index 078785904921a..318ae189c4281 100644 --- a/crates/ty_python_semantic/src/module_resolver/resolver.rs +++ b/crates/ty_python_semantic/src/module_resolver/resolver.rs @@ -236,6 +236,7 @@ impl SearchPaths { custom_typeshed: typeshed, site_packages_paths, real_stdlib_path, + safe_mode, } = settings; let mut static_paths = vec![]; @@ -244,12 +245,30 @@ impl SearchPaths { let path = canonicalize(path, system); tracing::debug!("Adding extra search-path `{path}`"); - static_paths.push(SearchPath::extra(system, path)?); + match SearchPath::extra(system, path) { + Ok(path) => static_paths.push(path), + Err(err) => { + if *safe_mode { + tracing::debug!("Skipping invalid extra search-path: {err}"); + } else { + return Err(err); + } + } + } } for src_root in src_roots { tracing::debug!("Adding first-party search path `{src_root}`"); - static_paths.push(SearchPath::first_party(system, src_root.to_path_buf())?); + match SearchPath::first_party(system, src_root.to_path_buf()) { + Ok(path) => static_paths.push(path), + Err(err) => { + if *safe_mode { + tracing::debug!("Skipping invalid first-party search-path: {err}"); + } else { + return Err(err); + } + } + } } let (typeshed_versions, stdlib_path) = if let Some(typeshed) = typeshed { @@ -258,18 +277,31 @@ impl SearchPaths { let versions_path = typeshed.join("stdlib/VERSIONS"); - let versions_content = system.read_to_string(&versions_path).map_err(|error| { - SearchPathValidationError::FailedToReadVersionsFile { - path: versions_path, - error, + let results = system + .read_to_string(&versions_path) + .map_err( + |error| SearchPathValidationError::FailedToReadVersionsFile { + path: versions_path, + error, + }, + ) + .and_then(|versions_content| Ok(versions_content.parse()?)) + .and_then(|parsed| Ok((parsed, SearchPath::custom_stdlib(system, &typeshed)?))); + + match results { + Ok(results) => results, + Err(err) => { + if settings.safe_mode { + tracing::debug!("Skipping custom-stdlib search-path: {err}"); + ( + vendored_typeshed_versions(vendored), + SearchPath::vendored_stdlib(), + ) + } else { + return Err(err); + } } - })?; - - let parsed: TypeshedVersions = versions_content.parse()?; - - let search_path = SearchPath::custom_stdlib(system, &typeshed)?; - - (parsed, search_path) + } } else { tracing::debug!("Using vendored stdlib"); ( @@ -279,7 +311,17 @@ impl SearchPaths { }; let real_stdlib_path = if let Some(path) = real_stdlib_path { - Some(SearchPath::real_stdlib(system, path.clone())?) + match SearchPath::real_stdlib(system, path.clone()) { + Ok(path) => Some(path), + Err(err) => { + if *safe_mode { + tracing::debug!("Skipping invalid real-stdlib search-path: {err}"); + None + } else { + return Err(err); + } + } + } } else { None }; @@ -288,7 +330,16 @@ impl SearchPaths { for path in site_packages_paths { tracing::debug!("Adding site-packages search path `{path}`"); - site_packages.push(SearchPath::site_packages(system, path.clone())?); + match SearchPath::site_packages(system, path.clone()) { + Ok(path) => site_packages.push(path), + Err(err) => { + if settings.safe_mode { + tracing::debug!("Skipping invalid real-stdlib search-path: {err}"); + } else { + return Err(err); + } + } + } } // TODO vendor typeshed's third-party stubs as well as the stdlib and diff --git a/crates/ty_python_semantic/src/program.rs b/crates/ty_python_semantic/src/program.rs index 8f06527951e1f..f7d1cc5c2251a 100644 --- a/crates/ty_python_semantic/src/program.rs +++ b/crates/ty_python_semantic/src/program.rs @@ -184,6 +184,12 @@ pub struct SearchPathSettings { /// We should ideally only ever use this for things like goto-definition, /// where typeshed isn't the right answer. pub real_stdlib_path: Option, + + /// Settings Failure Is Not An Error. + /// + /// This is used by the default database, which we are incentivized to make infallible, + /// while still trying to "do our best" to set things up properly where we can. + pub safe_mode: bool, } impl SearchPathSettings { @@ -201,6 +207,7 @@ impl SearchPathSettings { custom_typeshed: None, site_packages_paths: vec![], real_stdlib_path: None, + safe_mode: false, } } diff --git a/crates/ty_server/src/session.rs b/crates/ty_server/src/session.rs index 24ad0ef55e5c6..6e40cdbd8727c 100644 --- a/crates/ty_server/src/session.rs +++ b/crates/ty_server/src/session.rs @@ -540,7 +540,7 @@ impl Session { )); let db_with_default_settings = - ProjectMetadata::from_options(Options::default(), root, None) + ProjectMetadata::from_options(Options::safe(), root, None) .context("Failed to convert default options to metadata") .and_then(|metadata| ProjectDatabase::new(metadata, system)) .expect("Default configuration to be valid"); diff --git a/crates/ty_test/src/lib.rs b/crates/ty_test/src/lib.rs index f66eaaf26a48d..9617e1fea3de8 100644 --- a/crates/ty_test/src/lib.rs +++ b/crates/ty_test/src/lib.rs @@ -307,6 +307,7 @@ fn run_test( custom_typeshed: custom_typeshed_path.map(SystemPath::to_path_buf), site_packages_paths, real_stdlib_path: None, + safe_mode: false, } .to_search_paths(db.system(), db.vendored()) .expect("Failed to resolve search path settings"),