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!
This commit is contained in:
Aleksey Kladov 2020-10-30 19:38:29 +01:00
parent e7c8c600b7
commit 3b9548e163
3 changed files with 62 additions and 58 deletions

View file

@ -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))
}
}
}

View file

@ -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))?;

View file

@ -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(())
}