Skip to content

Commit 2968e35

Browse files
committed
Fix up debugger attach handlers
First off, the error messages were never actually displayed to the user because the RpcErrorException constructor takes three arguments. Now the second argument is always (correctly but annoyingly) null. Secondly, we do not support attaching to PowerShell Editor Services. It sure looked like we did (because we had special logic for it) but once attached, nothing worked. So it was half-baked. Now we throw an error if the user is trying to do that. Thirdly, because of that half-baked implementation, the process ID field was typed as a string (to support "current" as a shortcut) but that caused a mess here and an error in the VS Code client. Now it's just always an integer. (Same for the runspace ID.) Fourthly, a big mess was cleaned up by refactoring using functions, who'd have thought? Fifth and finally, superfluous version checking around PowerShell <5.1 was removed (as those versions are no longer supported whatsoever).
1 parent 4c39342 commit 2968e35

File tree

6 files changed

+180
-205
lines changed

6 files changed

+180
-205
lines changed

src/PowerShellEditorServices/Services/DebugAdapter/Handlers/DebuggerActionHandlers.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ public override Task<PauseResponse> Handle(PauseArguments request, CancellationT
5151
}
5252
catch (NotSupportedException e)
5353
{
54-
throw new RpcErrorException(0, e.Message);
54+
throw new RpcErrorException(0, null, e.Message);
5555
}
5656
}
5757
}

src/PowerShellEditorServices/Services/DebugAdapter/Handlers/LaunchAndAttachHandler.cs

+115-138
Original file line numberDiff line numberDiff line change
@@ -76,9 +76,9 @@ internal record PsesAttachRequestArguments : AttachRequestArguments
7676
{
7777
public string ComputerName { get; set; }
7878

79-
public string ProcessId { get; set; }
79+
public int ProcessId { get; set; }
8080

81-
public string RunspaceId { get; set; }
81+
public int RunspaceId { get; set; }
8282

8383
public string RunspaceName { get; set; }
8484

@@ -87,6 +87,7 @@ internal record PsesAttachRequestArguments : AttachRequestArguments
8787

8888
internal class LaunchAndAttachHandler : ILaunchHandler<PsesLaunchRequestArguments>, IAttachHandler<PsesAttachRequestArguments>, IOnDebugAdapterServerStarted
8989
{
90+
private static readonly int currentProcessId = System.Diagnostics.Process.GetCurrentProcess().Id;
9091
private static readonly Version s_minVersionForCustomPipeName = new(6, 2);
9192
private readonly ILogger<LaunchAndAttachHandler> _logger;
9293
private readonly BreakpointService _breakpointService;
@@ -190,7 +191,7 @@ public async Task<LaunchResponse> Handle(PsesLaunchRequestArguments request, Can
190191
&& !string.IsNullOrEmpty(request.Script)
191192
&& ScriptFile.IsUntitledPath(request.Script))
192193
{
193-
throw new RpcErrorException(0, "Running an Untitled file in a temporary Extension Terminal is currently not supported.");
194+
throw new RpcErrorException(0, null, "Running an Untitled file in a temporary Extension Terminal is currently not supported!");
194195
}
195196

196197
// If the current session is remote, map the script path to the remote
@@ -239,59 +240,26 @@ private async Task<AttachResponse> HandleImpl(PsesAttachRequestArguments request
239240
{
240241
// The debugger has officially started. We use this to later check if we should stop it.
241242
((PsesInternalHost)_executionService).DebugContext.IsActive = true;
242-
243243
_debugStateService.IsAttachSession = true;
244-
245244
_debugEventHandlerService.RegisterEventHandlers();
246245

247-
bool processIdIsSet = !string.IsNullOrEmpty(request.ProcessId) && request.ProcessId != "undefined";
246+
bool processIdIsSet = request.ProcessId != 0;
248247
bool customPipeNameIsSet = !string.IsNullOrEmpty(request.CustomPipeName) && request.CustomPipeName != "undefined";
249248

250-
PowerShellVersionDetails runspaceVersion = _runspaceContext.CurrentRunspace.PowerShellVersionDetails;
251-
252249
// If there are no host processes to attach to or the user cancels selection, we get a null for the process id.
253250
// This is not an error, just a request to stop the original "attach to" request.
254251
// Testing against "undefined" is a HACK because I don't know how to make "Cancel" on quick pick loading
255252
// to cancel on the VSCode side without sending an attachRequest with processId set to "undefined".
256253
if (!processIdIsSet && !customPipeNameIsSet)
257254
{
258-
_logger.LogInformation(
259-
$"Attach request aborted, received {request.ProcessId} for processId.");
260-
261-
throw new RpcErrorException(0, "User aborted attach to PowerShell host process.");
255+
string msg = $"User aborted attach to PowerShell host process: {request.ProcessId}";
256+
_logger.LogTrace(msg);
257+
throw new RpcErrorException(0, null, msg);
262258
}
263259

264-
if (request.ComputerName != null)
260+
if (!string.IsNullOrEmpty(request.ComputerName))
265261
{
266-
if (runspaceVersion.Version.Major < 4)
267-
{
268-
throw new RpcErrorException(0, $"Remote sessions are only available with PowerShell 4 and higher (current session is {runspaceVersion.Version}).");
269-
}
270-
else if (_runspaceContext.CurrentRunspace.RunspaceOrigin != RunspaceOrigin.Local)
271-
{
272-
throw new RpcErrorException(0, "Cannot attach to a process in a remote session when already in a remote session.");
273-
}
274-
275-
PSCommand enterPSSessionCommand = new PSCommand()
276-
.AddCommand("Enter-PSSession")
277-
.AddParameter("ComputerName", request.ComputerName);
278-
279-
try
280-
{
281-
await _executionService.ExecutePSCommandAsync(
282-
enterPSSessionCommand,
283-
cancellationToken,
284-
PowerShellExecutionOptions.ImmediateInteractive)
285-
.ConfigureAwait(false);
286-
}
287-
catch (Exception e)
288-
{
289-
string msg = $"Could not establish remote session to computer '{request.ComputerName}'";
290-
_logger.LogError(e, msg);
291-
throw new RpcErrorException(0, msg);
292-
}
293-
294-
_debugStateService.IsRemoteAttach = true;
262+
await AttachToComputer(request.ComputerName, cancellationToken).ConfigureAwait(false);
295263
}
296264

297265
// Set up a temporary runspace changed event handler so we can ensure
@@ -305,131 +273,62 @@ void RunspaceChangedHandler(object s, RunspaceChangedEventArgs _)
305273
runspaceChanged.TrySetResult(true);
306274
}
307275

308-
_executionService.RunspaceChanged += RunspaceChangedHandler;
309-
310-
if (processIdIsSet && int.TryParse(request.ProcessId, out int processId) && (processId > 0))
276+
if (processIdIsSet)
311277
{
312-
if (runspaceVersion.Version.Major < 5)
278+
if (request.ProcessId == currentProcessId)
313279
{
314-
throw new RpcErrorException(0, $"Attaching to a process is only available with PowerShell 5 and higher (current session is {runspaceVersion.Version}).");
280+
throw new RpcErrorException(0, null, $"Attaching to the Extension Terminal is not supported!");
315281
}
316282

317-
PSCommand enterPSHostProcessCommand = new PSCommand()
318-
.AddCommand("Enter-PSHostProcess")
319-
.AddParameter("Id", processId);
320-
321-
try
322-
{
323-
await _executionService.ExecutePSCommandAsync(
324-
enterPSHostProcessCommand,
325-
cancellationToken,
326-
PowerShellExecutionOptions.ImmediateInteractive)
327-
.ConfigureAwait(false);
328-
}
329-
catch (Exception e)
330-
{
331-
string msg = $"Could not attach to process with Id: '{request.ProcessId}'";
332-
_logger.LogError(e, msg);
333-
throw new RpcErrorException(0, msg);
334-
}
283+
_executionService.RunspaceChanged += RunspaceChangedHandler;
284+
await AttachToProcess(request.ProcessId, cancellationToken).ConfigureAwait(false);
285+
await runspaceChanged.Task.ConfigureAwait(false);
335286
}
336287
else if (customPipeNameIsSet)
337288
{
338-
if (runspaceVersion.Version < s_minVersionForCustomPipeName)
339-
{
340-
throw new RpcErrorException(0, $"Attaching to a process with CustomPipeName is only available with PowerShell 6.2 and higher (current session is {runspaceVersion.Version}).");
341-
}
342-
343-
PSCommand enterPSHostProcessCommand = new PSCommand()
344-
.AddCommand("Enter-PSHostProcess")
345-
.AddParameter("CustomPipeName", request.CustomPipeName);
346-
347-
try
348-
{
349-
await _executionService.ExecutePSCommandAsync(
350-
enterPSHostProcessCommand,
351-
cancellationToken,
352-
PowerShellExecutionOptions.ImmediateInteractive)
353-
.ConfigureAwait(false);
354-
}
355-
catch (Exception e)
356-
{
357-
string msg = $"Could not attach to process with CustomPipeName: '{request.CustomPipeName}'";
358-
_logger.LogError(e, msg);
359-
throw new RpcErrorException(0, msg);
360-
}
289+
_executionService.RunspaceChanged += RunspaceChangedHandler;
290+
await AttachToPipe(request.CustomPipeName, cancellationToken).ConfigureAwait(false);
291+
await runspaceChanged.Task.ConfigureAwait(false);
361292
}
362-
else if (request.ProcessId != "current")
293+
else
363294
{
364-
_logger.LogError(
365-
$"Attach request failed, '{request.ProcessId}' is an invalid value for the processId.");
366-
367-
throw new RpcErrorException(0, "A positive integer must be specified for the processId field.");
295+
throw new RpcErrorException(0, null, "Invalid configuration with no process ID nor custom pipe name!");
368296
}
369297

370-
await runspaceChanged.Task.ConfigureAwait(false);
371298

372299
// Execute the Debug-Runspace command but don't await it because it
373-
// will block the debug adapter initialization process. The
300+
// will block the debug adapter initialization process. The
374301
// InitializedEvent will be sent as soon as the RunspaceChanged
375302
// event gets fired with the attached runspace.
376-
377303
PSCommand debugRunspaceCmd = new PSCommand().AddCommand("Debug-Runspace");
378-
if (request.RunspaceName != null)
304+
if (!string.IsNullOrEmpty(request.RunspaceName))
379305
{
380-
PSCommand getRunspaceIdCommand = new PSCommand()
306+
PSCommand psCommand = new PSCommand()
381307
.AddCommand(@"Microsoft.PowerShell.Utility\Get-Runspace")
382308
.AddParameter("Name", request.RunspaceName)
383309
.AddCommand(@"Microsoft.PowerShell.Utility\Select-Object")
384310
.AddParameter("ExpandProperty", "Id");
385311

386-
try
387-
{
388-
IEnumerable<int?> ids = await _executionService.ExecutePSCommandAsync<int?>(
389-
getRunspaceIdCommand,
390-
cancellationToken)
391-
.ConfigureAwait(false);
392-
393-
foreach (int? id in ids)
394-
{
395-
_debugStateService.RunspaceId = id;
396-
break;
312+
IReadOnlyList<int> results = await _executionService.ExecutePSCommandAsync<int>(psCommand, cancellationToken).ConfigureAwait(false);
397313

398-
// TODO: If we don't end up setting this, we should throw
399-
}
400-
}
401-
catch (Exception getRunspaceException)
314+
if (results.Count == 0)
402315
{
403-
_logger.LogError(
404-
getRunspaceException,
405-
"Unable to determine runspace to attach to. Message: {message}",
406-
getRunspaceException.Message);
316+
throw new RpcErrorException(0, null, $"Could not find ID of runspace: {request.RunspaceName}");
407317
}
408318

409-
// TODO: We have the ID, why not just use that?
410-
debugRunspaceCmd.AddParameter("Name", request.RunspaceName);
319+
// Translate the runspace name to the runspace ID.
320+
request.RunspaceId = results[0];
411321
}
412-
else if (request.RunspaceId != null)
413-
{
414-
if (!int.TryParse(request.RunspaceId, out int runspaceId) || runspaceId <= 0)
415-
{
416-
_logger.LogError(
417-
$"Attach request failed, '{request.RunspaceId}' is an invalid value for the processId.");
418-
419-
throw new RpcErrorException(0, "A positive integer must be specified for the RunspaceId field.");
420-
}
421-
422-
_debugStateService.RunspaceId = runspaceId;
423322

424-
debugRunspaceCmd.AddParameter("Id", runspaceId);
425-
}
426-
else
323+
if (request.RunspaceId < 1)
427324
{
428-
_debugStateService.RunspaceId = 1;
429325

430-
debugRunspaceCmd.AddParameter("Id", 1);
326+
throw new RpcErrorException(0, null, "A positive integer must be specified for the RunspaceId!");
431327
}
432328

329+
_debugStateService.RunspaceId = request.RunspaceId;
330+
debugRunspaceCmd.AddParameter("Id", request.RunspaceId);
331+
433332
// Clear any existing breakpoints before proceeding
434333
await _breakpointService.RemoveAllBreakpointsAsync().ConfigureAwait(continueOnCapturedContext: false);
435334

@@ -438,11 +337,89 @@ await _executionService.ExecutePSCommandAsync(
438337
.ExecutePSCommandAsync(debugRunspaceCmd, CancellationToken.None, PowerShellExecutionOptions.ImmediateInteractive)
439338
.ContinueWith(OnExecutionCompletedAsync, TaskScheduler.Default);
440339

441-
if (runspaceVersion.Version.Major >= 7)
340+
_debugStateService.ServerStarted.TrySetResult(true);
341+
342+
return new AttachResponse();
343+
}
344+
345+
private async Task AttachToComputer(string computerName, CancellationToken cancellationToken)
346+
{
347+
_debugStateService.IsRemoteAttach = true;
348+
349+
if (_runspaceContext.CurrentRunspace.RunspaceOrigin != RunspaceOrigin.Local)
442350
{
443-
_debugStateService.ServerStarted.TrySetResult(true);
351+
throw new RpcErrorException(0, null, "Cannot attach to a process in a remote session when already in a remote session!");
352+
}
353+
354+
PSCommand psCommand = new PSCommand()
355+
.AddCommand("Enter-PSSession")
356+
.AddParameter("ComputerName", computerName);
357+
358+
try
359+
{
360+
await _executionService.ExecutePSCommandAsync(
361+
psCommand,
362+
cancellationToken,
363+
PowerShellExecutionOptions.ImmediateInteractive)
364+
.ConfigureAwait(false);
365+
}
366+
catch (Exception e)
367+
{
368+
string msg = $"Could not establish remote session to computer: {computerName}";
369+
_logger.LogError(e, msg);
370+
throw new RpcErrorException(0, null, msg);
371+
}
372+
}
373+
374+
private async Task AttachToProcess(int processId, CancellationToken cancellationToken)
375+
{
376+
PSCommand enterPSHostProcessCommand = new PSCommand()
377+
.AddCommand(@"Microsoft.PowerShell.Core\Enter-PSHostProcess")
378+
.AddParameter("Id", processId);
379+
380+
try
381+
{
382+
await _executionService.ExecutePSCommandAsync(
383+
enterPSHostProcessCommand,
384+
cancellationToken,
385+
PowerShellExecutionOptions.ImmediateInteractive)
386+
.ConfigureAwait(false);
387+
}
388+
catch (Exception e)
389+
{
390+
string msg = $"Could not attach to process with ID: {processId}";
391+
_logger.LogError(e, msg);
392+
throw new RpcErrorException(0, null, msg);
393+
}
394+
}
395+
396+
private async Task AttachToPipe(string pipeName, CancellationToken cancellationToken)
397+
{
398+
PowerShellVersionDetails runspaceVersion = _runspaceContext.CurrentRunspace.PowerShellVersionDetails;
399+
400+
if (runspaceVersion.Version < s_minVersionForCustomPipeName)
401+
{
402+
throw new RpcErrorException(0, null, $"Attaching to a process with CustomPipeName is only available with PowerShell 6.2 and higher. Current session is: {runspaceVersion.Version}");
403+
}
404+
405+
PSCommand enterPSHostProcessCommand = new PSCommand()
406+
.AddCommand(@"Microsoft.PowerShell.Core\Enter-PSHostProcess")
407+
.AddParameter("CustomPipeName", pipeName);
408+
409+
try
410+
{
411+
await _executionService.ExecutePSCommandAsync(
412+
enterPSHostProcessCommand,
413+
cancellationToken,
414+
PowerShellExecutionOptions.ImmediateInteractive)
415+
.ConfigureAwait(false);
416+
}
417+
catch (Exception e)
418+
{
419+
string msg = $"Could not attach to process with CustomPipeName: {pipeName}";
420+
_logger.LogError(e, msg);
421+
throw new RpcErrorException(0, null, msg);
444422
}
445-
return new AttachResponse();
446423
}
447424

448425
// PSES follows the following flow:

src/PowerShellEditorServices/Services/DebugAdapter/Handlers/SetVariableHandler.cs

+8-8
Original file line numberDiff line numberDiff line change
@@ -38,20 +38,20 @@ await _debugService.SetVariableAsync(
3838

3939
return new SetVariableResponse { Value = updatedValue };
4040
}
41-
catch (Exception ex) when (ex is ArgumentTransformationMetadataException or
41+
catch (Exception e) when (e is ArgumentTransformationMetadataException or
4242
InvalidPowerShellExpressionException or
4343
SessionStateUnauthorizedAccessException)
4444
{
4545
// Catch common, innocuous errors caused by the user supplying a value that can't be converted or the variable is not settable.
46-
_logger.LogTrace($"Failed to set variable: {ex.Message}");
47-
throw new RpcErrorException(0, ex.Message);
46+
string msg = $"Failed to set variable: {e.Message}";
47+
_logger.LogTrace(msg);
48+
throw new RpcErrorException(0, null, msg);
4849
}
49-
catch (Exception ex)
50+
catch (Exception e)
5051
{
51-
_logger.LogError($"Unexpected error setting variable: {ex.Message}");
52-
string msg =
53-
$"Unexpected error: {ex.GetType().Name} - {ex.Message} Please report this error to the PowerShellEditorServices project on GitHub.";
54-
throw new RpcErrorException(0, msg);
52+
string msg = $"Unexpected error setting variable: {e.Message}";
53+
_logger.LogError(msg);
54+
throw new RpcErrorException(0, null, msg);
5555
}
5656
}
5757
}

src/PowerShellEditorServices/Services/PowerShell/Handlers/IGetRunspaceHandler.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ internal interface IGetRunspaceHandler : IJsonRpcRequestHandler<GetRunspaceParam
1111

1212
internal class GetRunspaceParams : IRequest<RunspaceResponse[]>
1313
{
14-
public string ProcessId { get; set; }
14+
public int ProcessId { get; set; }
1515
}
1616

1717
internal class RunspaceResponse

0 commit comments

Comments
 (0)