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.
Log expected errors (ValueError, TypeError) at info level and unexpected exceptions with full traceback via pydev_log.exception.
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.