From 7f59e00ad74da0d9ee5c26ec3714ea4c60cc184b Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Sat, 24 Oct 2026 18:16:06 +0800 Subject: [PATCH] feat(lsp): Show configuration errors as "Not started", not "Crashed" A `launch_error` (e.g. the chosen jdtls JDK is too old) never starts the server, so reporting it as a crash was misleading. It is now its own status, `Misconfigured` ("Not started"), with the reason as the message; changing the settings retries as before. Run/Debug report the reason instead of "jdtls is not ready (Misconfigured)". Claude-Session: https://claude.ai/code/session_01SaPPsVPuk6ZTNsoUCNCbMv --- crates/ide-core/src/event.rs | 3 +++ crates/ide-lsp/src/jdtls.rs | 3 +++ crates/ide-lsp/src/launch.rs | 2 ++ crates/ide-lsp/src/lib.rs | 15 ++++++++++++--- crates/ide-lsp/tests/mock_server.rs | 10 ++++++---- crates/ide-lsp/tests/real_servers.rs | 2 +- src/bindings.ts | 7 ++++++- src/features/code-insight/ServerStatus.tsx | 2 +- src/features/code-insight/servers.ts | 3 ++- src/features/code-insight/views.test.ts | 1 + 10 files changed, 37 insertions(+), 11 deletions(-) diff --git a/crates/ide-core/src/event.rs b/crates/ide-core/src/event.rs index 3cfabb352..1db2bd6c4 100644 --- a/crates/ide-core/src/event.rs +++ b/crates/ide-core/src/event.rs @@ -106,6 +106,9 @@ pub enum ServerStatus { Crashed, Stopped, Missing, + /// Not started: its configuration can't launch it (e.g. no JDK new enough); + /// the message says what to change, and a settings change retries. + Misconfigured, } #[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize, Type)] diff --git a/crates/ide-lsp/src/jdtls.rs b/crates/ide-lsp/src/jdtls.rs index a3821b131..03c30b330 100644 --- a/crates/ide-lsp/src/jdtls.rs +++ b/crates/ide-lsp/src/jdtls.rs @@ -179,6 +179,9 @@ impl LspManager { ide_core::event::ServerStatus::Missing => { "jdtls is not installed (Settings › Languages & Frameworks › Language Servers)".to_owned() } + ide_core::event::ServerStatus::Misconfigured => { + server.status_message().unwrap_or_else(|| "jdtls cannot start".to_owned()) + } _ => format!("jdtls is not ready ({status:?})"), })) } diff --git a/crates/ide-lsp/src/launch.rs b/crates/ide-lsp/src/launch.rs index 728e10833..8e4c24b50 100644 --- a/crates/ide-lsp/src/launch.rs +++ b/crates/ide-lsp/src/launch.rs @@ -10,6 +10,8 @@ use std::sync::{Arc, OnceLock}; pub enum LaunchError { /// The server binary is not installed / not on `PATH`. Missing, + /// The configuration can't be launched (`LanguageServerConfig::launch_error`). + Config(String), Failed(String), } diff --git a/crates/ide-lsp/src/lib.rs b/crates/ide-lsp/src/lib.rs index bbc8e018f..90033735b 100644 --- a/crates/ide-lsp/src/lib.rs +++ b/crates/ide-lsp/src/lib.rs @@ -339,7 +339,7 @@ impl LspManager { if let Some(message) = &cfg.launch_error { // A missing binary still asks to be installed first. let missing = !me.launcher.available(&cfg, &root); - return Err(if missing { LaunchError::Missing } else { LaunchError::Failed(message.clone()) }); + return Err(if missing { LaunchError::Missing } else { LaunchError::Config(message.clone()) }); } me.launcher.launch(&cfg, &root, handler, Box::new(move |line| tail.push_stderr(line))) }) @@ -358,6 +358,13 @@ impl LspManager { } return; } + Err(LaunchError::Config(message)) => { + tracing::warn!("{}: {message}", server.name); + if server.finish_start(generation, Conn::Failed) { + server.set_status(&self.sink, ide_core::event::ServerStatus::Misconfigured, Some(message)); + } + return; + } Err(LaunchError::Failed(message)) => { tracing::warn!("{}: {message}", server.name); if server.finish_start(generation, Conn::Failed) { @@ -1604,8 +1611,10 @@ impl LspManager { s.reset_for_restart(); self.ensure_started(&s); } - // Missing / crashed servers retry with the new configuration (e.g. a new PATH). - ide_core::event::ServerStatus::Missing | ide_core::event::ServerStatus::Crashed => { + // Missing / crashed / misconfigured servers retry with the new configuration (e.g. a new PATH). + ide_core::event::ServerStatus::Missing + | ide_core::event::ServerStatus::Crashed + | ide_core::event::ServerStatus::Misconfigured => { s.reset_for_restart(); self.ensure_started(&s); } diff --git a/crates/ide-lsp/tests/mock_server.rs b/crates/ide-lsp/tests/mock_server.rs index c5f655217..8853ce800 100644 --- a/crates/ide-lsp/tests/mock_server.rs +++ b/crates/ide-lsp/tests/mock_server.rs @@ -710,13 +710,15 @@ async fn launch_error_fails_without_starting_the_server() { h.mgr.set_config_hook(Box::new(|cfg, _| cfg.launch_error = Some("needs JDK 21".into()))); assert!(!h.mgr.serves_path(&h.root.join("a.mk")), "batch inspection skips it"); h.open("a.mk", "x"); - h.wait_status("mock", ServerStatus::Crashed).await; + // Not a crash: nothing ran, the configuration has to change. + h.wait_status("mock", ServerStatus::Misconfigured).await; assert_eq!(h.launcher.state("mock").launches.load(Ordering::SeqCst), 0); - let crashed = h.events.lock().iter().find_map(|e| match e { - IdeEvent::LanguageServer { status: ServerStatus::Crashed, message, .. } => message.clone(), + let reason = h.events.lock().iter().find_map(|e| match e { + IdeEvent::LanguageServer { status: ServerStatus::Misconfigured, message, .. } => message.clone(), _ => None, }); - assert_eq!(crashed.as_deref(), Some("needs JDK 21")); + assert_eq!(reason.as_deref(), Some("needs JDK 21")); + assert!(!h.statuses("mock").contains(&ServerStatus::Crashed)); } #[tokio::test(flavor = "multi_thread")] diff --git a/crates/ide-lsp/tests/real_servers.rs b/crates/ide-lsp/tests/real_servers.rs index 582253b2b..42f23597a 100644 --- a/crates/ide-lsp/tests/real_servers.rs +++ b/crates/ide-lsp/tests/real_servers.rs @@ -124,7 +124,7 @@ async fn rust_analyzer_end_to_end() { mgr.did_open(doc, &main, &LanguageId::new("rust"), 1, MAIN_RS); retry("rust-analyzer ready", Duration::from_secs(60), || { let s = last_status(&events, "rust-analyzer"); - if let Some((ServerStatus::Crashed | ServerStatus::Missing, msg)) = &s { + if let Some((ServerStatus::Crashed | ServerStatus::Missing | ServerStatus::Misconfigured, msg)) = &s { panic!("rust-analyzer failed: {msg:?}"); } async move { matches!(s, Some((ServerStatus::Ready | ServerStatus::Busy, _))).then_some(()) } diff --git a/src/bindings.ts b/src/bindings.ts index 8d10b878b..71749fbc3 100644 --- a/src/bindings.ts +++ b/src/bindings.ts @@ -3397,7 +3397,12 @@ export type ServerInfo = { pid: number | null, }; -export type ServerStatus = "starting" | "ready" | "busy" | "crashed" | "stopped" | "missing"; +export type ServerStatus = "starting" | "ready" | "busy" | "crashed" | "stopped" | "missing" | +/** + * Not started: its configuration can't launch it (e.g. no JDK new enough); + * the message says what to change, and a settings change retries. + */ +"misconfigured"; /** Exception filters of one session (see [`DebugManager::exception_filters`]). */ export type SessionExceptionFilters = { diff --git a/src/features/code-insight/ServerStatus.tsx b/src/features/code-insight/ServerStatus.tsx index 503578874..4c521518f 100644 --- a/src/features/code-insight/ServerStatus.tsx +++ b/src/features/code-insight/ServerStatus.tsx @@ -89,6 +89,6 @@ export function ServerStatus() { function toneOf(status: string): "ok" | "busy" | "error" | "idle" { if (status === "ready") return "ok"; if (status === "starting" || status === "busy") return "busy"; - if (status === "crashed" || status === "missing") return "error"; + if (status === "crashed" || status === "missing" || status === "misconfigured") return "error"; return "idle"; } diff --git a/src/features/code-insight/servers.ts b/src/features/code-insight/servers.ts index aa91614d0..38f3ede09 100644 --- a/src/features/code-insight/servers.ts +++ b/src/features/code-insight/servers.ts @@ -126,12 +126,13 @@ export const STATUS_LABEL: Record = { crashed: "Crashed", stopped: "Stopped", missing: "Not installed", + misconfigured: "Not started", }; /** One-line summary for the status bar. */ export function serversSummary(servers: readonly ServerEntry[]): { text: string; tone: "ok" | "busy" | "error" | "idle" } | null { if (!servers.length) return null; - const bad = servers.filter((s) => s.status === "crashed" || s.status === "missing"); + const bad = servers.filter((s) => s.status === "crashed" || s.status === "missing" || s.status === "misconfigured"); const busy = servers.filter((s) => s.status === "starting" || s.status === "busy"); const tone = bad.length ? "error" : busy.length ? "busy" : servers.some((s) => s.status === "ready") ? "ok" : "idle"; const names = [...new Set(servers.map((s) => s.name))]; diff --git a/src/features/code-insight/views.test.ts b/src/features/code-insight/views.test.ts index 3b91b010e..5000bd96a 100644 --- a/src/features/code-insight/views.test.ts +++ b/src/features/code-insight/views.test.ts @@ -44,6 +44,7 @@ describe("serversSummary", () => { expect(serversSummary([s("rust-analyzer", "ready")])).toEqual({ text: "rust-analyzer", tone: "ok" }); expect(serversSummary([s("vtsls", "ready"), s("eslint", "starting")])).toEqual({ text: "vtsls +1", tone: "busy" }); expect(serversSummary([s("gopls", "missing")])!.tone).toBe("error"); + expect(serversSummary([s("jdtls", "misconfigured")])!.tone).toBe("error"); }); });