debugger_ui: Show a toast when setting breakpoints fails (#28815)

Release Notes:

- N/A

---------

Co-authored-by: Anthony Eid <hello@anthonyeid.me>
Co-authored-by: Anthony <anthony@zed.dev>

crates/project/src/debugger/breakpoint_store.rs

@@ -12,7 +12,7 @@ use rpc::{
     AnyProtoClient, TypedEnvelope,
     proto::{self},
 };
-use std::{hash::Hash, ops::Range, path::Path, sync::Arc};
+use std::{hash::Hash, ops::Range, path::Path, sync::Arc, u32};
 use text::{Point, PointUtf16};
 
 use crate::{Project, ProjectPath, buffer_store::BufferStore, worktree_store::WorktreeStore};

crates/project/src/debugger/dap_store.rs

@@ -23,7 +23,7 @@ use futures::{
 };
 use gpui::{
     App, AppContext, AsyncApp, BackgroundExecutor, Context, Entity, EventEmitter, SharedString,
-    Task,
+    Task, WeakEntity,
 };
 use http_client::HttpClient;
 use language::{BinaryStatus, LanguageRegistry, LanguageToolchainStore};
@@ -320,7 +320,7 @@ impl DapStore {
                     session.set_ignore_breakpoints(envelope.payload.ignore, cx)
                 })
             } else {
-                Task::ready(())
+                Task::ready(HashMap::default())
             }
         })?
         .await;
@@ -350,6 +350,7 @@ impl DapStore {
         &mut self,
         binary: DebugAdapterBinary,
         config: DebugTaskDefinition,
+        worktree: WeakEntity<Worktree>,
         parent_session: Option<Entity<Session>>,
         cx: &mut Context<Self>,
     ) -> (SessionId, Task<Result<Entity<Session>>>) {
@@ -373,6 +374,7 @@ impl DapStore {
             let start_client_task = this.update(cx, |this, cx| {
                 Session::local(
                     this.breakpoint_store.clone(),
+                    worktree.clone(),
                     session_id,
                     parent_session,
                     binary,
@@ -385,7 +387,7 @@ impl DapStore {
 
             let ret = this
                 .update(cx, |_, cx| {
-                    create_new_session(session_id, initialized_rx, start_client_task, cx)
+                    create_new_session(session_id, initialized_rx, start_client_task, worktree, cx)
                 })?
                 .await;
             ret
@@ -404,6 +406,16 @@ impl DapStore {
             return Task::ready(Err(anyhow!("Session not found")));
         };
 
+        let Some(worktree) = parent_session
+            .read(cx)
+            .as_local()
+            .map(|local| local.worktree().clone())
+        else {
+            return Task::ready(Err(anyhow!(
+                "Cannot handle start debugging request from remote end"
+            )));
+        };
+
         let args = serde_json::from_value::<StartDebuggingRequestArguments>(
             request.arguments.unwrap_or_default(),
         )
@@ -413,7 +425,7 @@ impl DapStore {
         binary.request_args = args;
 
         let new_session_task = self
-            .new_session(binary, config, Some(parent_session.clone()), cx)
+            .new_session(binary, config, worktree, Some(parent_session.clone()), cx)
             .1;
 
         let request_seq = request.seq;
@@ -742,6 +754,7 @@ fn create_new_session(
     session_id: SessionId,
     initialized_rx: oneshot::Receiver<()>,
     start_client_task: Task<Result<Entity<Session>, anyhow::Error>>,
+    worktree: WeakEntity<Worktree>,
     cx: &mut Context<DapStore>,
 ) -> Task<Result<Entity<Session>>> {
     let task = cx.spawn(async move |this, cx| {
@@ -771,7 +784,7 @@ fn create_new_session(
 
             session
                 .update(cx, |session, cx| {
-                    session.initialize_sequence(initialized_rx, cx)
+                    session.initialize_sequence(initialized_rx, this.clone(), cx)
                 })?
                 .await
         };
@@ -820,12 +833,13 @@ fn create_new_session(
 
                         let task = curr_session.update(cx, |session, cx| session.shutdown(cx));
 
+                        let worktree = worktree.clone();
                         cx.spawn(async move |this, cx| {
                             task.await;
 
                             this.update(cx, |this, cx| {
                                 this.sessions.remove(&session_id);
-                                this.new_session(binary, config, None, cx)
+                                this.new_session(binary, config, worktree, None, cx)
                             })?
                             .1
                             .await?;

crates/project/src/debugger/session.rs

@@ -9,6 +9,7 @@ use super::dap_command::{
     StepBackCommand, StepCommand, StepInCommand, StepOutCommand, TerminateCommand,
     TerminateThreadsCommand, ThreadsCommand, VariablesCommand,
 };
+use super::dap_store::DapStore;
 use anyhow::{Context as _, Result, anyhow};
 use collections::{HashMap, HashSet, IndexMap, IndexSet};
 use dap::adapters::DebugAdapterBinary;
@@ -26,6 +27,7 @@ use gpui::{
     App, AppContext, AsyncApp, BackgroundExecutor, Context, Entity, EventEmitter, SharedString,
     Task, WeakEntity,
 };
+
 use rpc::AnyProtoClient;
 use serde_json::{Value, json};
 use smol::stream::StreamExt;
@@ -42,6 +44,7 @@ use std::{
 use task::DebugTaskDefinition;
 use text::{PointUtf16, ToPointUtf16};
 use util::{ResultExt, merge_json_value_into};
+use worktree::Worktree;
 
 #[derive(Debug, Copy, Clone, Hash, PartialEq, PartialOrd, Ord, Eq)]
 #[repr(transparent)]
@@ -163,6 +166,7 @@ pub struct LocalMode {
     binary: DebugAdapterBinary,
     pub(crate) breakpoint_store: Entity<BreakpointStore>,
     tmp_breakpoint: Option<SourceBreakpoint>,
+    worktree: WeakEntity<Worktree>,
 }
 
 fn client_source(abs_path: &Path) -> dap::Source {
@@ -184,6 +188,7 @@ impl LocalMode {
     fn new(
         session_id: SessionId,
         parent_session: Option<Entity<Session>>,
+        worktree: WeakEntity<Worktree>,
         breakpoint_store: Entity<BreakpointStore>,
         config: DebugTaskDefinition,
         binary: DebugAdapterBinary,
@@ -194,6 +199,7 @@ impl LocalMode {
             session_id,
             parent_session,
             breakpoint_store,
+            worktree,
             config,
             binary,
             messages_tx,
@@ -206,6 +212,7 @@ impl LocalMode {
         session_id: SessionId,
         parent_session: Option<Entity<Session>>,
         breakpoint_store: Entity<BreakpointStore>,
+        worktree: WeakEntity<Worktree>,
         config: DebugTaskDefinition,
         binary: DebugAdapterBinary,
         messages_tx: futures::channel::mpsc::UnboundedSender<Message>,
@@ -240,6 +247,7 @@ impl LocalMode {
             let mut session = Self {
                 client,
                 breakpoint_store,
+                worktree,
                 tmp_breakpoint: None,
                 definition: config,
                 binary,
@@ -251,6 +259,10 @@ impl LocalMode {
         })
     }
 
+    pub(crate) fn worktree(&self) -> &WeakEntity<Worktree> {
+        &self.worktree
+    }
+
     fn unset_breakpoints_from_paths(&self, paths: &Vec<Arc<Path>>, cx: &mut App) -> Task<()> {
         let tasks: Vec<_> = paths
             .into_iter()
@@ -335,7 +347,12 @@ impl LocalMode {
         };
         self.request(arg, cx.background_executor().clone())
     }
-    fn send_source_breakpoints(&self, ignore_breakpoints: bool, cx: &App) -> Task<()> {
+
+    fn send_source_breakpoints(
+        &self,
+        ignore_breakpoints: bool,
+        cx: &App,
+    ) -> Task<HashMap<Arc<Path>, anyhow::Error>> {
         let mut breakpoint_tasks = Vec::new();
         let breakpoints = self
             .breakpoint_store
@@ -352,26 +369,25 @@ impl LocalMode {
                     .collect()
             };
 
-            breakpoint_tasks.push(self.request(
-                dap_command::SetBreakpoints {
-                    source: client_source(&path),
-                    source_modified: Some(false),
-                    breakpoints,
-                },
-                cx.background_executor().clone(),
-            ));
+            breakpoint_tasks.push(
+                self.request(
+                    dap_command::SetBreakpoints {
+                        source: client_source(&path),
+                        source_modified: Some(false),
+                        breakpoints,
+                    },
+                    cx.background_executor().clone(),
+                )
+                .map(|result| result.map_err(|e| (path, e))),
+            );
         }
 
         cx.background_spawn(async move {
             futures::future::join_all(breakpoint_tasks)
                 .await
-                .iter()
-                .for_each(|res| match res {
-                    Ok(_) => {}
-                    Err(err) => {
-                        log::warn!("Set breakpoints request failed: {}", err);
-                    }
-                });
+                .into_iter()
+                .filter_map(Result::err)
+                .collect::<HashMap<_, _>>()
         })
     }
 
@@ -389,6 +405,7 @@ impl LocalMode {
         &self,
         capabilities: &Capabilities,
         initialized_rx: oneshot::Receiver<()>,
+        dap_store: WeakEntity<DapStore>,
         cx: &App,
     ) -> Task<Result<()>> {
         let mut raw = self.binary.request_args.clone();
@@ -431,12 +448,40 @@ impl LocalMode {
             .unwrap_or_default();
         let configuration_sequence = cx.spawn({
             let this = self.clone();
+            let worktree = self.worktree().clone();
             async move |cx| {
                 initialized_rx.await?;
-                // todo(debugger) figure out if we want to handle a breakpoint response error
-                // This will probably consist of letting a user know that breakpoints failed to be set
-                cx.update(|cx| this.send_source_breakpoints(false, cx))?
+                let errors_by_path = cx
+                    .update(|cx| this.send_source_breakpoints(false, cx))?
                     .await;
+
+                dap_store.update(cx, |_, cx| {
+                    let Some(worktree) = worktree.upgrade() else {
+                        return;
+                    };
+
+                    for (path, error) in &errors_by_path {
+                        log::error!("failed to set breakpoints for {path:?}: {error}");
+                    }
+
+                    if let Some(failed_path) = errors_by_path.keys().next() {
+                        let failed_path = failed_path
+                            .strip_prefix(worktree.read(cx).abs_path())
+                            .unwrap_or(failed_path)
+                            .display();
+                        let message = format!(
+                            "Failed to set breakpoints for {failed_path}{}",
+                            match errors_by_path.len() {
+                                0 => unreachable!(),
+                                1 => "".into(),
+                                2 => " and 1 other path".into(),
+                                n => format!(" and {} other paths", n - 1),
+                            }
+                        );
+                        cx.emit(super::dap_store::DapStoreEvent::Notification(message));
+                    }
+                })?;
+
                 cx.update(|cx| {
                     this.send_exception_breakpoints(
                         exception_filters,
@@ -699,6 +744,7 @@ impl EventEmitter<SessionStateEvent> for Session {}
 impl Session {
     pub(crate) fn local(
         breakpoint_store: Entity<BreakpointStore>,
+        worktree: WeakEntity<Worktree>,
         session_id: SessionId,
         parent_session: Option<Entity<Session>>,
         binary: DebugAdapterBinary,
@@ -713,6 +759,7 @@ impl Session {
             let mode = LocalMode::new(
                 session_id,
                 parent_session.clone(),
+                worktree,
                 breakpoint_store.clone(),
                 config.clone(),
                 binary,
@@ -871,11 +918,12 @@ impl Session {
     pub(super) fn initialize_sequence(
         &mut self,
         initialize_rx: oneshot::Receiver<()>,
+        dap_store: WeakEntity<DapStore>,
         cx: &mut Context<Self>,
     ) -> Task<Result<()>> {
         match &self.mode {
             Mode::Local(local_mode) => {
-                local_mode.initialize_sequence(&self.capabilities, initialize_rx, cx)
+                local_mode.initialize_sequence(&self.capabilities, initialize_rx, dap_store, cx)
             }
             Mode::Remote(_) => Task::ready(Err(anyhow!("cannot initialize remote session"))),
         }
@@ -1292,13 +1340,20 @@ impl Session {
         self.ignore_breakpoints
     }
 
-    pub fn toggle_ignore_breakpoints(&mut self, cx: &mut App) -> Task<()> {
+    pub fn toggle_ignore_breakpoints(
+        &mut self,
+        cx: &mut App,
+    ) -> Task<HashMap<Arc<Path>, anyhow::Error>> {
         self.set_ignore_breakpoints(!self.ignore_breakpoints, cx)
     }
 
-    pub(crate) fn set_ignore_breakpoints(&mut self, ignore: bool, cx: &mut App) -> Task<()> {
+    pub(crate) fn set_ignore_breakpoints(
+        &mut self,
+        ignore: bool,
+        cx: &mut App,
+    ) -> Task<HashMap<Arc<Path>, anyhow::Error>> {
         if self.ignore_breakpoints == ignore {
-            return Task::ready(());
+            return Task::ready(HashMap::default());
         }
 
         self.ignore_breakpoints = ignore;

crates/project/src/project.rs

@@ -1462,7 +1462,7 @@ impl Project {
         config: DebugTaskDefinition,
         cx: &mut Context<Self>,
     ) -> Task<Result<Entity<Session>>> {
-        let Some(worktree) = self.worktrees(cx).next() else {
+        let Some(worktree) = self.worktrees(cx).find(|tree| tree.read(cx).is_visible()) else {
             return Task::ready(Err(anyhow!("Failed to find a worktree")));
         };
 
@@ -1501,7 +1501,7 @@ impl Project {
             let ret = this
                 .update(cx, |project, cx| {
                     project.dap_store.update(cx, |dap_store, cx| {
-                        dap_store.new_session(binary, config, None, cx)
+                        dap_store.new_session(binary, config, worktree.downgrade(), None, cx)
                     })
                 })?
                 .1