Prevent invalid scopes request from crashing debug session#2026
Prevent invalid scopes request from crashing debug session#2026pdepetro wants to merge 2 commits into
Conversation
If an unmapped or non-numeric frameId is sent in a scopes request, int() raises a ValueError that terminates the debug session. Wrap the scope construction in a try/except so that invalid frameIds return an empty scopes list instead, which is valid per the DAP spec.
| ] | ||
| try: | ||
| scopes = [ | ||
| Scope("Locals", ScopeRequest(int(variables_reference), "locals"), False, presentationHint="locals"), |
There was a problem hiding this comment.
Copilot generated:
except Exception is too broad. The PR description identifies ValueError from int() as the failure mode — catch (ValueError, TypeError) instead. The current catch will silently swallow any future bug in Scope() or ScopeRequest() constructors (e.g., AttributeError, KeyError), making those bugs invisible. All three technical reviewers flagged this independently. [unverified]
[verified]
| try: | ||
| scopes = [ | ||
| Scope("Locals", ScopeRequest(int(variables_reference), "locals"), False, presentationHint="locals"), | ||
| Scope("Globals", ScopeRequest(int(variables_reference), "globals"), False), |
There was a problem hiding this comment.
Copilot generated:
The except block has no logging. When this triggers in production, there's zero diagnostic trail — the user sees an empty Variables panel with no explanation. Add at minimum pydev_log.info('Invalid frameId %r in scopes request', variables_reference) so developers can trace why variables aren't showing. The Advocate notes the codebase has precedent for logging in exception handlers (e.g., line 534 uses pydev_log.exception). [unverified]
[verified]
| from _pydevd_bundle.pydevd_process_net_command_json import PyDevJsonCommandProcessor | ||
|
|
||
| processor = PyDevJsonCommandProcessor(pydevd_base_schema.from_json) | ||
| request = pydevd_schema.ScopesRequest( |
There was a problem hiding this comment.
Copilot generated:
-26
frameId="not_a_number" tests a scenario that likely can't occur via the DAP protocol (which defines frameId as integer). The more realistic failure mode is a valid integer that references a stale/dead frame (e.g., frameId=999999). Consider adding a test for that case — it would exercise a more likely production failure path. [unverified]
[verified]
There was a problem hiding this comment.
999999 is a valid int, so it wouldn't exercise what we're preventing here, which would be a non-integer value.
There was a problem hiding this comment.
The specific issue we've seen is that the client can send a scopes request using a DAP id that corresponds to a thread instead of a frame. The DAP to id translation converts that to something like pid_1996270_id_140192610058896 which raises an exception when passed to int().
Log expected errors (ValueError, TypeError) at info level and unexpected exceptions with full traceback via pydev_log.exception.
rchiodo
left a comment
rchiodo
left a comment
There was a problem hiding this comment.
Approved via Review Center.
2 participants
If an unmapped or non-numeric frameId is sent in a scopes request, int() raises a ValueError that terminates the debug session. Wrap the scope construction in a try/except so that invalid frameIds return an empty scopes list instead, which is valid per the DAP spec.