debugger: Fix stack frame filter not persisting between sessions (#44352)

This bug was caused by using two separate keys for writing/reading from
the KVP database. The bug didn't show up in my debug build because there
was an old entry of a valid key.

I added an integration test for this feature to prevent future
regressions as well.

Release Notes:

- debugger: Fix a bug where the stack frame filter state wouldn't
persist between sessions

crates/debugger_ui/src/session/running/stack_frame_list.rs

@@ -4,6 +4,7 @@ use std::time::Duration;
 
 use anyhow::{Context as _, Result, anyhow};
 use dap::StackFrameId;
+use dap::adapters::DebugAdapterName;
 use db::kvp::KEY_VALUE_STORE;
 use gpui::{
     Action, AnyElement, Entity, EventEmitter, FocusHandle, Focusable, FontWeight, ListState,
@@ -20,7 +21,7 @@ use project::debugger::breakpoint_store::ActiveStackFrame;
 use project::debugger::session::{Session, SessionEvent, StackFrame, ThreadStatus};
 use project::{ProjectItem, ProjectPath};
 use ui::{Tooltip, WithScrollbar, prelude::*};
-use workspace::{ItemHandle, Workspace};
+use workspace::{ItemHandle, Workspace, WorkspaceId};
 
 use super::RunningState;
 
@@ -58,6 +59,14 @@ impl From<StackFrameFilter> for String {
     }
 }
 
+pub(crate) fn stack_frame_filter_key(
+    adapter_name: &DebugAdapterName,
+    workspace_id: WorkspaceId,
+) -> String {
+    let database_id: i64 = workspace_id.into();
+    format!("stack-frame-list-filter-{}-{}", adapter_name.0, database_id)
+}
+
 pub struct StackFrameList {
     focus_handle: FocusHandle,
     _subscription: Subscription,
@@ -107,14 +116,18 @@ impl StackFrameList {
 
         let list_state = ListState::new(0, gpui::ListAlignment::Top, px(1000.));
 
-        let list_filter = KEY_VALUE_STORE
-            .read_kvp(&format!(
-                "stack-frame-list-filter-{}",
-                session.read(cx).adapter().0
-            ))
+        let list_filter = workspace
+            .read_with(cx, |workspace, _| workspace.database_id())
             .ok()
             .flatten()
-            .map(StackFrameFilter::from_str_or_default)
+            .and_then(|database_id| {
+                let key = stack_frame_filter_key(&session.read(cx).adapter(), database_id);
+                KEY_VALUE_STORE
+                    .read_kvp(&key)
+                    .ok()
+                    .flatten()
+                    .map(StackFrameFilter::from_str_or_default)
+            })
             .unwrap_or(StackFrameFilter::All);
 
         let mut this = Self {
@@ -807,15 +820,8 @@ impl StackFrameList {
             .ok()
             .flatten()
         {
-            let database_id: i64 = database_id.into();
-            let save_task = KEY_VALUE_STORE.write_kvp(
-                format!(
-                    "stack-frame-list-filter-{}-{}",
-                    self.session.read(cx).adapter().0,
-                    database_id,
-                ),
-                self.list_filter.into(),
-            );
+            let key = stack_frame_filter_key(&self.session.read(cx).adapter(), database_id);
+            let save_task = KEY_VALUE_STORE.write_kvp(key, self.list_filter.into());
             cx.background_spawn(save_task).detach();
         }
 

crates/debugger_ui/src/tests/stack_frame_list.rs

@@ -1,12 +1,15 @@
 use crate::{
     debugger_panel::DebugPanel,
-    session::running::stack_frame_list::{StackFrameEntry, StackFrameFilter},
+    session::running::stack_frame_list::{
+        StackFrameEntry, StackFrameFilter, stack_frame_filter_key,
+    },
     tests::{active_debug_session_panel, init_test, init_test_workspace, start_debug_session},
 };
 use dap::{
     StackFrame,
     requests::{Scopes, StackTrace, Threads},
 };
+use db::kvp::KEY_VALUE_STORE;
 use editor::{Editor, ToPoint as _};
 use gpui::{BackgroundExecutor, TestAppContext, VisualTestContext};
 use project::{FakeFs, Project};
@@ -1085,3 +1088,180 @@ async fn test_stack_frame_filter(executor: BackgroundExecutor, cx: &mut TestAppC
         );
     });
 }
+
+#[gpui::test]
+async fn test_stack_frame_filter_persistence(
+    executor: BackgroundExecutor,
+    cx: &mut TestAppContext,
+) {
+    init_test(cx);
+
+    let fs = FakeFs::new(executor.clone());
+
+    fs.insert_tree(
+        path!("/project"),
+        json!({
+           "src": {
+               "test.js": "function main() { console.log('hello'); }",
+           }
+        }),
+    )
+    .await;
+
+    let project = Project::test(fs.clone(), [path!("/project").as_ref()], cx).await;
+    let workspace = init_test_workspace(&project, cx).await;
+    let cx = &mut VisualTestContext::from_window(*workspace, cx);
+    workspace
+        .update(cx, |workspace, _, _| {
+            workspace.set_random_database_id();
+        })
+        .unwrap();
+
+    let threads_response = dap::ThreadsResponse {
+        threads: vec![dap::Thread {
+            id: 1,
+            name: "Thread 1".into(),
+        }],
+    };
+
+    let stack_trace_response = dap::StackTraceResponse {
+        stack_frames: vec![StackFrame {
+            id: 1,
+            name: "main".into(),
+            source: Some(dap::Source {
+                name: Some("test.js".into()),
+                path: Some(path!("/project/src/test.js").into()),
+                source_reference: None,
+                presentation_hint: None,
+                origin: None,
+                sources: None,
+                adapter_data: None,
+                checksums: None,
+            }),
+            line: 1,
+            column: 1,
+            end_line: None,
+            end_column: None,
+            can_restart: None,
+            instruction_pointer_reference: None,
+            module_id: None,
+            presentation_hint: None,
+        }],
+        total_frames: None,
+    };
+
+    let stopped_event = dap::StoppedEvent {
+        reason: dap::StoppedEventReason::Pause,
+        description: None,
+        thread_id: Some(1),
+        preserve_focus_hint: None,
+        text: None,
+        all_threads_stopped: None,
+        hit_breakpoint_ids: None,
+    };
+
+    let session = start_debug_session(&workspace, cx, |_| {}).unwrap();
+    let client = session.update(cx, |session, _| session.adapter_client().unwrap());
+    let adapter_name = session.update(cx, |session, _| session.adapter());
+
+    client.on_request::<Threads, _>({
+        let threads_response = threads_response.clone();
+        move |_, _| Ok(threads_response.clone())
+    });
+
+    client.on_request::<Scopes, _>(move |_, _| Ok(dap::ScopesResponse { scopes: vec![] }));
+
+    client.on_request::<StackTrace, _>({
+        let stack_trace_response = stack_trace_response.clone();
+        move |_, _| Ok(stack_trace_response.clone())
+    });
+
+    client
+        .fake_event(dap::messages::Events::Stopped(stopped_event.clone()))
+        .await;
+
+    cx.run_until_parked();
+
+    let stack_frame_list =
+        active_debug_session_panel(workspace, cx).update(cx, |debug_panel_item, cx| {
+            debug_panel_item
+                .running_state()
+                .update(cx, |state, _| state.stack_frame_list().clone())
+        });
+
+    stack_frame_list.update(cx, |stack_frame_list, _cx| {
+        assert_eq!(
+            stack_frame_list.list_filter(),
+            StackFrameFilter::All,
+            "Initial filter should be All"
+        );
+    });
+
+    stack_frame_list.update(cx, |stack_frame_list, cx| {
+        stack_frame_list
+            .toggle_frame_filter(Some(project::debugger::session::ThreadStatus::Stopped), cx);
+        assert_eq!(
+            stack_frame_list.list_filter(),
+            StackFrameFilter::OnlyUserFrames,
+            "Filter should be OnlyUserFrames after toggle"
+        );
+    });
+
+    cx.run_until_parked();
+
+    let workspace_id = workspace
+        .update(cx, |workspace, _window, _cx| workspace.database_id())
+        .ok()
+        .flatten()
+        .expect("workspace id has to be some for this test to work properly");
+
+    let key = stack_frame_filter_key(&adapter_name, workspace_id);
+    let stored_value = KEY_VALUE_STORE.read_kvp(&key).unwrap();
+    assert_eq!(
+        stored_value,
+        Some(StackFrameFilter::OnlyUserFrames.into()),
+        "Filter should be persisted in KVP store with key: {}",
+        key
+    );
+
+    client
+        .fake_event(dap::messages::Events::Terminated(None))
+        .await;
+    cx.run_until_parked();
+
+    let session2 = start_debug_session(&workspace, cx, |_| {}).unwrap();
+    let client2 = session2.update(cx, |session, _| session.adapter_client().unwrap());
+
+    client2.on_request::<Threads, _>({
+        let threads_response = threads_response.clone();
+        move |_, _| Ok(threads_response.clone())
+    });
+
+    client2.on_request::<Scopes, _>(move |_, _| Ok(dap::ScopesResponse { scopes: vec![] }));
+
+    client2.on_request::<StackTrace, _>({
+        let stack_trace_response = stack_trace_response.clone();
+        move |_, _| Ok(stack_trace_response.clone())
+    });
+
+    client2
+        .fake_event(dap::messages::Events::Stopped(stopped_event.clone()))
+        .await;
+
+    cx.run_until_parked();
+
+    let stack_frame_list2 =
+        active_debug_session_panel(workspace, cx).update(cx, |debug_panel_item, cx| {
+            debug_panel_item
+                .running_state()
+                .update(cx, |state, _| state.stack_frame_list().clone())
+        });
+
+    stack_frame_list2.update(cx, |stack_frame_list, _cx| {
+        assert_eq!(
+            stack_frame_list.list_filter(),
+            StackFrameFilter::OnlyUserFrames,
+            "Filter should be restored from KVP store in new session"
+        );
+    });
+}

crates/workspace/src/workspace.rs

@@ -6069,6 +6069,11 @@ impl Workspace {
             .on_action(cx.listener(Workspace::cancel))
     }
 
+    #[cfg(any(test, feature = "test-support"))]
+    pub fn set_random_database_id(&mut self) {
+        self.database_id = Some(WorkspaceId(Uuid::new_v4().as_u64_pair().0 as i64));
+    }
+
     #[cfg(any(test, feature = "test-support"))]
     pub fn test_new(project: Entity<Project>, window: &mut Window, cx: &mut Context<Self>) -> Self {
         use node_runtime::NodeRuntime;