Improve error typing.

4 files changed, 277 insertions, 232 deletions

diff --git a/helix-lsp/Cargo.toml b/helix-lsp/Cargo.toml
index b34c139f..e4956f0b 100644
--- a/helix-lsp/Cargo.toml
+++ b/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"
diff --git a/helix-lsp/src/client.rs b/helix-lsp/src/client.rs
new file mode 100644
index 00000000..93e137cb
--- /dev/null
+++ b/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!()
+    }
+}
diff --git a/helix-lsp/src/lib.rs b/helix-lsp/src/lib.rs
index f8c73902..c37222f1 100644
--- a/helix-lsp/src/lib.rs
+++ b/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;
-
-use serde::{Deserialize, Serialize};
 
-pub use lsp::Position;
-pub use lsp::Url;
+pub use client::Client;
+pub use lsp::{Position, Url};
 
-use smol::{
-    channel::{Receiver, Sender},
-    io::{BufReader, BufWriter},
-    // prelude::*,
-    process::{Child, ChildStderr, Command, Stdio},
-    Executor,
-};
+use serde::{Deserialize, Serialize};
+use thiserror::Error;
+
+#[derive(Error, Debug)]
+pub enum Error {
+    #[error("protocol error: {0}")]
+    Rpc(#[from] jsonrpc::Error),
+    #[error("failed to parse: {0}")]
+    Parse(#[from] serde_json::Error),
+    #[error("request timed out")]
+    Timeout,
+    #[error(transparent)]
+    Other(#[from] anyhow::Error),
+}
 
 pub mod util {
     use super::*;
@@ -68,209 +67,3 @@ impl 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!()
-    }
-}
diff --git a/helix-lsp/src/transport.rs b/helix-lsp/src/transport.rs
index 8915a925..38c3bb57 100644
--- a/helix-lsp/src/transport.rs
+++ b/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 @@ use smol::{
 
 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 @@ impl Transport {
     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 @@ impl Transport {
 
             let parts: Vec<&str> = header.split(": ").collect();
             if parts.len() != 2 {
-                // return Err(Error::new(ErrorKind::Other, "Failed to parse header"));
-                panic!()
+                return Err(std::io::Error::new(
+                    std::io::ErrorKind::Other,
+                    "Failed to parse header",
+                ));
             }
             headers.insert(parts[0].to_string(), parts[1].to_string());
         }
@@ -155,7 +158,13 @@ impl Transport {
                     .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(())