Auto merge of #18105 - Veykril:push-rquxwznuuwpu, r=Veykril
fix: Faulty notifications should not bring down the server Fixes https://github.com/rust-lang/rust-analyzer/issues/18055, if a client sends us an unregistered document path in a did save notification it would force us to exit the thread. That is obviously not great behavior, we should be more fallible here
This commit is contained in:
commit
628a3d78f1
@ -325,14 +325,14 @@ impl NotificationDispatcher<'_> {
|
||||
pub(crate) fn on_sync_mut<N>(
|
||||
&mut self,
|
||||
f: fn(&mut GlobalState, N::Params) -> anyhow::Result<()>,
|
||||
) -> anyhow::Result<&mut Self>
|
||||
) -> &mut Self
|
||||
where
|
||||
N: lsp_types::notification::Notification,
|
||||
N::Params: DeserializeOwned + Send + Debug,
|
||||
{
|
||||
let not = match self.not.take() {
|
||||
Some(it) => it,
|
||||
None => return Ok(self),
|
||||
None => return self,
|
||||
};
|
||||
|
||||
let _guard = tracing::info_span!("notification", method = ?not.method).entered();
|
||||
@ -344,7 +344,7 @@ pub(crate) fn on_sync_mut<N>(
|
||||
}
|
||||
Err(ExtractError::MethodMismatch(not)) => {
|
||||
self.not = Some(not);
|
||||
return Ok(self);
|
||||
return self;
|
||||
}
|
||||
};
|
||||
|
||||
@ -355,8 +355,10 @@ pub(crate) fn on_sync_mut<N>(
|
||||
version(),
|
||||
N::METHOD
|
||||
));
|
||||
f(self.global_state, params)?;
|
||||
Ok(self)
|
||||
if let Err(e) = f(self.global_state, params) {
|
||||
tracing::error!(handler = %N::METHOD, error = %e, "notification handler failed");
|
||||
}
|
||||
self
|
||||
}
|
||||
|
||||
pub(crate) fn finish(&mut self) {
|
||||
|
@ -196,7 +196,7 @@ fn run(mut self, inbox: Receiver<lsp_server::Message>) -> anyhow::Result<()> {
|
||||
) {
|
||||
return Ok(());
|
||||
}
|
||||
self.handle_event(event)?;
|
||||
self.handle_event(event);
|
||||
}
|
||||
|
||||
Err(anyhow::anyhow!("A receiver has been dropped, something panicked!"))
|
||||
@ -278,7 +278,7 @@ fn next_event(
|
||||
.map(Some)
|
||||
}
|
||||
|
||||
fn handle_event(&mut self, event: Event) -> anyhow::Result<()> {
|
||||
fn handle_event(&mut self, event: Event) {
|
||||
let loop_start = Instant::now();
|
||||
let _p = tracing::info_span!("GlobalState::handle_event", event = %event).entered();
|
||||
|
||||
@ -295,7 +295,7 @@ fn handle_event(&mut self, event: Event) -> anyhow::Result<()> {
|
||||
match event {
|
||||
Event::Lsp(msg) => match msg {
|
||||
lsp_server::Message::Request(req) => self.on_new_request(loop_start, req),
|
||||
lsp_server::Message::Notification(not) => self.on_notification(not)?,
|
||||
lsp_server::Message::Notification(not) => self.on_notification(not),
|
||||
lsp_server::Message::Response(resp) => self.complete_request(resp),
|
||||
},
|
||||
Event::QueuedTask(task) => {
|
||||
@ -487,7 +487,6 @@ fn handle_event(&mut self, event: Event) -> anyhow::Result<()> {
|
||||
"overly long loop turn took {loop_duration:?} (event handling took {event_handling_duration:?}): {event_dbg_msg}"
|
||||
));
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn prime_caches(&mut self, cause: String) {
|
||||
@ -1116,37 +1115,32 @@ fn on_request(&mut self, req: Request) {
|
||||
}
|
||||
|
||||
/// Handles an incoming notification.
|
||||
fn on_notification(&mut self, not: Notification) -> anyhow::Result<()> {
|
||||
fn on_notification(&mut self, not: Notification) {
|
||||
let _p =
|
||||
span!(Level::INFO, "GlobalState::on_notification", not.method = ?not.method).entered();
|
||||
use crate::handlers::notification as handlers;
|
||||
use lsp_types::notification as notifs;
|
||||
|
||||
NotificationDispatcher { not: Some(not), global_state: self }
|
||||
.on_sync_mut::<notifs::Cancel>(handlers::handle_cancel)?
|
||||
.on_sync_mut::<notifs::Cancel>(handlers::handle_cancel)
|
||||
.on_sync_mut::<notifs::WorkDoneProgressCancel>(
|
||||
handlers::handle_work_done_progress_cancel,
|
||||
)?
|
||||
.on_sync_mut::<notifs::DidOpenTextDocument>(handlers::handle_did_open_text_document)?
|
||||
.on_sync_mut::<notifs::DidChangeTextDocument>(
|
||||
handlers::handle_did_change_text_document,
|
||||
)?
|
||||
.on_sync_mut::<notifs::DidCloseTextDocument>(handlers::handle_did_close_text_document)?
|
||||
.on_sync_mut::<notifs::DidSaveTextDocument>(handlers::handle_did_save_text_document)?
|
||||
)
|
||||
.on_sync_mut::<notifs::DidOpenTextDocument>(handlers::handle_did_open_text_document)
|
||||
.on_sync_mut::<notifs::DidChangeTextDocument>(handlers::handle_did_change_text_document)
|
||||
.on_sync_mut::<notifs::DidCloseTextDocument>(handlers::handle_did_close_text_document)
|
||||
.on_sync_mut::<notifs::DidSaveTextDocument>(handlers::handle_did_save_text_document)
|
||||
.on_sync_mut::<notifs::DidChangeConfiguration>(
|
||||
handlers::handle_did_change_configuration,
|
||||
)?
|
||||
)
|
||||
.on_sync_mut::<notifs::DidChangeWorkspaceFolders>(
|
||||
handlers::handle_did_change_workspace_folders,
|
||||
)?
|
||||
.on_sync_mut::<notifs::DidChangeWatchedFiles>(
|
||||
handlers::handle_did_change_watched_files,
|
||||
)?
|
||||
.on_sync_mut::<lsp_ext::CancelFlycheck>(handlers::handle_cancel_flycheck)?
|
||||
.on_sync_mut::<lsp_ext::ClearFlycheck>(handlers::handle_clear_flycheck)?
|
||||
.on_sync_mut::<lsp_ext::RunFlycheck>(handlers::handle_run_flycheck)?
|
||||
.on_sync_mut::<lsp_ext::AbortRunTest>(handlers::handle_abort_run_test)?
|
||||
)
|
||||
.on_sync_mut::<notifs::DidChangeWatchedFiles>(handlers::handle_did_change_watched_files)
|
||||
.on_sync_mut::<lsp_ext::CancelFlycheck>(handlers::handle_cancel_flycheck)
|
||||
.on_sync_mut::<lsp_ext::ClearFlycheck>(handlers::handle_clear_flycheck)
|
||||
.on_sync_mut::<lsp_ext::RunFlycheck>(handlers::handle_run_flycheck)
|
||||
.on_sync_mut::<lsp_ext::AbortRunTest>(handlers::handle_abort_run_test)
|
||||
.finish();
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
Loading…
Reference in New Issue
Block a user