Skip to content

Commit

Permalink
Fix up debugger attach handlers
Browse files Browse the repository at this point in the history 
First off, the error messages were never actually displayed to the user
because the RpcErrorException constructor takes three arguments.
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).
  • Loading branch information
@andyleejordan
andyleejordan committed Jan 16, 2024
1 parent 4c39342 commit 12adcc0
Show file tree
Hide file tree
Showing 6 changed files with 179 additions and 179 deletions.
2 changes: 1 addition & 1 deletion src/PowerShellEditorServices/Services/DebugAdapter/Handlers/DebuggerActionHandlers.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public override Task<PauseResponse> Handle(PauseArguments request, CancellationT
}
catch (NotSupportedException e)
{
throw new RpcErrorException(0, e.Message);
throw new RpcErrorException(0, e, e.Message);
}
}
}
Expand Down
231 changes: 113 additions & 118 deletions src/PowerShellEditorServices/Services/DebugAdapter/Handlers/LaunchAndAttachHandler.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,17 +76,18 @@ internal record PsesAttachRequestArguments : AttachRequestArguments
{
public string ComputerName { get; set; }

public string ProcessId { get; set; }
public int ProcessId { get; set; }

public string RunspaceId { get; set; }
public int RunspaceId { get; set; }

public string RunspaceName { get; set; }

public string CustomPipeName { get; set; }
}

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

// If the current session is remote, map the script path to the remote
Expand Down Expand Up @@ -239,16 +240,12 @@ private async Task<AttachResponse> HandleImpl(PsesAttachRequestArguments request
{
// The debugger has officially started. We use this to later check if we should stop it.
((PsesInternalHost)_executionService).DebugContext.IsActive = true;

_debugStateService.IsAttachSession = true;

_debugEventHandlerService.RegisterEventHandlers();

bool processIdIsSet = !string.IsNullOrEmpty(request.ProcessId) && request.ProcessId != "undefined";
bool processIdIsSet = request.ProcessId != 0;
bool customPipeNameIsSet = !string.IsNullOrEmpty(request.CustomPipeName) && request.CustomPipeName != "undefined";

PowerShellVersionDetails runspaceVersion = _runspaceContext.CurrentRunspace.PowerShellVersionDetails;

// If there are no host processes to attach to or the user cancels selection, we get a null for the process id.
// This is not an error, just a request to stop the original "attach to" request.
// Testing against "undefined" is a HACK because I don't know how to make "Cancel" on quick pick loading
Expand All @@ -258,40 +255,12 @@ private async Task<AttachResponse> HandleImpl(PsesAttachRequestArguments request
_logger.LogInformation(
$"Attach request aborted, received {request.ProcessId} for processId.");

throw new RpcErrorException(0, "User aborted attach to PowerShell host process.");
throw new RpcErrorException(0, null, "User aborted attach to PowerShell host process.");
}

if (request.ComputerName != null)
if (!string.IsNullOrEmpty(request.ComputerName))
{
if (runspaceVersion.Version.Major < 4)
{
throw new RpcErrorException(0, $"Remote sessions are only available with PowerShell 4 and higher (current session is {runspaceVersion.Version}).");
}
else if (_runspaceContext.CurrentRunspace.RunspaceOrigin != RunspaceOrigin.Local)
{
throw new RpcErrorException(0, "Cannot attach to a process in a remote session when already in a remote session.");
}

PSCommand enterPSSessionCommand = new PSCommand()
.AddCommand("Enter-PSSession")
.AddParameter("ComputerName", request.ComputerName);

try
{
await _executionService.ExecutePSCommandAsync(
enterPSSessionCommand,
cancellationToken,
PowerShellExecutionOptions.ImmediateInteractive)
.ConfigureAwait(false);
}
catch (Exception e)
{
string msg = $"Could not establish remote session to computer '{request.ComputerName}'";
_logger.LogError(e, msg);
throw new RpcErrorException(0, msg);
}

_debugStateService.IsRemoteAttach = true;
await AttachToComputer(request.ComputerName, cancellationToken).ConfigureAwait(false);
}

// Set up a temporary runspace changed event handler so we can ensure
Expand All @@ -305,79 +274,38 @@ void RunspaceChangedHandler(object s, RunspaceChangedEventArgs _)
runspaceChanged.TrySetResult(true);
}

_executionService.RunspaceChanged += RunspaceChangedHandler;

if (processIdIsSet && int.TryParse(request.ProcessId, out int processId) && (processId > 0))
if (processIdIsSet)
{
if (runspaceVersion.Version.Major < 5)
// TODO: Implement support for breakpoints in runspaces in the extension terminal.
if (request.ProcessId == currentProcessId)
{
throw new RpcErrorException(0, $"Attaching to a process is only available with PowerShell 5 and higher (current session is {runspaceVersion.Version}).");
throw new RpcErrorException(0, null, $"Attaching to the Extension Terminal is not supported!");
}

PSCommand enterPSHostProcessCommand = new PSCommand()
.AddCommand("Enter-PSHostProcess")
.AddParameter("Id", processId);

try
{
await _executionService.ExecutePSCommandAsync(
enterPSHostProcessCommand,
cancellationToken,
PowerShellExecutionOptions.ImmediateInteractive)
.ConfigureAwait(false);
}
catch (Exception e)
{
string msg = $"Could not attach to process with Id: '{request.ProcessId}'";
_logger.LogError(e, msg);
throw new RpcErrorException(0, msg);
}
_executionService.RunspaceChanged += RunspaceChangedHandler;
await AttachToProcess(request.ProcessId, cancellationToken).ConfigureAwait(false);
await runspaceChanged.Task.ConfigureAwait(false);
}
else if (customPipeNameIsSet)
{
if (runspaceVersion.Version < s_minVersionForCustomPipeName)
{
throw new RpcErrorException(0, $"Attaching to a process with CustomPipeName is only available with PowerShell 6.2 and higher (current session is {runspaceVersion.Version}).");
}

PSCommand enterPSHostProcessCommand = new PSCommand()
.AddCommand("Enter-PSHostProcess")
.AddParameter("CustomPipeName", request.CustomPipeName);

try
{
await _executionService.ExecutePSCommandAsync(
enterPSHostProcessCommand,
cancellationToken,
PowerShellExecutionOptions.ImmediateInteractive)
.ConfigureAwait(false);
}
catch (Exception e)
{
string msg = $"Could not attach to process with CustomPipeName: '{request.CustomPipeName}'";
_logger.LogError(e, msg);
throw new RpcErrorException(0, msg);
}
_executionService.RunspaceChanged += RunspaceChangedHandler;
await AttachToPipe(request.CustomPipeName, cancellationToken).ConfigureAwait(false);
await runspaceChanged.Task.ConfigureAwait(false);
}
else if (request.ProcessId != "current")
else
{
_logger.LogError(
$"Attach request failed, '{request.ProcessId}' is an invalid value for the processId.");

throw new RpcErrorException(0, "A positive integer must be specified for the processId field.");
throw new RpcErrorException(0, null, "Invalid configuration with no process ID nor custom pipe name!");
}

await runspaceChanged.Task.ConfigureAwait(false);

// Execute the Debug-Runspace command but don't await it because it
// will block the debug adapter initialization process. The
// will block the debug adapter initialization process. The
// InitializedEvent will be sent as soon as the RunspaceChanged
// event gets fired with the attached runspace.

PSCommand debugRunspaceCmd = new PSCommand().AddCommand("Debug-Runspace");
if (request.RunspaceName != null)
if (!string.IsNullOrEmpty(request.RunspaceName))
{
PSCommand getRunspaceIdCommand = new PSCommand()
PSCommand psCommand = new PSCommand()
.AddCommand(@"Microsoft.PowerShell.Utility\Get-Runspace")
.AddParameter("Name", request.RunspaceName)
.AddCommand(@"Microsoft.PowerShell.Utility\Select-Object")
Expand All @@ -386,7 +314,7 @@ await _executionService.ExecutePSCommandAsync(
try
{
IEnumerable<int?> ids = await _executionService.ExecutePSCommandAsync<int?>(
getRunspaceIdCommand,
psCommand,
cancellationToken)
.ConfigureAwait(false);

Expand All @@ -395,38 +323,27 @@ await _executionService.ExecutePSCommandAsync(
_debugStateService.RunspaceId = id;
break;

// TODO: If we don't end up setting this, we should throw
// TODO: If we don't end up setting this, we should throw!
}
}
catch (Exception getRunspaceException)
catch (Exception e)
{
_logger.LogError(
getRunspaceException,
e,
"Unable to determine runspace to attach to. Message: {message}",
getRunspaceException.Message);
e.Message);
}

// TODO: We have the ID, why not just use that?
debugRunspaceCmd.AddParameter("Name", request.RunspaceName);
}
else if (request.RunspaceId != null)
else if (request.RunspaceId > 0)
{
if (!int.TryParse(request.RunspaceId, out int runspaceId) || runspaceId <= 0)
{
_logger.LogError(
$"Attach request failed, '{request.RunspaceId}' is an invalid value for the processId.");

throw new RpcErrorException(0, "A positive integer must be specified for the RunspaceId field.");
}

_debugStateService.RunspaceId = runspaceId;

debugRunspaceCmd.AddParameter("Id", runspaceId);
_debugStateService.RunspaceId = request.RunspaceId;
debugRunspaceCmd.AddParameter("Id", request.RunspaceId);
}
else
{
_debugStateService.RunspaceId = 1;

debugRunspaceCmd.AddParameter("Id", 1);
}

Expand All @@ -438,11 +355,89 @@ await _executionService.ExecutePSCommandAsync(
.ExecutePSCommandAsync(debugRunspaceCmd, CancellationToken.None, PowerShellExecutionOptions.ImmediateInteractive)
.ContinueWith(OnExecutionCompletedAsync, TaskScheduler.Default);

if (runspaceVersion.Version.Major >= 7)
_debugStateService.ServerStarted.TrySetResult(true);

return new AttachResponse();
}

private async Task AttachToComputer(string computerName, CancellationToken cancellationToken)
{
_debugStateService.IsRemoteAttach = true;

if (_runspaceContext.CurrentRunspace.RunspaceOrigin != RunspaceOrigin.Local)
{
_debugStateService.ServerStarted.TrySetResult(true);
throw new RpcErrorException(0, null, "Cannot attach to a process in a remote session when already in a remote session.");
}

PSCommand psCommand = new PSCommand()
.AddCommand("Enter-PSSession")
.AddParameter("ComputerName", computerName);

try
{
await _executionService.ExecutePSCommandAsync(
psCommand,
cancellationToken,
PowerShellExecutionOptions.ImmediateInteractive)
.ConfigureAwait(false);
}
catch (Exception e)
{
string msg = $"Could not establish remote session to computer {computerName}";
_logger.LogError(e, msg);
throw new RpcErrorException(0, e, msg);
}
}

private async Task AttachToProcess(int processId, CancellationToken cancellationToken)
{
PSCommand enterPSHostProcessCommand = new PSCommand()
.AddCommand(@"Microsoft.PowerShell.Core\Enter-PSHostProcess")
.AddParameter("Id", processId);

try
{
await _executionService.ExecutePSCommandAsync(
enterPSHostProcessCommand,
cancellationToken,
PowerShellExecutionOptions.ImmediateInteractive)
.ConfigureAwait(false);
}
catch (Exception e)
{
string msg = $"Could not attach to process with ID: {processId}";
_logger.LogError(e, msg);
throw new RpcErrorException(0, e, msg);
}
}

private async Task AttachToPipe(string pipeName, CancellationToken cancellationToken)
{
PowerShellVersionDetails runspaceVersion = _runspaceContext.CurrentRunspace.PowerShellVersionDetails;

if (runspaceVersion.Version < s_minVersionForCustomPipeName)
{
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}).");
}

PSCommand enterPSHostProcessCommand = new PSCommand()
.AddCommand(@"Microsoft.PowerShell.Core\Enter-PSHostProcess")
.AddParameter("CustomPipeName", pipeName);

try
{
await _executionService.ExecutePSCommandAsync(
enterPSHostProcessCommand,
cancellationToken,
PowerShellExecutionOptions.ImmediateInteractive)
.ConfigureAwait(false);
}
catch (Exception e)
{
string msg = $"Could not attach to process with CustomPipeName: {pipeName}";
_logger.LogError(e, msg);
throw new RpcErrorException(0, e, msg);
}
return new AttachResponse();
}

// PSES follows the following flow:
Expand Down
14 changes: 7 additions & 7 deletions src/PowerShellEditorServices/Services/DebugAdapter/Handlers/SetVariableHandler.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,20 +38,20 @@ await _debugService.SetVariableAsync(

return new SetVariableResponse { Value = updatedValue };
}
catch (Exception ex) when (ex is ArgumentTransformationMetadataException or
catch (Exception e) when (e is ArgumentTransformationMetadataException or
InvalidPowerShellExpressionException or
SessionStateUnauthorizedAccessException)
{
// Catch common, innocuous errors caused by the user supplying a value that can't be converted or the variable is not settable.
_logger.LogTrace($"Failed to set variable: {ex.Message}");
throw new RpcErrorException(0, ex.Message);
_logger.LogTrace($"Failed to set variable: {e.Message}");
throw new RpcErrorException(0, e, e.Message);
}
catch (Exception ex)
catch (Exception e)
{
_logger.LogError($"Unexpected error setting variable: {ex.Message}");
_logger.LogError($"Unexpected error setting variable: {e.Message}");
string msg =
$"Unexpected error: {ex.GetType().Name} - {ex.Message} Please report this error to the PowerShellEditorServices project on GitHub.";
throw new RpcErrorException(0, msg);
$"Unexpected error: {e.GetType().Name} - {e.Message} Please report this error to the PowerShellEditorServices project on GitHub.";
throw new RpcErrorException(0, e, msg);
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/PowerShellEditorServices/Services/PowerShell/Handlers/IGetRunspaceHandler.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ internal interface IGetRunspaceHandler : IJsonRpcRequestHandler<GetRunspaceParam

internal class GetRunspaceParams : IRequest<RunspaceResponse[]>
{
public string ProcessId { get; set; }
public int ProcessId { get; set; }
}

internal class RunspaceResponse
Expand Down
Loading

0 comments on commit 12adcc0

Please sign in to comment.