src/goDebugFactory: respond with error if dlv dap failed to start
DelveDAPOutputAdapter constructor begins spawning of dlv dap
process but it doesn't wait until the dlv dap process is up and
running. The success or failure of dlv dap process launch is
lazily detected when the client sends messages. Previously,
we just sent a TerminatedEvent and pop up an error notification.
This CL changes DelveDAPOutputAdapter to respond with
DebugProtocol.ErrorResponse, so the client can handle the error
- displaying the error with a modal popup
- changing focus so DEBUG CONSOLE becomes visible
And made startAndConnectToServer catches errors and never
throw an error (reject the promise). So, DelveDAPOutputAdapter's
'connected' will never be rejected and explicitly store the error
message. Previously this rejected promise caused warnings, i.e.,
rejected promise not handled within X seconds.
Did some clean up of unnecessary async or declaration.
And, fixed and reenabled logDest tests.
Change-Id: I57b7d52d83a82c506629e07b1dec65f9e02801bc
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/321472
Trust: Hyang-Ah Hana Kim <hyangah@gmail.com>
Trust: Suzy Mueller <suzmue@golang.org>
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Suzy Mueller <suzmue@golang.org>
diff --git a/src/goDebugFactory.ts b/src/goDebugFactory.ts
index 41613a5..b1341d3 100644
--- a/src/goDebugFactory.ts
+++ b/src/goDebugFactory.ts
@@ -15,6 +15,7 @@
import * as net from 'net';
import { getTool } from './goTools';
import { Logger, TimestampedLogger } from './goLogging';
+import { DebugProtocol } from 'vscode-debugprotocol';
export class GoDebugAdapterDescriptorFactory implements vscode.DebugAdapterDescriptorFactory {
constructor(private outputChannel?: vscode.OutputChannel) {}
@@ -198,30 +199,35 @@
this.connected = this.startAndConnectToServer();
}
- private connected: Promise<void>;
+ private connected: Promise<{ connected: boolean; reason?: any }>;
private dlvDapServer: ChildProcess;
private port: number;
private socket: net.Socket;
private terminatedOnError = false;
protected async sendMessageToServer(message: vscode.DebugProtocolMessage): Promise<void> {
- try {
- await this.connected;
+ const { connected, reason } = await this.connected;
+ if (connected) {
super.sendMessageToServer(message);
- } catch (err) {
- if (this.terminatedOnError) {
- return;
- }
+ return;
+ }
+ const errMsg = `Couldn't start dlv dap:\n${reason}`;
+ if (this.terminatedOnError) {
this.terminatedOnError = true;
- // If there was an error connecting, show an error message and send a terminated event
- // since we cannot start.
- if (err) {
- const errMsg = `Debug Error: ${err}`;
- this.outputEvent('stderr', errMsg);
- vscode.window.showErrorMessage(errMsg);
- }
+ this.outputEvent('stderr', errMsg);
this.sendMessageToClient(new TerminatedEvent());
}
+ if ((message as any).type === 'request') {
+ const req = message as DebugProtocol.Request;
+ this.sendMessageToClient({
+ seq: 0,
+ type: 'response',
+ request_seq: req.seq,
+ success: false,
+ command: req.command,
+ message: errMsg
+ });
+ }
}
async dispose(timeoutMS?: number) {
@@ -268,36 +274,42 @@
}
private async startAndConnectToServer() {
- const { port, host, dlvDapServer } = await startDapServer(
- this.config,
- (msg) => this.outputEvent('stdout', msg),
- (msg) => this.outputEvent('stderr', msg),
- (msg) => {
- this.outputEvent('console', msg);
- // Some log messages generated after vscode stops the debug session
- // may not appear in the DEBUG CONSOLE. For easier debugging, log
- // the messages through the logger that prints to Go Debug output
- // channel.
- this.logger?.info(msg);
- }
- );
- const socket = await new Promise<net.Socket>((resolve, reject) => {
- // eslint-disable-next-line prefer-const
- let timer: NodeJS.Timeout;
- const s = net.createConnection(port, host, () => {
- clearTimeout(timer);
- resolve(s);
+ try {
+ const { port, host, dlvDapServer } = await startDapServer(
+ this.config,
+ (msg) => this.outputEvent('stdout', msg),
+ (msg) => this.outputEvent('stderr', msg),
+ (msg) => {
+ this.outputEvent('console', msg);
+ // Some log messages generated after vscode stops the debug session
+ // may not appear in the DEBUG CONSOLE. For easier debugging, log
+ // the messages through the logger that prints to Go Debug output
+ // channel.
+ this.logger?.info(msg);
+ }
+ );
+ const socket = await new Promise<net.Socket>((resolve, reject) => {
+ // eslint-disable-next-line prefer-const
+ let timer: NodeJS.Timeout;
+ const s = net.createConnection(port, host, () => {
+ clearTimeout(timer);
+ resolve(s);
+ });
+ timer = setTimeout(() => {
+ reject('connection timeout');
+ s?.destroy();
+ }, 1000);
});
- timer = setTimeout(() => {
- reject('connection timeout');
- s?.destroy();
- }, 1000);
- });
- this.dlvDapServer = dlvDapServer;
- this.port = port;
- this.socket = socket;
- this.start(this.socket, this.socket);
+ this.dlvDapServer = dlvDapServer;
+ this.port = port;
+ this.socket = socket;
+ this.start(this.socket, this.socket);
+ } catch (err) {
+ return { connected: false, reason: err };
+ }
+ this.logger?.debug(`Running dlv dap server: port=${this.port} pid=${this.dlvDapServer.pid}\n`);
+ return { connected: true };
}
private outputEvent(dest: string, output: string, data?: any) {
@@ -305,11 +317,11 @@
}
}
-export async function startDapServer(
+async function startDapServer(
configuration: vscode.DebugConfiguration,
- log?: (msg: string) => void,
- logErr?: (msg: string) => void,
- logConsole?: (msg: string) => void
+ log: (msg: string) => void,
+ logErr: (msg: string) => void,
+ logConsole: (msg: string) => void
): Promise<{ port: number; host: string; dlvDapServer?: ChildProcessWithoutNullStreams }> {
const host = configuration.host || '127.0.0.1';
@@ -319,20 +331,11 @@
return { port: configuration.port, host };
}
const port = await getPort();
- if (!log) {
- log = appendToDebugConsole;
- }
- if (!logErr) {
- logErr = appendToDebugConsole;
- }
- if (!logConsole) {
- logConsole = appendToDebugConsole;
- }
const dlvDapServer = await spawnDlvDapServerProcess(configuration, host, port, log, logErr, logConsole);
return { dlvDapServer, port, host };
}
-async function spawnDlvDapServerProcess(
+function spawnDlvDapServerProcess(
launchArgs: vscode.DebugConfiguration,
host: string,
port: number,
@@ -350,12 +353,19 @@
logErr(
`Couldn't find dlv-dap at the Go tools path, ${process.env['GOPATH']}${
env['GOPATH'] ? ', ' + env['GOPATH'] : ''
- } or ${envPath}`
+ } or ${envPath}\n` +
+ 'Follow the setup instruction in https://github.com/golang/vscode-go/blob/master/docs/dlv-dap.md#getting-started.\n'
);
- throw new Error(
- 'Cannot find Delve debugger. Install from https://github.com/go-delve/delve & ensure it is in your Go tools path, "GOPATH/bin" or "PATH".'
- );
+ throw new Error('Cannot find Delve debugger (dlv dap)');
}
+ let dir = '';
+ try {
+ dir = parseProgramArgSync(launchArgs).dirname;
+ } catch (err) {
+ logErr(`Program arg: ${launchArgs.program}\n${err}\n`);
+ throw err; // rethrow so the caller knows it failed.
+ }
+
const dlvArgs = new Array<string>();
dlvArgs.push('dap');
// add user-specified dlv flags first. When duplicate flags are specified,
@@ -379,12 +389,12 @@
const logDest = launchArgs.logDest;
if (typeof logDest === 'number') {
- logErr('Using a file descriptor for `logDest` is not allowed.');
+ logErr(`Using a file descriptor for 'logDest' (${logDest}) is not allowed.\n`);
throw new Error('Using a file descriptor for `logDest` is not allowed.');
}
if (logDest && !path.isAbsolute(logDest)) {
logErr(
- 'Using a relative path for `logDest` is not allowed.\nSee [variables](https://code.visualstudio.com/docs/editor/variables-reference)'
+ `Using a relative path for 'logDest' (${logDest}) is not allowed.\nSee https://code.visualstudio.com/docs/editor/variables-reference if you want workspace-relative path.\n`
);
throw new Error('Using a relative path for `logDest` is not allowed');
}
@@ -397,14 +407,13 @@
const logDestStream = logDest ? fs.createWriteStream(logDest) : undefined;
- logConsole(`Running: ${dlvPath} ${dlvArgs.join(' ')}\n`);
+ logConsole(`Starting: ${dlvPath} ${dlvArgs.join(' ')}\n`);
- const dir = parseProgramArgSync(launchArgs).dirname;
// TODO(hyangah): determine the directories:
// run `dlv` => where dlv will create the default __debug_bin. (This won't work if the directory is not writable. Fix it)
// build program => 'program' directory. (This won't work for multimodule workspace. Fix it)
// run program => cwd (If test, make sure to run in the package directory.)
- return await new Promise<ChildProcess>((resolve, reject) => {
+ return new Promise<ChildProcess>((resolve, reject) => {
const p = spawn(dlvPath, dlvArgs, {
cwd: dir,
env,
@@ -521,8 +530,3 @@
const dirname = programIsDirectory ? program : path.dirname(program);
return { program, dirname, programIsDirectory };
}
-
-// appendToDebugConsole is declared as an exported const rather than a function, so it can be stubbbed in testing.
-export const appendToDebugConsole = (msg: string) => {
- console.error(msg);
-};
diff --git a/test/integration/goDebug.test.ts b/test/integration/goDebug.test.ts
index d2db3dc..2cf6194 100644
--- a/test/integration/goDebug.test.ts
+++ b/test/integration/goDebug.test.ts
@@ -1760,7 +1760,7 @@
});
});
- suite.skip('logDest attribute tests', () => {
+ suite('logDest attribute tests', () => {
const PROGRAM = path.join(DATA_ROOT, 'baseTest');
let tmpDir: string;
@@ -1808,13 +1808,13 @@
logDest
};
+ await initializeDebugConfig(config);
try {
- await initializeDebugConfig(config);
+ await dc.initializeRequest();
+ assert.fail('dlv dap started normally, wanted the invalid logDest to cause failure');
} catch (error) {
assert(error?.message.includes(wantedErrorMessage), `unexpected error: ${error}`);
- return;
}
- assert.fail('dlv dap started normally, wanted the invalid logDest to cause failure');
}
test('relative path as logDest triggers an error', async function () {
if (!isDlvDap || process.platform === 'win32') this.skip();
