Skip to content

Commit e831373

Browse files
committed
Fix up debugger configuration resolvers
Ok so we removed the notion of IDs (which are integers) being strings because that broke serialization with recent updates. But on inspection we realized the whole point was to be able to say "current" and have the attach configuration attach to the currently running Extension Terminal. Except that this was an unsupported, half-baked scenario (sure, it attached, but it couldn't actually be debugged properly). So now that throws warnings in both the client and server. The defaults for `processId` and `runspaceId` where changed to `null` (again, now they're just integers) which means to query for them. The supplied but half-baked configuration was removed.
1 parent d367094 commit e831373

File tree

3 files changed

+73
-64
lines changed

3 files changed

+73
-64
lines changed

package.json

+8-22
Original file line numberDiff line numberDiff line change
@@ -486,18 +486,7 @@
486486
"body": {
487487
"name": "PowerShell Attach to Host Process",
488488
"type": "PowerShell",
489-
"request": "attach",
490-
"runspaceId": 1
491-
}
492-
},
493-
{
494-
"label": "PowerShell: Attach Interactive Session Runspace",
495-
"description": "Open runspace picker to select runspace to attach debugger to",
496-
"body": {
497-
"name": "PowerShell Attach Interactive Session Runspace",
498-
"type": "PowerShell",
499-
"request": "attach",
500-
"processId": "current"
489+
"request": "attach"
501490
}
502491
},
503492
{
@@ -588,29 +577,26 @@
588577
"properties": {
589578
"computerName": {
590579
"type": "string",
591-
"description": "Optional: The computer name to which a remote session will be established. Works only on PowerShell 4 and above."
580+
"description": "Optional: The computer name to which a remote session will be established."
592581
},
593582
"processId": {
594-
"type": "string",
595-
"description": "The process id of the PowerShell host process to attach to. Works only on PowerShell 5 and above.",
583+
"type": "number",
584+
"description": "Optional: The ID of the PowerShell host process that should be attached.",
596585
"default": null
597586
},
598587
"runspaceId": {
599-
"type": [
600-
"string",
601-
"number"
602-
],
603-
"description": "Optional: The ID of the runspace to debug in the attached process. Defaults to 1. Works only on PowerShell 5 and above.",
588+
"type": "number",
589+
"description": "Optional: The ID of the runspace to debug in the attached process.",
604590
"default": null
605591
},
606592
"runspaceName": {
607593
"type": "string",
608-
"description": "Optional: The Name of the runspace to debug in the attached process. Works only on PowerShell 5 and above.",
594+
"description": "Optional: The name of the runspace to debug in the attached process.",
609595
"default": null
610596
},
611597
"customPipeName": {
612598
"type": "string",
613-
"description": "The custom pipe name of the PowerShell host process to attach to. Works only on PowerShell 6.2 and above.",
599+
"description": "The custom pipe name of the PowerShell host process to attach to.",
614600
"default": null
615601
}
616602
}

src/features/DebugSession.ts

+58-42
Original file line numberDiff line numberDiff line change
@@ -87,15 +87,14 @@ export const DebugConfigurations: Record<DebugConfig, DebugConfiguration> = {
8787
name: "PowerShell: Attach to PowerShell Host Process",
8888
type: "PowerShell",
8989
request: "attach",
90-
runspaceId: 1,
9190
},
9291
[DebugConfig.RunPester]: {
9392
name: "PowerShell: Run Pester Tests",
9493
type: "PowerShell",
9594
request: "launch",
9695
script: "Invoke-Pester",
9796
createTemporaryIntegratedConsole: true,
98-
attachDotnetDebugger: true
97+
attachDotnetDebugger: true,
9998
},
10099
[DebugConfig.ModuleInteractiveSession]: {
101100
name: "PowerShell: Module Interactive Session",
@@ -109,15 +108,15 @@ export const DebugConfigurations: Record<DebugConfig, DebugConfiguration> = {
109108
request: "launch",
110109
script: "Enter command to import your binary module, for example: \"Import-Module -Force ${workspaceFolder}/path/to/module.psd1|dll\"",
111110
createTemporaryIntegratedConsole: true,
112-
attachDotnetDebugger: true
111+
attachDotnetDebugger: true,
113112
},
114113
[DebugConfig.BinaryModulePester]: {
115114
name: "PowerShell: Binary Module Pester Tests",
116115
type: "PowerShell",
117116
request: "launch",
118117
script: "Invoke-Pester",
119118
createTemporaryIntegratedConsole: true,
120-
attachDotnetDebugger: true
119+
attachDotnetDebugger: true,
121120
}
122121
};
123122

@@ -131,7 +130,7 @@ export class DebugSessionFeature extends LanguageClientConsumer
131130

132131
constructor(context: ExtensionContext, private sessionManager: SessionManager, private logger: ILogger) {
133132
super();
134-
// This "activates" the debug adapter for use with You can only do this once.
133+
// This "activates" the debug adapter. You can only do this once.
135134
[
136135
DebugConfigurationProviderTriggerKind.Initial,
137136
DebugConfigurationProviderTriggerKind.Dynamic
@@ -276,11 +275,15 @@ export class DebugSessionFeature extends LanguageClientConsumer
276275
return resolvedConfig;
277276
}
278277

279-
// This is our factory entrypoint hook to when a debug session starts, and where we will lazy initialize everything needed to do the debugging such as a temporary console if required
278+
// This is our factory entrypoint hook to when a debug session starts, and
279+
// where we will lazy initialize everything needed to do the debugging such
280+
// as a temporary console if required.
281+
//
282+
// NOTE: A Promise meets the shape of a ProviderResult, which allows us to
283+
// make this method async.
280284
public async createDebugAdapterDescriptor(
281285
session: DebugSession,
282286
_executable: DebugAdapterExecutable | undefined): Promise<DebugAdapterDescriptor | undefined> {
283-
// NOTE: A Promise meets the shape of a ProviderResult, which allows us to make this method async.
284287

285288
await this.sessionManager.start();
286289

@@ -426,6 +429,7 @@ export class DebugSessionFeature extends LanguageClientConsumer
426429
this.logger.writeVerbose(`Dotnet attach debug configuration: ${JSON.stringify(dotnetAttachConfig, undefined, 2)}`);
427430
this.logger.writeVerbose(`Attached dotnet debugger to process: ${pid}`);
428431
}
432+
429433
return this.tempSessionDetails;
430434
}
431435

@@ -452,7 +456,7 @@ export class DebugSessionFeature extends LanguageClientConsumer
452456
};
453457
}
454458

455-
/** Fetches all available vscode launch configurations. This is abstracted out for easier testing */
459+
/** Fetches all available vscode launch configurations. This is abstracted out for easier testing. */
456460
private getLaunchConfigurations(): DebugConfiguration[] {
457461
return workspace.getConfiguration("launch").get<DebugConfiguration[]>("configurations") ?? [];
458462
}
@@ -480,6 +484,23 @@ export class DebugSessionFeature extends LanguageClientConsumer
480484
}
481485
}
482486

487+
// If we were given a stringified int from the user, or from the picker
488+
// command, we need to parse it here.
489+
if (typeof config.processId === "string" && config.processId != "current") {
490+
config.processId = parseInt(config.processId);
491+
}
492+
493+
// NOTE: We don't support attaching to the Extension Terminal, even
494+
// though in the past it looked like we did. The implementation was
495+
// half-baked and left things unusable.
496+
if (config.processId === "current" || config.processId === await this.sessionManager.getLanguageServerPid()) {
497+
// TODO: When (if ever) it's supported, we need to convert 0 and the
498+
// old notion of "current" to the actual process ID, like this:
499+
// config.processId = await this.sessionManager.getLanguageServerPid();
500+
void this.logger.writeAndShowError("Attaching to the PowerShell Extension terminal is not supported. Please use the 'PowerShell: Interactive Session' debug configuration instead.");
501+
return PREVENT_DEBUG_START_AND_OPEN_DEBUGCONFIG;
502+
}
503+
483504
if (!config.runspaceId && !config.runspaceName) {
484505
config.runspaceId = await commands.executeCommand("PowerShell.PickRunspace", config.processId);
485506
// No runspace selected. Cancel attach.
@@ -528,12 +549,13 @@ export class SpecifyScriptArgsFeature implements Disposable {
528549
if (text !== undefined) {
529550
await this.context.workspaceState.update(powerShellDbgScriptArgsKey, text);
530551
}
552+
531553
return text;
532554
}
533555
}
534556

535557
interface IProcessItem extends QuickPickItem {
536-
pid: string; // payload for the QuickPick UI
558+
processId: number; // payload for the QuickPick UI
537559
}
538560

539561
// eslint-disable-next-line @typescript-eslint/no-empty-interface
@@ -542,7 +564,7 @@ interface IGetPSHostProcessesArguments {
542564

543565
interface IPSHostProcessInfo {
544566
processName: string;
545-
processId: string;
567+
processId: number;
546568
appDomainName: string;
547569
mainWindowTitle: string;
548570
}
@@ -551,19 +573,16 @@ export const GetPSHostProcessesRequestType =
551573
new RequestType<IGetPSHostProcessesArguments, IPSHostProcessInfo[], string>("powerShell/getPSHostProcesses");
552574

553575
export class PickPSHostProcessFeature extends LanguageClientConsumer {
554-
555576
private command: Disposable;
556577
private waitingForClientToken?: CancellationTokenSource;
557578
private getLanguageClientResolve?: (value: LanguageClient) => void;
558579

559580
constructor(private logger: ILogger) {
560581
super();
561582

562-
this.command =
563-
commands.registerCommand("PowerShell.PickPSHostProcess", () => {
564-
return this.getLanguageClient()
565-
.then((_) => this.pickPSHostProcess(), (_) => undefined);
566-
});
583+
this.command = commands.registerCommand("PowerShell.PickPSHostProcess", async () => {
584+
return this.pickPSHostProcess();
585+
});
567586
}
568587

569588
public override setLanguageClient(languageClient: LanguageClient): void {
@@ -617,25 +636,24 @@ export class PickPSHostProcessFeature extends LanguageClientConsumer {
617636
}
618637
}
619638

620-
private async pickPSHostProcess(): Promise<string | undefined> {
639+
private async pickPSHostProcess(): Promise<number | undefined> {
640+
// We need the language client in order to send the request.
641+
await this.getLanguageClient();
642+
621643
// Start with the current PowerShell process in the list.
622-
const items: IProcessItem[] = [{
623-
label: "Current",
624-
description: "The current PowerShell Extension process.",
625-
pid: "current",
626-
}];
644+
const items: IProcessItem[] = [];
627645

628646
const response = await this.languageClient?.sendRequest(GetPSHostProcessesRequestType, {});
629647
for (const process of response ?? []) {
630648
let windowTitle = "";
631649
if (process.mainWindowTitle) {
632-
windowTitle = `, Title: ${process.mainWindowTitle}`;
650+
windowTitle = `, ${process.mainWindowTitle}`;
633651
}
634652

635653
items.push({
636654
label: process.processName,
637655
description: `PID: ${process.processId.toString()}${windowTitle}`,
638-
pid: process.processId,
656+
processId: process.processId,
639657
});
640658
}
641659

@@ -648,9 +666,10 @@ export class PickPSHostProcessFeature extends LanguageClientConsumer {
648666
matchOnDescription: true,
649667
matchOnDetail: true,
650668
};
669+
651670
const item = await window.showQuickPick(items, options);
652671

653-
return item ? item.pid : undefined;
672+
return item?.processId ?? undefined;
654673
}
655674

656675
private clearWaitingToken(): void {
@@ -660,7 +679,7 @@ export class PickPSHostProcessFeature extends LanguageClientConsumer {
660679
}
661680

662681
interface IRunspaceItem extends QuickPickItem {
663-
id: string; // payload for the QuickPick UI
682+
id: number; // payload for the QuickPick UI
664683
}
665684

666685
// eslint-disable-next-line @typescript-eslint/no-empty-interface
@@ -677,18 +696,16 @@ export const GetRunspaceRequestType =
677696
new RequestType<IGetRunspaceRequestArguments, IRunspace[], string>("powerShell/getRunspace");
678697

679698
export class PickRunspaceFeature extends LanguageClientConsumer {
680-
681699
private command: Disposable;
682700
private waitingForClientToken?: CancellationTokenSource;
683701
private getLanguageClientResolve?: (value: LanguageClient) => void;
684702

685703
constructor(private logger: ILogger) {
686704
super();
687-
this.command =
688-
commands.registerCommand("PowerShell.PickRunspace", (processId) => {
689-
return this.getLanguageClient()
690-
.then((_) => this.pickRunspace(processId), (_) => undefined);
691-
}, this);
705+
706+
this.command = commands.registerCommand("PowerShell.PickRunspace",
707+
async (processId) => { return this.pickRunspace(processId); },
708+
this);
692709
}
693710

694711
public override setLanguageClient(languageClient: LanguageClient): void {
@@ -734,28 +751,26 @@ export class PickRunspaceFeature extends LanguageClientConsumer {
734751
this.clearWaitingToken();
735752
reject();
736753

737-
void this.logger.writeAndShowError("Attach to PowerShell host process: PowerShell session took too long to start.");
754+
void this.logger.writeAndShowError(
755+
"Attach to PowerShell host process: PowerShell session took too long to start.");
738756
}
739757
}, 60000);
740758
},
741759
);
742760
}
743761
}
744762

745-
private async pickRunspace(processId: string): Promise<string | undefined> {
763+
private async pickRunspace(processId: number): Promise<number | undefined> {
764+
// We need the language client in order to send the request.
765+
await this.getLanguageClient();
766+
746767
const response = await this.languageClient?.sendRequest(GetRunspaceRequestType, { processId });
747768
const items: IRunspaceItem[] = [];
748769
for (const runspace of response ?? []) {
749-
// Skip default runspace
750-
if ((runspace.id === 1 || runspace.name === "PSAttachRunspace")
751-
&& processId === "current") {
752-
continue;
753-
}
754-
755770
items.push({
756771
label: runspace.name,
757772
description: `ID: ${runspace.id} - ${runspace.availability}`,
758-
id: runspace.id.toString(),
773+
id: runspace.id,
759774
});
760775
}
761776

@@ -764,9 +779,10 @@ export class PickRunspaceFeature extends LanguageClientConsumer {
764779
matchOnDescription: true,
765780
matchOnDetail: true,
766781
};
782+
767783
const item = await window.showQuickPick(items, options);
768784

769-
return item ? item.id : undefined;
785+
return item?.id ?? undefined;
770786
}
771787

772788
private clearWaitingToken(): void {

src/session.ts

+7
Original file line numberDiff line numberDiff line change
@@ -303,6 +303,13 @@ export class SessionManager implements Middleware {
303303
return this.sessionDetails;
304304
}
305305

306+
public async getLanguageServerPid(): Promise<number | undefined> {
307+
if (this.languageServerProcess === undefined) {
308+
void this.logger.writeAndShowError("PowerShell Extension Terminal unavailable!");
309+
}
310+
return this.languageServerProcess?.getPid();
311+
}
312+
306313
public getPowerShellVersionDetails(): IPowerShellVersionDetails | undefined {
307314
return this.versionDetails;
308315
}

0 commit comments

Comments
 (0)