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

Debug adapter doesn't send pathMappings to local container #14820

Closed
yajo opened this issue Nov 25, 2020 · 11 comments
Closed

Debug adapter doesn't send pathMappings to local container #14820

yajo opened this issue Nov 25, 2020 · 11 comments
Assignees
Labels
area-debugging bug Issue identified by VS Code Team member as probable bug needs PR Ready to be worked on

Comments

@yajo
Copy link

yajo commented Nov 25, 2020

Issue Type: Bug

This is how isLocalHost() function is implemented:

protected isLocalHost(hostName?: string) {
const LocalHosts = ['localhost', '127.0.0.1', '::1'];
return hostName && LocalHosts.indexOf(hostName.toLowerCase()) >= 0 ? true : false;
}

However, in my scenario, the python app I'm attaching to runs inside a docker container, and my dev laptop is the docker host. This means that I have to connect to localhost to debug the app, but localhost is not my local host; it's the container.

My guess is that there's a false positive in this code:

// If attaching to local host, then always map local root and remote roots.
if (this.isLocalHost(host)) {
pathMappings = this.fixUpPathMappings(pathMappings, workspaceFolder ? workspaceFolder.fsPath : '');
}

This is my launch configuration (there are several; the one that affects this issue is called "Attach Python debugger to running container"). You can see that it has lots of path mappings because this is a complex multi-root project. Also you can see that it connects to "host": "localhost".

If I enable debugpy logging inside the container, I will be able to see this attach request. Obviously, the breakpoints cannot resolve while debugging:

imagen

I can see that if I change my launch configuration to attach to "host": "0.0.0.0" instead of "host": "localhost" (which also happens to resolve to localhost but will return isLocalHost() -> false), this is the attach request that debugpy reports.

This looks more like what it should be. Now my breakpoints appear as properly mapped while debugging:

imagen

Note: randomly the problem just disappears and it connects properly with path mappings to localhost. Some colleagues with my same OS, the same project, commit and VSCode+extensions versions cannot reproduce. CC @joao-p-marques. However, I have noticed that it seems to work 100% of the time when using "host": "0.0.0.0". Weird bug... 😵


Extension version: 2020.11.371526539
VS Code version: Code 1.51.1 (e5a624b788d92b8d34d1392e4c4d9789406efe8f, 2020-11-10T23:31:29.624Z)
OS version: Linux x64 5.9.8-200.fc33.x86_64

System Info
Item Value
CPUs Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz (12 x 4000)
GPU Status 2d_canvas: enabled
flash_3d: enabled
flash_stage3d: enabled
flash_stage3d_baseline: enabled
gpu_compositing: enabled
multiple_raster_threads: enabled_on
oop_rasterization: disabled_off
opengl: enabled_on
protected_video_decode: unavailable_off
rasterization: disabled_software
skia_renderer: enabled_on
video_decode: unavailable_off
vulkan: disabled_off
webgl: enabled
webgl2: enabled
Load (avg) 2, 2, 2
Memory (System) 15.48GB (0.51GB free)
Process Argv --no-sandbox --unity-launch --crash-reporter-id 1b07b1f6-f187-42fc-aa9e-516fd1a4c83e
Screen Reader no
VM 0%
DESKTOP_SESSION gnome
XDG_CURRENT_DESKTOP GNOME
XDG_SESSION_DESKTOP gnome
XDG_SESSION_TYPE x11
@ghost ghost added the triage-needed Needs assignment to the proper sub-team label Nov 25, 2020
yajo pushed a commit to Tecnativa/doodba-copier-template that referenced this issue Nov 25, 2020
github-actions bot pushed a commit to Tecnativa/doodba-copier-template that referenced this issue Nov 25, 2020
github-actions bot pushed a commit to Tecnativa/doodba-copier-template that referenced this issue Nov 25, 2020
@brettcannon brettcannon added area-debugging reason-preexisting bug Issue identified by VS Code Team member as probable bug labels Nov 25, 2020
@ghost ghost removed the triage-needed Needs assignment to the proper sub-team label Nov 25, 2020
@karthiknadig
Copy link
Member

karthiknadig commented Nov 30, 2020

@yajo I am assuming you are launching the debugger with python -m debugpy --listen 5678, and mapping the port from the container to host using docker's port mapping.

As a workaround you should be able to use host as loopback. This should work in most cases, it will skip the naïve local host check.

@yajo
Copy link
Author

yajo commented Dec 1, 2020

I am assuming you are launching the debugger with python -m debypy --listen 5678, and mapping the port from the container to host using docker's port mapping.

Indeed.

As a workaround you should be able to use host as loopback. This should work in most cases, it will skip the naïve local host check.

That doesn't resolve:

$ ping loopback
ping: loopback: Name or service not known

@int19h
Copy link

int19h commented Dec 3, 2020

@karthiknadig Does pydevd still need the path mappings for localhost case?

@karthiknadig
Copy link
Member

@int19h But this is in the container, so mapping is needed right?

@int19h
Copy link

int19h commented Dec 3, 2020

Yes, but if I understand the issue correctly, the problem is that the fix-up code kicks in and replaces the manually configured path mappings. So if we don't actually need the fix-up anymore, removing it would fix this issue also.

But I'm not so sure about it... looking at the fix-up code, what it actually does is expand {workspaceFolder}, and also ensure that drive letters are lowercased in pathnames. This all sounds sensible, but then the question is: why are we only doing this for localhost attach? I suppose one could argue that C:/ is also a perfectly legal Unix path, so we don't want to touch that unless we know for sure that it's for Windows; but variable expansion is surely useful in all attach scenarios?

Anyway, I think the problem here is that it the fix-up code immediately returns an empty array if defaultLocalRoot is blank, instead of returning the original mappings. We must have regressed at some point, because it wasn't always doing that:

if (!debugConfiguration.pathMappings) {
debugConfiguration.pathMappings = [];
}
// This is for backwards compatibility.
if (debugConfiguration.localRoot && debugConfiguration.remoteRoot) {
debugConfiguration.pathMappings!.push({
localRoot: debugConfiguration.localRoot,
remoteRoot: debugConfiguration.remoteRoot
});
}
// If attaching to local host, then always map local root and remote roots.
if (workspaceFolder && debugConfiguration.host &&
['LOCALHOST', '127.0.0.1', '::1'].indexOf(debugConfiguration.host.toUpperCase()) >= 0) {
let configPathMappings;
if (debugConfiguration.pathMappings!.length === 0) {
configPathMappings = [{
localRoot: workspaceFolder.fsPath,
remoteRoot: workspaceFolder.fsPath
}];
} else {
// Expand ${workspaceFolder} variable first if necessary.
const systemVariables = new SystemVariables(workspaceFolder.fsPath);
configPathMappings = debugConfiguration.pathMappings.map(({ localRoot: mappedLocalRoot, remoteRoot }) => ({
localRoot: systemVariables.resolveAny(mappedLocalRoot),
remoteRoot
}));
}
// If on Windows, lowercase the drive letter for path mappings.
let pathMappings = configPathMappings;
if (this.platformService.isWindows) {
pathMappings = configPathMappings.map(({ localRoot: windowsLocalRoot, remoteRoot }) => {
let localRoot = windowsLocalRoot;
if (windowsLocalRoot.match(/^[A-Z]:/)) {
localRoot = `${windowsLocalRoot[0].toLowerCase()}${windowsLocalRoot.substr(1)}`;
}
return { localRoot, remoteRoot };
});
}
debugConfiguration.pathMappings = pathMappings;
}
this.sendTelemetry('attach', debugConfiguration);

@yajo
Copy link
Author

yajo commented Dec 4, 2020

Please notice that, after applying the workaround explained above, I hit microsoft/debugpy#482 which also has relation with some wrong path mappings. May both be related?

I appreciate a lot your interest in these issues, such a powerful tool like this is becoming a nightmare when debugging. 😿

@int19h
Copy link

int19h commented Dec 4, 2020

I don't think the two are directly related - it's just that unblocking you on this issue means that now debugpy gets to see the entirety of your path mappings, and we weren't handling the prefixes quite right when it's a complicated hierarchy. That issue already has a PR to fix it.

@yajo
Copy link
Author

yajo commented Dec 5, 2020

Nice, and it seems it got fixed already.

OK, then regarding thisissue, my question is: if you're developing locally (without containers), why in the world would you ever need any path mappings? 🤔

If there's no use for that, then possibly the best you can do is apply the mappings always, regardless it's localhost or not.

@int19h
Copy link

int19h commented Dec 7, 2020

If the app being debugged is running on the same file system as VSCode, you do not need them, generally speaking. But you can still use path mapping to translate filenames in that case - e.g. if you're running your code from site-packages, but want the editor to show the original repo.

@yajo
Copy link
Author

yajo commented Dec 7, 2020

I think that's weird, but in any case it seems that removing the specific restrictions for localhost is the way to go.

@karthiknadig
Copy link
Member

Closing as stale

@karthiknadig karthiknadig closed this as not planned Won't fix, can't repro, duplicate, stale Dec 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-debugging bug Issue identified by VS Code Team member as probable bug needs PR Ready to be worked on
Projects
None yet
Development

No branches or pull requests

5 participants