Fix evaluation of variables from chained exception frames#2018
Fix evaluation of variables from chained exception frames#2018pdepetro wants to merge 1 commit intomicrosoft:mainfrom
Conversation
|
|
||
| self._frame_id_to_main_thread_id[frame_id] = thread_id | ||
|
|
||
| # Also track frames from chained exceptions (e.g. __cause__ / __context__) |
There was a problem hiding this comment.
Copilot generated:
-419
Missing cycle detection on chained-frame walk (Medium-High). The while chained is not None loop follows chained_frames_list links without tracking visited nodes. If the exception chain is cyclic (possible pre-3.11 via __context__, or via manual __cause__ assignment), this hangs the debugger. CPython's own traceback.py uses a _seen set for this reason. Suggested fix:
seen = set()
chained = getattr(frames_list, 'chained_frames_list', None)
while chained is not None and id(chained) not in seen and len(chained) > 0:
seen.add(id(chained))
...
chained = getattr(chained, 'chained_frames_list', None)Even if create_frames_list_from_traceback currently prevents cycles, this is cheap defense-in-depth.
|
|
||
| self._frame_id_to_main_thread_id[frame_id] = thread_id | ||
|
|
||
| # Also track frames from chained exceptions (e.g. __cause__ / __context__) |
There was a problem hiding this comment.
Copilot generated:
-419
Duplicated frame registration logic (Medium). The 5-line per-frame registration block (_frame_id_to_frame, _FrameVariable, _variable_reference_to_frames_tracker, frame_ids_from_thread.append, _frame_id_to_main_thread_id) is duplicated verbatim from the primary loop ~10 lines above. Since this duplication is being introduced in this PR, now is the right time to extract a _register_frame(self, frame, thread_id, frame_ids_from_thread) helper called from both loops. This eliminates drift risk if the registration contract changes.
| # Also track frames from chained exceptions (e.g. __cause__ / __context__) | ||
| # so that variable evaluation works for chained exception frames displayed | ||
| # in the call stack. | ||
| chained = getattr(frames_list, 'chained_frames_list', None) |
There was a problem hiding this comment.
Copilot generated:
Potential issue with synthetic separator frames (Low). Chained exception stacks may contain synthetic separator frames (e.g., [Chained Exc: ...]). If these appear in chained_frames_list, calling _FrameVariable(self.py_db, frame, ...) on a non-real frame could raise or produce nonsensical variable listings. If separator objects do appear here, they should be skipped or handled gracefully.
| chained = getattr(frames_list, 'chained_frames_list', None) | ||
| while chained is not None and len(chained) > 0: | ||
| for frame in chained: | ||
| frame_id = id(frame) |
There was a problem hiding this comment.
Copilot generated:
frame_custom_thread_id not considered for chained frames (Low-Medium). The original primary-frame tracking code has frame_custom_thread_id logic, but the new chained-frame block unconditionally uses thread_id. If frame_custom_thread_id remaps the thread identity in some contexts, chained frames would map to the wrong thread. Worth verifying whether frame_custom_thread_id is ever non-None when chained exceptions are present.
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
When stopped on a chained exception, selecting a frame from the cause/context exception in the call stack would show "Unable to find thread to evaluate variable reference." for every variable. This happened because chained exception frames were included in the stackTrace DAP response but were never registered in the SuspendedFramesManager for variable lookup.
Modified _FramesTracker.track() to always walk the chained_frames_list and register those frames in the same tracker, enabling variable evaluation for chained exception frames.