Improve error typing.

Cargo.lock

@@ -100,9 +100,9 @@ dependencies = [
 
 [[package]]
 name = "async-net"
-version = "1.4.7"
+version = "1.5.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "ee4c3668eb091d781e97f0026b5289b457c77d407a85749a9bb4c057456c428f"
+checksum = "06de475c85affe184648202401d7622afb32f0f74e02192857d0201a16defbe5"
 dependencies = [
  "async-io",
  "blocking",
@@ -350,9 +350,9 @@ checksum = "5fc94b64bb39543b4e432f1790b6bf18e3ee3b74653c5449f63310e9a74b123c"
 
 [[package]]
 name = "futures-lite"
-version = "1.11.1"
+version = "1.11.2"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "381a7ad57b1bad34693f63f6f377e1abded7a9c85c9d3eb6771e11c60aaadab9"
+checksum = "5e6c079abfac3ab269e2927ec048dabc89d009ebfdda6b8ee86624f30c689658"
 dependencies = [
  "fastrand",
  "futures-core",
@@ -454,6 +454,7 @@ dependencies = [
  "serde_json",
  "shellexpand",
  "smol",
+ "thiserror",
  "url",
 ]
 
@@ -623,9 +624,9 @@ checksum = "3728d817d99e5ac407411fa471ff9800a778d88a24685968b36824eaf4bee400"
 
 [[package]]
 name = "mio"
-version = "0.7.3"
+version = "0.7.4"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "e53a6ea5f38c0a48ca42159868c6d8e1bd56c0451238856cc08d58563643bdc3"
+checksum = "f8f1c83949125de4a582aa2da15ae6324d91cf6a58a70ea407643941ff98f558"
 dependencies = [
  "libc",
  "log",
@@ -757,9 +758,9 @@ dependencies = [
 
 [[package]]
 name = "pin-project-lite"
-version = "0.1.10"
+version = "0.1.11"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "e555d9e657502182ac97b539fb3dae8b79cda19e3e4f8ffb5e8de4f18df93c95"
+checksum = "c917123afa01924fc84bb20c4c03f004d9c38e5127e3c039bbf7f4b9c76a2f6b"
 
 [[package]]
 name = "pin-utils"
@@ -1022,6 +1023,26 @@ dependencies = [
  "unicode-width",
 ]
 
+[[package]]
+name = "thiserror"
+version = "1.0.21"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "318234ffa22e0920fe9a40d7b8369b5f649d490980cf7aadcf1eb91594869b42"
+dependencies = [
+ "thiserror-impl",
+]
+
+[[package]]
+name = "thiserror-impl"
+version = "1.0.21"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "cae2447b6282786c3493999f40a9be2a6ad20cb8bd268b0a0dbf5a065535c0ab"
+dependencies = [
+ "proc-macro2",
+ "quote",
+ "syn",
+]
+
 [[package]]
 name = "thread_local"
 version = "1.0.1"
@@ -1139,9 +1160,9 @@ checksum = "cccddf32554fecc6acb585f82a32a72e28b48f8c4c1883ddfeeeaa96f7d8e519"
 
 [[package]]
 name = "wepoll-sys"
-version = "3.0.0"
+version = "3.0.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "142bc2cba3fe88be1a8fcb55c727fa4cd5b0cf2d7438722792e22f26f04bc1e0"
+checksum = "0fcb14dea929042224824779fbc82d9fab8d2e6d3cbc0ac404de8edf489e77ff"
 dependencies = [
  "cc",
 ]

helix-lsp/Cargo.toml

@@ -20,3 +20,4 @@ serde_json = "1.0"
 serde = { version = "1.0", features = ["derive"] }
 jsonrpc-core = "15.1"
 futures-util = "0.3"
+thiserror = "1.0.21"

helix-lsp/src/client.rs

@@ -0,0 +1,242 @@
+use crate::{
+    transport::{Payload, Transport},
+    Error, Notification,
+};
+
+type Result<T> = core::result::Result<T, Error>;
+
+use helix_core::{State, Transaction};
+
+// use std::collections::HashMap;
+
+use jsonrpc_core as jsonrpc;
+use lsp_types as lsp;
+use serde_json::Value;
+
+use smol::{
+    channel::{Receiver, Sender},
+    io::{BufReader, BufWriter},
+    // prelude::*,
+    process::{Child, ChildStderr, Command, Stdio},
+    Executor,
+};
+
+pub struct Client {
+    _process: Child,
+    stderr: BufReader<ChildStderr>,
+
+    outgoing: Sender<Payload>,
+    pub incoming: Receiver<Notification>,
+
+    pub request_counter: u64,
+
+    capabilities: Option<lsp::ServerCapabilities>,
+    // TODO: handle PublishDiagnostics Version
+    // diagnostics: HashMap<lsp::Url, Vec<lsp::Diagnostic>>,
+}
+
+impl Client {
+    pub fn start(ex: &Executor, cmd: &str, args: &[String]) -> Self {
+        let mut process = Command::new(cmd)
+            .args(args)
+            .stdin(Stdio::piped())
+            .stdout(Stdio::piped())
+            .stderr(Stdio::piped())
+            .spawn()
+            .expect("Failed to start language server");
+        // smol makes sure the process is reaped on drop, but using kill_on_drop(true) maybe?
+
+        // TODO: do we need bufreader/writer here? or do we use async wrappers on unblock?
+        let writer = BufWriter::new(process.stdin.take().expect("Failed to open stdin"));
+        let reader = BufReader::new(process.stdout.take().expect("Failed to open stdout"));
+        let stderr = BufReader::new(process.stderr.take().expect("Failed to open stderr"));
+
+        let (incoming, outgoing) = Transport::start(ex, reader, writer);
+
+        Client {
+            _process: process,
+            stderr,
+
+            outgoing,
+            incoming,
+
+            request_counter: 0,
+
+            capabilities: None,
+            // diagnostics: HashMap::new(),
+        }
+    }
+
+    fn next_request_id(&mut self) -> jsonrpc::Id {
+        let id = jsonrpc::Id::Num(self.request_counter);
+        self.request_counter += 1;
+        id
+    }
+
+    fn to_params(value: Value) -> Result<jsonrpc::Params> {
+        use jsonrpc::Params;
+
+        let params = match value {
+            Value::Null => Params::None,
+            Value::Bool(_) | Value::Number(_) | Value::String(_) => Params::Array(vec![value]),
+            Value::Array(vec) => Params::Array(vec),
+            Value::Object(map) => Params::Map(map),
+        };
+
+        Ok(params)
+    }
+
+    pub async fn request<R: lsp::request::Request>(
+        &mut self,
+        params: R::Params,
+    ) -> Result<R::Result>
+    where
+        R::Params: serde::Serialize,
+        R::Result: core::fmt::Debug, // TODO: temporary
+    {
+        let params = serde_json::to_value(params)?;
+
+        let request = jsonrpc::MethodCall {
+            jsonrpc: Some(jsonrpc::Version::V2),
+            id: self.next_request_id(),
+            method: R::METHOD.to_string(),
+            params: Self::to_params(params)?,
+        };
+
+        let (tx, rx) = smol::channel::bounded::<Result<Value>>(1);
+
+        self.outgoing
+            .send(Payload::Request {
+                chan: tx,
+                value: request,
+            })
+            .await
+            .map_err(|e| Error::Other(e.into()))?;
+
+        let response = rx.recv().await.map_err(|e| Error::Other(e.into()))??;
+
+        let response = serde_json::from_value(response)?;
+
+        // TODO: we should pass request to a sender thread via a channel
+        // so it can't be interleaved
+
+        // TODO: responses can be out of order, we need to register a single shot response channel
+
+        Ok(response)
+    }
+
+    pub async fn notify<R: lsp::notification::Notification>(
+        &mut self,
+        params: R::Params,
+    ) -> Result<()>
+    where
+        R::Params: serde::Serialize,
+    {
+        let params = serde_json::to_value(params)?;
+
+        let notification = jsonrpc::Notification {
+            jsonrpc: Some(jsonrpc::Version::V2),
+            method: R::METHOD.to_string(),
+            params: Self::to_params(params)?,
+        };
+
+        self.outgoing
+            .send(Payload::Notification(notification))
+            .await
+            .map_err(|e| Error::Other(e.into()))?;
+
+        Ok(())
+    }
+
+    // -------------------------------------------------------------------------------------------
+    // General messages
+    // -------------------------------------------------------------------------------------------
+
+    pub async fn initialize(&mut self) -> Result<()> {
+        // TODO: delay any requests that are triggered prior to initialize
+
+        #[allow(deprecated)]
+        let params = lsp::InitializeParams {
+            process_id: Some(u64::from(std::process::id())),
+            root_path: None,
+            // root_uri: Some(lsp_types::Url::parse("file://localhost/")?),
+            root_uri: None, // set to project root in the future
+            initialization_options: None,
+            capabilities: lsp::ClientCapabilities::default(),
+            trace: None,
+            workspace_folders: None,
+            client_info: None,
+        };
+
+        let response = self.request::<lsp::request::Initialize>(params).await?;
+        self.capabilities = Some(response.capabilities);
+
+        // next up, notify<initialized>
+        self.notify::<lsp::notification::Initialized>(lsp::InitializedParams {})
+            .await?;
+
+        Ok(())
+    }
+
+    pub async fn shutdown(&mut self) -> Result<()> {
+        self.request::<lsp::request::Shutdown>(()).await
+    }
+
+    pub async fn exit(&mut self) -> Result<()> {
+        self.notify::<lsp::notification::Exit>(()).await
+    }
+
+    // -------------------------------------------------------------------------------------------
+    // Text document
+    // -------------------------------------------------------------------------------------------
+
+    pub async fn text_document_did_open(&mut self, state: &State) -> Result<()> {
+        self.notify::<lsp::notification::DidOpenTextDocument>(lsp::DidOpenTextDocumentParams {
+            text_document: lsp::TextDocumentItem {
+                uri: lsp::Url::from_file_path(state.path().unwrap()).unwrap(),
+                language_id: "rust".to_string(), // TODO: hardcoded for now
+                version: state.version,
+                text: String::from(&state.doc),
+            },
+        })
+        .await
+    }
+
+    // TODO: trigger any time history.commit_revision happens
+    pub async fn text_document_did_change(
+        &mut self,
+        state: &State,
+        transaction: &Transaction,
+    ) -> Result<()> {
+        self.notify::<lsp::notification::DidChangeTextDocument>(lsp::DidChangeTextDocumentParams {
+            text_document: lsp::VersionedTextDocumentIdentifier::new(
+                lsp::Url::from_file_path(state.path().unwrap()).unwrap(),
+                state.version,
+            ),
+            content_changes: vec![lsp::TextDocumentContentChangeEvent {
+                // range = None -> whole document
+                range: None,        //Some(Range)
+                range_length: None, // u64 apparently deprecated
+                text: "".to_string(),
+            }], // TODO: probably need old_state here too?
+        })
+        .await
+    }
+
+    // TODO: impl into() TextDocumentIdentifier / VersionedTextDocumentIdentifier for State.
+
+    pub async fn text_document_did_close(&mut self, state: &State) -> Result<()> {
+        self.notify::<lsp::notification::DidCloseTextDocument>(lsp::DidCloseTextDocumentParams {
+            text_document: lsp::TextDocumentIdentifier::new(
+                lsp::Url::from_file_path(state.path().unwrap()).unwrap(),
+            ),
+        })
+        .await
+    }
+
+    // will_save / will_save_wait_until
+
+    pub async fn text_document_did_save(&mut self) -> anyhow::Result<()> {
+        unimplemented!()
+    }
+}

helix-lsp/src/lib.rs

@@ -1,27 +1,26 @@
+mod client;
 mod transport;
 
-use transport::{Payload, Transport};
-
-use helix_core::{State, Transaction};
-
-// use std::collections::HashMap;
-
 use jsonrpc_core as jsonrpc;
 use lsp_types as lsp;
-use serde_json::Value;
+
+pub use client::Client;
+pub use lsp::{Position, Url};
 
 use serde::{Deserialize, Serialize};
+use thiserror::Error;
 
-pub use lsp::Position;
+#[derive(Error, Debug)]
-pub use lsp::Url;
+pub enum Error {
-
+    #[error("protocol error: {0}")]
-use smol::{
+    Rpc(#[from] jsonrpc::Error),
-    channel::{Receiver, Sender},
+    #[error("failed to parse: {0}")]
-    io::{BufReader, BufWriter},
+    Parse(#[from] serde_json::Error),
-    // prelude::*,
+    #[error("request timed out")]
-    process::{Child, ChildStderr, Command, Stdio},
+    Timeout,
-    Executor,
+    #[error(transparent)]
-};
+    Other(#[from] anyhow::Error),
+}
 
 pub mod util {
     use super::*;
@@ -68,209 +67,3 @@ pub fn parse(method: &str, params: jsonrpc::Params) -> Notification {
         }
     }
 }
-
-pub struct Client {
-    _process: Child,
-    stderr: BufReader<ChildStderr>,
-
-    outgoing: Sender<Payload>,
-    pub incoming: Receiver<Notification>,
-
-    pub request_counter: u64,
-
-    capabilities: Option<lsp::ServerCapabilities>,
-    // TODO: handle PublishDiagnostics Version
-    // diagnostics: HashMap<lsp::Url, Vec<lsp::Diagnostic>>,
-}
-
-impl Client {
-    pub fn start(ex: &Executor, cmd: &str, args: &[String]) -> Self {
-        let mut process = Command::new(cmd)
-            .args(args)
-            .stdin(Stdio::piped())
-            .stdout(Stdio::piped())
-            .stderr(Stdio::piped())
-            .spawn()
-            .expect("Failed to start language server");
-        // smol makes sure the process is reaped on drop, but using kill_on_drop(true) maybe?
-
-        // TODO: do we need bufreader/writer here? or do we use async wrappers on unblock?
-        let writer = BufWriter::new(process.stdin.take().expect("Failed to open stdin"));
-        let reader = BufReader::new(process.stdout.take().expect("Failed to open stdout"));
-        let stderr = BufReader::new(process.stderr.take().expect("Failed to open stderr"));
-
-        let (incoming, outgoing) = Transport::start(ex, reader, writer);
-
-        Client {
-            _process: process,
-            stderr,
-
-            outgoing,
-            incoming,
-
-            request_counter: 0,
-
-            capabilities: None,
-            // diagnostics: HashMap::new(),
-        }
-    }
-
-    fn next_request_id(&mut self) -> jsonrpc::Id {
-        let id = jsonrpc::Id::Num(self.request_counter);
-        self.request_counter += 1;
-        id
-    }
-
-    fn to_params(value: Value) -> anyhow::Result<jsonrpc::Params> {
-        use jsonrpc::Params;
-
-        let params = match value {
-            Value::Null => Params::None,
-            Value::Bool(_) | Value::Number(_) | Value::String(_) => Params::Array(vec![value]),
-            Value::Array(vec) => Params::Array(vec),
-            Value::Object(map) => Params::Map(map),
-        };
-
-        Ok(params)
-    }
-
-    pub async fn request<R: lsp::request::Request>(
-        &mut self,
-        params: R::Params,
-    ) -> anyhow::Result<R::Result>
-    where
-        R::Params: serde::Serialize,
-        R::Result: core::fmt::Debug, // TODO: temporary
-    {
-        let params = serde_json::to_value(params)?;
-
-        let request = jsonrpc::MethodCall {
-            jsonrpc: Some(jsonrpc::Version::V2),
-            id: self.next_request_id(),
-            method: R::METHOD.to_string(),
-            params: Self::to_params(params)?,
-        };
-
-        let (tx, rx) = smol::channel::bounded::<anyhow::Result<Value>>(1);
-
-        self.outgoing
-            .send(Payload::Request {
-                chan: tx,
-                value: request,
-            })
-            .await?;
-
-        let response = rx.recv().await??;
-
-        let response = serde_json::from_value(response)?;
-
-        // TODO: we should pass request to a sender thread via a channel
-        // so it can't be interleaved
-
-        // TODO: responses can be out of order, we need to register a single shot response channel
-
-        Ok(response)
-    }
-
-    pub async fn notify<R: lsp::notification::Notification>(
-        &mut self,
-        params: R::Params,
-    ) -> anyhow::Result<()>
-    where
-        R::Params: serde::Serialize,
-    {
-        let params = serde_json::to_value(params)?;
-
-        let notification = jsonrpc::Notification {
-            jsonrpc: Some(jsonrpc::Version::V2),
-            method: R::METHOD.to_string(),
-            params: Self::to_params(params)?,
-        };
-
-        self.outgoing
-            .send(Payload::Notification(notification))
-            .await?;
-
-        Ok(())
-    }
-
-    // -------------------------------------------------------------------------------------------
-    // General messages
-    // -------------------------------------------------------------------------------------------
-
-    pub async fn initialize(&mut self) -> anyhow::Result<()> {
-        // TODO: delay any requests that are triggered prior to initialize
-
-        #[allow(deprecated)]
-        let params = lsp::InitializeParams {
-            process_id: Some(u64::from(std::process::id())),
-            root_path: None,
-            // root_uri: Some(lsp_types::Url::parse("file://localhost/")?),
-            root_uri: None, // set to project root in the future
-            initialization_options: None,
-            capabilities: lsp::ClientCapabilities::default(),
-            trace: None,
-            workspace_folders: None,
-            client_info: None,
-        };
-
-        let response = self.request::<lsp::request::Initialize>(params).await?;
-        self.capabilities = Some(response.capabilities);
-
-        // next up, notify<initialized>
-        self.notify::<lsp::notification::Initialized>(lsp::InitializedParams {})
-            .await?;
-
-        Ok(())
-    }
-
-    pub async fn shutdown(&mut self) -> anyhow::Result<()> {
-        self.request::<lsp::request::Shutdown>(()).await
-    }
-
-    pub async fn exit(&mut self) -> anyhow::Result<()> {
-        self.notify::<lsp::notification::Exit>(()).await
-    }
-
-    // -------------------------------------------------------------------------------------------
-    // Text document
-    // -------------------------------------------------------------------------------------------
-
-    pub async fn text_document_did_open(&mut self, state: &State) -> anyhow::Result<()> {
-        self.notify::<lsp::notification::DidOpenTextDocument>(lsp::DidOpenTextDocumentParams {
-            text_document: lsp::TextDocumentItem {
-                uri: lsp::Url::from_file_path(state.path().unwrap()).unwrap(),
-                language_id: "rust".to_string(), // TODO: hardcoded for now
-                version: state.version,
-                text: String::from(&state.doc),
-            },
-        })
-        .await
-    }
-
-    // TODO: trigger any time history.commit_revision happens
-    pub async fn text_document_did_change(
-        &mut self,
-        state: &State,
-        transaction: &Transaction,
-    ) -> anyhow::Result<()> {
-        self.notify::<lsp::notification::DidChangeTextDocument>(lsp::DidChangeTextDocumentParams {
-            text_document: lsp::VersionedTextDocumentIdentifier::new(
-                lsp::Url::from_file_path(state.path().unwrap()).unwrap(),
-                state.version,
-            ),
-            content_changes: vec![], // TODO: probably need old_state here too?
-        })
-        .await
-    }
-
-    pub async fn text_document_did_close(&mut self) -> anyhow::Result<()> {
-        unimplemented!()
-    }
-
-    // will_save / will_save_wait_until
-
-    pub async fn text_document_did_save(&mut self) -> anyhow::Result<()> {
-        unimplemented!()
-    }
-}

helix-lsp/src/transport.rs

@@ -1,9 +1,10 @@
 use std::collections::HashMap;
 
-use crate::{Message, Notification};
+use crate::{Error, Message, Notification};
+
+type Result<T> = core::result::Result<T, Error>;
 
 use jsonrpc_core as jsonrpc;
-use lsp_types as lsp;
 use serde_json::Value;
 
 use smol::prelude::*;
@@ -17,7 +18,7 @@
 
 pub(crate) enum Payload {
     Request {
-        chan: Sender<anyhow::Result<Value>>,
+        chan: Sender<Result<Value>>,
         value: jsonrpc::MethodCall,
     },
     Notification(jsonrpc::Notification),
@@ -27,7 +28,7 @@ pub(crate) struct Transport {
     incoming: Sender<Notification>, // TODO Notification | Call
     outgoing: Receiver<Payload>,
 
-    pending_requests: HashMap<jsonrpc::Id, Sender<anyhow::Result<Value>>>,
+    pending_requests: HashMap<jsonrpc::Id, Sender<Result<Value>>>,
     headers: HashMap<String, String>,
 
     writer: BufWriter<ChildStdin>,
@@ -60,7 +61,7 @@ pub fn start(
     async fn recv(
         reader: &mut (impl AsyncBufRead + Unpin),
         headers: &mut HashMap<String, String>,
-    ) -> Result<Message, std::io::Error> {
+    ) -> core::result::Result<Message, std::io::Error> {
         // read headers
         loop {
             let mut header = String::new();
@@ -74,8 +75,10 @@ async fn recv(
 
             let parts: Vec<&str> = header.split(": ").collect();
             if parts.len() != 2 {
-                // return Err(Error::new(ErrorKind::Other, "Failed to parse header"));
+                return Err(std::io::Error::new(
-                panic!()
+                    std::io::ErrorKind::Other,
+                    "Failed to parse header",
+                ));
             }
             headers.insert(parts[0].to_string(), parts[1].to_string());
         }
@@ -155,7 +158,13 @@ pub async fn recv_response(&mut self, output: jsonrpc::Output) -> anyhow::Result
                     .expect("pending_request with id not found!");
                 tx.send(Ok(result)).await?;
             }
-            jsonrpc::Output::Failure(_) => panic!("recv fail"),
+            jsonrpc::Output::Failure(jsonrpc::Failure { id, error, .. }) => {
+                let tx = self
+                    .pending_requests
+                    .remove(&id)
+                    .expect("pending_request with id not found!");
+                tx.send(Err(error.into())).await?;
+            }
             msg => unimplemented!("{:?}", msg),
         }
         Ok(())