diff --git a/crates/ruff/src/main.rs b/crates/ruff/src/main.rs index 94becf6841812..5ba8bd07d0807 100644 --- a/crates/ruff/src/main.rs +++ b/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() diff --git a/crates/ruff_server/src/message.rs b/crates/ruff_server/src/message.rs index 66ad75542ccbb..79d7c63ec347a 100644 --- a/crates/ruff_server/src/message.rs +++ b/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 diff --git a/crates/ruff_server/src/server.rs b/crates/ruff_server/src/server.rs index 8292a7dba142e..015ba9de3eddc 100644 --- a/crates/ruff_server/src/server.rs +++ b/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 = std::result::Result; @@ -133,6 +134,46 @@ impl Server { } pub fn run(self) -> crate::Result<()> { + type PanicHook = Box) + 'static + Sync + Send>; + struct RestorePanicHook { + hook: Option, + } + + 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,