From a5daba2b6936f1b2b56ec67d25b65a42f517201a Mon Sep 17 00:00:00 2001 From: Jane Lewis Date: Mon, 15 Apr 2024 03:03:08 -0700 Subject: [PATCH 1/5] Implement `showMessage` handler and `show_err_msg!` macro --- crates/ruff_server/src/lib.rs | 2 + crates/ruff_server/src/message.rs | 63 +++++++++++++++++++++++++++++++ crates/ruff_server/src/server.rs | 4 ++ 3 files changed, 69 insertions(+) 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..526272fdec0ac --- /dev/null +++ b/crates/ruff_server/src/message.rs @@ -0,0 +1,63 @@ +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: format!( + "The Ruff language server exited with a panic: {panic_info}" + ), + }) + .unwrap_or_default(), + }, + )); + } + + let backtrace = std::backtrace::Backtrace::force_capture(); + #[allow(clippy::print_stderr)] + { + eprintln!("{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"); +} + +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 dfdea1e252154..b21549b6f442b 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)?; From 6e0b6ca0c8083704e6bd971ceeb7451ce2bc18b2 Mon Sep 17 00:00:00 2001 From: Jane Lewis Date: Mon, 15 Apr 2024 03:04:43 -0700 Subject: [PATCH 2/5] Introduce `show_err_msg!` to significant error paths --- crates/ruff_server/src/server/api.rs | 8 ++++++++ .../src/server/api/requests/execute_command.rs | 3 ++- crates/ruff_server/src/session/settings.rs | 1 + 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/crates/ruff_server/src/server/api.rs b/crates/ruff_server/src/server/api.rs index d649379d48317..38166fdf68795 100644 --- a/crates/ruff_server/src/server/api.rs +++ b/crates/ruff_server/src/server/api.rs @@ -55,6 +55,7 @@ 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 error log for more details."); let result: Result<()> = Err(err); Task::immediate(id, result) }) @@ -84,6 +85,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 error log for more details."); Task::nothing() }) } @@ -122,6 +124,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 error log for more details."); } })) } @@ -140,6 +143,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 error log for more details."); } }) })) @@ -182,6 +186,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 error log 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 c8cdb7fdec05a..28b309b5978e0 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(), ) From 97f24bba6d10e7ebf360f56f9553aa4c5ba08eca Mon Sep 17 00:00:00 2001 From: Jane Lewis Date: Mon, 15 Apr 2024 19:13:52 -0700 Subject: [PATCH 3/5] Document `show_err_msg!` --- crates/ruff_server/src/message.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/ruff_server/src/message.rs b/crates/ruff_server/src/message.rs index 526272fdec0ac..1a14d0cbbad0e 100644 --- a/crates/ruff_server/src/message.rs +++ b/crates/ruff_server/src/message.rs @@ -56,6 +56,8 @@ pub(crate) fn show_message(message: String, message_type: lsp_types::MessageType .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) From 6cec60389ac734c3be320efa3ad26711f00d5c78 Mon Sep 17 00:00:00 2001 From: Jane Lewis Date: Mon, 15 Apr 2024 19:15:06 -0700 Subject: [PATCH 4/5] 'error log' -> 'logs' --- crates/ruff_server/src/server/api.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/crates/ruff_server/src/server/api.rs b/crates/ruff_server/src/server/api.rs index 38166fdf68795..e3ffde76c7512 100644 --- a/crates/ruff_server/src/server/api.rs +++ b/crates/ruff_server/src/server/api.rs @@ -55,7 +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 error log for more details."); + 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) }) @@ -85,7 +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 error log for more details."); + show_err_msg!("Ruff failed to handle a notification from the editor. Check the logs for more details."); Task::nothing() }) } @@ -124,7 +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 error log for more details."); + show_err_msg!("Ruff encountered a problem. Check the logs for more details."); } })) } @@ -143,7 +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 error log for more details."); + show_err_msg!("Ruff encountered a problem. Check the logs for more details."); } }) })) @@ -188,7 +190,7 @@ fn respond( { if let Err(err) = &result { tracing::error!("An error occurred with result ID {id}: {err}"); - show_err_msg!("Ruff encountered a problem. Check the error log for more details."); + 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}"); From 11c9136a5762ffd6edcdbcb136c9eeb07b148afa Mon Sep 17 00:00:00 2001 From: Jane Lewis Date: Tue, 16 Apr 2024 11:22:15 -0700 Subject: [PATCH 5/5] Print panic message with tracing::error --- crates/ruff_server/src/message.rs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/crates/ruff_server/src/message.rs b/crates/ruff_server/src/message.rs index 1a14d0cbbad0e..034771aea828c 100644 --- a/crates/ruff_server/src/message.rs +++ b/crates/ruff_server/src/message.rs @@ -22,8 +22,8 @@ pub(crate) fn init_messenger(client_sender: &ClientSender) { method: lsp_types::notification::ShowMessage::METHOD.into(), params: serde_json::to_value(lsp_types::ShowMessageParams { typ: lsp_types::MessageType::ERROR, - message: format!( - "The Ruff language server exited with a panic: {panic_info}" + message: String::from( + "The Ruff language server exited with a panic. See the logs for more details." ), }) .unwrap_or_default(), @@ -32,10 +32,7 @@ pub(crate) fn init_messenger(client_sender: &ClientSender) { } let backtrace = std::backtrace::Backtrace::force_capture(); - #[allow(clippy::print_stderr)] - { - eprintln!("{panic_info}\n{backtrace}"); - } + tracing::error!("{panic_info}\n{backtrace}"); })); }