Skip to content

Commit e9cb397

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 3ba25fe commit e9cb397

File tree

3 files changed

+67
-64
lines changed

3 files changed

+67
-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

+52-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,17 @@ export class DebugSessionFeature extends LanguageClientConsumer
480484
}
481485
}
482486

487+
// NOTE: We don't support attaching to the Extension Terminal, even
488+
// though in the past it looked like we did. The implementation was
489+
// half-baked and left things unusable.
490+
if (config.processId === "current" || config.processId === await this.sessionManager.getLanguageServerPid()) {
491+
// TODO: When (if ever) it's supported, we need to convert 0 and the
492+
// old notion of "current" to the actual process ID, like this:
493+
// config.processId = await this.sessionManager.getLanguageServerPid();
494+
void this.logger.writeAndShowError("Attaching to the PowerShell Extension terminal is not supported. Please use the 'PowerShell: Interactive Session' debug configuration instead.");
495+
return PREVENT_DEBUG_START_AND_OPEN_DEBUGCONFIG;
496+
}
497+
483498
if (!config.runspaceId && !config.runspaceName) {
484499
config.runspaceId = await commands.executeCommand("PowerShell.PickRunspace", config.processId);
485500
// No runspace selected. Cancel attach.
@@ -528,12 +543,13 @@ export class SpecifyScriptArgsFeature implements Disposable {
528543
if (text !== undefined) {
529544
await this.context.workspaceState.update(powerShellDbgScriptArgsKey, text);
530545
}
546+
531547
return text;
532548
}
533549
}
534550

535551
interface IProcessItem extends QuickPickItem {
536-
pid: string; // payload for the QuickPick UI
552+
processId: number; // payload for the QuickPick UI
537553
}
538554

539555
// eslint-disable-next-line @typescript-eslint/no-empty-interface
@@ -542,7 +558,7 @@ interface IGetPSHostProcessesArguments {
542558

543559
interface IPSHostProcessInfo {
544560
processName: string;
545-
processId: string;
561+
processId: number;
546562
appDomainName: string;
547563
mainWindowTitle: string;
548564
}
@@ -551,19 +567,16 @@ export const GetPSHostProcessesRequestType =
551567
new RequestType<IGetPSHostProcessesArguments, IPSHostProcessInfo[], string>("powerShell/getPSHostProcesses");
552568

553569
export class PickPSHostProcessFeature extends LanguageClientConsumer {
554-
555570
private command: Disposable;
556571
private waitingForClientToken?: CancellationTokenSource;
557572
private getLanguageClientResolve?: (value: LanguageClient) => void;
558573

559574
constructor(private logger: ILogger) {
560575
super();
561576

562-
this.command =
563-
commands.registerCommand("PowerShell.PickPSHostProcess", () => {
564-
return this.getLanguageClient()
565-
.then((_) => this.pickPSHostProcess(), (_) => undefined);
566-
});
577+
this.command = commands.registerCommand("PowerShell.PickPSHostProcess", async () => {
578+
return this.pickPSHostProcess();
579+
});
567580
}
568581

569582
public override setLanguageClient(languageClient: LanguageClient): void {
@@ -617,25 +630,24 @@ export class PickPSHostProcessFeature extends LanguageClientConsumer {
617630
}
618631
}
619632

620-
private async pickPSHostProcess(): Promise<string | undefined> {
633+
private async pickPSHostProcess(): Promise<number | undefined> {
634+
// We need the language client in order to send the request.
635+
await this.getLanguageClient();
636+
621637
// 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-
}];
638+
const items: IProcessItem[] = [];
627639

628640
const response = await this.languageClient?.sendRequest(GetPSHostProcessesRequestType, {});
629641
for (const process of response ?? []) {
630642
let windowTitle = "";
631643
if (process.mainWindowTitle) {
632-
windowTitle = `, Title: ${process.mainWindowTitle}`;
644+
windowTitle = `, ${process.mainWindowTitle}`;
633645
}
634646

635647
items.push({
636648
label: process.processName,
637649
description: `PID: ${process.processId.toString()}${windowTitle}`,
638-
pid: process.processId,
650+
processId: process.processId,
639651
});
640652
}
641653

@@ -648,9 +660,10 @@ export class PickPSHostProcessFeature extends LanguageClientConsumer {
648660
matchOnDescription: true,
649661
matchOnDetail: true,
650662
};
663+
651664
const item = await window.showQuickPick(items, options);
652665

653-
return item ? item.pid : undefined;
666+
return item?.processId ?? undefined;
654667
}
655668

656669
private clearWaitingToken(): void {
@@ -660,7 +673,7 @@ export class PickPSHostProcessFeature extends LanguageClientConsumer {
660673
}
661674

662675
interface IRunspaceItem extends QuickPickItem {
663-
id: string; // payload for the QuickPick UI
676+
id: number; // payload for the QuickPick UI
664677
}
665678

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

679692
export class PickRunspaceFeature extends LanguageClientConsumer {
680-
681693
private command: Disposable;
682694
private waitingForClientToken?: CancellationTokenSource;
683695
private getLanguageClientResolve?: (value: LanguageClient) => void;
684696

685697
constructor(private logger: ILogger) {
686698
super();
687-
this.command =
688-
commands.registerCommand("PowerShell.PickRunspace", (processId) => {
689-
return this.getLanguageClient()
690-
.then((_) => this.pickRunspace(processId), (_) => undefined);
691-
}, this);
699+
700+
this.command = commands.registerCommand("PowerShell.PickRunspace",
701+
async (processId) => { return this.pickRunspace(processId); },
702+
this);
692703
}
693704

694705
public override setLanguageClient(languageClient: LanguageClient): void {
@@ -734,28 +745,26 @@ export class PickRunspaceFeature extends LanguageClientConsumer {
734745
this.clearWaitingToken();
735746
reject();
736747

737-
void this.logger.writeAndShowError("Attach to PowerShell host process: PowerShell session took too long to start.");
748+
void this.logger.writeAndShowError(
749+
"Attach to PowerShell host process: PowerShell session took too long to start.");
738750
}
739751
}, 60000);
740752
},
741753
);
742754
}
743755
}
744756

745-
private async pickRunspace(processId: string): Promise<string | undefined> {
757+
private async pickRunspace(processId: number): Promise<number | undefined> {
758+
// We need the language client in order to send the request.
759+
await this.getLanguageClient();
760+
746761
const response = await this.languageClient?.sendRequest(GetRunspaceRequestType, { processId });
747762
const items: IRunspaceItem[] = [];
748763
for (const runspace of response ?? []) {
749-
// Skip default runspace
750-
if ((runspace.id === 1 || runspace.name === "PSAttachRunspace")
751-
&& processId === "current") {
752-
continue;
753-
}
754-
755764
items.push({
756765
label: runspace.name,
757766
description: `ID: ${runspace.id} - ${runspace.availability}`,
758-
id: runspace.id.toString(),
767+
id: runspace.id,
759768
});
760769
}
761770

@@ -764,9 +773,10 @@ export class PickRunspaceFeature extends LanguageClientConsumer {
764773
matchOnDescription: true,
765774
matchOnDetail: true,
766775
};
776+
767777
const item = await window.showQuickPick(items, options);
768778

769-
return item ? item.id : undefined;
779+
return item?.id ?? undefined;
770780
}
771781

772782
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)