From d8a3d6f37890734e7bb273a4d6bf9e36fd2bbc74 Mon Sep 17 00:00:00 2001
From: Aleksey Kladov <aleksey.kladov@gmail.com>
Date: Tue, 31 Aug 2021 19:01:39 +0300
Subject: [PATCH] internal: cleanup proc macro server error handlig

When dealing with proc macros, there are two very different kinds of
errors:

* first, usual errors of "proc macro panicked on this particular input"
* second, the proc macro server might day if the user, eg, kills it

First kind of errors are expected and are a normal output, while the
second kind are genuine IO-errors.

For this reason, we use a curious nested result here: `Result<Result<T,
E1>, E2>` pattern, which is 100% inspired by http://sled.rs/errors.html
---
 crates/proc_macro_api/src/lib.rs              | 136 +++++++++++-------
 crates/proc_macro_api/src/msg.rs              | 112 ++++++++++-----
 .../proc_macro_api/src/{rpc => msg}/flat.rs   |   0
 crates/proc_macro_api/src/process.rs          |  58 +++-----
 crates/proc_macro_api/src/rpc.rs              | 107 --------------
 .../proc_macro_srv/src/abis/abi_1_47/mod.rs   |   2 +-
 .../proc_macro_srv/src/abis/abi_1_55/mod.rs   |   2 +-
 .../proc_macro_srv/src/abis/abi_1_56/mod.rs   |   2 +-
 crates/proc_macro_srv/src/cli.rs              |  23 +--
 crates/proc_macro_srv/src/lib.rs              |  28 ++--
 crates/proc_macro_srv/src/tests/utils.rs      |   9 +-
 crates/rust-analyzer/src/reload.rs            |  34 ++++-
 12 files changed, 242 insertions(+), 271 deletions(-)
 rename crates/proc_macro_api/src/{rpc => msg}/flat.rs (100%)
 delete mode 100644 crates/proc_macro_api/src/rpc.rs

diff --git a/crates/proc_macro_api/src/lib.rs b/crates/proc_macro_api/src/lib.rs
index 55754325f6f..20c5ffaebd1 100644
--- a/crates/proc_macro_api/src/lib.rs
+++ b/crates/proc_macro_api/src/lib.rs
@@ -7,25 +7,32 @@
 
 pub mod msg;
 mod process;
-mod rpc;
 mod version;
 
-use paths::{AbsPath, AbsPathBuf};
+use paths::AbsPathBuf;
 use std::{
     ffi::OsStr,
-    io,
+    fmt, io,
     sync::{Arc, Mutex},
 };
 
-use tt::{SmolStr, Subtree};
+use serde::{Deserialize, Serialize};
+use tt::Subtree;
 
-use crate::process::ProcMacroProcessSrv;
-
-pub use rpc::{
-    flat::FlatTree, ExpansionResult, ExpansionTask, ListMacrosResult, ListMacrosTask, ProcMacroKind,
+use crate::{
+    msg::{ExpandMacro, FlatTree, PanicMessage},
+    process::ProcMacroProcessSrv,
 };
+
 pub use version::{read_dylib_info, RustCInfo};
 
+#[derive(Copy, Clone, Eq, PartialEq, Debug, Serialize, Deserialize)]
+pub enum ProcMacroKind {
+    CustomDerive,
+    FuncLike,
+    Attr,
+}
+
 /// A handle to an external process which load dylibs with macros (.so or .dll)
 /// and runs actual macro expansion functions.
 #[derive(Debug)]
@@ -39,6 +46,26 @@ pub struct ProcMacroServer {
     process: Arc<Mutex<ProcMacroProcessSrv>>,
 }
 
+pub struct MacroDylib {
+    path: AbsPathBuf,
+}
+
+impl MacroDylib {
+    // FIXME: this is buggy due to TOCTOU, we should check the version in the
+    // macro process instead.
+    pub fn new(path: AbsPathBuf) -> io::Result<MacroDylib> {
+        let _p = profile::span("MacroDylib::new");
+
+        let info = version::read_dylib_info(&path)?;
+        if info.version.0 < 1 || info.version.1 < 47 {
+            let msg = format!("proc-macro {} built by {:#?} is not supported by Rust Analyzer, please update your rust version.", path.display(), info);
+            return Err(io::Error::new(io::ErrorKind::InvalidData, msg));
+        }
+
+        Ok(MacroDylib { path })
+    }
+}
+
 /// A handle to a specific macro (a `#[proc_macro]` annotated function).
 ///
 /// It exists withing a context of a specific [`ProcMacroProcess`] -- currently
@@ -47,7 +74,7 @@ pub struct ProcMacroServer {
 pub struct ProcMacro {
     process: Arc<Mutex<ProcMacroProcessSrv>>,
     dylib_path: AbsPathBuf,
-    name: SmolStr,
+    name: String,
     kind: ProcMacroKind,
 }
 
@@ -61,6 +88,25 @@ impl PartialEq for ProcMacro {
     }
 }
 
+pub struct ServerError {
+    pub message: String,
+    pub io: Option<io::Error>,
+}
+
+impl fmt::Display for ServerError {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "{}", self.message)?;
+        if let Some(io) = &self.io {
+            write!(f, ": {}", io)?;
+        }
+        Ok(())
+    }
+}
+
+pub struct MacroPanic {
+    pub message: String,
+}
+
 impl ProcMacroServer {
     /// Spawns an external process as the proc macro server and returns a client connected to it.
     pub fn spawn(
@@ -71,45 +117,27 @@ impl ProcMacroServer {
         Ok(ProcMacroServer { process: Arc::new(Mutex::new(process)) })
     }
 
-    pub fn load_dylib(&self, dylib_path: &AbsPath) -> Vec<ProcMacro> {
+    pub fn load_dylib(
+        &self,
+        dylib: MacroDylib,
+    ) -> Result<Result<Vec<ProcMacro>, String>, ServerError> {
         let _p = profile::span("ProcMacroClient::by_dylib_path");
-        match version::read_dylib_info(dylib_path) {
-            Ok(info) => {
-                if info.version.0 < 1 || info.version.1 < 47 {
-                    eprintln!("proc-macro {} built by {:#?} is not supported by Rust Analyzer, please update your rust version.", dylib_path.display(), info);
-                }
-            }
-            Err(err) => {
-                eprintln!(
-                    "proc-macro {} failed to find the given version. Reason: {}",
-                    dylib_path.display(),
-                    err
-                );
-            }
-        }
+        let macros =
+            self.process.lock().unwrap_or_else(|e| e.into_inner()).find_proc_macros(&dylib.path)?;
 
-        let macros = match self
-            .process
-            .lock()
-            .unwrap_or_else(|e| e.into_inner())
-            .find_proc_macros(dylib_path)
-        {
-            Err(err) => {
-                eprintln!("Failed to find proc macros. Error: {:#?}", err);
-                return vec![];
-            }
-            Ok(macros) => macros,
-        };
+        let res = macros.map(|macros| {
+            macros
+                .into_iter()
+                .map(|(name, kind)| ProcMacro {
+                    process: self.process.clone(),
+                    name: name.into(),
+                    kind,
+                    dylib_path: dylib.path.clone(),
+                })
+                .collect()
+        });
 
-        macros
-            .into_iter()
-            .map(|(name, kind)| ProcMacro {
-                process: self.process.clone(),
-                name: name.into(),
-                kind,
-                dylib_path: dylib_path.to_path_buf(),
-            })
-            .collect()
+        Ok(res)
     }
 }
 
@@ -127,8 +155,8 @@ impl ProcMacro {
         subtree: &Subtree,
         attr: Option<&Subtree>,
         env: Vec<(String, String)>,
-    ) -> Result<Subtree, tt::ExpansionError> {
-        let task = ExpansionTask {
+    ) -> Result<Result<Subtree, PanicMessage>, ServerError> {
+        let task = ExpandMacro {
             macro_body: FlatTree::new(subtree),
             macro_name: self.name.to_string(),
             attributes: attr.map(FlatTree::new),
@@ -136,11 +164,13 @@ impl ProcMacro {
             env,
         };
 
-        let result: ExpansionResult = self
-            .process
-            .lock()
-            .unwrap_or_else(|e| e.into_inner())
-            .send_task(msg::Request::ExpansionMacro(task))?;
-        Ok(result.expansion.to_subtree())
+        let request = msg::Request::ExpandMacro(task);
+        let response = self.process.lock().unwrap_or_else(|e| e.into_inner()).send_task(request)?;
+        match response {
+            msg::Response::ExpandMacro(it) => Ok(it.map(|it| it.to_subtree())),
+            msg::Response::ListMacros { .. } => {
+                Err(ServerError { message: "unexpected response".to_string(), io: None })
+            }
+        }
     }
 }
diff --git a/crates/proc_macro_api/src/msg.rs b/crates/proc_macro_api/src/msg.rs
index 04248e04478..e9764a7dc6a 100644
--- a/crates/proc_macro_api/src/msg.rs
+++ b/crates/proc_macro_api/src/msg.rs
@@ -1,57 +1,53 @@
 //! Defines messages for cross-process message passing based on `ndjson` wire protocol
+pub(crate) mod flat;
 
 use std::{
-    convert::TryFrom,
     io::{self, BufRead, Write},
+    path::PathBuf,
 };
 
 use serde::{de::DeserializeOwned, Deserialize, Serialize};
 
-use crate::{
-    rpc::{ListMacrosResult, ListMacrosTask},
-    ExpansionResult, ExpansionTask,
-};
+use crate::ProcMacroKind;
+
+pub use crate::msg::flat::FlatTree;
 
 #[derive(Debug, Serialize, Deserialize)]
 pub enum Request {
-    ListMacro(ListMacrosTask),
-    ExpansionMacro(ExpansionTask),
+    ListMacros { dylib_path: PathBuf },
+    ExpandMacro(ExpandMacro),
 }
 
 #[derive(Debug, Serialize, Deserialize)]
 pub enum Response {
-    Error(ResponseError),
-    ListMacro(ListMacrosResult),
-    ExpansionMacro(ExpansionResult),
+    ListMacros(Result<Vec<(String, ProcMacroKind)>, String>),
+    ExpandMacro(Result<FlatTree, PanicMessage>),
 }
 
-macro_rules! impl_try_from_response {
-    ($ty:ty, $tag:ident) => {
-        impl TryFrom<Response> for $ty {
-            type Error = &'static str;
-            fn try_from(value: Response) -> Result<Self, Self::Error> {
-                match value {
-                    Response::$tag(res) => Ok(res),
-                    _ => Err(concat!("Failed to convert response to ", stringify!($tag))),
-                }
-            }
-        }
-    };
-}
+#[derive(Debug, Serialize, Deserialize)]
+pub struct PanicMessage(pub String);
 
-impl_try_from_response!(ListMacrosResult, ListMacro);
-impl_try_from_response!(ExpansionResult, ExpansionMacro);
+#[derive(Debug, Serialize, Deserialize)]
+pub struct ExpandMacro {
+    /// Argument of macro call.
+    ///
+    /// In custom derive this will be a struct or enum; in attribute-like macro - underlying
+    /// item; in function-like macro - the macro body.
+    pub macro_body: FlatTree,
 
-#[derive(Debug, Serialize, Deserialize, Clone)]
-pub struct ResponseError {
-    pub code: ErrorCode,
-    pub message: String,
-}
+    /// Name of macro to expand.
+    ///
+    /// In custom derive this is the name of the derived trait (`Serialize`, `Getters`, etc.).
+    /// In attribute-like and function-like macros - single name of macro itself (`show_streams`).
+    pub macro_name: String,
 
-#[derive(Debug, Serialize, Deserialize, Clone)]
-pub enum ErrorCode {
-    ServerErrorEnd,
-    ExpansionError,
+    /// Possible attributes for the attribute-like macros.
+    pub attributes: Option<FlatTree>,
+
+    pub lib: PathBuf,
+
+    /// Environment variables to set during macro expansion.
+    pub env: Vec<(String, String)>,
 }
 
 pub trait Message: Serialize + DeserializeOwned {
@@ -108,3 +104,51 @@ fn write_json(out: &mut impl Write, msg: &str) -> io::Result<()> {
     out.flush()?;
     Ok(())
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use tt::*;
+
+    fn fixture_token_tree() -> Subtree {
+        let mut subtree = Subtree::default();
+        subtree
+            .token_trees
+            .push(TokenTree::Leaf(Ident { text: "struct".into(), id: TokenId(0) }.into()));
+        subtree
+            .token_trees
+            .push(TokenTree::Leaf(Ident { text: "Foo".into(), id: TokenId(1) }.into()));
+        subtree.token_trees.push(TokenTree::Leaf(Leaf::Literal(Literal {
+            text: "Foo".into(),
+            id: TokenId::unspecified(),
+        })));
+        subtree.token_trees.push(TokenTree::Leaf(Leaf::Punct(Punct {
+            char: '@',
+            id: TokenId::unspecified(),
+            spacing: Spacing::Joint,
+        })));
+        subtree.token_trees.push(TokenTree::Subtree(Subtree {
+            delimiter: Some(Delimiter { id: TokenId(2), kind: DelimiterKind::Brace }),
+            token_trees: vec![],
+        }));
+        subtree
+    }
+
+    #[test]
+    fn test_proc_macro_rpc_works() {
+        let tt = fixture_token_tree();
+        let task = ExpandMacro {
+            macro_body: FlatTree::new(&tt),
+            macro_name: Default::default(),
+            attributes: None,
+            lib: std::env::current_dir().unwrap(),
+            env: Default::default(),
+        };
+
+        let json = serde_json::to_string(&task).unwrap();
+        // println!("{}", json);
+        let back: ExpandMacro = serde_json::from_str(&json).unwrap();
+
+        assert_eq!(tt, back.macro_body.to_subtree());
+    }
+}
diff --git a/crates/proc_macro_api/src/rpc/flat.rs b/crates/proc_macro_api/src/msg/flat.rs
similarity index 100%
rename from crates/proc_macro_api/src/rpc/flat.rs
rename to crates/proc_macro_api/src/msg/flat.rs
diff --git a/crates/proc_macro_api/src/process.rs b/crates/proc_macro_api/src/process.rs
index 2fa5d65be28..16741fc0ad1 100644
--- a/crates/proc_macro_api/src/process.rs
+++ b/crates/proc_macro_api/src/process.rs
@@ -1,7 +1,6 @@
 //! Handle process life-time and message passing for proc-macro client
 
 use std::{
-    convert::{TryFrom, TryInto},
     ffi::{OsStr, OsString},
     io::{self, BufRead, BufReader, Write},
     process::{Child, ChildStdin, ChildStdout, Command, Stdio},
@@ -11,8 +10,8 @@ use paths::{AbsPath, AbsPathBuf};
 use stdx::JodChild;
 
 use crate::{
-    msg::{ErrorCode, Message, Request, Response, ResponseError},
-    rpc::{ListMacrosResult, ListMacrosTask, ProcMacroKind},
+    msg::{Message, Request, Response},
+    ProcMacroKind, ServerError,
 };
 
 #[derive(Debug)]
@@ -38,42 +37,22 @@ impl ProcMacroProcessSrv {
     pub(crate) fn find_proc_macros(
         &mut self,
         dylib_path: &AbsPath,
-    ) -> Result<Vec<(String, ProcMacroKind)>, tt::ExpansionError> {
-        let task = ListMacrosTask { lib: dylib_path.to_path_buf().into() };
+    ) -> Result<Result<Vec<(String, ProcMacroKind)>, String>, ServerError> {
+        let request = Request::ListMacros { dylib_path: dylib_path.to_path_buf().into() };
 
-        let result: ListMacrosResult = self.send_task(Request::ListMacro(task))?;
-        Ok(result.macros)
+        let response = self.send_task(request)?;
+
+        match response {
+            Response::ListMacros(it) => Ok(it),
+            Response::ExpandMacro { .. } => {
+                Err(ServerError { message: "unexpected response".to_string(), io: None })
+            }
+        }
     }
 
-    pub(crate) fn send_task<R>(&mut self, req: Request) -> Result<R, tt::ExpansionError>
-    where
-        R: TryFrom<Response, Error = &'static str>,
-    {
+    pub(crate) fn send_task(&mut self, req: Request) -> Result<Response, ServerError> {
         let mut buf = String::new();
-        let res = match send_request(&mut self.stdin, &mut self.stdout, req, &mut buf) {
-            Ok(res) => res,
-            Err(err) => {
-                let result = self.process.child.try_wait();
-                tracing::error!(
-                    "proc macro server crashed, server process state: {:?}, server request error: {:?}",
-                    result,
-                    err
-                );
-                let res = Response::Error(ResponseError {
-                    code: ErrorCode::ServerErrorEnd,
-                    message: "proc macro server crashed".into(),
-                });
-                Some(res)
-            }
-        };
-
-        match res {
-            Some(Response::Error(err)) => Err(tt::ExpansionError::ExpansionError(err.message)),
-            Some(res) => Ok(res.try_into().map_err(|err| {
-                tt::ExpansionError::Unknown(format!("Fail to get response, reason : {:#?} ", err))
-            })?),
-            None => Err(tt::ExpansionError::Unknown("Empty result".into())),
-        }
+        send_request(&mut self.stdin, &mut self.stdout, req, &mut buf)
     }
 }
 
@@ -118,7 +97,10 @@ fn send_request(
     mut reader: &mut impl BufRead,
     req: Request,
     buf: &mut String,
-) -> io::Result<Option<Response>> {
-    req.write(&mut writer)?;
-    Response::read(&mut reader, buf)
+) -> Result<Response, ServerError> {
+    req.write(&mut writer)
+        .map_err(|err| ServerError { message: "failed to write request".into(), io: Some(err) })?;
+    let res = Response::read(&mut reader, buf)
+        .map_err(|err| ServerError { message: "failed to read response".into(), io: Some(err) })?;
+    res.ok_or_else(|| ServerError { message: "server exited".into(), io: None })
 }
diff --git a/crates/proc_macro_api/src/rpc.rs b/crates/proc_macro_api/src/rpc.rs
deleted file mode 100644
index 31d0aa56d3b..00000000000
--- a/crates/proc_macro_api/src/rpc.rs
+++ /dev/null
@@ -1,107 +0,0 @@
-//! Data structure serialization related stuff for RPC
-//!
-//! Defines all necessary rpc serialization data structures,
-//! which includes `tt` related data and some task messages.
-//! Although adding `Serialize` and `Deserialize` traits to `tt` directly seems
-//! to be much easier, we deliberately duplicate `tt` structs with `#[serde(with = "XXDef")]`
-//! for separation of code responsibility.
-pub(crate) mod flat;
-
-use std::path::PathBuf;
-
-use serde::{Deserialize, Serialize};
-
-use crate::rpc::flat::FlatTree;
-
-#[derive(Clone, Eq, PartialEq, Debug, Serialize, Deserialize)]
-pub struct ListMacrosTask {
-    pub lib: PathBuf,
-}
-
-#[derive(Copy, Clone, Eq, PartialEq, Debug, Serialize, Deserialize)]
-pub enum ProcMacroKind {
-    CustomDerive,
-    FuncLike,
-    Attr,
-}
-
-#[derive(Clone, Eq, PartialEq, Debug, Default, Serialize, Deserialize)]
-pub struct ListMacrosResult {
-    pub macros: Vec<(String, ProcMacroKind)>,
-}
-
-#[derive(Debug, Serialize, Deserialize)]
-pub struct ExpansionTask {
-    /// Argument of macro call.
-    ///
-    /// In custom derive this will be a struct or enum; in attribute-like macro - underlying
-    /// item; in function-like macro - the macro body.
-    pub macro_body: FlatTree,
-
-    /// Name of macro to expand.
-    ///
-    /// In custom derive this is the name of the derived trait (`Serialize`, `Getters`, etc.).
-    /// In attribute-like and function-like macros - single name of macro itself (`show_streams`).
-    pub macro_name: String,
-
-    /// Possible attributes for the attribute-like macros.
-    pub attributes: Option<FlatTree>,
-
-    pub lib: PathBuf,
-
-    /// Environment variables to set during macro expansion.
-    pub env: Vec<(String, String)>,
-}
-
-#[derive(Debug, Serialize, Deserialize)]
-pub struct ExpansionResult {
-    pub expansion: FlatTree,
-}
-
-#[cfg(test)]
-mod tests {
-    use super::*;
-    use tt::*;
-
-    fn fixture_token_tree() -> Subtree {
-        let mut subtree = Subtree::default();
-        subtree
-            .token_trees
-            .push(TokenTree::Leaf(Ident { text: "struct".into(), id: TokenId(0) }.into()));
-        subtree
-            .token_trees
-            .push(TokenTree::Leaf(Ident { text: "Foo".into(), id: TokenId(1) }.into()));
-        subtree.token_trees.push(TokenTree::Leaf(Leaf::Literal(Literal {
-            text: "Foo".into(),
-            id: TokenId::unspecified(),
-        })));
-        subtree.token_trees.push(TokenTree::Leaf(Leaf::Punct(Punct {
-            char: '@',
-            id: TokenId::unspecified(),
-            spacing: Spacing::Joint,
-        })));
-        subtree.token_trees.push(TokenTree::Subtree(Subtree {
-            delimiter: Some(Delimiter { id: TokenId(2), kind: DelimiterKind::Brace }),
-            token_trees: vec![],
-        }));
-        subtree
-    }
-
-    #[test]
-    fn test_proc_macro_rpc_works() {
-        let tt = fixture_token_tree();
-        let task = ExpansionTask {
-            macro_body: FlatTree::new(&tt),
-            macro_name: Default::default(),
-            attributes: None,
-            lib: std::env::current_dir().unwrap(),
-            env: Default::default(),
-        };
-
-        let json = serde_json::to_string(&task).unwrap();
-        // println!("{}", json);
-        let back: ExpansionTask = serde_json::from_str(&json).unwrap();
-
-        assert_eq!(tt, back.macro_body.to_subtree());
-    }
-}
diff --git a/crates/proc_macro_srv/src/abis/abi_1_47/mod.rs b/crates/proc_macro_srv/src/abis/abi_1_47/mod.rs
index 6bbdcc58686..b944143a079 100644
--- a/crates/proc_macro_srv/src/abis/abi_1_47/mod.rs
+++ b/crates/proc_macro_srv/src/abis/abi_1_47/mod.rs
@@ -7,8 +7,8 @@ mod proc_macro;
 #[allow(dead_code)]
 #[doc(hidden)]
 mod rustc_server;
-use libloading::Library;
 
+use libloading::Library;
 use proc_macro_api::ProcMacroKind;
 
 use super::PanicMessage;
diff --git a/crates/proc_macro_srv/src/abis/abi_1_55/mod.rs b/crates/proc_macro_srv/src/abis/abi_1_55/mod.rs
index 81260162075..81c55cf10c7 100644
--- a/crates/proc_macro_srv/src/abis/abi_1_55/mod.rs
+++ b/crates/proc_macro_srv/src/abis/abi_1_55/mod.rs
@@ -7,8 +7,8 @@ mod proc_macro;
 #[allow(dead_code)]
 #[doc(hidden)]
 mod rustc_server;
-use libloading::Library;
 
+use libloading::Library;
 use proc_macro_api::ProcMacroKind;
 
 use super::PanicMessage;
diff --git a/crates/proc_macro_srv/src/abis/abi_1_56/mod.rs b/crates/proc_macro_srv/src/abis/abi_1_56/mod.rs
index 921beca1438..96b71d3f218 100644
--- a/crates/proc_macro_srv/src/abis/abi_1_56/mod.rs
+++ b/crates/proc_macro_srv/src/abis/abi_1_56/mod.rs
@@ -7,8 +7,8 @@ mod proc_macro;
 #[allow(dead_code)]
 #[doc(hidden)]
 mod rustc_server;
-use libloading::Library;
 
+use libloading::Library;
 use proc_macro_api::ProcMacroKind;
 
 use super::PanicMessage;
diff --git a/crates/proc_macro_srv/src/cli.rs b/crates/proc_macro_srv/src/cli.rs
index fe3665110db..f1e131c135d 100644
--- a/crates/proc_macro_srv/src/cli.rs
+++ b/crates/proc_macro_srv/src/cli.rs
@@ -1,8 +1,9 @@
 //! Driver for proc macro server
+use std::io;
+
+use proc_macro_api::msg::{self, Message};
 
 use crate::ProcMacroSrv;
-use proc_macro_api::msg::{self, Message};
-use std::io;
 
 pub fn run() -> io::Result<()> {
     let mut srv = ProcMacroSrv::default();
@@ -10,22 +11,12 @@ pub fn run() -> io::Result<()> {
 
     while let Some(req) = read_request(&mut buf)? {
         let res = match req {
-            msg::Request::ListMacro(task) => srv.list_macros(&task).map(msg::Response::ListMacro),
-            msg::Request::ExpansionMacro(task) => {
-                srv.expand(task).map(msg::Response::ExpansionMacro)
+            msg::Request::ListMacros { dylib_path } => {
+                msg::Response::ListMacros(srv.list_macros(&dylib_path))
             }
+            msg::Request::ExpandMacro(task) => msg::Response::ExpandMacro(srv.expand(task)),
         };
-
-        let msg = res.unwrap_or_else(|err| {
-            msg::Response::Error(msg::ResponseError {
-                code: msg::ErrorCode::ExpansionError,
-                message: err,
-            })
-        });
-
-        if let Err(err) = write_response(msg) {
-            eprintln!("Write message error: {}", err);
-        }
+        write_response(res)?
     }
 
     Ok(())
diff --git a/crates/proc_macro_srv/src/lib.rs b/crates/proc_macro_srv/src/lib.rs
index c60dd2efc53..6f3943870e6 100644
--- a/crates/proc_macro_srv/src/lib.rs
+++ b/crates/proc_macro_srv/src/lib.rs
@@ -12,10 +12,8 @@
 #![allow(unreachable_pub)]
 
 mod dylib;
-
 mod abis;
 
-use proc_macro_api::{ExpansionResult, ExpansionTask, FlatTree, ListMacrosResult, ListMacrosTask};
 use std::{
     collections::{hash_map::Entry, HashMap},
     env, fs,
@@ -23,14 +21,22 @@ use std::{
     time::SystemTime,
 };
 
+use proc_macro_api::{
+    msg::{ExpandMacro, FlatTree, PanicMessage},
+    ProcMacroKind,
+};
+
 #[derive(Default)]
 pub(crate) struct ProcMacroSrv {
     expanders: HashMap<(PathBuf, SystemTime), dylib::Expander>,
 }
 
 impl ProcMacroSrv {
-    pub fn expand(&mut self, task: ExpansionTask) -> Result<ExpansionResult, String> {
-        let expander = self.expander(task.lib.as_ref())?;
+    pub fn expand(&mut self, task: ExpandMacro) -> Result<FlatTree, PanicMessage> {
+        let expander = self.expander(task.lib.as_ref()).map_err(|err| {
+            debug_assert!(false, "should list macros before asking to expand");
+            PanicMessage(format!("failed to load macro: {}", err))
+        })?;
 
         let mut prev_env = HashMap::new();
         for (k, v) in &task.env {
@@ -51,15 +57,15 @@ impl ProcMacroSrv {
             }
         }
 
-        match result {
-            Ok(expansion) => Ok(ExpansionResult { expansion }),
-            Err(msg) => Err(format!("proc-macro panicked: {}", msg)),
-        }
+        result.map_err(PanicMessage)
     }
 
-    pub fn list_macros(&mut self, task: &ListMacrosTask) -> Result<ListMacrosResult, String> {
-        let expander = self.expander(task.lib.as_ref())?;
-        Ok(ListMacrosResult { macros: expander.list_macros() })
+    pub(crate) fn list_macros(
+        &mut self,
+        dylib_path: &Path,
+    ) -> Result<Vec<(String, ProcMacroKind)>, String> {
+        let expander = self.expander(dylib_path)?;
+        Ok(expander.list_macros())
     }
 
     fn expander(&mut self, path: &Path) -> Result<&dylib::Expander, String> {
diff --git a/crates/proc_macro_srv/src/tests/utils.rs b/crates/proc_macro_srv/src/tests/utils.rs
index e5520174488..f8cbf70b619 100644
--- a/crates/proc_macro_srv/src/tests/utils.rs
+++ b/crates/proc_macro_srv/src/tests/utils.rs
@@ -3,7 +3,6 @@
 use crate::dylib;
 use crate::ProcMacroSrv;
 use expect_test::Expect;
-use proc_macro_api::ListMacrosTask;
 use std::str::FromStr;
 
 pub mod fixtures {
@@ -40,9 +39,9 @@ fn assert_expand_impl(macro_name: &str, input: &str, attr: Option<&str>, expect:
     expect.assert_eq(&format!("{:?}", res));
 }
 
-pub fn list() -> Vec<String> {
-    let task = ListMacrosTask { lib: fixtures::proc_macro_test_dylib_path() };
+pub(crate) fn list() -> Vec<String> {
+    let dylib_path = fixtures::proc_macro_test_dylib_path();
     let mut srv = ProcMacroSrv::default();
-    let res = srv.list_macros(&task).unwrap();
-    res.macros.into_iter().map(|(name, kind)| format!("{} [{:?}]", name, kind)).collect()
+    let res = srv.list_macros(&dylib_path).unwrap();
+    res.into_iter().map(|(name, kind)| format!("{} [{:?}]", name, kind)).collect()
 }
diff --git a/crates/rust-analyzer/src/reload.rs b/crates/rust-analyzer/src/reload.rs
index 7e419ba839d..0a1226af5ea 100644
--- a/crates/rust-analyzer/src/reload.rs
+++ b/crates/rust-analyzer/src/reload.rs
@@ -7,7 +7,7 @@ use ide::Change;
 use ide_db::base_db::{
     CrateGraph, Env, ProcMacro, ProcMacroExpander, ProcMacroKind, SourceRoot, VfsPath,
 };
-use proc_macro_api::ProcMacroServer;
+use proc_macro_api::{MacroDylib, ProcMacroServer};
 use project_model::{ProjectWorkspace, WorkspaceBuildScripts};
 use vfs::{file_set::FileSetConfig, AbsPath, AbsPathBuf, ChangeKind};
 
@@ -557,10 +557,32 @@ impl SourceRootConfig {
 }
 
 pub(crate) fn load_proc_macro(client: Option<&ProcMacroServer>, path: &AbsPath) -> Vec<ProcMacro> {
+    let dylib = match MacroDylib::new(path.to_path_buf()) {
+        Ok(it) => it,
+        Err(err) => {
+            // FIXME: that's not really right -- we store this error in a
+            // persistent status.
+            tracing::warn!("failed to load proc macro: {}", err);
+            return Vec::new();
+        }
+    };
+
     return client
-        .map(|it| it.load_dylib(path))
-        .unwrap_or_default()
+        .map(|it| it.load_dylib(dylib))
         .into_iter()
+        .flat_map(|it| match it {
+            Ok(Ok(macros)) => macros,
+            Err(err) => {
+                tracing::error!("proc macro server crashed: {}", err);
+                Vec::new()
+            }
+            Ok(Err(err)) => {
+                // FIXME: that's not really right -- we store this error in a
+                // persistent status.
+                tracing::warn!("failed to load proc macro: {}", err);
+                Vec::new()
+            }
+        })
         .map(expander_to_proc_macro)
         .collect();
 
@@ -586,7 +608,11 @@ pub(crate) fn load_proc_macro(client: Option<&ProcMacroServer>, path: &AbsPath)
             env: &Env,
         ) -> Result<tt::Subtree, tt::ExpansionError> {
             let env = env.iter().map(|(k, v)| (k.to_string(), v.to_string())).collect();
-            self.0.expand(subtree, attrs, env)
+            match self.0.expand(subtree, attrs, env) {
+                Ok(Ok(subtree)) => Ok(subtree),
+                Ok(Err(err)) => Err(tt::ExpansionError::ExpansionError(err.0)),
+                Err(err) => Err(tt::ExpansionError::Unknown(err.to_string())),
+            }
         }
     }
 }