Skip to content

Cherry pick debugpy fixes to release #10654

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Mar 19, 2020
Merged

Cherry pick debugpy fixes to release #10654

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions news/1 Enhancements/10437.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Support reverse connection ("listen" in launch.json) from debug adapter to VSCode.
39 changes: 35 additions & 4 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1363,15 +1363,46 @@
},
"attach": {
"properties": {
"connect": {
"type": "object",
"label": "Attach by connecting to debugpy over a socket.",
"properties": {
"port": {
"type": "number",
"description": "Port to connect to."
},
"host": {
"type": "string",
"description": "Hostname or IP address to connect to.",
"default": "127.0.0.1"
}
},
"required": ["port"]
},
"listen": {
"type": "object",
"label": "Attach by listening for incoming socket connection from debugpy",
"properties": {
"port": {
"type": "number",
"description": "Port to listen on."
},
"host": {
"type": "string",
"description": "Hostname or IP address of the interface to listen on.",
"default": "127.0.0.1"
}
},
"required": ["port"]
},
"port": {
"type": "number",
"description": "Debug port to attach",
"default": 0
"description": "Port to connect to."
},
"host": {
"type": "string",
"description": "IP Address of the of remote server (default is localhost or use 127.0.0.1).",
"default": "localhost"
"description": "Hostname or IP address to connect to.",
"default": "127.0.0.1"
},
"pathMappings": {
"type": "array",
Expand Down
88 changes: 50 additions & 38 deletions src/client/debugger/extension/adapter/factory.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,48 +38,60 @@ export class DebugAdapterDescriptorFactory implements IDebugAdapterDescriptorFac
const configuration = session.configuration as LaunchRequestArguments | AttachRequestArguments;

if (this.experimentsManager.inExperiment(DebugAdapterNewPtvsd.experiment)) {
const isAttach = configuration.request === 'attach';
const port = configuration.port ?? 0;
// When processId is provided we may have to inject the debugger into the process.
// This is done by the debug adapter, so we need to start it. The adapter will handle injecting the debugger when it receives the attach request.
const processId = configuration.processId ?? 0;

if (isAttach && processId === 0) {
if (port === 0) {
throw new Error('Port or processId must be specified for request type attach');
} else {
return new DebugAdapterServer(port, configuration.host);
// There are four distinct scenarios here:
//
// 1. "launch";
// 2. "attach" with "processId";
// 3. "attach" with "listen";
// 4. "attach" with "connect" (or legacy "host"/"port");
//
// For the first three, we want to spawn the debug adapter directly.
// For the last one, the adapter is already listening on the specified socket.
// When "debugServer" is used, the standard adapter factory takes care of it - no need to check here.

if (configuration.request === 'attach') {
if (configuration.connect !== undefined) {
return new DebugAdapterServer(
configuration.connect.port,
configuration.connect.host ?? '127.0.0.1'
);
} else if (configuration.port !== undefined) {
return new DebugAdapterServer(configuration.port, configuration.host ?? '127.0.0.1');
} else if (configuration.listen === undefined && configuration.processId === undefined) {
throw new Error('"request":"attach" requires either "connect", "listen", or "processId"');
}
} else {
const pythonPath = await this.getPythonPath(configuration, session.workspaceFolder);
// If logToFile is set in the debug config then pass --log-dir <path-to-extension-dir> when launching the debug adapter.
}

const pythonPath = await this.getPythonPath(configuration, session.workspaceFolder);
if (pythonPath.length !== 0) {
if (configuration.request === 'attach' && configuration.processId !== undefined) {
sendTelemetryEvent(EventName.DEBUGGER_ATTACH_TO_LOCAL_PROCESS);
}

// "logToFile" is not handled directly by the adapter - instead, we need to pass
// the corresponding CLI switch when spawning it.
const logArgs = configuration.logToFile ? ['--log-dir', EXTENSION_ROOT_DIR] : [];

if (configuration.debugAdapterPath !== undefined) {
return new DebugAdapterExecutable(pythonPath, [configuration.debugAdapterPath, ...logArgs]);
}

const debuggerPathToUse = path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'lib', 'python', 'debugpy');

if (pythonPath.length !== 0) {
if (processId) {
sendTelemetryEvent(EventName.DEBUGGER_ATTACH_TO_LOCAL_PROCESS);
}

if (configuration.debugAdapterPath) {
return new DebugAdapterExecutable(pythonPath, [configuration.debugAdapterPath, ...logArgs]);
}

if (await this.useNewDebugger(pythonPath)) {
sendTelemetryEvent(EventName.DEBUG_ADAPTER_USING_WHEELS_PATH, undefined, { usingWheels: true });
return new DebugAdapterExecutable(pythonPath, [
path.join(debuggerPathToUse, 'wheels', 'debugpy', 'adapter'),
...logArgs
]);
} else {
sendTelemetryEvent(EventName.DEBUG_ADAPTER_USING_WHEELS_PATH, undefined, {
usingWheels: false
});
return new DebugAdapterExecutable(pythonPath, [
path.join(debuggerPathToUse, 'no_wheels', 'debugpy', 'adapter'),
...logArgs
]);
}
if (await this.useNewDebugger(pythonPath)) {
sendTelemetryEvent(EventName.DEBUG_ADAPTER_USING_WHEELS_PATH, undefined, { usingWheels: true });
return new DebugAdapterExecutable(pythonPath, [
path.join(debuggerPathToUse, 'wheels', 'debugpy', 'adapter'),
...logArgs
]);
} else {
sendTelemetryEvent(EventName.DEBUG_ADAPTER_USING_WHEELS_PATH, undefined, {
usingWheels: false
});
return new DebugAdapterExecutable(pythonPath, [
path.join(debuggerPathToUse, 'no_wheels', 'debugpy', 'adapter'),
...logArgs
]);
}
}
} else {
Expand Down
43 changes: 32 additions & 11 deletions src/test/debugger/extension/adapter/factory.unit.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,37 +206,58 @@ suite('Debugging - Adapter Factory', () => {
assert.deepEqual(descriptor, nodeExecutable);
});

test('Return Debug Adapter server if in DA experiment, configuration is attach and port is specified', async () => {
test('Return Debug Adapter server if in DA experiment, request is "attach", and port is specified directly', async () => {
const session = createSession({ request: 'attach', port: 5678, host: 'localhost' });
const debugServer = new DebugAdapterServer(session.configuration.port, session.configuration.host);

when(spiedInstance.inExperiment(DebugAdapterNewPtvsd.experiment)).thenReturn(true);
const descriptor = await factory.createDebugAdapterDescriptor(session, nodeExecutable);

// Interpreter not needed for attach
// Interpreter not needed for host/port
verify(interpreterService.getInterpreters(anything())).never();
assert.deepEqual(descriptor, debugServer);
});

test('Throw error if in DA experiment, configuration is attach, port is 0 and process ID is not specified', async () => {
const session = createSession({ request: 'attach', port: 0, host: 'localhost' });
test('Return Debug Adapter server if in DA experiment, request is "attach", and connect is specified', async () => {
const session = createSession({ request: 'attach', connect: { port: 5678, host: 'localhost' } });
const debugServer = new DebugAdapterServer(
session.configuration.connect.port,
session.configuration.connect.host
);

when(spiedInstance.inExperiment(DebugAdapterNewPtvsd.experiment)).thenReturn(true);
const promise = factory.createDebugAdapterDescriptor(session, nodeExecutable);
const descriptor = await factory.createDebugAdapterDescriptor(session, nodeExecutable);

await expect(promise).to.eventually.be.rejectedWith(
'Port or processId must be specified for request type attach'
);
// Interpreter not needed for connect
verify(interpreterService.getInterpreters(anything())).never();
assert.deepEqual(descriptor, debugServer);
});

test('Return Debug Adapter executable if in DA experiment, request is "attach", and listen is specified', async () => {
const session = createSession({ request: 'attach', listen: { port: 5678, host: 'localhost' } });
const debugExecutable = new DebugAdapterExecutable(pythonPath, [ptvsdAdapterPathWithWheels]);

when(spiedInstance.inExperiment(DebugAdapterNewPtvsd.experiment)).thenReturn(true);
when(interpreterService.getActiveInterpreter(anything())).thenResolve(interpreter);

const descriptor = await factory.createDebugAdapterDescriptor(session, nodeExecutable);
assert.deepEqual(descriptor, debugExecutable);
});

test('Throw error if in DA experiment, configuration is attach and port and process ID are not specified', async () => {
const session = createSession({ request: 'attach', port: undefined, processId: undefined });
test('Throw error if in DA experiment, request is "attach", and neither port, processId, listen, nor connect is specified', async () => {
const session = createSession({
request: 'attach',
port: undefined,
processId: undefined,
listen: undefined,
connect: undefined
});

when(spiedInstance.inExperiment(DebugAdapterNewPtvsd.experiment)).thenReturn(true);
const promise = factory.createDebugAdapterDescriptor(session, nodeExecutable);

await expect(promise).to.eventually.be.rejectedWith(
'Port or processId must be specified for request type attach'
'"request":"attach" requires either "connect", "listen", or "processId"'
);
});

Expand Down