Skip to content

Commit

Permalink
debug: allow debugAdapter=dlv-dap with remote attach mode
Browse files Browse the repository at this point in the history 
If debug adapter is not specified, still default to `legacy` for remote attach in case users haven't updated their tools to newer version of dlv that supports this via DAP. We can flip the default in a couple of months. In the meantime, users will need to specify directly if they want to use `dlv-dap` adapter. There is no good way to detect the version of a running server, so we will always allow them to proceed, while providing a warning about the right version at session start-up and at shutdown via Go Debug output channel if we detect that the connection failed with no responses.

After this change, we will have the following behavior:
attach + remote + debugAdapter=   => legacy
attach + remote + debugAdapter=dlv-dap => dlv-dap + version warnings
attach + remote + debugAdapter=legacy => legacy

attach + !remote + debugAdapter=dlv-dap + port => warning to drop port if not using external server
launch + !remote + debugAdapter=dlv-dap + port => warning to drop port if not using external server
launch + remote  + debugAdapter=dlv-dap + port => error from dlv server

Updates #1861

Change-Id: Ia2e6b35f9d401ea2e719a65c78d9cf8e5ef90c24
GitHub-Last-Rev: 3a0ce53
GitHub-Pull-Request: #1873
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/360974
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
Reviewed-by: Polina Sokolova <polina@google.com>
Trust: Hyang-Ah Hana Kim <hyangah@gmail.com>
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: kokoro <noreply+kokoro@google.com>
  • Loading branch information
@polinasok
polinasok committed Nov 4, 2021
1 parent a445ec3 commit b2c00ac
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 22 deletions.
29 changes: 16 additions & 13 deletions src/goDebugConfiguration.ts
View file
Expand Up @@ -155,21 +155,24 @@ export class GoDebugConfigurationProvider implements vscode.DebugConfigurationPr
}
}
if (!debugConfiguration['debugAdapter']) {
// for local mode, default to dlv-dap.
// For local modes, default to dlv-dap. For remote - to legacy for now.
debugConfiguration['debugAdapter'] = debugConfiguration['mode'] !== 'remote' ? 'dlv-dap' : 'legacy';
}
if (debugConfiguration['debugAdapter'] === 'dlv-dap' && debugConfiguration['mode'] === 'remote') {
this.showWarning(
'ignoreDlvDAPInRemoteModeWarning',
"debugAdapter type of 'dlv-dap' with mode 'remote' is unsupported. Fall back to the 'legacy' debugAdapter for 'remote' mode."
);
debugConfiguration['debugAdapter'] = 'legacy';
}
if (debugConfiguration['debugAdapter'] === 'dlv-dap' && debugConfiguration['port']) {
this.showWarning(
'ignorePortUsedInDlvDapWarning',
"`port` is used with the 'dlv-dap' debugAdapter to support [launching the debug adapter server externally](https://github.com/golang/vscode-go/blob/master/docs/debugging.md#running-debugee-externally). Remove 'host' and 'port' from your launch.json if you are not launching a DAP server."
);
if (debugConfiguration['debugAdapter'] === 'dlv-dap') {
if (debugConfiguration['mode'] === 'remote') {
// This is only possible if a user explicitely requests this combination. Let them, with a warning.
// They need to use dlv at version 'v1.7.3-0.20211026171155-b48ceec161d5' or later,
// but we have no way of detectng that with an external server.
this.showWarning(
'ignoreDlvDAPInRemoteModeWarning',
"'remote' mode with 'dlv-dap' debugAdapter must connect to an external `dlv --headless` server @ v1.7.3 or later. Older versions will fail with \"error layer=rpc rpc:invalid character 'C' looking for beginning of value\" logged to the terminal.\n"
);
} else if (debugConfiguration['port']) {
this.showWarning(
'ignorePortUsedInDlvDapWarning',
"`port` with 'dlv-dap' debugAdapter connects to [an external `dlv dap` server](https://github.com/golang/vscode-go/blob/master/docs/debugging.md#running-debugee-externally) to launch a program or attach to a process. Remove 'host' and 'port' from your launch.json if you have not launched a 'dlv dap' server."
);
}
}

const debugAdapter = debugConfiguration['debugAdapter'] === 'dlv-dap' ? 'dlv-dap' : 'dlv';
Expand Down
34 changes: 28 additions & 6 deletions src/goDebugFactory.ts
View file
Expand Up @@ -38,11 +38,13 @@ export class GoDebugAdapterDescriptorFactory implements vscode.DebugAdapterDescr
private async createDebugAdapterDescriptorDlvDap(
configuration: vscode.DebugConfiguration
): Promise<vscode.ProviderResult<vscode.DebugAdapterDescriptor>> {
const logger = new TimestampedLogger(configuration.trace, this.outputChannel);
logger.debug(`Config: ${JSON.stringify(configuration)}\n`);
if (configuration.port) {
return new vscode.DebugAdapterServer(configuration.port, configuration.host ?? '127.0.0.1');
const host = configuration.host ?? '127.0.0.1';
logger.info(`Connecting to DAP server at ${host}:${configuration.port}\n`);
return new vscode.DebugAdapterServer(configuration.port, host);
}
const logger = new TimestampedLogger(configuration.trace, this.outputChannel);
logger.debug(`Config: ${JSON.stringify(configuration)}`);
const d = new DelveDAPOutputAdapter(configuration, logger);
return new vscode.DebugAdapterInlineImplementation(d);
}
Expand All @@ -57,13 +59,33 @@ export class GoDebugAdapterTrackerFactory implements vscode.DebugAdapterTrackerF
return null;
}
const logger = new TimestampedLogger(session.configuration?.trace || 'off', this.outputChannel);
let requestsSent = 0;
let responsesReceived = 0;
return {
onWillStartSession: () =>
logger.debug(`session ${session.id} will start with ${JSON.stringify(session.configuration)}\n`),
onWillReceiveMessage: (message: any) => logger.trace(`client -> ${JSON.stringify(message)}\n`),
onDidSendMessage: (message: any) => logger.trace(`client <- ${JSON.stringify(message)}\n`),
onWillReceiveMessage: (message: any) => {
logger.trace(`client -> ${JSON.stringify(message)}\n`);
requestsSent++;
},
onDidSendMessage: (message: any) => {
logger.trace(`client <- ${JSON.stringify(message)}\n`);
responsesReceived++;
},
onError: (error: Error) => logger.error(`error: ${error}\n`),
onWillStopSession: () => logger.debug(`session ${session.id} will stop\n`),
onWillStopSession: () => {
if (
session.configuration.debugAdapter === 'dlv-dap' &&
session.configuration.mode === 'remote' &&
requestsSent > 0 &&
responsesReceived === 0 // happens when the rpc server doesn't understand DAP
) {
logger.warn(
"'remote' mode with 'dlv-dap' debugAdapter must connect to an external headless server started with dlv @ v1.7.3 or later.\n"
);
}
logger.debug(`session ${session.id} will stop\n`);
},
onExit: (code: number | undefined, signal: string | undefined) =>
logger.info(`debug adapter exited: (code: ${code}, signal: ${signal})\n`)
};
Expand Down
6 changes: 3 additions & 3 deletions test/integration/goDebugConfiguration.test.ts
View file
Expand Up @@ -887,13 +887,13 @@ suite('Debug Configuration Default DebugAdapter', () => {
cwd: '/path'
};

const want = 'legacy'; // remote mode works only with legacy mode.
const want = 'legacy'; // Remote mode defaults to legacy mode.
debugConfigProvider.resolveDebugConfiguration(undefined, config);
const resolvedConfig = config as any;
assert.strictEqual(resolvedConfig['debugAdapter'], want);
});

test('debugAdapter=dlv-dap should be ignored for remote mode', () => {
test('debugAdapter=dlv-dap is allowed with remote mode', () => {
const config = {
name: 'Attach',
type: 'go',
Expand All @@ -904,7 +904,7 @@ suite('Debug Configuration Default DebugAdapter', () => {
cwd: '/path'
};

const want = 'legacy'; // remote mode works only with legacy mode.
const want = 'dlv-dap'; // If requested, dlv-dap is preserved.
debugConfigProvider.resolveDebugConfiguration(undefined, config);
const resolvedConfig = config as any;
assert.strictEqual(resolvedConfig['debugAdapter'], want);
Expand Down

0 comments on commit b2c00ac

Please sign in to comment.