From 7ae6cad52ed001a903577e30fb71e346fd831af7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bla=C5=BE=20Hrastnik?= <blaz@mxxn.io>
Date: Wed, 11 May 2022 11:00:55 +0900
Subject: [PATCH] Don't panic on LSP parsing errors

This made sense initially when the implementation was still new (so we
got user reports more frequently), but a parsing error now generally
signifies a language server isn't properly implementing the spec.
---
 helix-lsp/src/lib.rs          | 49 +++++++++++------------------------
 helix-term/src/application.rs | 24 +++++++++++++----
 2 files changed, 34 insertions(+), 39 deletions(-)

diff --git a/helix-lsp/src/lib.rs b/helix-lsp/src/lib.rs
index 84fb00fe7da1..f33646c82177 100644
--- a/helix-lsp/src/lib.rs
+++ b/helix-lsp/src/lib.rs
@@ -40,6 +40,8 @@ pub enum Error {
     StreamClosed,
     #[error("LSP not defined")]
     LspNotDefined,
+    #[error("Unhandled")]
+    Unhandled,
     #[error(transparent)]
     Other(#[from] anyhow::Error),
 }
@@ -226,34 +228,27 @@ pub enum MethodCall {
 }
 
 impl MethodCall {
-    pub fn parse(method: &str, params: jsonrpc::Params) -> Option<MethodCall> {
+    pub fn parse(method: &str, params: jsonrpc::Params) -> Result<MethodCall> {
         use lsp::request::Request;
         let request = match method {
             lsp::request::WorkDoneProgressCreate::METHOD => {
-                let params: lsp::WorkDoneProgressCreateParams = params
-                    .parse()
-                    .expect("Failed to parse WorkDoneCreate params");
+                let params: lsp::WorkDoneProgressCreateParams = params.parse()?;
                 Self::WorkDoneProgressCreate(params)
             }
             lsp::request::ApplyWorkspaceEdit::METHOD => {
-                let params: lsp::ApplyWorkspaceEditParams = params
-                    .parse()
-                    .expect("Failed to parse ApplyWorkspaceEdit params");
+                let params: lsp::ApplyWorkspaceEditParams = params.parse()?;
                 Self::ApplyWorkspaceEdit(params)
             }
             lsp::request::WorkspaceFoldersRequest::METHOD => Self::WorkspaceFolders,
             lsp::request::WorkspaceConfiguration::METHOD => {
-                let params: lsp::ConfigurationParams = params
-                    .parse()
-                    .expect("Failed to parse WorkspaceConfiguration params");
+                let params: lsp::ConfigurationParams = params.parse()?;
                 Self::WorkspaceConfiguration(params)
             }
             _ => {
-                log::warn!("unhandled lsp request: {}", method);
-                return None;
+                return Err(Error::Unhandled);
             }
         };
-        Some(request)
+        Ok(request)
     }
 }
 
@@ -268,48 +263,34 @@ pub enum Notification {
 }
 
 impl Notification {
-    pub fn parse(method: &str, params: jsonrpc::Params) -> Option<Notification> {
+    pub fn parse(method: &str, params: jsonrpc::Params) -> Result<Notification> {
         use lsp::notification::Notification as _;
 
         let notification = match method {
             lsp::notification::Initialized::METHOD => Self::Initialized,
             lsp::notification::PublishDiagnostics::METHOD => {
-                let params: lsp::PublishDiagnosticsParams = params
-                    .parse()
-                    .map_err(|err| {
-                        log::error!(
-                            "received malformed PublishDiagnostic from Language Server: {}",
-                            err
-                        )
-                    })
-                    .ok()?;
-
-                // TODO: need to loop over diagnostics and distinguish them by URI
+                let params: lsp::PublishDiagnosticsParams = params.parse()?;
                 Self::PublishDiagnostics(params)
             }
 
             lsp::notification::ShowMessage::METHOD => {
-                let params: lsp::ShowMessageParams = params.parse().ok()?;
-
+                let params: lsp::ShowMessageParams = params.parse()?;
                 Self::ShowMessage(params)
             }
             lsp::notification::LogMessage::METHOD => {
-                let params: lsp::LogMessageParams = params.parse().ok()?;
-
+                let params: lsp::LogMessageParams = params.parse()?;
                 Self::LogMessage(params)
             }
             lsp::notification::Progress::METHOD => {
-                let params: lsp::ProgressParams = params.parse().ok()?;
-
+                let params: lsp::ProgressParams = params.parse()?;
                 Self::ProgressMessage(params)
             }
             _ => {
-                log::error!("unhandled LSP notification: {}", method);
-                return None;
+                return Err(Error::Unhandled);
             }
         };
 
-        Some(notification)
+        Ok(notification)
     }
 }
 
diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs
index 91caade79c5f..2dfccf0435ec 100644
--- a/helix-term/src/application.rs
+++ b/helix-term/src/application.rs
@@ -399,8 +399,14 @@ impl Application {
         match call {
             Call::Notification(helix_lsp::jsonrpc::Notification { method, params, .. }) => {
                 let notification = match Notification::parse(&method, params) {
-                    Some(notification) => notification,
-                    None => return,
+                    Ok(notification) => notification,
+                    Err(err) => {
+                        log::error!(
+                            "received malformed notification from Language Server: {}",
+                            err
+                        );
+                        return;
+                    }
                 };
 
                 match notification {
@@ -613,9 +619,17 @@ impl Application {
                 method, params, id, ..
             }) => {
                 let call = match MethodCall::parse(&method, params) {
-                    Some(call) => call,
-                    None => {
-                        error!("Method not found {}", method);
+                    Ok(call) => call,
+                    Err(helix_lsp::Error::Unhandled) => {
+                        error!("Language Server: Method not found {}", method);
+                        return;
+                    }
+                    Err(err) => {
+                        log::error!(
+                            "received malformed method call from Language Server: {}: {}",
+                            method,
+                            err
+                        );
                         return;
                     }
                 };