Skip to content

internal: Improve error message when the proc-macro server unexpectedly exits #17526

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions crates/proc-macro-api/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,10 @@ impl ProcMacroServer {
Err(message) => Err(ServerError { message, io: None }),
}
}

pub fn exited(&self) -> Option<&ServerError> {
self.process.exited()
}
}

impl ProcMacro {
Expand Down
80 changes: 51 additions & 29 deletions crates/proc-macro-api/src/process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@

use std::{
io::{self, BufRead, BufReader, Read, Write},
panic::AssertUnwindSafe,
process::{Child, ChildStdin, ChildStdout, Command, Stdio},
sync::{Arc, Mutex},
sync::{Arc, Mutex, OnceLock},
};

use paths::AbsPath;
Expand All @@ -22,15 +23,15 @@ pub(crate) struct ProcMacroProcessSrv {
state: Mutex<ProcessSrvState>,
version: u32,
mode: SpanMode,
/// Populated when the server exits.
exited: OnceLock<AssertUnwindSafe<ServerError>>,
}

#[derive(Debug)]
struct ProcessSrvState {
process: Process,
stdin: ChildStdin,
stdout: BufReader<ChildStdout>,
/// Populated when the server exits.
server_exited: Option<ServerError>,
}

impl ProcMacroProcessSrv {
Expand All @@ -44,9 +45,10 @@ impl ProcMacroProcessSrv {
let (stdin, stdout) = process.stdio().expect("couldn't access child stdio");

io::Result::Ok(ProcMacroProcessSrv {
state: Mutex::new(ProcessSrvState { process, stdin, stdout, server_exited: None }),
state: Mutex::new(ProcessSrvState { process, stdin, stdout }),
version: 0,
mode: SpanMode::Id,
exited: OnceLock::new(),
})
};
let mut srv = create_srv(true)?;
Expand Down Expand Up @@ -77,6 +79,10 @@ impl ProcMacroProcessSrv {
}
}

pub(crate) fn exited(&self) -> Option<&ServerError> {
self.exited.get().map(|it| &it.0)
}

pub(crate) fn version(&self) -> u32 {
self.version
}
Expand Down Expand Up @@ -118,36 +124,52 @@ impl ProcMacroProcessSrv {
}

pub(crate) fn send_task(&self, req: Request) -> Result<Response, ServerError> {
let state = &mut *self.state.lock().unwrap();
if let Some(server_error) = &state.server_exited {
return Err(server_error.clone());
if let Some(server_error) = self.exited.get() {
return Err(server_error.0.clone());
}

let state = &mut *self.state.lock().unwrap();
let mut buf = String::new();
send_request(&mut state.stdin, &mut state.stdout, req, &mut buf).map_err(|e| {
if e.io.as_ref().map(|it| it.kind()) == Some(io::ErrorKind::BrokenPipe) {
match state.process.child.try_wait() {
Ok(None) => e,
Ok(Some(status)) => {
let mut msg = String::new();
if !status.success() {
if let Some(stderr) = state.process.child.stderr.as_mut() {
_ = stderr.read_to_string(&mut msg);
send_request(&mut state.stdin, &mut state.stdout, req, &mut buf)
.and_then(|res| {
res.ok_or_else(|| {
let message = "proc-macro server did not respond with data".to_owned();
ServerError {
io: Some(Arc::new(io::Error::new(
io::ErrorKind::BrokenPipe,
message.clone(),
))),
message,
}
})
})
.map_err(|e| {
if e.io.as_ref().map(|it| it.kind()) == Some(io::ErrorKind::BrokenPipe) {
match state.process.child.try_wait() {
Ok(None) | Err(_) => e,
Ok(Some(status)) => {
let mut msg = String::new();
if !status.success() {
if let Some(stderr) = state.process.child.stderr.as_mut() {
_ = stderr.read_to_string(&mut msg);
}
}
let server_error = ServerError {
message: format!(
"proc-macro server exited with {status}{}{msg}",
if msg.is_empty() { "" } else { ": " }
),
io: None,
};
// `AssertUnwindSafe` is fine here, we already correct initialized
// server_error at this point.
self.exited.get_or_init(|| AssertUnwindSafe(server_error)).0.clone()
}
let server_error = ServerError {
message: format!("server exited with {status}: {msg}"),
io: None,
};
state.server_exited = Some(server_error.clone());
server_error
}
Err(_) => e,
} else {
e
}
} else {
e
}
})
})
}
}

Expand Down Expand Up @@ -201,7 +223,7 @@ fn send_request(
mut reader: &mut impl BufRead,
req: Request,
buf: &mut String,
) -> Result<Response, ServerError> {
) -> Result<Option<Response>, ServerError> {
req.write(write_json, &mut writer).map_err(|err| ServerError {
message: "failed to write request".into(),
io: Some(Arc::new(err)),
Expand All @@ -210,5 +232,5 @@ fn send_request(
message: "failed to read response".into(),
io: Some(Arc::new(err)),
})?;
res.ok_or_else(|| ServerError { message: "server exited".into(), io: None })
Ok(res)
}
2 changes: 2 additions & 0 deletions crates/proc-macro-srv/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,8 @@ fn expand_ra_span(
let macro_body = macro_body.to_subtree_resolved(CURRENT_API_VERSION, &span_data_table);
let attributes =
attributes.map(|it| it.to_subtree_resolved(CURRENT_API_VERSION, &span_data_table));
// Note, we spawn a new thread here so that thread locals allocation don't accumulate (this
// includes the proc-macro symbol interner)
let result = thread::scope(|s| {
let thread = thread::Builder::new()
.stack_size(EXPANDER_STACK_SIZE)
Expand Down
30 changes: 22 additions & 8 deletions crates/rust-analyzer/src/reload.rs
Original file line number Diff line number Diff line change
Expand Up @@ -184,14 +184,28 @@ impl GlobalState {
message.push_str(err);
message.push_str("\n\n");
};
if let Some(Err(err)) = proc_macro_client {
status.health |= lsp_ext::Health::Warning;
format_to!(
message,
"Failed spawning proc-macro server for workspace `{}`: {err}",
ws.manifest_or_root()
);
message.push_str("\n\n");
match proc_macro_client {
Some(Err(err)) => {
status.health |= lsp_ext::Health::Warning;
format_to!(
message,
"Failed spawning proc-macro server for workspace `{}`: {err}",
ws.manifest_or_root()
);
message.push_str("\n\n");
}
Some(Ok(client)) => {
if let Some(err) = client.exited() {
status.health |= lsp_ext::Health::Warning;
format_to!(
message,
"proc-macro server for workspace `{}` exited: {err}",
ws.manifest_or_root()
);
message.push_str("\n\n");
}
}
_ => (),
}
}
}
Expand Down