Merge #6416

6416: Respond with JSON-RPC error if we failed to deserialize request r=matklad a=matklad

bors r+
🤖

Co-authored-by: Aleksey Kladov <[email protected]>

3 files changed, 62 insertions, 58 deletions

diff --git a/crates/rust-analyzer/src/dispatch.rs b/crates/rust-analyzer/src/dispatch.rs
index 7a87515e9..81b85a269 100644
--- a/crates/rust-analyzer/src/dispatch.rs
+++ b/crates/rust-analyzer/src/dispatch.rs
@@ -28,17 +28,16 @@ impl<'a> RequestDispatcher<'a> {
     {
         let (id, params) = match self.parse::<R>() {
             Some(it) => it,
-            None => {
-                return Ok(self);
-            }
+            None => return Ok(self),
         };
         let world = panic::AssertUnwindSafe(&mut *self.global_state);
+
         let response = panic::catch_unwind(move || {
             let _pctx = stdx::panic_context::enter(format!("request: {} {:#?}", R::METHOD, params));
             let result = f(world.0, params);
             result_to_response::<R>(id, result)
         })
-        .map_err(|_| format!("sync task {:?} panicked", R::METHOD))?;
+        .map_err(|_err| format!("sync task {:?} panicked", R::METHOD))?;
         self.global_state.respond(response);
         Ok(self)
     }
@@ -47,7 +46,7 @@ impl<'a> RequestDispatcher<'a> {
     pub(crate) fn on<R>(
         &mut self,
         f: fn(GlobalStateSnapshot, R::Params) -> Result<R::Result>,
-    ) -> Result<&mut Self>
+    ) -> &mut Self
     where
         R: lsp_types::request::Request + 'static,
         R::Params: DeserializeOwned + Send + fmt::Debug + 'static,
@@ -55,9 +54,7 @@ impl<'a> RequestDispatcher<'a> {
     {
         let (id, params) = match self.parse::<R>() {
             Some(it) => it,
-            None => {
-                return Ok(self);
-            }
+            None => return self,
         };
 
         self.global_state.task_pool.handle.spawn({
@@ -71,7 +68,7 @@ impl<'a> RequestDispatcher<'a> {
             }
         });
 
-        Ok(self)
+        self
     }
 
     pub(crate) fn finish(&mut self) {
@@ -82,7 +79,7 @@ impl<'a> RequestDispatcher<'a> {
                 lsp_server::ErrorCode::MethodNotFound as i32,
                 "unknown request".to_string(),
             );
-            self.global_state.respond(response)
+            self.global_state.respond(response);
         }
     }
 
@@ -91,15 +88,24 @@ impl<'a> RequestDispatcher<'a> {
         R: lsp_types::request::Request + 'static,
         R::Params: DeserializeOwned + 'static,
     {
-        let req = self.req.take()?;
-        let (id, params) = match req.extract::<R::Params>(R::METHOD) {
-            Ok(it) => it,
-            Err(req) => {
-                self.req = Some(req);
+        let req = match &self.req {
+            Some(req) if req.method == R::METHOD => self.req.take().unwrap(),
+            _ => return None,
+        };
+
+        let res = crate::from_json(R::METHOD, req.params);
+        match res {
+            Ok(params) => return Some((req.id, params)),
+            Err(err) => {
+                let response = lsp_server::Response::new_err(
+                    req.id,
+                    lsp_server::ErrorCode::InvalidParams as i32,
+                    err.to_string(),
+                );
+                self.global_state.respond(response);
                 return None;
             }
-        };
-        Some((id, params))
+        }
     }
 }
 
diff --git a/crates/rust-analyzer/src/lib.rs b/crates/rust-analyzer/src/lib.rs
index 87f72b497..ad08f1afb 100644
--- a/crates/rust-analyzer/src/lib.rs
+++ b/crates/rust-analyzer/src/lib.rs
@@ -37,14 +37,16 @@ mod document;
 pub mod lsp_ext;
 pub mod config;
 
-use serde::de::DeserializeOwned;
-
-pub type Result<T, E = Box<dyn std::error::Error + Send + Sync>> = std::result::Result<T, E>;
-pub use crate::{caps::server_capabilities, main_loop::main_loop};
 use ide::AnalysisHost;
+use serde::de::DeserializeOwned;
 use std::fmt;
 use vfs::Vfs;
 
+pub use crate::{caps::server_capabilities, main_loop::main_loop};
+
+pub type Error = Box<dyn std::error::Error + Send + Sync>;
+pub type Result<T, E = Error> = std::result::Result<T, E>;
+
 pub fn from_json<T: DeserializeOwned>(what: &'static str, json: serde_json::Value) -> Result<T> {
     let res = T::deserialize(&json)
         .map_err(|e| format!("Failed to deserialize {}: {}; {}", what, e, json))?;
diff --git a/crates/rust-analyzer/src/main_loop.rs b/crates/rust-analyzer/src/main_loop.rs
index ff855fe1a..53f8ca194 100644
--- a/crates/rust-analyzer/src/main_loop.rs
+++ b/crates/rust-analyzer/src/main_loop.rs
@@ -403,53 +403,49 @@ impl GlobalState {
                 handlers::handle_matching_brace(s.snapshot(), p)
             })?
             .on_sync::<lsp_ext::MemoryUsage>(|s, p| handlers::handle_memory_usage(s, p))?
-            .on::<lsp_ext::AnalyzerStatus>(handlers::handle_analyzer_status)?
-            .on::<lsp_ext::SyntaxTree>(handlers::handle_syntax_tree)?
-            .on::<lsp_ext::ExpandMacro>(handlers::handle_expand_macro)?
-            .on::<lsp_ext::ParentModule>(handlers::handle_parent_module)?
-            .on::<lsp_ext::Runnables>(handlers::handle_runnables)?
-            .on::<lsp_ext::InlayHints>(handlers::handle_inlay_hints)?
-            .on::<lsp_ext::CodeActionRequest>(handlers::handle_code_action)?
-            .on::<lsp_ext::ResolveCodeActionRequest>(handlers::handle_resolve_code_action)?
-            .on::<lsp_ext::HoverRequest>(handlers::handle_hover)?
-            .on::<lsp_ext::ExternalDocs>(handlers::handle_open_docs)?
-            .on::<lsp_types::request::OnTypeFormatting>(handlers::handle_on_type_formatting)?
-            .on::<lsp_types::request::DocumentSymbolRequest>(handlers::handle_document_symbol)?
-            .on::<lsp_types::request::WorkspaceSymbol>(handlers::handle_workspace_symbol)?
-            .on::<lsp_types::request::GotoDefinition>(handlers::handle_goto_definition)?
-            .on::<lsp_types::request::GotoImplementation>(handlers::handle_goto_implementation)?
-            .on::<lsp_types::request::GotoTypeDefinition>(handlers::handle_goto_type_definition)?
-            .on::<lsp_types::request::Completion>(handlers::handle_completion)?
-            .on::<lsp_types::request::CodeLensRequest>(handlers::handle_code_lens)?
-            .on::<lsp_types::request::CodeLensResolve>(handlers::handle_code_lens_resolve)?
-            .on::<lsp_types::request::FoldingRangeRequest>(handlers::handle_folding_range)?
-            .on::<lsp_types::request::SignatureHelpRequest>(handlers::handle_signature_help)?
-            .on::<lsp_types::request::PrepareRenameRequest>(handlers::handle_prepare_rename)?
-            .on::<lsp_types::request::Rename>(handlers::handle_rename)?
-            .on::<lsp_types::request::References>(handlers::handle_references)?
-            .on::<lsp_types::request::Formatting>(handlers::handle_formatting)?
-            .on::<lsp_types::request::DocumentHighlightRequest>(
-                handlers::handle_document_highlight,
-            )?
-            .on::<lsp_types::request::CallHierarchyPrepare>(
-                handlers::handle_call_hierarchy_prepare,
-            )?
+            .on::<lsp_ext::AnalyzerStatus>(handlers::handle_analyzer_status)
+            .on::<lsp_ext::SyntaxTree>(handlers::handle_syntax_tree)
+            .on::<lsp_ext::ExpandMacro>(handlers::handle_expand_macro)
+            .on::<lsp_ext::ParentModule>(handlers::handle_parent_module)
+            .on::<lsp_ext::Runnables>(handlers::handle_runnables)
+            .on::<lsp_ext::InlayHints>(handlers::handle_inlay_hints)
+            .on::<lsp_ext::CodeActionRequest>(handlers::handle_code_action)
+            .on::<lsp_ext::ResolveCodeActionRequest>(handlers::handle_resolve_code_action)
+            .on::<lsp_ext::HoverRequest>(handlers::handle_hover)
+            .on::<lsp_ext::ExternalDocs>(handlers::handle_open_docs)
+            .on::<lsp_types::request::OnTypeFormatting>(handlers::handle_on_type_formatting)
+            .on::<lsp_types::request::DocumentSymbolRequest>(handlers::handle_document_symbol)
+            .on::<lsp_types::request::WorkspaceSymbol>(handlers::handle_workspace_symbol)
+            .on::<lsp_types::request::GotoDefinition>(handlers::handle_goto_definition)
+            .on::<lsp_types::request::GotoImplementation>(handlers::handle_goto_implementation)
+            .on::<lsp_types::request::GotoTypeDefinition>(handlers::handle_goto_type_definition)
+            .on::<lsp_types::request::Completion>(handlers::handle_completion)
+            .on::<lsp_types::request::CodeLensRequest>(handlers::handle_code_lens)
+            .on::<lsp_types::request::CodeLensResolve>(handlers::handle_code_lens_resolve)
+            .on::<lsp_types::request::FoldingRangeRequest>(handlers::handle_folding_range)
+            .on::<lsp_types::request::SignatureHelpRequest>(handlers::handle_signature_help)
+            .on::<lsp_types::request::PrepareRenameRequest>(handlers::handle_prepare_rename)
+            .on::<lsp_types::request::Rename>(handlers::handle_rename)
+            .on::<lsp_types::request::References>(handlers::handle_references)
+            .on::<lsp_types::request::Formatting>(handlers::handle_formatting)
+            .on::<lsp_types::request::DocumentHighlightRequest>(handlers::handle_document_highlight)
+            .on::<lsp_types::request::CallHierarchyPrepare>(handlers::handle_call_hierarchy_prepare)
             .on::<lsp_types::request::CallHierarchyIncomingCalls>(
                 handlers::handle_call_hierarchy_incoming,
-            )?
+            )
             .on::<lsp_types::request::CallHierarchyOutgoingCalls>(
                 handlers::handle_call_hierarchy_outgoing,
-            )?
+            )
             .on::<lsp_types::request::SemanticTokensFullRequest>(
                 handlers::handle_semantic_tokens_full,
-            )?
+            )
             .on::<lsp_types::request::SemanticTokensFullDeltaRequest>(
                 handlers::handle_semantic_tokens_full_delta,
-            )?
+            )
             .on::<lsp_types::request::SemanticTokensRangeRequest>(
                 handlers::handle_semantic_tokens_range,
-            )?
-            .on::<lsp_ext::Ssr>(handlers::handle_ssr)?
+            )
+            .on::<lsp_ext::Ssr>(handlers::handle_ssr)
             .finish();
         Ok(())
     }