From ecbdffc84f1165323f256e8485ae84320550c759 Mon Sep 17 00:00:00 2001 From: Anthony Eid <56899983+Anthony-Eid@users.noreply.github.com> Date: Fri, 31 Oct 2025 19:02:51 -0400 Subject: [PATCH] debugger: Fix Debugpy attach with connect session startup (#41690) Closes #38345, #34882, #33280 Debugpy has four distinct configuration scenarios, which are: 1. launch 2. attach with process id 3. attach with listen 4. attach with connect Spawning Debugpy directly works with the first three scenarios but not with "attach with connect". Which requires host/port arguments being passed in both with an attach request and when starting up Debugpy. This PR passes in the right arguments when spawning Debugpy in an attach with connect scenario, thus fixing the bug. The VsCode extension comment that explains this: https://github.com/microsoft/vscode-python-debugger/blob/98f5b93ee4259f99b679d7c45163ba7babcbff6d/src/extension/debugger/adapter/factory.ts#L43-L51 Release Notes: - debugger: Fix Python attach-based sessions not working with `connect` or `port` arguments --- Cargo.lock | 3 + crates/dap_adapters/Cargo.toml | 4 + crates/dap_adapters/src/dap_adapters.rs | 62 ++++++ crates/dap_adapters/src/python.rs | 240 ++++++++++++++++++++++-- 4 files changed, 295 insertions(+), 14 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 25a22e64c6db0632ca1357cebe02f0bbe04fa0a8..ec55e4af77f78a9476b147744a9973d758d0e630 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4528,12 +4528,15 @@ dependencies = [ "fs", "futures 0.3.31", "gpui", + "http_client", "json_dotpath", "language", "log", + "node_runtime", "paths", "serde", "serde_json", + "settings", "smol", "task", "util", diff --git a/crates/dap_adapters/Cargo.toml b/crates/dap_adapters/Cargo.toml index 253674c0f3da16574b4303faf679abeb310756d8..7bdf39c74a43165d252d5d53d2afef776bf63f29 100644 --- a/crates/dap_adapters/Cargo.toml +++ b/crates/dap_adapters/Cargo.toml @@ -41,6 +41,10 @@ util.workspace = true [dev-dependencies] dap = { workspace = true, features = ["test-support"] } +fs = { workspace = true, features = ["test-support"] } gpui = { workspace = true, features = ["test-support"] } +http_client.workspace = true +node_runtime.workspace = true +settings = { workspace = true, features = ["test-support"] } task = { workspace = true, features = ["test-support"] } util = { workspace = true, features = ["test-support"] } diff --git a/crates/dap_adapters/src/dap_adapters.rs b/crates/dap_adapters/src/dap_adapters.rs index a4e6beb2495ebe1eec9f08ddb8394b498c0ae410..d8a706ba414af2c9e0beb1cffe8357bcece1dc52 100644 --- a/crates/dap_adapters/src/dap_adapters.rs +++ b/crates/dap_adapters/src/dap_adapters.rs @@ -4,6 +4,8 @@ mod go; mod javascript; mod python; +#[cfg(test)] +use std::path::PathBuf; use std::sync::Arc; use anyhow::Result; @@ -38,3 +40,63 @@ pub fn init(cx: &mut App) { } }) } + +#[cfg(test)] +struct MockDelegate { + worktree_root: PathBuf, +} + +#[cfg(test)] +impl MockDelegate { + fn new() -> Arc { + Arc::new(Self { + worktree_root: PathBuf::from("/tmp/test"), + }) + } +} + +#[cfg(test)] +#[async_trait::async_trait] +impl adapters::DapDelegate for MockDelegate { + fn worktree_id(&self) -> settings::WorktreeId { + settings::WorktreeId::from_usize(0) + } + + fn worktree_root_path(&self) -> &std::path::Path { + &self.worktree_root + } + + fn http_client(&self) -> Arc { + unimplemented!("Not needed for tests") + } + + fn node_runtime(&self) -> node_runtime::NodeRuntime { + unimplemented!("Not needed for tests") + } + + fn toolchain_store(&self) -> Arc { + unimplemented!("Not needed for tests") + } + + fn fs(&self) -> Arc { + unimplemented!("Not needed for tests") + } + + fn output_to_console(&self, _msg: String) {} + + async fn which(&self, _command: &std::ffi::OsStr) -> Option { + None + } + + async fn read_text_file(&self, _path: &util::rel_path::RelPath) -> Result { + Ok(String::new()) + } + + async fn shell_env(&self) -> collections::HashMap { + collections::HashMap::default() + } + + fn is_headless(&self) -> bool { + false + } +} diff --git a/crates/dap_adapters/src/python.rs b/crates/dap_adapters/src/python.rs index 66005db77029bd28c66f458bef7f1d2a1ad7a685..e718f66c78099044baed837da0ddc7bfa96ffa1c 100644 --- a/crates/dap_adapters/src/python.rs +++ b/crates/dap_adapters/src/python.rs @@ -23,6 +23,11 @@ use std::{ use util::command::new_smol_command; use util::{ResultExt, paths::PathStyle, rel_path::RelPath}; +enum DebugpyLaunchMode<'a> { + Normal, + AttachWithConnect { host: Option<&'a str> }, +} + #[derive(Default)] pub(crate) struct PythonDebugAdapter { base_venv_path: OnceCell, String>>, @@ -36,10 +41,11 @@ impl PythonDebugAdapter { const LANGUAGE_NAME: &'static str = "Python"; - async fn generate_debugpy_arguments( - host: &Ipv4Addr, + async fn generate_debugpy_arguments<'a>( + host: &'a Ipv4Addr, port: u16, - user_installed_path: Option<&Path>, + launch_mode: DebugpyLaunchMode<'a>, + user_installed_path: Option<&'a Path>, user_args: Option>, ) -> Result> { let mut args = if let Some(user_installed_path) = user_installed_path { @@ -62,7 +68,20 @@ impl PythonDebugAdapter { args.extend(if let Some(args) = user_args { args } else { - vec![format!("--host={}", host), format!("--port={}", port)] + match launch_mode { + DebugpyLaunchMode::Normal => { + vec![format!("--host={}", host), format!("--port={}", port)] + } + DebugpyLaunchMode::AttachWithConnect { host } => { + let mut args = vec!["connect".to_string()]; + + if let Some(host) = host { + args.push(format!("{host}:")); + } + args.push(format!("{port}")); + args + } + } }); Ok(args) } @@ -315,7 +334,46 @@ impl PythonDebugAdapter { user_env: Option>, python_from_toolchain: Option, ) -> Result { - let tcp_connection = config.tcp_connection.clone().unwrap_or_default(); + let mut tcp_connection = config.tcp_connection.clone().unwrap_or_default(); + + let (config_port, config_host) = config + .config + .get("connect") + .map(|value| { + ( + value + .get("port") + .and_then(|val| val.as_u64().map(|p| p as u16)), + value.get("host").and_then(|val| val.as_str()), + ) + }) + .unwrap_or_else(|| { + ( + config + .config + .get("port") + .and_then(|port| port.as_u64().map(|p| p as u16)), + config.config.get("host").and_then(|host| host.as_str()), + ) + }); + + let is_attach_with_connect = if config + .config + .get("request") + .is_some_and(|val| val.as_str().is_some_and(|request| request == "attach")) + { + if tcp_connection.host.is_some() && config_host.is_some() { + bail!("Cannot have two different hosts in debug configuration") + } else if tcp_connection.port.is_some() && config_port.is_some() { + bail!("Cannot have two different ports in debug configuration") + } + + tcp_connection.port = config_port; + DebugpyLaunchMode::AttachWithConnect { host: config_host } + } else { + DebugpyLaunchMode::Normal + }; + let (host, port, timeout) = crate::configure_tcp_connection(tcp_connection).await?; let python_path = if let Some(toolchain) = python_from_toolchain { @@ -330,6 +388,7 @@ impl PythonDebugAdapter { let arguments = Self::generate_debugpy_arguments( &host, port, + is_attach_with_connect, user_installed_path.as_deref(), user_args, ) @@ -824,7 +883,148 @@ mod tests { use util::path; use super::*; - use std::{net::Ipv4Addr, path::PathBuf}; + use task::TcpArgumentsTemplate; + + #[gpui::test] + async fn test_tcp_connection_conflict_with_connect_args() { + let adapter = PythonDebugAdapter { + base_venv_path: OnceCell::new(), + debugpy_whl_base_path: OnceCell::new(), + }; + + let config_with_port_conflict = json!({ + "request": "attach", + "connect": { + "port": 5679 + } + }); + + let tcp_connection = TcpArgumentsTemplate { + host: None, + port: Some(5678), + timeout: None, + }; + + let task_def = DebugTaskDefinition { + label: "test".into(), + adapter: PythonDebugAdapter::ADAPTER_NAME.into(), + config: config_with_port_conflict, + tcp_connection: Some(tcp_connection.clone()), + }; + + let result = adapter + .get_installed_binary( + &MockDelegate::new(), + &task_def, + None, + None, + None, + Some("python3".to_string()), + ) + .await; + + assert!(result.is_err()); + assert!( + result + .unwrap_err() + .to_string() + .contains("Cannot have two different ports") + ); + + let host = Ipv4Addr::new(127, 0, 0, 1); + let config_with_host_conflict = json!({ + "request": "attach", + "connect": { + "host": "192.168.1.1", + "port": 5678 + } + }); + + let tcp_connection_with_host = TcpArgumentsTemplate { + host: Some(host), + port: None, + timeout: None, + }; + + let task_def_host = DebugTaskDefinition { + label: "test".into(), + adapter: PythonDebugAdapter::ADAPTER_NAME.into(), + config: config_with_host_conflict, + tcp_connection: Some(tcp_connection_with_host), + }; + + let result_host = adapter + .get_installed_binary( + &MockDelegate::new(), + &task_def_host, + None, + None, + None, + Some("python3".to_string()), + ) + .await; + + assert!(result_host.is_err()); + assert!( + result_host + .unwrap_err() + .to_string() + .contains("Cannot have two different hosts") + ); + } + + #[gpui::test] + async fn test_attach_with_connect_mode_generates_correct_arguments() { + let host = Ipv4Addr::new(127, 0, 0, 1); + let port = 5678; + + let args_without_host = PythonDebugAdapter::generate_debugpy_arguments( + &host, + port, + DebugpyLaunchMode::AttachWithConnect { host: None }, + None, + None, + ) + .await + .unwrap(); + + let expected_suffix = path!("debug_adapters/Debugpy/debugpy/adapter"); + assert!(args_without_host[0].ends_with(expected_suffix)); + assert_eq!(args_without_host[1], "connect"); + assert_eq!(args_without_host[2], "5678"); + + let args_with_host = PythonDebugAdapter::generate_debugpy_arguments( + &host, + port, + DebugpyLaunchMode::AttachWithConnect { + host: Some("192.168.1.100"), + }, + None, + None, + ) + .await + .unwrap(); + + assert!(args_with_host[0].ends_with(expected_suffix)); + assert_eq!(args_with_host[1], "connect"); + assert_eq!(args_with_host[2], "192.168.1.100:"); + assert_eq!(args_with_host[3], "5678"); + + let args_normal = PythonDebugAdapter::generate_debugpy_arguments( + &host, + port, + DebugpyLaunchMode::Normal, + None, + None, + ) + .await + .unwrap(); + + assert!(args_normal[0].ends_with(expected_suffix)); + assert_eq!(args_normal[1], "--host=127.0.0.1"); + assert_eq!(args_normal[2], "--port=5678"); + assert!(!args_normal.contains(&"connect".to_string())); + } #[gpui::test] async fn test_debugpy_install_path_cases() { @@ -833,15 +1033,25 @@ mod tests { // Case 1: User-defined debugpy path (highest precedence) let user_path = PathBuf::from("/custom/path/to/debugpy/src/debugpy/adapter"); - let user_args = - PythonDebugAdapter::generate_debugpy_arguments(&host, port, Some(&user_path), None) - .await - .unwrap(); + let user_args = PythonDebugAdapter::generate_debugpy_arguments( + &host, + port, + DebugpyLaunchMode::Normal, + Some(&user_path), + None, + ) + .await + .unwrap(); - // Case 2: Venv-installed debugpy (uses -m debugpy.adapter) - let venv_args = PythonDebugAdapter::generate_debugpy_arguments(&host, port, None, None) - .await - .unwrap(); + let venv_args = PythonDebugAdapter::generate_debugpy_arguments( + &host, + port, + DebugpyLaunchMode::Normal, + None, + None, + ) + .await + .unwrap(); assert_eq!(user_args[0], "/custom/path/to/debugpy/src/debugpy/adapter"); assert_eq!(user_args[1], "--host=127.0.0.1"); @@ -856,6 +1066,7 @@ mod tests { let user_args = PythonDebugAdapter::generate_debugpy_arguments( &host, port, + DebugpyLaunchMode::Normal, Some(&user_path), Some(vec!["foo".into()]), ) @@ -864,6 +1075,7 @@ mod tests { let venv_args = PythonDebugAdapter::generate_debugpy_arguments( &host, port, + DebugpyLaunchMode::Normal, None, Some(vec!["foo".into()]), )