From f7eb5213a53774c615166313f147ca320df14700 Mon Sep 17 00:00:00 2001 From: Remco Smits Date: Sun, 27 Oct 2024 12:16:09 +0100 Subject: [PATCH] Allow users to configure `host`, `port` and `timeout` for build in TCP adapters (#56) * Remove space in cargo.toml * Add TCPHost for Javascript and PHP adapter * Allow falling back to first open port * Add some more docs to TCPHost * Fix cached binaries prevent from having multiple debug sessions for one adapters This was an issue, because we stored the port arg inside the DebugAdapterBinary which was cached so could not change anymore. Co-authored-by: Anthony Eid * Add default setting for tcp timeout Co-Authored-By: Anthony Eid <56899983+Anthony-Eid@users.noreply.github.com> --------- Co-authored-by: Anthony Eid Co-authored-by: Anthony Eid <56899983+Anthony-Eid@users.noreply.github.com> --- crates/dap/src/adapters.rs | 37 +++++++--- crates/dap/src/debugger_settings.rs | 5 ++ crates/dap/src/transport.rs | 73 +++++++++++-------- crates/dap_adapters/Cargo.toml | 1 - crates/dap_adapters/src/custom.rs | 21 ++++-- crates/dap_adapters/src/dap_adapters.rs | 11 +-- crates/dap_adapters/src/javascript.rs | 25 ++++--- crates/dap_adapters/src/lldb.rs | 1 - crates/dap_adapters/src/php.rs | 28 ++++--- crates/dap_adapters/src/python.rs | 1 - crates/debugger_tools/src/dap_log.rs | 2 +- crates/debugger_ui/src/debugger_panel_item.rs | 9 ++- crates/project/src/dap_store.rs | 22 +++--- crates/task/src/debug_format.rs | 32 +++++--- 14 files changed, 166 insertions(+), 102 deletions(-) diff --git a/crates/dap/src/adapters.rs b/crates/dap/src/adapters.rs index 691a20e7a52bb7..0ed5a7cd15bed5 100644 --- a/crates/dap/src/adapters.rs +++ b/crates/dap/src/adapters.rs @@ -28,7 +28,7 @@ pub trait DapDelegate { fn http_client(&self) -> Option>; fn node_runtime(&self) -> Option; fn fs(&self) -> Arc; - fn cached_binaries(&self) -> Arc>>; + fn updated_adapters(&self) -> Arc>>; fn update_status(&self, dap_name: DebugAdapterName, status: DapStatus); } @@ -192,9 +192,15 @@ pub trait DebugAdapter: 'static + Send + Sync { delegate: &dyn DapDelegate, config: &DebugAdapterConfig, ) -> Result { - if let Some(binary) = delegate.cached_binaries().lock().await.get(&self.name()) { + if delegate + .updated_adapters() + .lock() + .await + .contains(&self.name()) + { log::info!("Using cached debug adapter binary {}", self.name()); - return Ok(binary.clone()); + + return self.get_installed_binary(delegate, config).await; } log::info!("Getting latest version of debug adapter {}", self.name()); @@ -208,29 +214,40 @@ pub trait DebugAdapter: 'static + Send + Sync { .as_ref() .is_ok_and(|binary| binary.version == version.tag_name) { - let binary = binary?; - delegate - .cached_binaries() + .updated_adapters() .lock_arc() .await - .insert(self.name(), binary.clone()); + .insert(self.name()); - return Ok(binary); + return Ok(binary?); } delegate.update_status(self.name(), DapStatus::Downloading); self.install_binary(version, delegate).await?; binary = self.get_installed_binary(delegate, config).await; + } else { + log::error!( + "Failed getting latest version of debug adapter {}", + self.name() + ); } + if binary.is_err() { + delegate.update_status( + self.name(), + DapStatus::Failed { + error: format!("Failed to download {}", self.name()), + }, + ); + } let binary = binary?; delegate - .cached_binaries() + .updated_adapters() .lock_arc() .await - .insert(self.name(), binary.clone()); + .insert(self.name()); Ok(binary) } diff --git a/crates/dap/src/debugger_settings.rs b/crates/dap/src/debugger_settings.rs index 0d5a744a849365..4594a1785039e5 100644 --- a/crates/dap/src/debugger_settings.rs +++ b/crates/dap/src/debugger_settings.rs @@ -19,6 +19,10 @@ pub struct DebuggerSettings { /// /// Default: true pub button: bool, + /// Time in milliseconds until timeout error when connecting to a TCP debug adapter + /// + /// Default: 2000ms + pub timeout: u64, } impl Default for DebuggerSettings { @@ -27,6 +31,7 @@ impl Default for DebuggerSettings { button: true, save_breakpoints: true, stepping_granularity: SteppingGranularity::Line, + timeout: 2000, } } } diff --git a/crates/dap/src/transport.rs b/crates/dap/src/transport.rs index b78e1e5acc66ae..e68d1590c2a349 100644 --- a/crates/dap/src/transport.rs +++ b/crates/dap/src/transport.rs @@ -6,6 +6,7 @@ use dap_types::{ }; use futures::{select, AsyncBufRead, AsyncReadExt as _, AsyncWrite, FutureExt as _}; use gpui::AsyncAppContext; +use settings::Settings as _; use smallvec::SmallVec; use smol::{ channel::{unbounded, Receiver, Sender}, @@ -23,7 +24,7 @@ use std::{ }; use task::TCPHost; -use crate::adapters::DebugAdapterBinary; +use crate::{adapters::DebugAdapterBinary, debugger_settings::DebuggerSettings}; pub type IoHandler = Box; @@ -361,27 +362,36 @@ pub trait Transport: 'static + Send + Sync { ) -> Result; fn has_adapter_logs(&self) -> bool; + + fn clone_box(&self) -> Box; } +#[derive(Clone)] pub struct TcpTransport { - config: TCPHost, + port: u16, + host: Ipv4Addr, + timeout: Option, } impl TcpTransport { - pub fn new(config: TCPHost) -> Self { - Self { config } + pub fn new(host: Ipv4Addr, port: u16, timeout: Option) -> Self { + Self { + port, + host, + timeout, + } } /// Get an open port to use with the tcp client when not supplied by debug config - async fn get_open_port(host: Ipv4Addr) -> Option { - Some( - TcpListener::bind(SocketAddrV4::new(host, 0)) - .await - .ok()? - .local_addr() - .ok()? - .port(), - ) + pub async fn port(host: &TCPHost) -> Result { + if let Some(port) = host.port { + Ok(port) + } else { + Ok(TcpListener::bind(SocketAddrV4::new(host.host(), 0)) + .await? + .local_addr()? + .port()) + } } } @@ -392,16 +402,6 @@ impl Transport for TcpTransport { binary: &DebugAdapterBinary, cx: &mut AsyncAppContext, ) -> Result { - let host_address = self - .config - .host - .unwrap_or_else(|| Ipv4Addr::new(127, 0, 0, 1)); - - let mut port = self.config.port; - if port.is_none() { - port = Self::get_open_port(host_address).await; - } - let mut command = process::Command::new(&binary.command); if let Some(args) = &binary.arguments { @@ -422,16 +422,16 @@ impl Transport for TcpTransport { .spawn() .with_context(|| "failed to start debug adapter.")?; - let address = SocketAddrV4::new( - host_address, - port.ok_or(anyhow!("Port is required to connect to TCP server"))?, - ); + let address = SocketAddrV4::new(self.host, self.port); - let timeout = self.config.timeout.unwrap_or(2000); + let timeout = self.timeout.unwrap_or_else(|| { + cx.update(|cx| DebuggerSettings::get_global(cx).timeout) + .unwrap_or(2000u64) + }); let (rx, tx) = select! { _ = cx.background_executor().timer(Duration::from_millis(timeout)).fuse() => { - return Err(anyhow!("Connection to tcp DAP timeout")) + return Err(anyhow!(format!("Connection to TCP DAP timeout {}:{}", self.host, self.port))) }, result = cx.spawn(|cx| async move { loop { @@ -444,7 +444,11 @@ impl Transport for TcpTransport { } }).fuse() => result }; - log::info!("Debug adapter has connected to tcp server"); + log::info!( + "Debug adapter has connected to TCP server {}:{}", + self.host, + self.port + ); Ok(TransportParams::new( Box::new(tx), @@ -456,8 +460,13 @@ impl Transport for TcpTransport { fn has_adapter_logs(&self) -> bool { true } + + fn clone_box(&self) -> Box { + Box::new(self.clone()) + } } +#[derive(Clone)] pub struct StdioTransport {} impl StdioTransport { @@ -514,4 +523,8 @@ impl Transport for StdioTransport { fn has_adapter_logs(&self) -> bool { false } + + fn clone_box(&self) -> Box { + Box::new(self.clone()) + } } diff --git a/crates/dap_adapters/Cargo.toml b/crates/dap_adapters/Cargo.toml index 883e8cfe3f20bf..73d5e8168f5a98 100644 --- a/crates/dap_adapters/Cargo.toml +++ b/crates/dap_adapters/Cargo.toml @@ -5,7 +5,6 @@ edition = "2021" publish = false license = "GPL-3.0-or-later" - [lints] workspace = true diff --git a/crates/dap_adapters/src/custom.rs b/crates/dap_adapters/src/custom.rs index 6da45ea1351660..8aa6a5612446cc 100644 --- a/crates/dap_adapters/src/custom.rs +++ b/crates/dap_adapters/src/custom.rs @@ -6,16 +6,26 @@ use task::DebugAdapterConfig; use crate::*; -#[derive(Debug, Eq, PartialEq, Clone)] pub(crate) struct CustomDebugAdapter { custom_args: CustomArgs, + transport: Box, } impl CustomDebugAdapter { const ADAPTER_NAME: &'static str = "custom_dap"; - pub(crate) fn new(custom_args: CustomArgs) -> Self { - CustomDebugAdapter { custom_args } + pub(crate) async fn new(custom_args: CustomArgs) -> Result { + Ok(CustomDebugAdapter { + transport: match &custom_args.connection { + DebugConnectionType::TCP(host) => Box::new(TcpTransport::new( + host.host(), + TcpTransport::port(&host).await?, + host.timeout, + )), + DebugConnectionType::STDIO => Box::new(StdioTransport::new()), + }, + custom_args, + }) } } @@ -26,10 +36,7 @@ impl DebugAdapter for CustomDebugAdapter { } fn transport(&self) -> Box { - match &self.custom_args.connection { - DebugConnectionType::STDIO => Box::new(StdioTransport::new()), - DebugConnectionType::TCP(tcp_host) => Box::new(TcpTransport::new(tcp_host.clone())), - } + self.transport.clone_box() } async fn fetch_latest_adapter_version(&self, _: &dyn DapDelegate) -> Result { diff --git a/crates/dap_adapters/src/dap_adapters.rs b/crates/dap_adapters/src/dap_adapters.rs index e099f92430fec9..2b4caa771d0c1e 100644 --- a/crates/dap_adapters/src/dap_adapters.rs +++ b/crates/dap_adapters/src/dap_adapters.rs @@ -16,17 +16,18 @@ use lldb::LldbDebugAdapter; use php::PhpDebugAdapter; use python::PythonDebugAdapter; use serde_json::{json, Value}; -use std::fmt::Debug; use task::{CustomArgs, DebugAdapterConfig, DebugAdapterKind, DebugConnectionType, TCPHost}; -pub fn build_adapter(adapter_config: &DebugAdapterConfig) -> Result> { +pub async fn build_adapter(adapter_config: &DebugAdapterConfig) -> Result> { match &adapter_config.kind { DebugAdapterKind::Custom(start_args) => { - Ok(Box::new(CustomDebugAdapter::new(start_args.clone()))) + Ok(Box::new(CustomDebugAdapter::new(start_args.clone()).await?)) } DebugAdapterKind::Python => Ok(Box::new(PythonDebugAdapter::new())), - DebugAdapterKind::PHP => Ok(Box::new(PhpDebugAdapter::new())), - DebugAdapterKind::Javascript => Ok(Box::new(JsDebugAdapter::new())), + DebugAdapterKind::PHP(host) => Ok(Box::new(PhpDebugAdapter::new(host.clone()).await?)), + DebugAdapterKind::Javascript(host) => { + Ok(Box::new(JsDebugAdapter::new(host.clone()).await?)) + } DebugAdapterKind::Lldb => Ok(Box::new(LldbDebugAdapter::new())), } } diff --git a/crates/dap_adapters/src/javascript.rs b/crates/dap_adapters/src/javascript.rs index 2c59a9ae2cbd59..4b47243b43fa57 100644 --- a/crates/dap_adapters/src/javascript.rs +++ b/crates/dap_adapters/src/javascript.rs @@ -1,16 +1,25 @@ +use std::net::Ipv4Addr; + use dap::transport::{TcpTransport, Transport}; use crate::*; -#[derive(Debug, Eq, PartialEq, Clone)] -pub(crate) struct JsDebugAdapter {} +pub(crate) struct JsDebugAdapter { + port: u16, + host: Ipv4Addr, + timeout: Option, +} impl JsDebugAdapter { const ADAPTER_NAME: &'static str = "vscode-js-debug"; const ADAPTER_PATH: &'static str = "src/dapDebugServer.js"; - pub(crate) fn new() -> Self { - JsDebugAdapter {} + pub(crate) async fn new(host: TCPHost) -> Result { + Ok(JsDebugAdapter { + host: host.host(), + timeout: host.timeout, + port: TcpTransport::port(&host).await?, + }) } } @@ -21,11 +30,7 @@ impl DebugAdapter for JsDebugAdapter { } fn transport(&self) -> Box { - Box::new(TcpTransport::new(TCPHost { - port: Some(8133), - host: None, - timeout: None, - })) + Box::new(TcpTransport::new(self.host, self.port, self.timeout)) } async fn fetch_latest_adapter_version( @@ -73,7 +78,7 @@ impl DebugAdapter for JsDebugAdapter { .into_owned(), arguments: Some(vec![ adapter_path.join(Self::ADAPTER_PATH).into(), - "8133".into(), + self.port.to_string().into(), ]), envs: None, version, diff --git a/crates/dap_adapters/src/lldb.rs b/crates/dap_adapters/src/lldb.rs index 10ff48638cb2cf..47dfd3a25f43d8 100644 --- a/crates/dap_adapters/src/lldb.rs +++ b/crates/dap_adapters/src/lldb.rs @@ -5,7 +5,6 @@ use task::DebugAdapterConfig; use crate::*; -#[derive(Debug, Eq, PartialEq, Clone)] pub(crate) struct LldbDebugAdapter {} impl LldbDebugAdapter { diff --git a/crates/dap_adapters/src/php.rs b/crates/dap_adapters/src/php.rs index a5aa647822a4d4..4e5ccb92d08c9f 100644 --- a/crates/dap_adapters/src/php.rs +++ b/crates/dap_adapters/src/php.rs @@ -1,16 +1,24 @@ use dap::transport::{TcpTransport, Transport}; +use std::net::Ipv4Addr; use crate::*; -#[derive(Debug, Eq, PartialEq, Clone)] -pub(crate) struct PhpDebugAdapter {} +pub(crate) struct PhpDebugAdapter { + port: u16, + host: Ipv4Addr, + timeout: Option, +} impl PhpDebugAdapter { const ADAPTER_NAME: &'static str = "vscode-php-debug"; const ADAPTER_PATH: &'static str = "out/phpDebug.js"; - pub(crate) fn new() -> Self { - PhpDebugAdapter {} + pub(crate) async fn new(host: TCPHost) -> Result { + Ok(PhpDebugAdapter { + port: TcpTransport::port(&host).await?, + host: host.host(), + timeout: host.timeout, + }) } } @@ -21,11 +29,7 @@ impl DebugAdapter for PhpDebugAdapter { } fn transport(&self) -> Box { - Box::new(TcpTransport::new(TCPHost { - port: Some(8132), - host: None, - timeout: None, - })) + Box::new(TcpTransport::new(self.host, self.port, self.timeout)) } async fn fetch_latest_adapter_version( @@ -34,7 +38,7 @@ impl DebugAdapter for PhpDebugAdapter { ) -> Result { let github_repo = GithubRepo { repo_name: Self::ADAPTER_NAME.into(), - repo_owner: "xdebug".to_string(), + repo_owner: "xdebug".into(), }; adapters::fetch_latest_adapter_version_from_github(github_repo, delegate).await @@ -62,7 +66,7 @@ impl DebugAdapter for PhpDebugAdapter { .file_name() .and_then(|file_name| file_name.to_str()) .and_then(|file_name| file_name.strip_prefix(&file_name_prefix)) - .ok_or_else(|| anyhow!("Php debug adapter has invalid file name"))? + .ok_or_else(|| anyhow!("PHP debug adapter has invalid file name"))? .to_string(); Ok(DebugAdapterBinary { @@ -73,7 +77,7 @@ impl DebugAdapter for PhpDebugAdapter { .into_owned(), arguments: Some(vec![ adapter_path.join(Self::ADAPTER_PATH).into(), - "--server=8132".into(), + format!("--server={}", self.port).into(), ]), envs: None, version, diff --git a/crates/dap_adapters/src/python.rs b/crates/dap_adapters/src/python.rs index 136b95c0972776..2a3a6cb2ded439 100644 --- a/crates/dap_adapters/src/python.rs +++ b/crates/dap_adapters/src/python.rs @@ -2,7 +2,6 @@ use dap::transport::{StdioTransport, Transport}; use crate::*; -#[derive(Debug, Eq, PartialEq, Clone)] pub(crate) struct PythonDebugAdapter {} impl PythonDebugAdapter { diff --git a/crates/debugger_tools/src/dap_log.rs b/crates/debugger_tools/src/dap_log.rs index 74622d3d03e51c..5f907c3ec0dbd0 100644 --- a/crates/debugger_tools/src/dap_log.rs +++ b/crates/debugger_tools/src/dap_log.rs @@ -520,7 +520,7 @@ impl DapLogView { .debug_clients(cx) .map(|client| DapMenuItem { client_id: client.id(), - client_name: client.config().kind.to_string(), + client_name: client.config().kind.diplay_name().into(), selected_entry: self.current_view.map_or(LogKind::Adapter, |(_, kind)| kind), has_adapter_logs: client.has_adapter_logs(), }) diff --git a/crates/debugger_ui/src/debugger_panel_item.rs b/crates/debugger_ui/src/debugger_panel_item.rs index 853255fb0bb3a7..4e5eb1fefcf879 100644 --- a/crates/debugger_ui/src/debugger_panel_item.rs +++ b/crates/debugger_ui/src/debugger_panel_item.rs @@ -496,8 +496,9 @@ impl Item for DebugPanelItem { _: &WindowContext, ) -> AnyElement { Label::new(format!( - "{:?} - Thread {}", - self.client_kind, self.thread_id + "{} - Thread {}", + self.client_kind.diplay_name(), + self.thread_id )) .color(if params.selected { Color::Default @@ -509,8 +510,8 @@ impl Item for DebugPanelItem { fn tab_tooltip_text(&self, cx: &AppContext) -> Option { Some(SharedString::from(format!( - "{:?} Thread {} - {:?}", - self.client_kind, + "{} Thread {} - {:?}", + self.client_kind.diplay_name(), self.thread_id, self.thread_state.read(cx).status, ))) diff --git a/crates/project/src/dap_store.rs b/crates/project/src/dap_store.rs index 9bca55c0075f2f..e80c50be6d8607 100644 --- a/crates/project/src/dap_store.rs +++ b/crates/project/src/dap_store.rs @@ -1,7 +1,6 @@ use crate::ProjectPath; use anyhow::{anyhow, Context as _, Result}; -use collections::HashSet; -use dap::adapters::{DapStatus, DebugAdapterBinary, DebugAdapterName}; +use dap::adapters::{DapStatus, DebugAdapterName}; use dap::client::{DebugAdapterClient, DebugAdapterClientId}; use dap::messages::{Message, Response}; use dap::requests::{ @@ -33,7 +32,7 @@ use serde_json::{json, Value}; use settings::WorktreeId; use smol::lock::Mutex; use std::{ - collections::{BTreeMap, HashMap}, + collections::{BTreeMap, HashMap, HashSet}, hash::{Hash, Hasher}, path::{Path, PathBuf}, sync::{ @@ -69,7 +68,7 @@ pub struct DebugPosition { pub struct DapStore { next_client_id: AtomicUsize, clients: HashMap, - cached_binaries: Arc>>, + updated_adapters: Arc>>, breakpoints: BTreeMap>, capabilities: HashMap, active_debug_line: Option<(ProjectPath, DebugPosition)>, @@ -94,7 +93,7 @@ impl DapStore { Self { active_debug_line: None, clients: Default::default(), - cached_binaries: Default::default(), + updated_adapters: Default::default(), breakpoints: Default::default(), capabilities: HashMap::default(), next_client_id: Default::default(), @@ -252,13 +251,14 @@ impl DapStore { self.http_client.clone(), self.node_runtime.clone(), self.fs.clone(), - self.cached_binaries.clone(), + self.updated_adapters.clone(), self.languages.clone(), ); let start_client_task = cx.spawn(|this, mut cx| async move { let dap_store = this.clone(); let adapter = Arc::new( build_adapter(&config) + .await .context("Creating debug adapter") .log_err()?, ); @@ -1239,7 +1239,7 @@ pub struct DapAdapterDelegate { fs: Arc, http_client: Option>, node_runtime: Option, - cached_binaries: Arc>>, + udpated_adapters: Arc>>, languages: Arc, } @@ -1248,14 +1248,14 @@ impl DapAdapterDelegate { http_client: Option>, node_runtime: Option, fs: Arc, - cached_binaries: Arc>>, + udpated_adapters: Arc>>, languages: Arc, ) -> Self { Self { fs, http_client, node_runtime, - cached_binaries, + udpated_adapters, languages, } } @@ -1274,8 +1274,8 @@ impl dap::adapters::DapDelegate for DapAdapterDelegate { self.fs.clone() } - fn cached_binaries(&self) -> Arc>> { - self.cached_binaries.clone() + fn updated_adapters(&self) -> Arc>> { + self.udpated_adapters.clone() } fn update_status(&self, dap_name: DebugAdapterName, status: dap::adapters::DapStatus) { diff --git a/crates/task/src/debug_format.rs b/crates/task/src/debug_format.rs index 6aef1745c2e3de..62dafa2e358dcc 100644 --- a/crates/task/src/debug_format.rs +++ b/crates/task/src/debug_format.rs @@ -16,13 +16,26 @@ impl Default for DebugConnectionType { #[derive(Default, Deserialize, Serialize, PartialEq, Eq, JsonSchema, Clone, Debug)] pub struct TCPHost { /// The port that the debug adapter is listening on + /// + /// Default: We will try to find an open port pub port: Option, /// The host that the debug adapter is listening too + /// + /// Default: 127.0.0.1 pub host: Option, - /// The max amount of time to connect to a tcp DAP before returning an error + /// The max amount of time in milliseconds to connect to a tcp DAP before returning an error + /// + /// Default: 2000ms pub timeout: Option, } +impl TCPHost { + /// Get the host or fallback to the default host + pub fn host(&self) -> Ipv4Addr { + self.host.unwrap_or_else(|| Ipv4Addr::new(127, 0, 0, 1)) + } +} + /// Represents the type that will determine which request to call on the debug adapter #[derive(Default, Deserialize, Serialize, PartialEq, Eq, JsonSchema, Clone, Debug)] #[serde(rename_all = "lowercase")] @@ -44,22 +57,23 @@ pub enum DebugAdapterKind { /// Use debugpy Python, /// Use vscode-php-debug - PHP, + PHP(TCPHost), /// Use vscode-js-debug - Javascript, + Javascript(TCPHost), /// Use lldb Lldb, } -impl std::fmt::Display for DebugAdapterKind { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.write_str(match self { +impl DebugAdapterKind { + /// Returns the display name for the adapter kind + pub fn diplay_name(&self) -> &str { + match self { Self::Custom(_) => "Custom", Self::Python => "Python", - Self::PHP => "PHP", - Self::Javascript => "JavaScript", + Self::PHP(_) => "PHP", + Self::Javascript(_) => "JavaScript", Self::Lldb => "LLDB", - }) + } } }