fabioz / PyDev.Debugger

Sources for the debugger used in PyDev, PyCharm and VSCode Python
Eclipse Public License 1.0
434 stars 122 forks source link

Handle exception thrown when executing invalid expression #276

Closed lukejriddle closed 1 month ago

lukejriddle commented 8 months ago

If an invalid expression is evaluated, the attempt to exec said expression results in an exception. Since the code omits an except clause and only does try finally, the exception is not properly discarded without a return, break, or continue statement: reference.

This resulted in the debugger hanging indefinitely when an invalid expression was evaluated. The exception would show in the console, but the stepping/continuing would stop working.

~This PR fixes this issue by adding the except clause and logging the exception. This allows the debugger to continue executing.~

This PR fixes this issue by properly handling the exception in the calling method (see comments below)

lukejriddle commented 8 months ago

I should note that this only happens when internal_set_expression_json is called -- as part of a setExpression DAP call -- and that this issue doesn't occur when internal_evaluate_expression_json is called -- from the repl.

This is because the former expects a return value from evaluate_expression, but that function will never return anything since is_exec=True. This results in the error check always failing. The latter properly wraps it in a try except.

lukejriddle commented 8 months ago

It also appears that if is_error were ever actually true, which it never will be since the function doesn't return anything, there'd be a runtime exception. Creating a SetExpressionResponseBody is done improperly; it expects value in the constructor.

Same for these lines. I'm guessing this condition is never true, which makes sense from the comment.

lukejriddle commented 8 months ago

Added a new commit to address the above ^

fabioz commented 1 month ago

Sorry for the long time to review this. The change seems fine. Do you think you can fix the conflicts and resubmit?

lukejriddle commented 1 month ago

Done, thanks.

fabioz commented 1 month ago

Just merged. Thanks for the PR.