debugger: Make exception breakpoints persistent (#34014)

Piotr Osiewicz created

Closes #33053
Release Notes:

- Exception breakpoint state is now persisted across debugging sessions.

Change summary

crates/debugger_ui/src/session/running/breakpoint_list.rs | 117 ++++++--
crates/project/src/debugger/dap_store.rs                  | 117 ++++----
crates/project/src/debugger/session.rs                    |  60 ++--
3 files changed, 178 insertions(+), 116 deletions(-)

Detailed changes

crates/debugger_ui/src/session/running/breakpoint_list.rs 🔗

@@ -5,7 +5,8 @@ use std::{
     time::Duration,
 };
 
-use dap::{Capabilities, ExceptionBreakpointsFilter};
+use dap::{Capabilities, ExceptionBreakpointsFilter, adapters::DebugAdapterName};
+use db::kvp::KEY_VALUE_STORE;
 use editor::Editor;
 use gpui::{
     Action, AppContext, ClickEvent, Entity, FocusHandle, Focusable, MouseButton, ScrollStrategy,
@@ -16,6 +17,7 @@ use project::{
     Project,
     debugger::{
         breakpoint_store::{BreakpointEditAction, BreakpointStore, SourceBreakpoint},
+        dap_store::{DapStore, PersistedAdapterOptions},
         session::Session,
     },
     worktree_store::WorktreeStore,
@@ -48,6 +50,7 @@ pub(crate) enum SelectedBreakpointKind {
 pub(crate) struct BreakpointList {
     workspace: WeakEntity<Workspace>,
     breakpoint_store: Entity<BreakpointStore>,
+    dap_store: Entity<DapStore>,
     worktree_store: Entity<WorktreeStore>,
     scrollbar_state: ScrollbarState,
     breakpoints: Vec<BreakpointEntry>,
@@ -59,6 +62,7 @@ pub(crate) struct BreakpointList {
     selected_ix: Option<usize>,
     input: Entity<Editor>,
     strip_mode: Option<ActiveBreakpointStripMode>,
+    serialize_exception_breakpoints_task: Option<Task<anyhow::Result<()>>>,
 }
 
 impl Focusable for BreakpointList {
@@ -85,24 +89,34 @@ impl BreakpointList {
         let project = project.read(cx);
         let breakpoint_store = project.breakpoint_store();
         let worktree_store = project.worktree_store();
+        let dap_store = project.dap_store();
         let focus_handle = cx.focus_handle();
         let scroll_handle = UniformListScrollHandle::new();
         let scrollbar_state = ScrollbarState::new(scroll_handle.clone());
 
-        cx.new(|cx| Self {
-            breakpoint_store,
-            worktree_store,
-            scrollbar_state,
-            breakpoints: Default::default(),
-            hide_scrollbar_task: None,
-            show_scrollbar: false,
-            workspace,
-            session,
-            focus_handle,
-            scroll_handle,
-            selected_ix: None,
-            input: cx.new(|cx| Editor::single_line(window, cx)),
-            strip_mode: None,
+        let adapter_name = session.as_ref().map(|session| session.read(cx).adapter());
+        cx.new(|cx| {
+            let this = Self {
+                breakpoint_store,
+                dap_store,
+                worktree_store,
+                scrollbar_state,
+                breakpoints: Default::default(),
+                hide_scrollbar_task: None,
+                show_scrollbar: false,
+                workspace,
+                session,
+                focus_handle,
+                scroll_handle,
+                selected_ix: None,
+                input: cx.new(|cx| Editor::single_line(window, cx)),
+                strip_mode: None,
+                serialize_exception_breakpoints_task: None,
+            };
+            if let Some(name) = adapter_name {
+                _ = this.deserialize_exception_breakpoints(name, cx);
+            }
+            this
         })
     }
 
@@ -404,12 +418,8 @@ impl BreakpointList {
                 self.edit_line_breakpoint(path, row, BreakpointEditAction::InvertState, cx);
             }
             BreakpointEntryKind::ExceptionBreakpoint(exception_breakpoint) => {
-                if let Some(session) = &self.session {
-                    let id = exception_breakpoint.id.clone();
-                    session.update(cx, |session, cx| {
-                        session.toggle_exception_breakpoint(&id, cx);
-                    });
-                }
+                let id = exception_breakpoint.id.clone();
+                self.toggle_exception_breakpoint(&id, cx);
             }
         }
         cx.notify();
@@ -480,6 +490,64 @@ impl BreakpointList {
         cx.notify();
     }
 
+    fn toggle_exception_breakpoint(&mut self, id: &str, cx: &mut Context<Self>) {
+        if let Some(session) = &self.session {
+            session.update(cx, |this, cx| {
+                this.toggle_exception_breakpoint(&id, cx);
+            });
+            cx.notify();
+            const EXCEPTION_SERIALIZATION_INTERVAL: Duration = Duration::from_secs(1);
+            self.serialize_exception_breakpoints_task = Some(cx.spawn(async move |this, cx| {
+                cx.background_executor()
+                    .timer(EXCEPTION_SERIALIZATION_INTERVAL)
+                    .await;
+                this.update(cx, |this, cx| this.serialize_exception_breakpoints(cx))?
+                    .await?;
+                Ok(())
+            }));
+        }
+    }
+
+    fn kvp_key(adapter_name: &str) -> String {
+        format!("debug_adapter_`{adapter_name}`_persistence")
+    }
+    fn serialize_exception_breakpoints(
+        &mut self,
+        cx: &mut Context<Self>,
+    ) -> Task<anyhow::Result<()>> {
+        if let Some(session) = self.session.as_ref() {
+            let key = {
+                let session = session.read(cx);
+                let name = session.adapter().0;
+                Self::kvp_key(&name)
+            };
+            let settings = self.dap_store.update(cx, |this, cx| {
+                this.sync_adapter_options(session, cx);
+            });
+            let value = serde_json::to_string(&settings);
+
+            cx.background_executor()
+                .spawn(async move { KEY_VALUE_STORE.write_kvp(key, value?).await })
+        } else {
+            return Task::ready(Result::Ok(()));
+        }
+    }
+
+    fn deserialize_exception_breakpoints(
+        &self,
+        adapter_name: DebugAdapterName,
+        cx: &mut Context<Self>,
+    ) -> anyhow::Result<()> {
+        let Some(val) = KEY_VALUE_STORE.read_kvp(&Self::kvp_key(&adapter_name))? else {
+            return Ok(());
+        };
+        let value: PersistedAdapterOptions = serde_json::from_str(&val)?;
+        self.dap_store
+            .update(cx, |this, _| this.set_adapter_options(adapter_name, value));
+
+        Ok(())
+    }
+
     fn hide_scrollbar(&mut self, window: &mut Window, cx: &mut Context<Self>) {
         const SCROLLBAR_SHOW_INTERVAL: Duration = Duration::from_secs(1);
         self.hide_scrollbar_task = Some(cx.spawn_in(window, async move |panel, cx| {
@@ -988,12 +1056,7 @@ impl ExceptionBreakpoint {
                     let list = list.clone();
                     move |_, _, cx| {
                         list.update(cx, |this, cx| {
-                            if let Some(session) = &this.session {
-                                session.update(cx, |this, cx| {
-                                    this.toggle_exception_breakpoint(&id, cx);
-                                });
-                                cx.notify();
-                            }
+                            this.toggle_exception_breakpoint(&id, cx);
                         })
                         .ok();
                     }

crates/project/src/debugger/dap_store.rs 🔗

@@ -14,15 +14,13 @@ use anyhow::{Context as _, Result, anyhow};
 use async_trait::async_trait;
 use collections::HashMap;
 use dap::{
-    Capabilities, CompletionItem, CompletionsArguments, DapRegistry, DebugRequest,
-    EvaluateArguments, EvaluateArgumentsContext, EvaluateResponse, Source, StackFrameId,
+    Capabilities, DapRegistry, DebugRequest, EvaluateArgumentsContext, StackFrameId,
     adapters::{
         DapDelegate, DebugAdapterBinary, DebugAdapterName, DebugTaskDefinition, TcpArguments,
     },
     client::SessionId,
     inline_value::VariableLookupKind,
     messages::Message,
-    requests::{Completions, Evaluate},
 };
 use fs::Fs;
 use futures::{
@@ -40,6 +38,7 @@ use rpc::{
     AnyProtoClient, TypedEnvelope,
     proto::{self},
 };
+use serde::{Deserialize, Serialize};
 use settings::{Settings, SettingsLocation, WorktreeId};
 use std::{
     borrow::Borrow,
@@ -93,10 +92,23 @@ pub struct DapStore {
     worktree_store: Entity<WorktreeStore>,
     sessions: BTreeMap<SessionId, Entity<Session>>,
     next_session_id: u32,
+    adapter_options: BTreeMap<DebugAdapterName, Arc<PersistedAdapterOptions>>,
 }
 
 impl EventEmitter<DapStoreEvent> for DapStore {}
 
+#[derive(Clone, Serialize, Deserialize)]
+pub struct PersistedExceptionBreakpoint {
+    pub enabled: bool,
+}
+
+/// Represents best-effort serialization of adapter state during last session (e.g. watches)
+#[derive(Clone, Default, Serialize, Deserialize)]
+pub struct PersistedAdapterOptions {
+    /// Which exception breakpoints were enabled during the last session with this adapter?
+    pub exception_breakpoints: BTreeMap<String, PersistedExceptionBreakpoint>,
+}
+
 impl DapStore {
     pub fn init(client: &AnyProtoClient, cx: &mut App) {
         static ADD_LOCATORS: Once = Once::new();
@@ -173,6 +185,7 @@ impl DapStore {
             breakpoint_store,
             worktree_store,
             sessions: Default::default(),
+            adapter_options: Default::default(),
         }
     }
 
@@ -520,65 +533,6 @@ impl DapStore {
         ))
     }
 
-    pub fn evaluate(
-        &self,
-        session_id: &SessionId,
-        stack_frame_id: u64,
-        expression: String,
-        context: EvaluateArgumentsContext,
-        source: Option<Source>,
-        cx: &mut Context<Self>,
-    ) -> Task<Result<EvaluateResponse>> {
-        let Some(client) = self
-            .session_by_id(session_id)
-            .and_then(|client| client.read(cx).adapter_client())
-        else {
-            return Task::ready(Err(anyhow!("Could not find client: {:?}", session_id)));
-        };
-
-        cx.background_executor().spawn(async move {
-            client
-                .request::<Evaluate>(EvaluateArguments {
-                    expression: expression.clone(),
-                    frame_id: Some(stack_frame_id),
-                    context: Some(context),
-                    format: None,
-                    line: None,
-                    column: None,
-                    source,
-                })
-                .await
-        })
-    }
-
-    pub fn completions(
-        &self,
-        session_id: &SessionId,
-        stack_frame_id: u64,
-        text: String,
-        completion_column: u64,
-        cx: &mut Context<Self>,
-    ) -> Task<Result<Vec<CompletionItem>>> {
-        let Some(client) = self
-            .session_by_id(session_id)
-            .and_then(|client| client.read(cx).adapter_client())
-        else {
-            return Task::ready(Err(anyhow!("Could not find client: {:?}", session_id)));
-        };
-
-        cx.background_executor().spawn(async move {
-            Ok(client
-                .request::<Completions>(CompletionsArguments {
-                    frame_id: Some(stack_frame_id),
-                    line: None,
-                    text,
-                    column: completion_column,
-                })
-                .await?
-                .targets)
-        })
-    }
-
     pub fn resolve_inline_value_locations(
         &self,
         session: Entity<Session>,
@@ -853,6 +807,45 @@ impl DapStore {
             })
         })
     }
+
+    pub fn sync_adapter_options(
+        &mut self,
+        session: &Entity<Session>,
+        cx: &App,
+    ) -> Arc<PersistedAdapterOptions> {
+        let session = session.read(cx);
+        let adapter = session.adapter();
+        let exceptions = session.exception_breakpoints();
+        let exception_breakpoints = exceptions
+            .map(|(exception, enabled)| {
+                (
+                    exception.filter.clone(),
+                    PersistedExceptionBreakpoint { enabled: *enabled },
+                )
+            })
+            .collect();
+        let options = Arc::new(PersistedAdapterOptions {
+            exception_breakpoints,
+        });
+        self.adapter_options.insert(adapter, options.clone());
+        options
+    }
+
+    pub fn set_adapter_options(
+        &mut self,
+        adapter: DebugAdapterName,
+        options: PersistedAdapterOptions,
+    ) {
+        self.adapter_options.insert(adapter, Arc::new(options));
+    }
+
+    pub fn adapter_options(&self, name: &str) -> Option<Arc<PersistedAdapterOptions>> {
+        self.adapter_options.get(name).cloned()
+    }
+
+    pub fn all_adapter_options(&self) -> &BTreeMap<DebugAdapterName, Arc<PersistedAdapterOptions>> {
+        &self.adapter_options
+    }
 }
 
 #[derive(Clone)]

crates/project/src/debugger/session.rs 🔗

@@ -409,17 +409,6 @@ impl RunningMode {
         };
 
         let configuration_done_supported = ConfigurationDone::is_supported(capabilities);
-        let exception_filters = capabilities
-            .exception_breakpoint_filters
-            .as_ref()
-            .map(|exception_filters| {
-                exception_filters
-                    .iter()
-                    .filter(|filter| filter.default == Some(true))
-                    .cloned()
-                    .collect::<Vec<_>>()
-            })
-            .unwrap_or_default();
         // From spec (on initialization sequence):
         // client sends a setExceptionBreakpoints request if one or more exceptionBreakpointFilters have been defined (or if supportsConfigurationDoneRequest is not true)
         //
@@ -434,10 +423,20 @@ impl RunningMode {
             .unwrap_or_default();
         let this = self.clone();
         let worktree = self.worktree().clone();
+        let mut filters = capabilities
+            .exception_breakpoint_filters
+            .clone()
+            .unwrap_or_default();
         let configuration_sequence = cx.spawn({
-            async move |_, cx| {
-                let breakpoint_store =
-                    dap_store.read_with(cx, |dap_store, _| dap_store.breakpoint_store().clone())?;
+            async move |session, cx| {
+                let adapter_name = session.read_with(cx, |this, _| this.adapter())?;
+                let (breakpoint_store, adapter_defaults) =
+                    dap_store.read_with(cx, |dap_store, _| {
+                        (
+                            dap_store.breakpoint_store().clone(),
+                            dap_store.adapter_options(&adapter_name),
+                        )
+                    })?;
                 initialized_rx.await?;
                 let errors_by_path = cx
                     .update(|cx| this.send_source_breakpoints(false, &breakpoint_store, cx))?
@@ -471,7 +470,25 @@ impl RunningMode {
                 })?;
 
                 if should_send_exception_breakpoints {
-                    this.send_exception_breakpoints(exception_filters, supports_exception_filters)
+                    _ = session.update(cx, |this, _| {
+                        filters.retain(|filter| {
+                            let is_enabled = if let Some(defaults) = adapter_defaults.as_ref() {
+                                defaults
+                                    .exception_breakpoints
+                                    .get(&filter.filter)
+                                    .map(|options| options.enabled)
+                                    .unwrap_or_else(|| filter.default.unwrap_or_default())
+                            } else {
+                                filter.default.unwrap_or_default()
+                            };
+                            this.exception_breakpoints
+                                .entry(filter.filter.clone())
+                                .or_insert_with(|| (filter.clone(), is_enabled));
+                            is_enabled
+                        });
+                    });
+
+                    this.send_exception_breakpoints(filters, supports_exception_filters)
                         .await
                         .ok();
                 }
@@ -1233,18 +1250,7 @@ impl Session {
                     Ok(capabilities) => {
                         this.update(cx, |session, cx| {
                             session.capabilities = capabilities;
-                            let filters = session
-                                .capabilities
-                                .exception_breakpoint_filters
-                                .clone()
-                                .unwrap_or_default();
-                            for filter in filters {
-                                let default = filter.default.unwrap_or_default();
-                                session
-                                    .exception_breakpoints
-                                    .entry(filter.filter.clone())
-                                    .or_insert_with(|| (filter, default));
-                            }
+
                             cx.emit(SessionEvent::CapabilitiesLoaded);
                         })?;
                         return Ok(());