Fix a couple of bugs in remote browser debugging implementation (#40225)

Follow-up to #39248 

- Correctly forward ports over SSH, including the port from the debug
scenario's `url`
- Give the companion time to start up, instead of bailing if the first
connection attempt fails

Release Notes:

- Fixed not being able to launch a browser debugging session in an SSH
project.

crates/project/src/debugger/session.rs

@@ -14,12 +14,13 @@ use super::dap_command::{
     TerminateCommand, TerminateThreadsCommand, ThreadsCommand, VariablesCommand,
 };
 use super::dap_store::DapStore;
-use anyhow::{Context as _, Result, anyhow};
+use anyhow::{Context as _, Result, anyhow, bail};
 use base64::Engine;
 use collections::{HashMap, HashSet, IndexMap};
 use dap::adapters::{DebugAdapterBinary, DebugAdapterName};
 use dap::messages::Response;
 use dap::requests::{Request, RunInTerminal, StartDebugging};
+use dap::transport::TcpTransport;
 use dap::{
     Capabilities, ContinueArguments, EvaluateArgumentsContext, Module, Source, StackFrameId,
     SteppingGranularity, StoppedEvent, VariableReference,
@@ -47,12 +48,14 @@ use remote::RemoteClient;
 use rpc::ErrorExt;
 use serde::{Deserialize, Serialize};
 use serde_json::Value;
-use smol::net::TcpListener;
+use smol::net::{TcpListener, TcpStream};
 use std::any::TypeId;
 use std::collections::BTreeMap;
+use std::net::Ipv4Addr;
 use std::ops::RangeInclusive;
 use std::path::PathBuf;
 use std::process::Stdio;
+use std::time::Duration;
 use std::u64;
 use std::{
     any::Any,
@@ -63,6 +66,7 @@ use std::{
 };
 use task::TaskContext;
 use text::{PointUtf16, ToPointUtf16};
+use url::Url;
 use util::command::new_smol_command;
 use util::{ResultExt, debug_panic, maybe};
 use worktree::Worktree;
@@ -2768,31 +2772,42 @@ impl Session {
 
         let mut console_output = self.console_output(cx);
         let task = cx.spawn(async move |this, cx| {
-            let (dap_port, _child) =
-                if remote_client.read_with(cx, |client, _| client.shares_network_interface())? {
-                    (request.server_port, None)
-                } else {
-                    let port = {
-                        let listener = TcpListener::bind("127.0.0.1:0")
-                            .await
-                            .context("getting port for DAP")?;
-                        listener.local_addr()?.port()
-                    };
-                    let child = remote_client.update(cx, |client, _| {
-                        let command = client.build_forward_port_command(
-                            port,
-                            "localhost".into(),
-                            request.server_port,
-                        )?;
-                        let child = new_smol_command(command.program)
-                            .args(command.args)
-                            .envs(command.env)
-                            .spawn()
-                            .context("spawning port forwarding process")?;
-                        anyhow::Ok(child)
-                    })??;
-                    (port, Some(child))
-                };
+            let forward_ports_process = if remote_client
+                .read_with(cx, |client, _| client.shares_network_interface())?
+            {
+                request.other.insert(
+                    "proxyUri".into(),
+                    format!("127.0.0.1:{}", request.server_port).into(),
+                );
+                None
+            } else {
+                let port = TcpTransport::unused_port(Ipv4Addr::LOCALHOST)
+                    .await
+                    .context("getting port for DAP")?;
+                request
+                    .other
+                    .insert("proxyUri".into(), format!("127.0.0.1:{port}").into());
+                let mut port_forwards = vec![(port, "localhost".to_owned(), request.server_port)];
+
+                if let Some(value) = request.params.get("url")
+                    && let Some(url) = value.as_str()
+                    && let Some(url) = Url::parse(url).ok()
+                    && let Some(frontend_port) = url.port()
+                {
+                    port_forwards.push((frontend_port, "localhost".to_owned(), frontend_port));
+                }
+
+                let child = remote_client.update(cx, |client, _| {
+                    let command = client.build_forward_ports_command(port_forwards)?;
+                    let child = new_smol_command(command.program)
+                        .args(command.args)
+                        .envs(command.env)
+                        .spawn()
+                        .context("spawning port forwarding process")?;
+                    anyhow::Ok(child)
+                })??;
+                Some(child)
+            };
 
             let mut companion_process = None;
             let companion_port =
@@ -2814,14 +2829,17 @@ impl Session {
                         }
                     }
                 };
-            this.update(cx, |this, cx| {
-                this.companion_port = Some(companion_port);
-                let Some(mut child) = companion_process else {
-                    return;
-                };
-                if let Some(stderr) = child.stderr.take() {
+
+            let mut background_tasks = Vec::new();
+            if let Some(mut forward_ports_process) = forward_ports_process {
+                background_tasks.push(cx.spawn(async move |_| {
+                    forward_ports_process.status().await.log_err();
+                }));
+            };
+            if let Some(mut companion_process) = companion_process {
+                if let Some(stderr) = companion_process.stderr.take() {
                     let mut console_output = console_output.clone();
-                    this.background_tasks.push(cx.spawn(async move |_, _| {
+                    background_tasks.push(cx.spawn(async move |_| {
                         let mut stderr = BufReader::new(stderr);
                         let mut line = String::new();
                         while let Ok(n) = stderr.read_line(&mut line).await
@@ -2835,9 +2853,9 @@ impl Session {
                         }
                     }));
                 }
-                this.background_tasks.push(cx.spawn({
+                background_tasks.push(cx.spawn({
                     let mut console_output = console_output.clone();
-                    async move |_, _| match child.status().await {
+                    async move |_| match companion_process.status().await {
                         Ok(status) => {
                             if status.success() {
                                 console_output
@@ -2860,17 +2878,33 @@ impl Session {
                                 .ok();
                         }
                     }
-                }))
-            })?;
+                }));
+            }
 
-            request
-                .other
-                .insert("proxyUri".into(), format!("127.0.0.1:{dap_port}").into());
             // TODO pass wslInfo as needed
 
+            let companion_address = format!("127.0.0.1:{companion_port}");
+            let mut companion_started = false;
+            for _ in 0..10 {
+                if TcpStream::connect(&companion_address).await.is_ok() {
+                    companion_started = true;
+                    break;
+                }
+                cx.background_executor()
+                    .timer(Duration::from_millis(100))
+                    .await;
+            }
+            if !companion_started {
+                console_output
+                    .send("Browser companion failed to start".into())
+                    .await
+                    .ok();
+                bail!("Browser companion failed to start");
+            }
+
             let response = http_client
                 .post_json(
-                    &format!("http://127.0.0.1:{companion_port}/launch-and-attach"),
+                    &format!("http://{companion_address}/launch-and-attach"),
                     serde_json::to_string(&request)
                         .context("serializing request")?
                         .into(),
@@ -2895,6 +2929,11 @@ impl Session {
                 }
             }
 
+            this.update(cx, |this, _| {
+                this.background_tasks.extend(background_tasks);
+                this.companion_port = Some(companion_port);
+            })?;
+
             anyhow::Ok(())
         });
         self.background_tasks.push(cx.spawn(async move |_, _| {
@@ -2926,15 +2965,16 @@ impl Session {
     }
 }
 
-#[derive(Serialize, Deserialize)]
+#[derive(Serialize, Deserialize, Debug)]
 #[serde(rename_all = "camelCase")]
 struct LaunchBrowserInCompanionParams {
     server_port: u16,
+    params: HashMap<String, serde_json::Value>,
     #[serde(flatten)]
     other: HashMap<String, serde_json::Value>,
 }
 
-#[derive(Serialize, Deserialize)]
+#[derive(Serialize, Deserialize, Debug)]
 #[serde(rename_all = "camelCase")]
 struct KillCompanionBrowserParams {
     launch_id: u64,

crates/remote/src/remote_client.rs

@@ -836,16 +836,14 @@ impl RemoteClient {
         connection.build_command(program, args, env, working_dir, port_forward)
     }
 
-    pub fn build_forward_port_command(
+    pub fn build_forward_ports_command(
         &self,
-        local_port: u16,
-        host: String,
-        remote_port: u16,
+        forwards: Vec<(u16, String, u16)>,
     ) -> Result<CommandTemplate> {
         let Some(connection) = self.remote_connection() else {
             return Err(anyhow!("no ssh connection"));
         };
-        connection.build_forward_port_command(local_port, host, remote_port)
+        connection.build_forward_ports_command(forwards)
     }
 
     pub fn upload_directory(
@@ -1116,11 +1114,9 @@ pub(crate) trait RemoteConnection: Send + Sync {
         working_dir: Option<String>,
         port_forward: Option<(u16, String, u16)>,
     ) -> Result<CommandTemplate>;
-    fn build_forward_port_command(
+    fn build_forward_ports_command(
         &self,
-        local_port: u16,
-        remote: String,
-        remote_port: u16,
+        forwards: Vec<(u16, String, u16)>,
     ) -> Result<CommandTemplate>;
     fn connection_options(&self) -> RemoteConnectionOptions;
     fn path_style(&self) -> PathStyle;
@@ -1551,19 +1547,17 @@ mod fake {
             })
         }
 
-        fn build_forward_port_command(
+        fn build_forward_ports_command(
             &self,
-            local_port: u16,
-            host: String,
-            remote_port: u16,
+            forwards: Vec<(u16, String, u16)>,
         ) -> anyhow::Result<CommandTemplate> {
             Ok(CommandTemplate {
                 program: "ssh".into(),
-                args: vec![
-                    "-N".into(),
-                    "-L".into(),
-                    format!("{local_port}:{host}:{remote_port}"),
-                ],
+                args: std::iter::once("-N".to_owned())
+                    .chain(forwards.into_iter().map(|(local_port, host, remote_port)| {
+                        format!("{local_port}:{host}:{remote_port}")
+                    }))
+                    .collect(),
                 env: Default::default(),
             })
         }

crates/remote/src/transport/ssh.rs

@@ -146,19 +146,20 @@ impl RemoteConnection for SshRemoteConnection {
         )
     }
 
-    fn build_forward_port_command(
+    fn build_forward_ports_command(
         &self,
-        local_port: u16,
-        host: String,
-        remote_port: u16,
+        forwards: Vec<(u16, String, u16)>,
     ) -> Result<CommandTemplate> {
+        let Self { socket, .. } = self;
+        let mut args = socket.ssh_args();
+        args.push("-N".into());
+        for (local_port, host, remote_port) in forwards {
+            args.push("-L".into());
+            args.push(format!("{local_port}:{host}:{remote_port}"));
+        }
         Ok(CommandTemplate {
             program: "ssh".into(),
-            args: vec![
-                "-N".into(),
-                "-L".into(),
-                format!("{local_port}:{host}:{remote_port}"),
-            ],
+            args,
             env: Default::default(),
         })
     }

crates/remote/src/transport/wsl.rs

@@ -485,11 +485,9 @@ impl RemoteConnection for WslRemoteConnection {
         })
     }
 
-    fn build_forward_port_command(
+    fn build_forward_ports_command(
         &self,
-        _: u16,
-        _: String,
-        _: u16,
+        _: Vec<(u16, String, u16)>,
     ) -> anyhow::Result<CommandTemplate> {
         Err(anyhow!("WSL shares a network interface with the host"))
     }