Shutdown language servers better (#35038)

Kirill Bulatov created 4 months ago

Follow-up of https://github.com/zed-industries/zed/pull/33417

* adjust prettier mock LSP to handle `shutdown` and `exit` messages
* removed another `?.log_err()` backtrace from logs and improved the
logging info
* always handle the last parts of the shutdown logic even if the
shutdown response had failed

Release Notes:

- N/A

Change summary

crates/lsp/src/lsp.rs                  | 58 ++++++++++++++-------------
crates/prettier/src/prettier_server.js |  4 +
2 files changed, 34 insertions(+), 28 deletions(-)

Detailed changes

crates/lsp/src/lsp.rs

@@ -877,39 +877,41 @@ impl LanguageServer {
 
             let server = self.server.clone();
             let name = self.name.clone();
+            let server_id = self.server_id;
             let mut timer = self.executor.timer(SERVER_SHUTDOWN_TIMEOUT).fuse();
-            Some(
-                async move {
-                    log::debug!("language server shutdown started");
-
-                    select! {
-                        request_result = shutdown_request.fuse() => {
-                            match request_result {
-                                ConnectionResult::Timeout => {
-                                    log::warn!("timeout waiting for language server {name} to shutdown");
-                                },
-                                ConnectionResult::ConnectionReset => {},
-                                ConnectionResult::Result(r) => r?,
-                            }
+            Some(async move {
+                log::debug!("language server shutdown started");
+
+                select! {
+                    request_result = shutdown_request.fuse() => {
+                        match request_result {
+                            ConnectionResult::Timeout => {
+                                log::warn!("timeout waiting for language server {name} (id {server_id}) to shutdown");
+                            },
+                            ConnectionResult::ConnectionReset => {
+                                log::warn!("language server {name} (id {server_id}) closed the shutdown request connection");
+                            },
+                            ConnectionResult::Result(Err(e)) => {
+                                log::error!("Shutdown request failure, server {name} (id {server_id}): {e:#}");
+                            },
+                            ConnectionResult::Result(Ok(())) => {}
                         }
-
-                        _ = timer => {
-                            log::info!("timeout waiting for language server {name} to shutdown");
-                        },
                     }
 
-                    response_handlers.lock().take();
-                    Self::notify_internal::<notification::Exit>(&outbound_tx, &()).ok();
-                    outbound_tx.close();
-                    output_done.recv().await;
-                    server.lock().take().map(|mut child| child.kill());
-                    log::debug!("language server shutdown finished");
-
-                    drop(tasks);
-                    anyhow::Ok(())
+                    _ = timer => {
+                        log::info!("timeout waiting for language server {name} (id {server_id}) to shutdown");
+                    },
                 }
-                .log_err(),
-            )
+
+                response_handlers.lock().take();
+                Self::notify_internal::<notification::Exit>(&outbound_tx, &()).ok();
+                outbound_tx.close();
+                output_done.recv().await;
+                server.lock().take().map(|mut child| child.kill());
+                drop(tasks);
+                log::debug!("language server shutdown finished");
+                Some(())
+            })
         } else {
             None
         }

crates/prettier/src/prettier_server.js

@@ -152,6 +152,10 @@ async function handleMessage(message, prettier) {
     throw new Error(`Message method is undefined: ${JSON.stringify(message)}`);
   } else if (method == "initialized") {
     return;
+  } else if (method === "shutdown") {
+    sendResponse({ result: {} });
+  } else if (method == "exit") {
+    process.exit(0);
   }
 
   if (id === undefined) {