From 28826044510a455df4f32cd9ad974878831e7db4 Mon Sep 17 00:00:00 2001 From: Jane Lewis Date: Tue, 16 Apr 2024 11:32:53 -0700 Subject: [PATCH] `ruff server`: Important errors are now shown as popups (#10951) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Fixes #10866. Introduces the `show_err_msg!` macro which will send a message to be shown as a popup to the client via the `window/showMessage` LSP method. ## Test Plan Insert various `show_err_msg!` calls in common code paths (for example, at the beginning of `event_loop`) and confirm that these messages appear in your editor. To test that panicking works correctly, add this to the top of the `fn run` definition in `crates/ruff_server/src/server/api/requests/execute_command.rs`: ```rust panic!("This should appear"); ``` Then, try running a command like `Ruff: Format document` from the command palette (`Ctrl/Cmd+Shift+P`). You should see the following messages appear: ![Screenshot 2024-04-16 at 11 20 57 AM](https://github.com/astral-sh/ruff/assets/19577865/ae430da6-82c3-4841-a419-664ff34034e8) --- crates/ruff_server/src/lib.rs | 2 + crates/ruff_server/src/message.rs | 62 +++++++++++++++++++ crates/ruff_server/src/server.rs | 4 ++ crates/ruff_server/src/server/api.rs | 10 +++ .../server/api/requests/execute_command.rs | 3 +- crates/ruff_server/src/session/settings.rs | 1 + 6 files changed, 81 insertions(+), 1 deletion(-) create mode 100644 crates/ruff_server/src/message.rs diff --git a/crates/ruff_server/src/lib.rs b/crates/ruff_server/src/lib.rs index 4814436678f6f..5ec26c9d399a6 100644 --- a/crates/ruff_server/src/lib.rs +++ b/crates/ruff_server/src/lib.rs @@ -8,6 +8,8 @@ mod edit; mod fix; mod format; mod lint; +#[macro_use] +mod message; mod server; mod session; diff --git a/crates/ruff_server/src/message.rs b/crates/ruff_server/src/message.rs new file mode 100644 index 0000000000000..034771aea828c --- /dev/null +++ b/crates/ruff_server/src/message.rs @@ -0,0 +1,62 @@ +use std::sync::OnceLock; + +use lsp_types::notification::Notification; + +use crate::server::ClientSender; + +static MESSENGER: OnceLock = OnceLock::new(); + +pub(crate) fn init_messenger(client_sender: &ClientSender) { + MESSENGER + .set(client_sender.clone()) + .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}"); + })); +} + +pub(crate) fn show_message(message: String, message_type: lsp_types::MessageType) { + MESSENGER + .get() + .expect("messenger should be 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"); +} + +/// Sends an error to the client with a formatted message. The error is sent in a +/// `window/showMessage` notification. +macro_rules! show_err_msg { + ($msg:expr$(, $($arg:tt),*)?) => { + crate::message::show_message(::core::format_args!($msg, $($($arg),*)?).to_string(), lsp_types::MessageType::ERROR) + }; +} diff --git a/crates/ruff_server/src/server.rs b/crates/ruff_server/src/server.rs index 0374217fb95b3..647e42f353eab 100644 --- a/crates/ruff_server/src/server.rs +++ b/crates/ruff_server/src/server.rs @@ -30,6 +30,8 @@ mod api; mod client; mod schedule; +pub(crate) use client::ClientSender; + pub(crate) type Result = std::result::Result; pub struct Server { @@ -44,6 +46,8 @@ impl Server { pub fn new(worker_threads: NonZeroUsize) -> crate::Result { let (conn, threads) = lsp::Connection::stdio(); + crate::message::init_messenger(&conn.sender); + let (id, params) = conn.initialize_start()?; let init_params: types::InitializeParams = serde_json::from_value(params)?; diff --git a/crates/ruff_server/src/server/api.rs b/crates/ruff_server/src/server/api.rs index d649379d48317..e3ffde76c7512 100644 --- a/crates/ruff_server/src/server/api.rs +++ b/crates/ruff_server/src/server/api.rs @@ -55,6 +55,9 @@ pub(super) fn request<'a>(req: server::Request) -> Task<'a> { } .unwrap_or_else(|err| { tracing::error!("Encountered error when routing request with ID {id}: {err}"); + show_err_msg!( + "Ruff failed to handle a request from the editor. Check the logs for more details." + ); let result: Result<()> = Err(err); Task::immediate(id, result) }) @@ -84,6 +87,7 @@ pub(super) fn notification<'a>(notif: server::Notification) -> Task<'a> { } .unwrap_or_else(|err| { tracing::error!("Encountered error when routing notification: {err}"); + show_err_msg!("Ruff failed to handle a notification from the editor. Check the logs for more details."); Task::nothing() }) } @@ -122,6 +126,7 @@ fn local_notification_task<'a, N: traits::SyncNotificationHandler>( Ok(Task::local(move |session, notifier, _, _| { if let Err(err) = N::run(session, notifier, params) { tracing::error!("An error occurred while running {id}: {err}"); + show_err_msg!("Ruff encountered a problem. Check the logs for more details."); } })) } @@ -140,6 +145,7 @@ fn background_notification_thread<'a, N: traits::BackgroundDocumentNotificationH Box::new(move |notifier, _| { if let Err(err) = N::run_with_snapshot(snapshot, notifier, params) { tracing::error!("An error occurred while running {id}: {err}"); + show_err_msg!("Ruff encountered a problem. Check the logs for more details."); } }) })) @@ -182,6 +188,10 @@ fn respond( ) where Req: traits::RequestHandler, { + if let Err(err) = &result { + tracing::error!("An error occurred with result ID {id}: {err}"); + show_err_msg!("Ruff encountered a problem. Check the logs for more details."); + } if let Err(err) = responder.respond(id, result) { tracing::error!("Failed to send response: {err}"); } diff --git a/crates/ruff_server/src/server/api/requests/execute_command.rs b/crates/ruff_server/src/server/api/requests/execute_command.rs index 758358e325022..10ee29443357f 100644 --- a/crates/ruff_server/src/server/api/requests/execute_command.rs +++ b/crates/ruff_server/src/server/api/requests/execute_command.rs @@ -145,7 +145,8 @@ fn apply_edit( let reason = response .failure_reason .unwrap_or_else(|| String::from("unspecified reason")); - tracing::error!("Failed to apply workspace edit: {}", reason); + tracing::error!("Failed to apply workspace edit: {reason}"); + show_err_msg!("Ruff was unable to apply edits: {reason}"); } Task::nothing() }, diff --git a/crates/ruff_server/src/session/settings.rs b/crates/ruff_server/src/session/settings.rs index a29692a63c419..de30b922d75cf 100644 --- a/crates/ruff_server/src/session/settings.rs +++ b/crates/ruff_server/src/session/settings.rs @@ -100,6 +100,7 @@ impl AllSettings { serde_json::from_value(options) .map_err(|err| { tracing::error!("Failed to deserialize initialization options: {err}. Falling back to default client settings..."); + show_err_msg!("Ruff received invalid client settings - falling back to default client settings."); }) .unwrap_or_default(), )