Respond with JSON-RPC error if we failed to deserialize request

Historically, we intentinally violated JSON-RPC spec here by hard
crashing. The idea was to poke both the clients and servers to fix
stuff.

However, this is confusing for server implementors, and falls down in
one important place -- protocol extension are not always backwards
compatible, which causes crashes simply due to version mismatch. We
had once such case with our own extension, and one for semantic
tokens.

So let's be less adventerous and just err on the err side!

diff --git a/crates/rust-analyzer/src/dispatch.rs b/crates/rust-analyzer/src/dispatch.rs
index 7a87515e9aef0cac14177a68ce281c82886c1b36..81b85a269cb2570264057d806e6270d9da2584a0 100644 (file)
--- a/crates/rust-analyzer/src/dispatch.rs
+++ b/crates/rust-analyzer/src/dispatch.rs
@@ -28,17 +28,16 @@ pub(crate) fn on_sync<R>(
     {
         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 @@ pub(crate) fn on_sync<R>(
     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 @@ pub(crate) fn on<R>(
     {
         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 @@ pub(crate) fn on<R>(
             }
         });
 
-        Ok(self)
+        self
     }
 
     pub(crate) fn finish(&mut self) {
@@ -82,7 +79,7 @@ pub(crate) fn finish(&mut self) {
                 lsp_server::ErrorCode::MethodNotFound as i32,
                 "unknown request".to_string(),
             );
-            self.global_state.respond(response)
+            self.global_state.respond(response);
         }
     }
 
@@ -91,15 +88,24 @@ fn parse<R>(&mut self) -> Option<(lsp_server::RequestId, R::Params)>
         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 87f72b497467a791fe36b7cee68d93d09062eeaa..ad08f1afb5abb1e79b35a6b8f613b998f410ef1d 100644 (file)
--- a/crates/rust-analyzer/src/lib.rs
+++ b/crates/rust-analyzer/src/lib.rs
@@ -37,14 +37,16 @@ macro_rules! eprintln {
 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 ff855fe1a6425ba0941feb6b5b55c2de83d9fe6a..53f8ca1946b5a7f27bf93803839896c90d606910 100644 (file)
--- a/crates/rust-analyzer/src/main_loop.rs
+++ b/crates/rust-analyzer/src/main_loop.rs
@@ -403,53 +403,49 @@ fn on_request(&mut self, request_received: Instant, req: Request) -> Result<()>
                 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(())
     }