Allow users to configure host, port and timeout for build in TC…

…P 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 <hello@anthonyeid.me>

* Add default setting for tcp timeout

Co-Authored-By: Anthony Eid <56899983+Anthony-Eid@users.noreply.github.com>

---------

Co-authored-by: Anthony Eid <hello@anthonyeid.me>
Co-authored-by: Anthony Eid <56899983+Anthony-Eid@users.noreply.github.com>

crates/dap/src/adapters.rs

@@ -28,7 +28,7 @@ pub trait DapDelegate {
     fn http_client(&self) -> Option<Arc<dyn HttpClient>>;
     fn node_runtime(&self) -> Option<NodeRuntime>;
     fn fs(&self) -> Arc<dyn Fs>;
-    fn cached_binaries(&self) -> Arc<Mutex<HashMap<DebugAdapterName, DebugAdapterBinary>>>;
+    fn updated_adapters(&self) -> Arc<Mutex<HashSet<DebugAdapterName>>>;
     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<DebugAdapterBinary> {
-        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)
     }

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,
         }
     }
 }

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<dyn Send + FnMut(IoKind, &str)>;
 
@@ -361,27 +362,36 @@ pub trait Transport: 'static + Send + Sync {
     ) -> Result<TransportParams>;
 
     fn has_adapter_logs(&self) -> bool;
+
+    fn clone_box(&self) -> Box<dyn Transport>;
 }
 
+#[derive(Clone)]
 pub struct TcpTransport {
-    config: TCPHost,
+    port: u16,
+    host: Ipv4Addr,
+    timeout: Option<u64>,
 }
 
 impl TcpTransport {
-    pub fn new(config: TCPHost) -> Self {
-        Self { config }
+    pub fn new(host: Ipv4Addr, port: u16, timeout: Option<u64>) -> 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<u16> {
-        Some(
-            TcpListener::bind(SocketAddrV4::new(host, 0))
-                .await
-                .ok()?
-                .local_addr()
-                .ok()?
-                .port(),
-        )
+    pub async fn port(host: &TCPHost) -> Result<u16> {
+        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<TransportParams> {
-        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<dyn Transport> {
+        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<dyn Transport> {
+        Box::new(self.clone())
+    }
 }

crates/dap_adapters/Cargo.toml

@@ -5,7 +5,6 @@ edition =  "2021"
 publish =  false
 license = "GPL-3.0-or-later"
 
-
 [lints]
 workspace = true
 

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<dyn Transport>,
 }
 
 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<Self> {
+        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<dyn Transport> {
-        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<AdapterVersion> {

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<Box<dyn DebugAdapter>> {
+pub async fn build_adapter(adapter_config: &DebugAdapterConfig) -> Result<Box<dyn DebugAdapter>> {
     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())),
     }
 }

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<u64>,
+}
 
 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<Self> {
+        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<dyn Transport> {
-        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,

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 {