Add Watch Pane support for the notebook debugger#1282
Open
lionel- wants to merge 4 commits into
Open
Conversation
ba17a11 to
87ea6f3
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addresses posit-dev/positron#13323
The debugger support for notebooks and other Jupyter apps implemented in #1170 and #1171 was quite comprehensive except for one handler:
evaluate, which powers the Watch Pane.The reason it was delayed is that this handler was written for the Console path with an asynchronous approach. The handler needs to evaluate R code in the console and, by the time we get the Evaluate DAP request when the frontend refreshes the Wath Pane contents after R stops at a breakpoint (or
browser()call), R might have resumed evaluation already. That could be either because the user typednvery fast or because they evaluated multiple top-level expresssions in one go and R stops at each of them. To support this case gracefully, the Evaluate handler is executed via an Idle task that only fires when R is no longer busy. Once it completes, a response is sent to the frontend with the result. This happens asynchronously: other DAP requests might come in and be handled while the Evaluate idle task is in flight.This approach did not work out for the Jupyter layer because the handling happens on the Control socket (used for interrupts, shutdowns, and DAP messaging) and expects a quick response right away. While the protocol is silent on whether responses can be asynchronous, reference implementations assume serial handling, both on the frontend side (e.g. VS Code) and the backend side (ipykernel).
This PR solves this by adding a new "try-idle" R task that fires under 200ms if R becomes idle in that time frame, and use that in the Jupyter handler for Evaluate. In my initial implementation there was no timeout and the task would only succeed if the Console was parked in its event loop right at that moment. However that approach had two races that the timeout-waiting fixes:
I picked 200ms arbitrarily, it felt like the maximum amount of time that is reasonable to defer other important Control messages like interrupts or shut downs when R is truly busy and can't service our request.