From 3b8fe6d50037b7d7d5d4ce6289a6c0dcb18ad19b Mon Sep 17 00:00:00 2001
From: Lukas Wirth <lukastw97@gmail.com>
Date: Thu, 12 Sep 2024 08:03:14 +0200
Subject: [PATCH] fix: Faulty notifications should not bring down the server

---
 crates/rust-analyzer/src/handlers/dispatch.rs | 12 +++---
 crates/rust-analyzer/src/main_loop.rs         | 40 ++++++++-----------
 2 files changed, 24 insertions(+), 28 deletions(-)

diff --git a/crates/rust-analyzer/src/handlers/dispatch.rs b/crates/rust-analyzer/src/handlers/dispatch.rs
index f03de8ce0f..ed7bf27843 100644
--- a/crates/rust-analyzer/src/handlers/dispatch.rs
+++ b/crates/rust-analyzer/src/handlers/dispatch.rs
@@ -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 @@ impl NotificationDispatcher<'_> {
             }
             Err(ExtractError::MethodMismatch(not)) => {
                 self.not = Some(not);
-                return Ok(self);
+                return self;
             }
         };
 
@@ -355,8 +355,10 @@ impl NotificationDispatcher<'_> {
             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) {
diff --git a/crates/rust-analyzer/src/main_loop.rs b/crates/rust-analyzer/src/main_loop.rs
index 12df61ad4c..8355923025 100644
--- a/crates/rust-analyzer/src/main_loop.rs
+++ b/crates/rust-analyzer/src/main_loop.rs
@@ -196,7 +196,7 @@ impl GlobalState {
             ) {
                 return Ok(());
             }
-            self.handle_event(event)?;
+            self.handle_event(event);
         }
 
         Err(anyhow::anyhow!("A receiver has been dropped, something panicked!"))
@@ -278,7 +278,7 @@ impl GlobalState {
         .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 @@ impl GlobalState {
         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 @@ impl GlobalState {
                 "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 @@ impl GlobalState {
     }
 
     /// 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(())
     }
 }