Skip to content

Commit b6a3630

Browse files
committed
src/goDebugFactory: connect after createDebugAdapterDescriptor
This is a partial revert of https://go-review.googlesource.com/c/vscode-go/+/313049 CL/313049 attempted to complete the launch + connect before returning from createDebugAdapterDescriptor call. The intention was to detect the problems early enough and simplify the code path. But it turned out VSCode does not fully initialize the debug session and its associated resources such as a Debug Console before createDebugAdapterDescriptor is complete. As a result, any log messages before the return are dropped from Debug Console, which hurts usability/discoverability. Revert the cl, but instead of lazily calling startAndConnectToServer upon the very first sendMessageToServer call, call it from the constructor. We will need it anyway even when we disconnect without sending any message before. logDest test suite is temporarily disabled because the test depends on createDebugAdapterDescriptor's failure when invalid logDest is specified. Change-Id: I980dd27e265133274b8f98bbfddc3a40160e115e Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/320432 Trust: Hyang-Ah Hana Kim <[email protected]> Run-TryBot: Hyang-Ah Hana Kim <[email protected]> TryBot-Result: kokoro <[email protected]> Reviewed-by: Suzy Mueller <[email protected]>
1 parent cbdd328 commit b6a3630

File tree

2 files changed

+26
-5
lines changed

2 files changed

+26
-5
lines changed

src/goDebugFactory.ts

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@ export class GoDebugAdapterDescriptorFactory implements vscode.DebugAdapterDescr
4141
}
4242
const logger = new TimestampedLogger(configuration.trace, this.outputChannel);
4343
const d = new DelveDAPOutputAdapter(configuration, logger);
44-
await d.startAndConnectToServer();
4544
return new vscode.DebugAdapterInlineImplementation(d);
4645
}
4746
}
@@ -196,14 +195,33 @@ export class ProxyDebugAdapter implements vscode.DebugAdapter {
196195
export class DelveDAPOutputAdapter extends ProxyDebugAdapter {
197196
constructor(private config: vscode.DebugConfiguration, logger?: Logger) {
198197
super(logger);
198+
this.connected = this.startAndConnectToServer();
199199
}
200200

201+
private connected: Promise<void>;
201202
private dlvDapServer: ChildProcess;
202203
private port: number;
203204
private socket: net.Socket;
205+
private terminatedOnError = false;
204206

205207
protected async sendMessageToServer(message: vscode.DebugProtocolMessage): Promise<void> {
206-
super.sendMessageToServer(message);
208+
try {
209+
await this.connected;
210+
super.sendMessageToServer(message);
211+
} catch (err) {
212+
if (this.terminatedOnError) {
213+
return;
214+
}
215+
this.terminatedOnError = true;
216+
// If there was an error connecting, show an error message and send a terminated event
217+
// since we cannot start.
218+
if (err) {
219+
const errMsg = `Debug Error: ${err}`;
220+
this.outputEvent('stderr', errMsg);
221+
vscode.window.showErrorMessage(errMsg);
222+
}
223+
this.sendMessageToClient(new TerminatedEvent());
224+
}
207225
}
208226

209227
async dispose(timeoutMS?: number) {
@@ -213,6 +231,10 @@ export class DelveDAPOutputAdapter extends ProxyDebugAdapter {
213231
if (!this.dlvDapServer) {
214232
return;
215233
}
234+
if (this.connected === undefined) {
235+
return;
236+
}
237+
this.connected = undefined;
216238

217239
if (timeoutMS === undefined || timeoutMS < 0) {
218240
timeoutMS = 1_000;
@@ -245,7 +267,7 @@ export class DelveDAPOutputAdapter extends ProxyDebugAdapter {
245267
});
246268
}
247269

248-
public async startAndConnectToServer() {
270+
private async startAndConnectToServer() {
249271
const { port, host, dlvDapServer } = await startDapServer(
250272
this.config,
251273
(msg) => this.outputEvent('stdout', msg),

test/integration/goDebug.test.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1760,7 +1760,7 @@ const testAll = (ctx: Mocha.Context, isDlvDap: boolean) => {
17601760
});
17611761
});
17621762

1763-
suite('logDest attribute tests', () => {
1763+
suite.skip('logDest attribute tests', () => {
17641764
const PROGRAM = path.join(DATA_ROOT, 'baseTest');
17651765

17661766
let tmpDir: string;
@@ -2033,7 +2033,6 @@ suite('Go Debug Adapter Tests (dlv-dap)', function () {
20332033
class DelveDAPDebugAdapterOnSocket extends proxy.DelveDAPOutputAdapter {
20342034
static async create(config: DebugConfiguration) {
20352035
const d = new DelveDAPDebugAdapterOnSocket(config);
2036-
await d.startAndConnectToServer();
20372036
return d;
20382037
}
20392038

0 commit comments

Comments
 (0)