Skip to content

Commit

Permalink
ruff server: Important errors are now shown as popups (#10951)
Browse files Browse the repository at this point in the history
## 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)
  • Loading branch information
snowsignal authored Apr 16, 2024
1 parent eab3c4e commit 2882604
Show file tree
Hide file tree
Showing 6 changed files with 81 additions and 1 deletion.
2 changes: 2 additions & 0 deletions crates/ruff_server/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ mod edit;
mod fix;
mod format;
mod lint;
#[macro_use]
mod message;
mod server;
mod session;

Expand Down
62 changes: 62 additions & 0 deletions crates/ruff_server/src/message.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
use std::sync::OnceLock;

use lsp_types::notification::Notification;

use crate::server::ClientSender;

static MESSENGER: OnceLock<ClientSender> = 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)
};
}
4 changes: 4 additions & 0 deletions crates/ruff_server/src/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ mod api;
mod client;
mod schedule;

pub(crate) use client::ClientSender;

pub(crate) type Result<T> = std::result::Result<T, api::Error>;

pub struct Server {
Expand All @@ -44,6 +46,8 @@ impl Server {
pub fn new(worker_threads: NonZeroUsize) -> crate::Result<Self> {
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)?;
Expand Down
10 changes: 10 additions & 0 deletions crates/ruff_server/src/server/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
Expand Down Expand Up @@ -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()
})
}
Expand Down Expand Up @@ -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.");
}
}))
}
Expand All @@ -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.");
}
})
}))
Expand Down Expand Up @@ -182,6 +188,10 @@ fn respond<Req>(
) 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}");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
},
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_server/src/session/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
)
Expand Down

0 comments on commit 2882604

Please sign in to comment.