From 63cc90cd2c0d26697d9175fda8eb01a09af65921 Mon Sep 17 00:00:00 2001 From: Anthony Eid <56899983+Anthony-Eid@users.noreply.github.com> Date: Sun, 7 Dec 2025 20:43:30 -0500 Subject: [PATCH] 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 --- .../src/session/running/stack_frame_list.rs | 38 ++-- .../debugger_ui/src/tests/stack_frame_list.rs | 182 +++++++++++++++++- crates/workspace/src/workspace.rs | 5 + 3 files changed, 208 insertions(+), 17 deletions(-) diff --git a/crates/debugger_ui/src/session/running/stack_frame_list.rs b/crates/debugger_ui/src/session/running/stack_frame_list.rs index a715e2248d14e253a9762c1bcf9f50c1db09d64c..4dffb57a792cb5a5ed6bcc8003a8fa6f3b9af9de 100644 --- a/crates/debugger_ui/src/session/running/stack_frame_list.rs +++ b/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 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(); } diff --git a/crates/debugger_ui/src/tests/stack_frame_list.rs b/crates/debugger_ui/src/tests/stack_frame_list.rs index 05e638e2321bb6fcb4504a8bc8c81123f1b09a33..445d5a01d9f062501c889a9d5aa920de6f037914 100644 --- a/crates/debugger_ui/src/tests/stack_frame_list.rs +++ b/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::({ + let threads_response = threads_response.clone(); + move |_, _| Ok(threads_response.clone()) + }); + + client.on_request::(move |_, _| Ok(dap::ScopesResponse { scopes: vec![] })); + + client.on_request::({ + 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::({ + let threads_response = threads_response.clone(); + move |_, _| Ok(threads_response.clone()) + }); + + client2.on_request::(move |_, _| Ok(dap::ScopesResponse { scopes: vec![] })); + + client2.on_request::({ + 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" + ); + }); +} diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index b1ad520493b4869d646a76df4a0e576646253117..c79159e3dcb282c9622d01f1ee6cea39a5fd0f03 100644 --- a/crates/workspace/src/workspace.rs +++ b/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, window: &mut Window, cx: &mut Context) -> Self { use node_runtime::NodeRuntime;