Skip to content

Fix evaluation of variables from chained exception frames#2018

Open
pdepetro wants to merge 1 commit intomicrosoft:mainfrom
pdepetro:main
Open

Fix evaluation of variables from chained exception frames#2018
pdepetro wants to merge 1 commit intomicrosoft:mainfrom
pdepetro:main

Conversation

@pdepetro
Copy link
Copy Markdown
Contributor

@pdepetro pdepetro commented Apr 7, 2026

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.

@pdepetro pdepetro requested a review from a team as a code owner April 7, 2026 19:22

self._frame_id_to_main_thread_id[frame_id] = thread_id

# Also track frames from chained exceptions (e.g. __cause__ / __context__)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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__)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@rchiodo
Copy link
Copy Markdown
Contributor

rchiodo commented Apr 7, 2026

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants