From b23b27631091e44445c829cc8fc8fe4a5c910fc8 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Mon, 11 Apr 2022 14:38:30 +0200 Subject: [PATCH] internal: Show more project building errors to the user --- crates/flycheck/src/lib.rs | 2 +- crates/project_model/src/build_scripts.rs | 22 +++++--- crates/project_model/src/workspace.rs | 4 +- crates/rust-analyzer/src/bin/main.rs | 4 +- crates/rust-analyzer/src/global_state.rs | 6 ++- crates/rust-analyzer/src/lsp_utils.rs | 20 +++++++ crates/rust-analyzer/src/main_loop.rs | 12 ++--- crates/rust-analyzer/src/reload.rs | 63 +++++++++++------------ 8 files changed, 82 insertions(+), 51 deletions(-) diff --git a/crates/flycheck/src/lib.rs b/crates/flycheck/src/lib.rs index 4c84c62016d..2cd19952678 100644 --- a/crates/flycheck/src/lib.rs +++ b/crates/flycheck/src/lib.rs @@ -329,7 +329,7 @@ fn run(self, command: Command) -> io::Result<()> { Ok(output) if output.status.success() => Ok(()), Ok(output) => { Err(io::Error::new(io::ErrorKind::Other, format!( - "Cargo watcher failed, the command produced no valid metadata (exit code: {:?})\nCargo's stderr output:\n{}", + "Cargo watcher failed, the command produced no valid metadata (exit code: {:?}):\n{}", output.status, error ))) } diff --git a/crates/project_model/src/build_scripts.rs b/crates/project_model/src/build_scripts.rs index d96c135ba5e..5aa23331a13 100644 --- a/crates/project_model/src/build_scripts.rs +++ b/crates/project_model/src/build_scripts.rs @@ -7,11 +7,11 @@ //! here, but it covers procedural macros as well. use std::{ + io, path::PathBuf, process::{Command, Stdio}, }; -use anyhow::Result; use cargo_metadata::{camino::Utf8Path, Message}; use la_arena::ArenaMap; use paths::AbsPathBuf; @@ -80,7 +80,7 @@ pub(crate) fn run( config: &CargoConfig, workspace: &CargoWorkspace, progress: &dyn Fn(String), - ) -> Result { + ) -> io::Result { let mut cmd = Self::build_command(config); if config.wrap_rustc_in_build_scripts { @@ -107,12 +107,12 @@ pub(crate) fn run( by_id.insert(workspace[package].id.clone(), package); } - let mut callback_err = None; + let mut cfg_err = None; let mut stderr = String::new(); let output = stdx::process::streaming_output( cmd, &mut |line| { - if callback_err.is_some() { + if cfg_err.is_some() { return; } @@ -126,7 +126,7 @@ pub(crate) fn run( match message { Message::BuildScriptExecuted(message) => { let package = match by_id.get(&message.package_id.repr) { - Some(it) => *it, + Some(&it) => it, None => return, }; let cfgs = { @@ -135,7 +135,7 @@ pub(crate) fn run( match cfg.parse::() { Ok(it) => acc.push(it), Err(err) => { - callback_err = Some(anyhow::format_err!( + cfg_err = Some(format!( "invalid cfg from cargo-metadata: {}", err )); @@ -191,6 +191,11 @@ pub(crate) fn run( for package in workspace.packages() { let package_build_data = &mut res.outputs[package]; + tracing::info!( + "{} BuildScriptOutput: {:?}", + workspace[package].manifest.parent().display(), + package_build_data, + ); // inject_cargo_env(package, package_build_data); if let Some(out_dir) = &package_build_data.out_dir { // NOTE: cargo and rustc seem to hide non-UTF-8 strings from env! and option_env!() @@ -200,6 +205,11 @@ pub(crate) fn run( } } + if let Some(cfg_err) = cfg_err { + stderr.push_str(&cfg_err); + stderr.push('\n'); + } + if !output.status.success() { if stderr.is_empty() { stderr = "cargo check failed".to_string(); diff --git a/crates/project_model/src/workspace.rs b/crates/project_model/src/workspace.rs index 3ceb0469394..1330a869509 100644 --- a/crates/project_model/src/workspace.rs +++ b/crates/project_model/src/workspace.rs @@ -256,7 +256,9 @@ pub fn run_build_scripts( ) -> Result { match self { ProjectWorkspace::Cargo { cargo, .. } => { - WorkspaceBuildScripts::run(config, cargo, progress) + WorkspaceBuildScripts::run(config, cargo, progress).with_context(|| { + format!("Failed to run build scripts for {}", &cargo.workspace_root().display()) + }) } ProjectWorkspace::Json { .. } | ProjectWorkspace::DetachedFiles { .. } => { Ok(WorkspaceBuildScripts::default()) diff --git a/crates/rust-analyzer/src/bin/main.rs b/crates/rust-analyzer/src/bin/main.rs index a4e985f43b3..15ae0e64806 100644 --- a/crates/rust-analyzer/src/bin/main.rs +++ b/crates/rust-analyzer/src/bin/main.rs @@ -119,7 +119,9 @@ fn setup_logging(log_file: Option<&Path>) -> Result<()> { None => None, }; let filter = env::var("RA_LOG").ok(); - logger::Logger::new(log_file, filter.as_deref()).install()?; + // deliberately enable all `error` logs if the user has not set RA_LOG, as there is usually useful + // information in there for debugging + logger::Logger::new(log_file, filter.as_deref().or(Some("error"))).install()?; profile::init(); diff --git a/crates/rust-analyzer/src/global_state.rs b/crates/rust-analyzer/src/global_state.rs index 02482d58893..8b47ef02830 100644 --- a/crates/rust-analyzer/src/global_state.rs +++ b/crates/rust-analyzer/src/global_state.rs @@ -190,6 +190,7 @@ pub(crate) fn process_changes(&mut self) -> bool { for file in changed_files { if !file.is_created_or_deleted() { + // FIXME: https://github.com/rust-analyzer/rust-analyzer/issues/11357 let crates = self.analysis_host.raw_database().relevant_crates(file.file_id); let crate_graph = self.analysis_host.raw_database().crate_graph(); @@ -255,6 +256,7 @@ pub(crate) fn send_request( let request = self.req_queue.outgoing.register(R::METHOD.to_string(), params, handler); self.send(request.into()); } + pub(crate) fn complete_request(&mut self, response: lsp_server::Response) { let handler = self .req_queue @@ -281,6 +283,7 @@ pub(crate) fn register_request( .incoming .register(request.id.clone(), (request.method.clone(), request_received)); } + pub(crate) fn respond(&mut self, response: lsp_server::Response) { if let Some((method, start)) = self.req_queue.incoming.complete(response.id.clone()) { if let Some(err) = &response.error { @@ -294,6 +297,7 @@ pub(crate) fn respond(&mut self, response: lsp_server::Response) { self.send(response.into()); } } + pub(crate) fn cancel(&mut self, request_id: lsp_server::RequestId) { if let Some(response) = self.req_queue.incoming.cancel(request_id) { self.send(response.into()); @@ -307,7 +311,7 @@ fn send(&mut self, message: lsp_server::Message) { impl Drop for GlobalState { fn drop(&mut self) { - self.analysis_host.request_cancellation() + self.analysis_host.request_cancellation(); } } diff --git a/crates/rust-analyzer/src/lsp_utils.rs b/crates/rust-analyzer/src/lsp_utils.rs index b09c411908a..460ae4ef4da 100644 --- a/crates/rust-analyzer/src/lsp_utils.rs +++ b/crates/rust-analyzer/src/lsp_utils.rs @@ -47,6 +47,26 @@ pub(crate) fn show_message(&mut self, typ: lsp_types::MessageType, message: Stri ) } + /// Sends a notification to the client containing the error `message`. + /// If `additional_info` is [`Some`], appends a note to the notification telling to check the logs. + /// This will always log `message` + `additional_info` to the server's error log. + pub(crate) fn show_and_log_error(&mut self, message: String, additional_info: Option) { + let mut message = message; + match additional_info { + Some(additional_info) => { + tracing::error!("{}\n\n{}", &message, &additional_info); + if tracing::enabled!(tracing::Level::ERROR) { + message.push_str("\n\nCheck the server logs for additional info."); + } + } + None => tracing::error!("{}", &message), + } + + self.send_notification::( + lsp_types::ShowMessageParams { typ: lsp_types::MessageType::ERROR, message }, + ) + } + /// rust-analyzer is resilient -- if it fails, this doesn't usually affect /// the user experience. Part of that is that we deliberately hide panics /// from the user. diff --git a/crates/rust-analyzer/src/main_loop.rs b/crates/rust-analyzer/src/main_loop.rs index e305212165c..de44ba5e077 100644 --- a/crates/rust-analyzer/src/main_loop.rs +++ b/crates/rust-analyzer/src/main_loop.rs @@ -111,10 +111,7 @@ fn run(mut self, inbox: Receiver) -> Result<()> { && self.config.detached_files().is_empty() && self.config.notifications().cargo_toml_not_found { - self.show_message( - lsp_types::MessageType::ERROR, - "rust-analyzer failed to discover workspace".to_string(), - ); + self.show_and_log_error("rust-analyzer failed to discover workspace".to_string(), None); }; if self.config.did_save_text_document_dynamic_registration() { @@ -406,9 +403,9 @@ fn handle_event(&mut self, event: Event) -> Result<()> { flycheck::Progress::DidCancel => (Progress::End, None), flycheck::Progress::DidFinish(result) => { if let Err(err) = result { - self.show_message( - lsp_types::MessageType::ERROR, - format!("cargo check failed: {}", err), + self.show_and_log_error( + "cargo check failed".to_string(), + Some(err.to_string()), ); } (Progress::End, None) @@ -564,7 +561,6 @@ fn on_request(&mut self, request_received: Instant, req: Request) -> Result<()> if self.workspaces.is_empty() && !self.is_quiescent() { self.respond(lsp_server::Response::new_err( req.id, - // FIXME: i32 should impl From (from() guarantees lossless conversion) lsp_server::ErrorCode::ContentModified as i32, "waiting for cargo metadata or cargo check".to_owned(), )); diff --git a/crates/rust-analyzer/src/reload.rs b/crates/rust-analyzer/src/reload.rs index eecc83e02ac..5189d94eae9 100644 --- a/crates/rust-analyzer/src/reload.rs +++ b/crates/rust-analyzer/src/reload.rs @@ -72,9 +72,10 @@ pub(crate) fn current_status(&self) -> lsp_ext::ServerStatusParams { status.message = Some("Reload required due to source changes of a procedural macro.".into()) } - if let Some(error) = self.fetch_build_data_error() { + if let Err(_) = self.fetch_build_data_error() { status.health = lsp_ext::Health::Warning; - status.message = Some(error) + status.message = + Some("Failed to run build scripts of some packages, check the logs.".to_string()); } if !self.config.cargo_autoreload() && self.is_quiescent() @@ -84,7 +85,7 @@ pub(crate) fn current_status(&self) -> lsp_ext::ServerStatusParams { status.message = Some("Workspace reload required".to_string()) } - if let Some(error) = self.fetch_workspace_error() { + if let Err(error) = self.fetch_workspace_error() { status.health = lsp_ext::Health::Error; status.message = Some(error) } @@ -167,8 +168,8 @@ pub(crate) fn switch_workspaces(&mut self) { let _p = profile::span("GlobalState::switch_workspaces"); tracing::info!("will switch workspaces"); - if let Some(error_message) = self.fetch_workspace_error() { - tracing::error!("failed to switch workspaces: {}", error_message); + if let Err(error_message) = self.fetch_workspace_error() { + self.show_and_log_error(error_message, None); if !self.workspaces.is_empty() { // It only makes sense to switch to a partially broken workspace // if we don't have any workspace at all yet. @@ -176,8 +177,11 @@ pub(crate) fn switch_workspaces(&mut self) { } } - if let Some(error_message) = self.fetch_build_data_error() { - tracing::error!("failed to switch build data: {}", error_message); + if let Err(error) = self.fetch_build_data_error() { + self.show_and_log_error( + "rust-analyzer failed to run build scripts".to_string(), + Some(error), + ); } let workspaces = self @@ -277,20 +281,18 @@ fn eq_ignore_build_data<'a>( let project_folders = ProjectFolders::new(&self.workspaces, &files_config.exclude); if self.proc_macro_client.is_none() { - self.proc_macro_client = match self.config.proc_macro_srv() { - None => None, - Some((path, args)) => match ProcMacroServer::spawn(path.clone(), args) { - Ok(it) => Some(it), + if let Some((path, args)) = self.config.proc_macro_srv() { + match ProcMacroServer::spawn(path.clone(), args) { + Ok(it) => self.proc_macro_client = Some(it), Err(err) => { tracing::error!( "Failed to run proc_macro_srv from path {}, error: {:?}", path.display(), err ); - None } - }, - }; + } + } } let watch = match files_config.watcher { @@ -348,7 +350,7 @@ fn eq_ignore_build_data<'a>( tracing::info!("did switch workspaces"); } - fn fetch_workspace_error(&self) -> Option { + fn fetch_workspace_error(&self) -> Result<(), String> { let mut buf = String::new(); for ws in self.fetch_workspaces_queue.last_op_result() { @@ -358,35 +360,30 @@ fn fetch_workspace_error(&self) -> Option { } if buf.is_empty() { - return None; + return Ok(()); } - Some(buf) + Err(buf) } - fn fetch_build_data_error(&self) -> Option { - let mut buf = "rust-analyzer failed to run build scripts:\n".to_string(); - let mut has_errors = false; + fn fetch_build_data_error(&self) -> Result<(), String> { + let mut buf = String::new(); for ws in &self.fetch_build_data_queue.last_op_result().1 { match ws { - Ok(data) => { - if let Some(err) = data.error() { - has_errors = true; - stdx::format_to!(buf, "{:#}\n", err); - } - } - Err(err) => { - has_errors = true; - stdx::format_to!(buf, "{:#}\n", err); - } + Ok(data) => match data.error() { + Some(stderr) => stdx::format_to!(buf, "{:#}\n", stderr), + _ => (), + }, + // io errors + Err(err) => stdx::format_to!(buf, "{:#}\n", err), } } - if has_errors { - Some(buf) + if buf.is_empty() { + Ok(()) } else { - None + Err(buf) } } -- 2.44.0