Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cell debug doesn't stop at breakpoints when VSCode is run on Windows but a remote kernel runs on Unix #11396

Closed
adam-zlatniczki opened this issue Sep 16, 2022 · 4 comments · Fixed by #11436
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug notebook-debugging

Comments

@adam-zlatniczki
Copy link
Contributor

Environment data

  • VS Code version: 1.71.2
  • Jupyter Extension version (available under the Extensions sidebar): v2022.8.1002431955
  • Python Extension version (available under the Extensions sidebar): v2022.14.0
  • OS (Windows | Mac | Linux distro) and version: Windows 10 Enterprise 19044.1889
  • Python and/or Anaconda version: Python 3.9.12, Anaconda 4.14.0
  • Type of virtual environment used (N/A | venv | virtualenv | conda | ...): conda
  • Jupyter server running: Remote

Expected behaviour

After setting a breakpoint in a Jupyter Notebook cell and clicking on Debug cell, the debugger should stop at the breakpoint and show runtime info.

Actual behaviour

Instead of stopping on the breakpoint, VSCode simply executes the cell. The behavior is due to a bug in the kernelDebugAdapter, see later.

Steps to reproduce:

Start VSCode on a Windows machine and connect to a kernel that runs for example on Ubuntu (or some other unix system), then try debugging a cell.

Logs

The issue was evident from the debugpy logs on the server. In a nutshell: even though the kernel was running on unix, the SetBreakpoint requests included paths of the form "\tmp\kernel_id\dumped_cell_content_id.py", and the responses were File Not Found exceptions - as it could be expected.

Solution

The problem stems from path normalization in the dumpCell function of the kernelDebugAdapter. It normalizes the path based on the OS where VSCode runs, which doesn't necessarily coincide with the OS where the kernel runs. The normalization is unnecessary, as on the VSCode side the path on the kernel is simply a key in a hashmap - it works just as fine with whatever path the kernel returned, but in return when VSCode sends the path back to the kernel this way it's guaranteed that the path exists.

I'll be sending a pull-request with the solution when the change passes the CI.

@adam-zlatniczki adam-zlatniczki added the bug Issue identified by VS Code Team member as probable bug label Sep 16, 2022
@github-actions github-actions bot added the triage-needed Issue needs to be triaged label Sep 16, 2022
adam-zlatniczki added a commit to adam-zlatniczki/vscode-jupyter that referenced this issue Sep 16, 2022
* kernelDebugAdapter shouldn't try to convert the file path that it
  receives from the kernel based on the OS where VSCode is run, because
  it leads to files not being found on the kernel side, which in turn
  breaks debugging of a cell
@amunger amunger assigned roblourens and unassigned amunger Sep 16, 2022
@amunger amunger added notebook-debugging and removed triage-needed Issue needs to be triaged labels Sep 16, 2022
@roblourens
Copy link
Member

Instead of calling the generic normalize method, we can do a simple check for whether the path looks like a windows path with a drive letter (matches [a-z]:\) and call the correct normalize method.

If you want to continue working on it @adam-zlatniczki, doing it this way shouldn't be too hard, I would take a PR for that. You can send a new one or I can reopen the previous one.

but I saw
something interesting: the notebook uri looks like file:///c:/... Notice
that there's an extra /,

Not sure which URI you are looking at - that pattern is actually formed correctly. If you are looking at the URI of the notebook itself (local, on the windows machine) then that seems correct to me.

making the uri translate to a file located at
/c:/..., which is clearly an invalid windows path

That however shouldn't happen. If some code uses .path instead of .fsPath, that could happen

@adam-zlatniczki
Copy link
Contributor Author

Sure, I enjoy tinkering with the problem, it's a nice challenge and exercise :) I'll do another fix and PR for the Jupyter notebook debugger, and keep investigating the interactive window debugger. I'll have limited time in the next two weeks, but hopefully I can dig up something useful.

adam-zlatniczki pushed a commit to adam-zlatniczki/vscode-jupyter that referenced this issue Sep 22, 2022
* During notebook cell debugging, apply path normalization on the path of the dumped tmp file only if the kernel runs on a Windows machine.
adam-zlatniczki pushed a commit to adam-zlatniczki/vscode-jupyter that referenced this issue Sep 23, 2022
* During notebook cell debugging, apply path normalization on the path of the dumped tmp file only if the kernel runs on a Windows machine.
adam-zlatniczki pushed a commit to adam-zlatniczki/vscode-jupyter that referenced this issue Sep 23, 2022
* During notebook cell debugging, apply path normalization on the path of the dumped tmp file only if the kernel runs on a Windows machine.
@adam-zlatniczki
Copy link
Contributor Author

It seems that I might have misunderstood something up to this point. The interactive window debugger isn't even supposed to stop at the breakpoints, right?

@roblourens
Copy link
Member

The IW debugger is what runs when you click "Debug Cell" in a .py notebook file (you may have to set "jupyter.forceIPyKernelDebugger": true,)

adam-zlatniczki pushed a commit to adam-zlatniczki/vscode-jupyter that referenced this issue Oct 2, 2022
* During notebook cell debugging, apply path normalization on the path of the dumped tmp file only if the kernel runs on a Windows machine.
roblourens added a commit that referenced this issue Oct 4, 2022
* Fix for issue #11396

* During notebook cell debugging, apply path normalization on the path of the dumped tmp file only if the kernel runs on a Windows machine.

* Rename path helper

Co-authored-by: Ádám Zlatniczki <adam.zlatniczki@ericsson.com>
Co-authored-by: Rob Lourens <roblourens@gmail.com>
roblourens added a commit that referenced this issue Oct 5, 2022
roblourens added a commit that referenced this issue Oct 5, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 26, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug notebook-debugging
Projects
None yet
3 participants