Make server panic hook more error resilient (#12610)

Author: MichaReiser

crates/ruff/src/main.rs

@@ -87,14 +87,19 @@ pub fn main() -> ExitCode {
         Err(err) => {
             #[allow(clippy::print_stderr)]
             {
+                use std::io::Write;
+
+                // Use `writeln` instead of `eprintln` to avoid panicking when the stderr pipe is broken.
+                let mut stderr = std::io::stderr().lock();
+
                 // This communicates that this isn't a linter error but ruff itself hard-errored for
                 // some reason (e.g. failed to resolve the configuration)
-                eprintln!("{}", "ruff failed".red().bold());
+                writeln!(stderr, "{}", "ruff failed".red().bold()).ok();
                 // Currently we generally only see one error, but e.g. with io errors when resolving
                 // the configuration it is help to chain errors ("resolving configuration failed" ->
                 // "failed to read file: subdir/pyproject.toml")
                 for cause in err.chain() {
-                    eprintln!("  {} {cause}", "Cause:".bold());
+                    writeln!(stderr, "  {} {cause}", "Cause:".bold()).ok();
                 }
             }
             ExitStatus::Error.into()

crates/ruff_server/src/message.rs

@@ -1,6 +1,6 @@
-use std::sync::OnceLock;
-
+use anyhow::Context;
 use lsp_types::notification::Notification;
+use std::sync::OnceLock;
 
 use crate::server::ClientSender;
 
@@ -10,53 +10,31 @@ pub(crate) fn init_messenger(client_sender: ClientSender) {
     MESSENGER
         .set(client_sender)
         .expect("messenger should only be initialized once");
-
-    // unregister any previously registered panic hook
-    let _ = std::panic::take_hook();
-
-    // When we panic, try to notify the client.
-    std::panic::set_hook(Box::new(move |panic_info| {
-        if let Some(messenger) = MESSENGER.get() {
-            let _ = messenger.send(lsp_server::Message::Notification(
-                lsp_server::Notification {
-                    method: lsp_types::notification::ShowMessage::METHOD.into(),
-                    params: serde_json::to_value(lsp_types::ShowMessageParams {
-                        typ: lsp_types::MessageType::ERROR,
-                        message: String::from(
-                            "The Ruff language server exited with a panic. See the logs for more details."
-                        ),
-                    })
-                    .unwrap_or_default(),
-                },
-            ));
-        }
-
-        let backtrace = std::backtrace::Backtrace::force_capture();
-        tracing::error!("{panic_info}\n{backtrace}");
-        #[allow(clippy::print_stderr)]
-        {
-            // we also need to print to stderr directly in case tracing hasn't
-            // been initialized.
-            eprintln!("{panic_info}\n{backtrace}");
-        }
-    }));
 }
 
 pub(crate) fn show_message(message: String, message_type: lsp_types::MessageType) {
+    try_show_message(message, message_type).unwrap();
+}
+
+pub(super) fn try_show_message(
+    message: String,
+    message_type: lsp_types::MessageType,
+) -> crate::Result<()> {
     MESSENGER
         .get()
-        .expect("messenger should be initialized")
+        .ok_or_else(|| anyhow::anyhow!("messenger not initialized"))?
         .send(lsp_server::Message::Notification(
             lsp_server::Notification {
                 method: lsp_types::notification::ShowMessage::METHOD.into(),
                 params: serde_json::to_value(lsp_types::ShowMessageParams {
                     typ: message_type,
                     message,
-                })
-                .unwrap(),
+                })?,
             },
         ))
-        .expect("message should send");
+        .context("Failed to send message")?;
+
+    Ok(())
 }
 
 /// Sends an error to the client with a formatted message. The error is sent in a

crates/ruff_server/src/server.rs

@@ -1,10 +1,10 @@
 //! Scheduling, I/O, and API endpoints.
 
-use std::num::NonZeroUsize;
-use std::str::FromStr;
-
 use lsp_server as lsp;
 use lsp_types as types;
+use std::num::NonZeroUsize;
+use std::panic::PanicInfo;
+use std::str::FromStr;
 use types::ClientCapabilities;
 use types::CodeActionKind;
 use types::CodeActionOptions;
@@ -36,6 +36,7 @@ mod client;
 mod connection;
 mod schedule;
 
+use crate::message::try_show_message;
 pub(crate) use connection::ClientSender;
 
 pub(crate) type Result<T> = std::result::Result<T, api::Error>;
@@ -133,6 +134,46 @@ impl Server {
     }
 
     pub fn run(self) -> crate::Result<()> {
+        type PanicHook = Box<dyn Fn(&PanicInfo<'_>) + 'static + Sync + Send>;
+        struct RestorePanicHook {
+            hook: Option<PanicHook>,
+        }
+
+        impl Drop for RestorePanicHook {
+            fn drop(&mut self) {
+                if let Some(hook) = self.hook.take() {
+                    std::panic::set_hook(hook);
+                }
+            }
+        }
+
+        // unregister any previously registered panic hook
+        // The hook will be restored when this function exits.
+        let _ = RestorePanicHook {
+            hook: Some(std::panic::take_hook()),
+        };
+
+        // When we panic, try to notify the client.
+        std::panic::set_hook(Box::new(move |panic_info| {
+            use std::io::Write;
+
+            let backtrace = std::backtrace::Backtrace::force_capture();
+            tracing::error!("{panic_info}\n{backtrace}");
+
+            // we also need to print to stderr directly for when using `$logTrace` because
+            // the message won't be sent to the client.
+            // But don't use `eprintln` because `eprintln` itself may panic if the pipe is broken.
+            let mut stderr = std::io::stderr().lock();
+            writeln!(stderr, "{panic_info}\n{backtrace}").ok();
+
+            try_show_message(
+                "The Ruff language server exited with a panic. See the logs for more details."
+                    .to_string(),
+                lsp_types::MessageType::ERROR,
+            )
+            .ok();
+        }));
+
         event_loop_thread(move || {
             Self::event_loop(
                 &self.connection,