debugger: Show errors loading stack (#32658)

- **TEMP**
- **Show errors loading stack frames**
- **Stop cloning every DAP response unnecessarily**

Closes #ISSUE

Release Notes:

- debugger: Show errors loading stack frames.

<img width="1840" alt="Screenshot 2025-06-12 at 23 53 42"
src="https://github.com/user-attachments/assets/310d3046-f34c-4964-acef-f9742441c9db"
/>
This commit is contained in:
Conrad Irwin 2025-06-13 00:05:57 -06:00 committed by GitHub
parent bcd79331b9
commit f63ae4388d
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 123 additions and 86 deletions

View file

@ -36,6 +36,7 @@ pub struct StackFrameList {
opened_stack_frame_id: Option<StackFrameId>, opened_stack_frame_id: Option<StackFrameId>,
scrollbar_state: ScrollbarState, scrollbar_state: ScrollbarState,
list_state: ListState, list_state: ListState,
error: Option<SharedString>,
_refresh_task: Task<()>, _refresh_task: Task<()>,
} }
@ -82,6 +83,7 @@ impl StackFrameList {
state, state,
_subscription, _subscription,
entries: Default::default(), entries: Default::default(),
error: None,
selected_ix: None, selected_ix: None,
opened_stack_frame_id: None, opened_stack_frame_id: None,
list_state, list_state,
@ -113,21 +115,19 @@ impl StackFrameList {
.collect::<Vec<_>>() .collect::<Vec<_>>()
} }
fn stack_frames(&self, cx: &mut App) -> Vec<StackFrame> { fn stack_frames(&self, cx: &mut App) -> Result<Vec<StackFrame>> {
self.state if let Ok(Some(thread_id)) = self.state.read_with(cx, |state, _| state.thread_id) {
.read_with(cx, |state, _| state.thread_id) self.session
.ok() .update(cx, |this, cx| this.stack_frames(thread_id, cx))
.flatten() } else {
.map(|thread_id| { Ok(Vec::default())
self.session }
.update(cx, |this, cx| this.stack_frames(thread_id, cx))
})
.unwrap_or_default()
} }
#[cfg(test)] #[cfg(test)]
pub(crate) fn dap_stack_frames(&self, cx: &mut App) -> Vec<dap::StackFrame> { pub(crate) fn dap_stack_frames(&self, cx: &mut App) -> Vec<dap::StackFrame> {
self.stack_frames(cx) self.stack_frames(cx)
.unwrap_or_default()
.into_iter() .into_iter()
.map(|stack_frame| stack_frame.dap.clone()) .map(|stack_frame| stack_frame.dap.clone())
.collect() .collect()
@ -149,7 +149,7 @@ impl StackFrameList {
let debounce = this let debounce = this
.update(cx, |this, cx| { .update(cx, |this, cx| {
let new_stack_frames = this.stack_frames(cx); let new_stack_frames = this.stack_frames(cx);
new_stack_frames.is_empty() && !this.entries.is_empty() new_stack_frames.unwrap_or_default().is_empty() && !this.entries.is_empty()
}) })
.ok() .ok()
.unwrap_or_default(); .unwrap_or_default();
@ -185,7 +185,18 @@ impl StackFrameList {
let mut first_stack_frame = None; let mut first_stack_frame = None;
let mut first_not_subtle_frame = None; let mut first_not_subtle_frame = None;
let stack_frames = self.stack_frames(cx); let stack_frames = match self.stack_frames(cx) {
Ok(stack_frames) => stack_frames,
Err(e) => {
self.error = Some(format!("{}", e).into());
self.entries.clear();
self.selected_ix = None;
self.list_state.reset(0);
cx.emit(StackFrameListEvent::BuiltEntries);
cx.notify();
return;
}
};
for stack_frame in &stack_frames { for stack_frame in &stack_frames {
match stack_frame.dap.presentation_hint { match stack_frame.dap.presentation_hint {
Some(dap::StackFramePresentationHint::Deemphasize) => { Some(dap::StackFramePresentationHint::Deemphasize) => {
@ -212,8 +223,7 @@ impl StackFrameList {
if !collapsed_entries.is_empty() { if !collapsed_entries.is_empty() {
entries.push(StackFrameEntry::Collapsed(collapsed_entries.clone())); entries.push(StackFrameEntry::Collapsed(collapsed_entries.clone()));
} }
self.entries = entries;
std::mem::swap(&mut self.entries, &mut entries);
if let Some(ix) = first_not_subtle_frame if let Some(ix) = first_not_subtle_frame
.or(first_stack_frame) .or(first_stack_frame)
@ -657,7 +667,7 @@ impl StackFrameList {
} }
fn render_list(&mut self, _window: &mut Window, _cx: &mut Context<Self>) -> impl IntoElement { fn render_list(&mut self, _window: &mut Window, _cx: &mut Context<Self>) -> impl IntoElement {
list(self.list_state.clone()).size_full() div().p_1().child(list(self.list_state.clone()).size_full())
} }
} }
@ -666,12 +676,27 @@ impl Render for StackFrameList {
div() div()
.track_focus(&self.focus_handle) .track_focus(&self.focus_handle)
.size_full() .size_full()
.p_1()
.on_action(cx.listener(Self::select_next)) .on_action(cx.listener(Self::select_next))
.on_action(cx.listener(Self::select_previous)) .on_action(cx.listener(Self::select_previous))
.on_action(cx.listener(Self::select_first)) .on_action(cx.listener(Self::select_first))
.on_action(cx.listener(Self::select_last)) .on_action(cx.listener(Self::select_last))
.on_action(cx.listener(Self::confirm)) .on_action(cx.listener(Self::confirm))
.when_some(self.error.clone(), |el, error| {
el.child(
h_flex()
.bg(cx.theme().status().warning_background)
.border_b_1()
.border_color(cx.theme().status().warning_border)
.pl_1()
.child(Icon::new(IconName::Warning).color(Color::Warning))
.gap_2()
.child(
Label::new(error)
.size(LabelSize::Small)
.color(Color::Warning),
),
)
})
.child(self.render_list(window, cx)) .child(self.render_list(window, cx))
.child(self.render_vertical_scrollbar(cx)) .child(self.render_vertical_scrollbar(cx))
} }

View file

@ -36,6 +36,7 @@ use gpui::{
Task, WeakEntity, Task, WeakEntity,
}; };
use rpc::ErrorExt;
use serde_json::Value; use serde_json::Value;
use smol::stream::StreamExt; use smol::stream::StreamExt;
use std::any::TypeId; use std::any::TypeId;
@ -108,6 +109,7 @@ impl ThreadStatus {
pub struct Thread { pub struct Thread {
dap: dap::Thread, dap: dap::Thread,
stack_frames: Vec<StackFrame>, stack_frames: Vec<StackFrame>,
stack_frames_error: Option<anyhow::Error>,
_has_stopped: bool, _has_stopped: bool,
} }
@ -116,6 +118,7 @@ impl From<dap::Thread> for Thread {
Self { Self {
dap, dap,
stack_frames: Default::default(), stack_frames: Default::default(),
stack_frames_error: None,
_has_stopped: false, _has_stopped: false,
} }
} }
@ -1388,12 +1391,7 @@ impl Session {
fn fetch<T: DapCommand + PartialEq + Eq + Hash>( fn fetch<T: DapCommand + PartialEq + Eq + Hash>(
&mut self, &mut self,
request: T, request: T,
process_result: impl FnOnce( process_result: impl FnOnce(&mut Self, Result<T::Response>, &mut Context<Self>) + 'static,
&mut Self,
Result<T::Response>,
&mut Context<Self>,
) -> Option<T::Response>
+ 'static,
cx: &mut Context<Self>, cx: &mut Context<Self>,
) { ) {
const { const {
@ -1422,7 +1420,10 @@ impl Session {
&self.capabilities, &self.capabilities,
&self.mode, &self.mode,
command, command,
process_result, |this, result, cx| {
process_result(this, result, cx);
None
},
cx, cx,
); );
let task = cx let task = cx
@ -1438,17 +1439,6 @@ impl Session {
} }
} }
pub async fn request2<T: DapCommand + PartialEq + Eq + Hash>(
&self,
request: T,
) -> Result<T::Response> {
if !T::is_supported(&self.capabilities) {
anyhow::bail!("DAP request {:?} is not supported", request);
}
self.mode.request_dap(request).await
}
fn request_inner<T: DapCommand + PartialEq + Eq + Hash>( fn request_inner<T: DapCommand + PartialEq + Eq + Hash>(
capabilities: &Capabilities, capabilities: &Capabilities,
mode: &Mode, mode: &Mode,
@ -1535,18 +1525,18 @@ impl Session {
self.fetch( self.fetch(
dap_command::ThreadsCommand, dap_command::ThreadsCommand,
|this, result, cx| { |this, result, cx| {
let result = result.log_err()?; let Some(result) = result.log_err() else {
return;
};
this.threads = result this.threads = result
.iter() .into_iter()
.map(|thread| (ThreadId(thread.id), Thread::from(thread.clone()))) .map(|thread| (ThreadId(thread.id), Thread::from(thread.clone())))
.collect(); .collect();
this.invalidate_command_type::<StackTraceCommand>(); this.invalidate_command_type::<StackTraceCommand>();
cx.emit(SessionEvent::Threads); cx.emit(SessionEvent::Threads);
cx.notify(); cx.notify();
Some(result)
}, },
cx, cx,
); );
@ -1566,13 +1556,13 @@ impl Session {
self.fetch( self.fetch(
dap_command::ModulesCommand, dap_command::ModulesCommand,
|this, result, cx| { |this, result, cx| {
let result = result.log_err()?; let Some(result) = result.log_err() else {
return;
};
this.modules = result.iter().cloned().collect(); this.modules = result;
cx.emit(SessionEvent::Modules); cx.emit(SessionEvent::Modules);
cx.notify(); cx.notify();
Some(result)
}, },
cx, cx,
); );
@ -1651,11 +1641,12 @@ impl Session {
self.fetch( self.fetch(
dap_command::LoadedSourcesCommand, dap_command::LoadedSourcesCommand,
|this, result, cx| { |this, result, cx| {
let result = result.log_err()?; let Some(result) = result.log_err() else {
this.loaded_sources = result.iter().cloned().collect(); return;
};
this.loaded_sources = result;
cx.emit(SessionEvent::LoadedSources); cx.emit(SessionEvent::LoadedSources);
cx.notify(); cx.notify();
Some(result)
}, },
cx, cx,
); );
@ -1961,7 +1952,11 @@ impl Session {
.detach(); .detach();
} }
pub fn stack_frames(&mut self, thread_id: ThreadId, cx: &mut Context<Self>) -> Vec<StackFrame> { pub fn stack_frames(
&mut self,
thread_id: ThreadId,
cx: &mut Context<Self>,
) -> Result<Vec<StackFrame>> {
if self.thread_states.thread_status(thread_id) == ThreadStatus::Stopped if self.thread_states.thread_status(thread_id) == ThreadStatus::Stopped
&& self.requests.contains_key(&ThreadsCommand.type_id()) && self.requests.contains_key(&ThreadsCommand.type_id())
&& self.threads.contains_key(&thread_id) && self.threads.contains_key(&thread_id)
@ -1977,48 +1972,63 @@ impl Session {
levels: None, levels: None,
}, },
move |this, stack_frames, cx| { move |this, stack_frames, cx| {
let stack_frames = stack_frames.log_err()?; let entry =
this.threads
let entry = this.threads.entry(thread_id).and_modify(|thread| { .entry(thread_id)
thread.stack_frames = .and_modify(|thread| match &stack_frames {
stack_frames.iter().cloned().map(StackFrame::from).collect(); Ok(stack_frames) => {
}); thread.stack_frames = stack_frames
.iter()
.cloned()
.map(StackFrame::from)
.collect();
thread.stack_frames_error = None;
}
Err(error) => {
thread.stack_frames.clear();
thread.stack_frames_error = Some(error.cloned());
}
});
debug_assert!( debug_assert!(
matches!(entry, indexmap::map::Entry::Occupied(_)), matches!(entry, indexmap::map::Entry::Occupied(_)),
"Sent request for thread_id that doesn't exist" "Sent request for thread_id that doesn't exist"
); );
if let Ok(stack_frames) = stack_frames {
this.stack_frames.extend( this.stack_frames.extend(
stack_frames stack_frames
.iter() .into_iter()
.filter(|frame| { .filter(|frame| {
// Workaround for JavaScript debug adapter sending out "fake" stack frames for delineating await points. This is fine, // Workaround for JavaScript debug adapter sending out "fake" stack frames for delineating await points. This is fine,
// except that they always use an id of 0 for it, which collides with other (valid) stack frames. // except that they always use an id of 0 for it, which collides with other (valid) stack frames.
!(frame.id == 0 !(frame.id == 0
&& frame.line == 0 && frame.line == 0
&& frame.column == 0 && frame.column == 0
&& frame.presentation_hint && frame.presentation_hint
== Some(StackFramePresentationHint::Label)) == Some(StackFramePresentationHint::Label))
}) })
.cloned() .map(|frame| (frame.id, StackFrame::from(frame))),
.map(|frame| (frame.id, StackFrame::from(frame))), );
); }
this.invalidate_command_type::<ScopesCommand>(); this.invalidate_command_type::<ScopesCommand>();
this.invalidate_command_type::<VariablesCommand>(); this.invalidate_command_type::<VariablesCommand>();
cx.emit(SessionEvent::StackTrace); cx.emit(SessionEvent::StackTrace);
cx.notify();
Some(stack_frames)
}, },
cx, cx,
); );
} }
self.threads match self.threads.get(&thread_id) {
.get(&thread_id) Some(thread) => {
.map(|thread| thread.stack_frames.clone()) if let Some(error) = &thread.stack_frames_error {
.unwrap_or_default() Err(error.cloned())
} else {
Ok(thread.stack_frames.clone())
}
}
None => Ok(Vec::new()),
}
} }
pub fn scopes(&mut self, stack_frame_id: u64, cx: &mut Context<Self>) -> &[dap::Scope] { pub fn scopes(&mut self, stack_frame_id: u64, cx: &mut Context<Self>) -> &[dap::Scope] {
@ -2030,9 +2040,11 @@ impl Session {
self.fetch( self.fetch(
ScopesCommand { stack_frame_id }, ScopesCommand { stack_frame_id },
move |this, scopes, cx| { move |this, scopes, cx| {
let scopes = scopes.log_err()?; let Some(scopes) = scopes.log_err() else {
return
};
for scope in scopes .iter(){ for scope in scopes.iter() {
this.variables(scope.variables_reference, cx); this.variables(scope.variables_reference, cx);
} }
@ -2040,7 +2052,7 @@ impl Session {
.stack_frames .stack_frames
.entry(stack_frame_id) .entry(stack_frame_id)
.and_modify(|stack_frame| { .and_modify(|stack_frame| {
stack_frame.scopes = scopes.clone(); stack_frame.scopes = scopes;
}); });
cx.emit(SessionEvent::Variables); cx.emit(SessionEvent::Variables);
@ -2049,8 +2061,6 @@ impl Session {
matches!(entry, indexmap::map::Entry::Occupied(_)), matches!(entry, indexmap::map::Entry::Occupied(_)),
"Sent scopes request for stack_frame_id that doesn't exist or hasn't been fetched" "Sent scopes request for stack_frame_id that doesn't exist or hasn't been fetched"
); );
Some(scopes)
}, },
cx, cx,
); );
@ -2092,13 +2102,14 @@ impl Session {
self.fetch( self.fetch(
command, command,
move |this, variables, cx| { move |this, variables, cx| {
let variables = variables.log_err()?; let Some(variables) = variables.log_err() else {
this.variables return;
.insert(variables_reference, variables.clone()); };
this.variables.insert(variables_reference, variables);
cx.emit(SessionEvent::Variables); cx.emit(SessionEvent::Variables);
cx.emit(SessionEvent::InvalidateInlineValue); cx.emit(SessionEvent::InvalidateInlineValue);
Some(variables)
}, },
cx, cx,
); );
@ -2209,9 +2220,10 @@ impl Session {
self.fetch( self.fetch(
LocationsCommand { reference }, LocationsCommand { reference },
move |this, response, _| { move |this, response, _| {
let response = response.log_err()?; let Some(response) = response.log_err() else {
this.locations.insert(reference, response.clone()); return;
Some(response) };
this.locations.insert(reference, response);
}, },
cx, cx,
); );