fix(lsp): hang when caching failed (#24651)

Closes #24600

cli/lsp/language_server.rs

@@ -74,7 +74,6 @@ use super::lsp_custom::TaskDefinition;
 use super::npm::CliNpmSearchApi;
 use super::parent_process_checker;
 use super::performance::Performance;
-use super::performance::PerformanceMark;
 use super::refactor;
 use super::registries::ModuleRegistry;
 use super::resolver::LspResolver;
@@ -122,6 +121,7 @@ impl RootCertStoreProvider for LspRootCertStoreProvider {
 
 #[derive(Debug, Clone)]
 pub struct LanguageServer {
+  client: Client,
   pub inner: Arc<tokio::sync::RwLock<Inner>>,
   /// This is used to block out standard request handling until the complete
   /// user configuration has been fetched. This is done in the `initialized`
@@ -130,6 +130,7 @@ pub struct LanguageServer {
   /// `workspace/configuration` requests in the `initialize` handler. See:
   /// https://github.com/Microsoft/language-server-protocol/issues/567#issuecomment-2085131917
   init_flag: AsyncFlag,
+  performance: Arc<Performance>,
   shutdown_flag: AsyncFlag,
 }
 
@@ -223,9 +224,15 @@ pub struct Inner {
 
 impl LanguageServer {
   pub fn new(client: Client, shutdown_flag: AsyncFlag) -> Self {
+    let performance = Arc::new(Performance::default());
     Self {
-      inner: Arc::new(tokio::sync::RwLock::new(Inner::new(client))),
+      client: client.clone(),
+      inner: Arc::new(tokio::sync::RwLock::new(Inner::new(
+        client,
+        performance.clone(),
+      ))),
       init_flag: Default::default(),
+      performance,
       shutdown_flag,
     }
   }
@@ -238,9 +245,6 @@ impl LanguageServer {
     referrer: ModuleSpecifier,
     force_global_cache: bool,
   ) -> LspResult<Option<Value>> {
-    if !self.init_flag.is_raised() {
-      self.init_flag.wait_raised().await;
-    }
     async fn create_graph_for_caching(
       cli_options: CliOptions,
       roots: Vec<ModuleSpecifier>,
@@ -285,55 +289,46 @@ impl LanguageServer {
       Ok(())
     }
 
-    // prepare the cache inside the lock
+    if !self.init_flag.is_raised() {
-    let maybe_prepare_cache_result = {
+      self.init_flag.wait_raised().await;
-      let mut inner = self.inner.write().await;
-      match inner.prepare_cache(specifiers, referrer, force_global_cache) {
-        Ok(maybe_cache_result) => maybe_cache_result,
-        Err(err) => {
-          lsp_warn!("Error preparing caching: {:#}", err);
-          self
-            .inner
-            .read()
-            .await
-            .client
-            .show_message(MessageType::WARNING, err);
-          return Err(LspError::internal_error());
-        }
-      }
-    };
-    if let Some(result) = maybe_prepare_cache_result {
-      // cache outside the lock
-      let cli_options = result.cli_options;
-      let roots = result.roots;
-      let open_docs = result.open_docs;
-      let handle = spawn(async move {
-        create_graph_for_caching(cli_options, roots, open_docs).await
-      });
-      if let Err(err) = handle.await.unwrap() {
-        lsp_warn!("Error caching: {:#}", err);
-        self
-          .inner
-          .read()
-          .await
-          .client
-          .show_message(MessageType::WARNING, err);
-      }
-
-      // now get the lock back to update with the new information
-      let mut inner = self.inner.write().await;
-      inner.resolver.did_cache();
-      inner.refresh_npm_specifiers().await;
-      inner.diagnostics_server.invalidate_all();
-      inner.project_changed([], true);
-      inner
-        .ts_server
-        .cleanup_semantic_cache(inner.snapshot())
-        .await;
-      inner.send_diagnostics_update();
-      inner.send_testing_update();
-      inner.performance.measure(result.mark);
     }
+
+    // prepare the cache inside the lock
+    let mark = self
+      .performance
+      .mark_with_args("lsp.cache", (&specifiers, &referrer));
+    let prepare_cache_result = self.inner.write().await.prepare_cache(
+      specifiers,
+      referrer,
+      force_global_cache,
+    );
+
+    match prepare_cache_result {
+      Ok(result) => {
+        // cache outside the lock
+        let cli_options = result.cli_options;
+        let roots = result.roots;
+        let open_docs = result.open_docs;
+        let handle = spawn(async move {
+          create_graph_for_caching(cli_options, roots, open_docs).await
+        });
+
+        if let Err(err) = handle.await.unwrap() {
+          lsp_warn!("Error caching: {:#}", err);
+          self.client.show_message(MessageType::WARNING, err);
+        }
+
+        // now get the lock back to update with the new information
+        self.inner.write().await.post_cache().await;
+        self.performance.measure(mark);
+      }
+      Err(err) => {
+        lsp_warn!("Error preparing caching: {:#}", err);
+        self.client.show_message(MessageType::WARNING, err);
+        return Err(LspError::internal_error());
+      }
+    }
+
     Ok(Some(json!(true)))
   }
 
@@ -377,20 +372,7 @@ impl LanguageServer {
     if !self.init_flag.is_raised() {
       self.init_flag.wait_raised().await;
     }
-    let inner = self.inner.read().await;
+    self.inner.read().await.test_run_request(params).await
-    if let Some(testing_server) = &inner.maybe_testing_server {
-      match params.map(serde_json::from_value) {
-        Some(Ok(params)) => {
-          testing_server
-            .run_request(params, inner.config.workspace_settings().clone())
-            .await
-        }
-        Some(Err(err)) => Err(LspError::invalid_params(err.to_string())),
-        None => Err(LspError::invalid_params("Missing parameters")),
-      }
-    } else {
-      Err(LspError::invalid_request())
-    }
   }
 
   pub async fn test_run_cancel_request(
@@ -400,16 +382,7 @@ impl LanguageServer {
     if !self.init_flag.is_raised() {
       self.init_flag.wait_raised().await;
     }
-    if let Some(testing_server) = &self.inner.read().await.maybe_testing_server
+    self.inner.read().await.test_run_cancel_request(params)
-    {
-      match params.map(serde_json::from_value) {
-        Some(Ok(params)) => testing_server.run_cancel_request(params),
-        Some(Err(err)) => Err(LspError::invalid_params(err.to_string())),
-        None => Err(LspError::invalid_params("Missing parameters")),
-      }
-    } else {
-      Err(LspError::invalid_request())
-    }
   }
 
   pub async fn virtual_text_document(
@@ -438,10 +411,9 @@ impl LanguageServer {
   }
 
   pub async fn refresh_configuration(&self) {
-    let (client, folders, capable) = {
+    let (folders, capable) = {
       let inner = self.inner.read().await;
       (
-        inner.client.clone(),
         inner.config.workspace_folders.clone(),
         inner.config.workspace_configuration_capable(),
       )
@@ -452,7 +424,8 @@ impl LanguageServer {
       for (_, folder) in folders.as_ref() {
         scopes.push(Some(folder.uri.clone()));
       }
-      let configs = client
+      let configs = self
+        .client
         .when_outside_lsp_lock()
         .workspace_configuration(scopes)
         .await;
@@ -467,8 +440,10 @@ impl LanguageServer {
         for (folder_uri, _) in folders.as_ref() {
           folder_settings.push((folder_uri.clone(), configs.next().unwrap()));
         }
-        let mut inner = self.inner.write().await;
+        self
-        inner
+          .inner
+          .write()
+          .await
           .config
           .set_workspace_settings(unscoped, folder_settings);
       }
@@ -477,7 +452,7 @@ impl LanguageServer {
 }
 
 impl Inner {
-  fn new(client: Client) -> Self {
+  fn new(client: Client, performance: Arc<Performance>) -> Self {
     let cache = LspCache::default();
     let http_client_provider = Arc::new(HttpClientProvider::new(None, None));
     let module_registry = ModuleRegistry::new(
@@ -489,7 +464,6 @@ impl Inner {
     let npm_search_api =
       CliNpmSearchApi::new(module_registry.file_fetcher.clone());
     let documents = Documents::default();
-    let performance = Arc::new(Performance::default());
     let config = Config::default();
     let ts_server = Arc::new(TsServer::new(performance.clone()));
     let diagnostics_state = Arc::new(DiagnosticsState::default());
@@ -1181,6 +1155,36 @@ impl Inner {
     self.performance.measure(mark);
   }
 
+  async fn did_change_configuration(
+    &mut self,
+    params: DidChangeConfigurationParams,
+  ) {
+    if !self.config.workspace_configuration_capable() {
+      let config = params.settings.as_object().map(|settings| {
+        let deno =
+          serde_json::to_value(settings.get(SETTINGS_SECTION)).unwrap();
+        let javascript =
+          serde_json::to_value(settings.get("javascript")).unwrap();
+        let typescript =
+          serde_json::to_value(settings.get("typescript")).unwrap();
+        WorkspaceSettings::from_raw_settings(deno, javascript, typescript)
+      });
+      if let Some(settings) = config {
+        self.config.set_workspace_settings(settings, vec![]);
+      }
+    };
+    self.update_debug_flag();
+    self.update_global_cache().await;
+    self.refresh_workspace_files();
+    self.refresh_config_tree().await;
+    self.update_cache();
+    self.refresh_resolver().await;
+    self.refresh_documents_config().await;
+    self.diagnostics_server.invalidate_all();
+    self.send_diagnostics_update();
+    self.send_testing_update();
+  }
+
   async fn did_change_watched_files(
     &mut self,
     params: DidChangeWatchedFilesParams,
@@ -2997,98 +3001,17 @@ impl tower_lsp::LanguageServer for LanguageServer {
 
   async fn initialized(&self, _: InitializedParams) {
     self.refresh_configuration().await;
-    let mut registrations = Vec::with_capacity(2);
+    let (registrations, http_client) = {
-    let (client, http_client) = {
       let mut inner = self.inner.write().await;
-      init_log_file(inner.config.log_file());
+      let registrations = inner.initialized().await;
-      inner.update_debug_flag();
-      inner.update_global_cache().await;
-      inner.refresh_workspace_files();
-      inner.refresh_config_tree().await;
-      inner.update_cache();
-      inner.refresh_resolver().await;
-      inner.refresh_documents_config().await;
       inner.task_queue.start(self.clone());
-      self.init_flag.raise();
+      (registrations, inner.http_client_provider.clone())
-      if inner.config.did_change_watched_files_capable() {
-        // we are going to watch all the JSON files in the workspace, and the
-        // notification handler will pick up any of the changes of those files we
-        // are interested in.
-        let options = DidChangeWatchedFilesRegistrationOptions {
-          watchers: vec![FileSystemWatcher {
-            glob_pattern: GlobPattern::String(
-              "**/*.{json,jsonc,lock}".to_string(),
-            ),
-            kind: None,
-          }],
-        };
-        registrations.push(Registration {
-          id: "workspace/didChangeWatchedFiles".to_string(),
-          method: "workspace/didChangeWatchedFiles".to_string(),
-          register_options: Some(serde_json::to_value(options).unwrap()),
-        });
-      }
-      if inner.config.will_rename_files_capable() {
-        let options = FileOperationRegistrationOptions {
-          filters: vec![FileOperationFilter {
-            scheme: Some("file".to_string()),
-            pattern: FileOperationPattern {
-              glob: "**/*".to_string(),
-              matches: None,
-              options: None,
-            },
-          }],
-        };
-        registrations.push(Registration {
-          id: "workspace/willRenameFiles".to_string(),
-          method: "workspace/willRenameFiles".to_string(),
-          register_options: Some(serde_json::to_value(options).unwrap()),
-        });
-      }
-
-      if inner.config.testing_api_capable() {
-        let test_server = testing::TestServer::new(
-          inner.client.clone(),
-          inner.performance.clone(),
-          inner.config.root_uri().cloned(),
-        );
-        inner.maybe_testing_server = Some(test_server);
-      }
-
-      let mut config_events = vec![];
-      for (scope_uri, config_data) in inner.config.tree.data_by_scope().iter() {
-        if let Some(config_file) = config_data.maybe_deno_json() {
-          config_events.push(lsp_custom::DenoConfigurationChangeEvent {
-            scope_uri: scope_uri.clone(),
-            file_uri: config_file.specifier.clone(),
-            typ: lsp_custom::DenoConfigurationChangeType::Added,
-            configuration_type: lsp_custom::DenoConfigurationType::DenoJson,
-          });
-        }
-        if let Some(package_json) = config_data.maybe_pkg_json() {
-          config_events.push(lsp_custom::DenoConfigurationChangeEvent {
-            scope_uri: scope_uri.clone(),
-            file_uri: package_json.specifier(),
-            typ: lsp_custom::DenoConfigurationChangeType::Added,
-            configuration_type: lsp_custom::DenoConfigurationType::PackageJson,
-          });
-        }
-      }
-      if !config_events.is_empty() {
-        inner
-          .client
-          .send_did_change_deno_configuration_notification(
-            lsp_custom::DidChangeDenoConfigurationNotificationParams {
-              changes: config_events,
-            },
-          );
-      }
-
-      (inner.client.clone(), inner.http_client_provider.clone())
     };
+    self.init_flag.raise();
 
     for registration in registrations {
-      if let Err(err) = client
+      if let Err(err) = self
+        .client
         .when_outside_lsp_lock()
         .register_capability(vec![registration])
         .await
@@ -3098,6 +3021,7 @@ impl tower_lsp::LanguageServer for LanguageServer {
     }
 
     if upgrade_check_enabled() {
+      let client = self.client.clone();
       // spawn to avoid lsp send/sync requirement, but also just
       // to ensure this initialized method returns quickly
       spawn(async move {
@@ -3162,39 +3086,17 @@ impl tower_lsp::LanguageServer for LanguageServer {
     if !self.init_flag.is_raised() {
       self.init_flag.wait_raised().await;
     }
-    let mark = {
+    let mark = self
-      let inner = self.inner.read().await;
+      .performance
-      inner
+      .mark_with_args("lsp.did_change_configuration", &params);
-        .performance
-        .mark_with_args("lsp.did_change_configuration", &params)
-    };
     self.refresh_configuration().await;
-    let mut inner = self.inner.write().await;
+    self
-    if !inner.config.workspace_configuration_capable() {
+      .inner
-      let config = params.settings.as_object().map(|settings| {
+      .write()
-        let deno =
+      .await
-          serde_json::to_value(settings.get(SETTINGS_SECTION)).unwrap();
+      .did_change_configuration(params)
-        let javascript =
+      .await;
-          serde_json::to_value(settings.get("javascript")).unwrap();
+    self.performance.measure(mark);
-        let typescript =
-          serde_json::to_value(settings.get("typescript")).unwrap();
-        WorkspaceSettings::from_raw_settings(deno, javascript, typescript)
-      });
-      if let Some(settings) = config {
-        inner.config.set_workspace_settings(settings, vec![]);
-      }
-    };
-    inner.update_debug_flag();
-    inner.update_global_cache().await;
-    inner.refresh_workspace_files();
-    inner.refresh_config_tree().await;
-    inner.update_cache();
-    inner.refresh_resolver().await;
-    inner.refresh_documents_config().await;
-    inner.diagnostics_server.invalidate_all();
-    inner.send_diagnostics_update();
-    inner.send_testing_update();
-    inner.performance.measure(mark);
   }
 
   async fn did_change_watched_files(
@@ -3219,43 +3121,22 @@ impl tower_lsp::LanguageServer for LanguageServer {
     if !self.init_flag.is_raised() {
       self.init_flag.wait_raised().await;
     }
-    let mark = {
+    let mark = self
-      let mut inner = self.inner.write().await;
+      .performance
-      let mark = inner
+      .mark_with_args("lsp.did_change_workspace_folders", &params);
-        .performance
+    self
-        .mark_with_args("lsp.did_change_workspace_folders", &params);
+      .inner
-      let mut workspace_folders = params
+      .write()
-        .event
+      .await
-        .added
+      .pre_did_change_workspace_folders(params);
-        .into_iter()
-        .map(|folder| {
-          (
-            inner.url_map.normalize_url(&folder.uri, LspUrlKind::Folder),
-            folder,
-          )
-        })
-        .collect::<Vec<(ModuleSpecifier, WorkspaceFolder)>>();
-      for (specifier, folder) in inner.config.workspace_folders.as_ref() {
-        if !params.event.removed.is_empty()
-          && params.event.removed.iter().any(|f| f.uri == folder.uri)
-        {
-          continue;
-        }
-        workspace_folders.push((specifier.clone(), folder.clone()));
-      }
-      inner.config.set_workspace_folders(workspace_folders);
-      mark
-    };
     self.refresh_configuration().await;
-    let mut inner = self.inner.write().await;
+    self
-    inner.refresh_workspace_files();
+      .inner
-    inner.refresh_config_tree().await;
+      .write()
-    inner.refresh_resolver().await;
+      .await
-    inner.refresh_documents_config().await;
+      .post_did_change_workspace_folders()
-    inner.diagnostics_server.invalidate_all();
+      .await;
-    inner.send_diagnostics_update();
+    self.performance.measure(mark);
-    inner.send_testing_update();
-    inner.performance.measure(mark);
   }
 
   async fn document_symbol(
@@ -3517,20 +3398,101 @@ struct PrepareCacheResult {
   cli_options: CliOptions,
   roots: Vec<ModuleSpecifier>,
   open_docs: Vec<Arc<Document>>,
-  mark: PerformanceMark,
 }
 
 // These are implementations of custom commands supported by the LSP
 impl Inner {
+  async fn initialized(&mut self) -> Vec<Registration> {
+    let mut registrations = Vec::with_capacity(2);
+    init_log_file(self.config.log_file());
+    self.update_debug_flag();
+    self.update_global_cache().await;
+    self.refresh_workspace_files();
+    self.refresh_config_tree().await;
+    self.update_cache();
+    self.refresh_resolver().await;
+    self.refresh_documents_config().await;
+
+    if self.config.did_change_watched_files_capable() {
+      // we are going to watch all the JSON files in the workspace, and the
+      // notification handler will pick up any of the changes of those files we
+      // are interested in.
+      let options = DidChangeWatchedFilesRegistrationOptions {
+        watchers: vec![FileSystemWatcher {
+          glob_pattern: GlobPattern::String(
+            "**/*.{json,jsonc,lock}".to_string(),
+          ),
+          kind: None,
+        }],
+      };
+      registrations.push(Registration {
+        id: "workspace/didChangeWatchedFiles".to_string(),
+        method: "workspace/didChangeWatchedFiles".to_string(),
+        register_options: Some(serde_json::to_value(options).unwrap()),
+      });
+    }
+    if self.config.will_rename_files_capable() {
+      let options = FileOperationRegistrationOptions {
+        filters: vec![FileOperationFilter {
+          scheme: Some("file".to_string()),
+          pattern: FileOperationPattern {
+            glob: "**/*".to_string(),
+            matches: None,
+            options: None,
+          },
+        }],
+      };
+      registrations.push(Registration {
+        id: "workspace/willRenameFiles".to_string(),
+        method: "workspace/willRenameFiles".to_string(),
+        register_options: Some(serde_json::to_value(options).unwrap()),
+      });
+    }
+
+    if self.config.testing_api_capable() {
+      let test_server = testing::TestServer::new(
+        self.client.clone(),
+        self.performance.clone(),
+        self.config.root_uri().cloned(),
+      );
+      self.maybe_testing_server = Some(test_server);
+    }
+
+    let mut config_events = vec![];
+    for (scope_uri, config_data) in self.config.tree.data_by_scope().iter() {
+      if let Some(config_file) = config_data.maybe_deno_json() {
+        config_events.push(lsp_custom::DenoConfigurationChangeEvent {
+          scope_uri: scope_uri.clone(),
+          file_uri: config_file.specifier.clone(),
+          typ: lsp_custom::DenoConfigurationChangeType::Added,
+          configuration_type: lsp_custom::DenoConfigurationType::DenoJson,
+        });
+      }
+      if let Some(package_json) = config_data.maybe_pkg_json() {
+        config_events.push(lsp_custom::DenoConfigurationChangeEvent {
+          scope_uri: scope_uri.clone(),
+          file_uri: package_json.specifier(),
+          typ: lsp_custom::DenoConfigurationChangeType::Added,
+          configuration_type: lsp_custom::DenoConfigurationType::PackageJson,
+        });
+      }
+    }
+    if !config_events.is_empty() {
+      self.client.send_did_change_deno_configuration_notification(
+        lsp_custom::DidChangeDenoConfigurationNotificationParams {
+          changes: config_events,
+        },
+      );
+    }
+    registrations
+  }
+
   fn prepare_cache(
     &mut self,
     specifiers: Vec<ModuleSpecifier>,
     referrer: ModuleSpecifier,
     force_global_cache: bool,
-  ) -> Result<Option<PrepareCacheResult>, AnyError> {
+  ) -> Result<PrepareCacheResult, AnyError> {
-    let mark = self
-      .performance
-      .mark_with_args("lsp.cache", (&specifiers, &referrer));
     let config_data = self.config.tree.data_for_specifier(&referrer);
     let mut roots = if !specifiers.is_empty() {
       specifiers
@@ -3612,12 +3574,57 @@ impl Inner {
     )?;
 
     let open_docs = self.documents.documents(DocumentsFilter::OpenDiagnosable);
-    Ok(Some(PrepareCacheResult {
+    Ok(PrepareCacheResult {
       cli_options,
       open_docs,
       roots,
-      mark,
+    })
-    }))
+  }
+
+  async fn post_cache(&mut self) {
+    self.resolver.did_cache();
+    self.refresh_npm_specifiers().await;
+    self.diagnostics_server.invalidate_all();
+    self.project_changed([], true);
+    self.ts_server.cleanup_semantic_cache(self.snapshot()).await;
+    self.send_diagnostics_update();
+    self.send_testing_update();
+  }
+
+  fn pre_did_change_workspace_folders(
+    &mut self,
+    params: DidChangeWorkspaceFoldersParams,
+  ) {
+    let mut workspace_folders = params
+      .event
+      .added
+      .into_iter()
+      .map(|folder| {
+        (
+          self.url_map.normalize_url(&folder.uri, LspUrlKind::Folder),
+          folder,
+        )
+      })
+      .collect::<Vec<(ModuleSpecifier, WorkspaceFolder)>>();
+    for (specifier, folder) in self.config.workspace_folders.as_ref() {
+      if !params.event.removed.is_empty()
+        && params.event.removed.iter().any(|f| f.uri == folder.uri)
+      {
+        continue;
+      }
+      workspace_folders.push((specifier.clone(), folder.clone()));
+    }
+    self.config.set_workspace_folders(workspace_folders);
+  }
+
+  async fn post_did_change_workspace_folders(&mut self) {
+    self.refresh_workspace_files();
+    self.refresh_config_tree().await;
+    self.refresh_resolver().await;
+    self.refresh_documents_config().await;
+    self.diagnostics_server.invalidate_all();
+    self.send_diagnostics_update();
+    self.send_testing_update();
   }
 
   fn get_performance(&self) -> Value {
@@ -3625,6 +3632,40 @@ impl Inner {
     json!({ "averages": averages })
   }
 
+  async fn test_run_request(
+    &self,
+    params: Option<Value>,
+  ) -> LspResult<Option<Value>> {
+    if let Some(testing_server) = &self.maybe_testing_server {
+      match params.map(serde_json::from_value) {
+        Some(Ok(params)) => {
+          testing_server
+            .run_request(params, self.config.workspace_settings().clone())
+            .await
+        }
+        Some(Err(err)) => Err(LspError::invalid_params(err.to_string())),
+        None => Err(LspError::invalid_params("Missing parameters")),
+      }
+    } else {
+      Err(LspError::invalid_request())
+    }
+  }
+
+  fn test_run_cancel_request(
+    &self,
+    params: Option<Value>,
+  ) -> LspResult<Option<Value>> {
+    if let Some(testing_server) = &self.maybe_testing_server {
+      match params.map(serde_json::from_value) {
+        Some(Ok(params)) => testing_server.run_cancel_request(params),
+        Some(Err(err)) => Err(LspError::invalid_params(err.to_string())),
+        None => Err(LspError::invalid_params("Missing parameters")),
+      }
+    } else {
+      Err(LspError::invalid_request())
+    }
+  }
+
   fn task_definitions(&self) -> LspResult<Vec<TaskDefinition>> {
     let mut result = vec![];
     for config_file in self.config.tree.config_files() {