GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — Implementation for Invoke-Command step-in remote debugging by PaulHigin · Pull Request #3015 · PowerShell/PowerShell

Conversation

Paul Higinbotham (PaulHigin) commented Jan 19, 2017

Copy link

Copy Markdown

Contributor

These changes provide the ability to debug remote running scripts started with the Invoke-Command cmdlet. The design is event based and provides new public events that allow subscribers to be notified when an Invoke-Command remote session is ready for debugging. Since Invoke-Command allows running scripts on multiple targets at once (fan-out) the notification event is raised for each remote session as it becomes ready for debugging. The subscriber to these events will be a script debugger implementation (such as PowerShell console, ISE, or VSCode) and will handle all debugging details such as simultaneously debugging multiple remote sessions at once in separate windows.

But these changes also include an internal implementation which is used by default if host debuggers don't want to handle the debugging details. This internal implementation is what PowerShell console, ISE uses so they can have this new behavior without having to modify their debugger implementations. The internal implementation serializes each remote session of Invoke-Command so that they can be debugged one at a time. The remote session debugger is "pushed" onto the internal debugger stack so that debugging transitions to the remote session. Existing debugging commands work so that the "quit" debugging command will stop the current remote session script from running and allow the next remote session to be debugged. Similarly the "continue" debugging command allows the script to continue running outside step mode and again go to the next remote session for debugging. The "stepout" debugging command steps out of all Invoke-Command remote sessions and lets the script continue to run for each remote session in parallel as they are normally run.

The purpose of Invoke-Command step-in remote debugging is allow seamless debugging of a local script that calls Invoke-Command on remote targets. But there is also a new Invoke-Command "-RemoteDebug" parameter that lets you Invoke-Command on the command line and have it drop directly into the debugger.

An example from the PowerShell command line looks like this:

PS C:\> C:\TestICM.ps1
Entering debug mode. Use h or ? for help.

Hit Command breakpoint on 'Invoke-Command'

At C:\TestICM.ps1:2 char:1
+ Invoke-Command -cn $computerName,paulhig-3 -File c:\LinuxScript.ps1
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[DBG]: PS C:\>> list

    1:  $computerName = "localhost"
    2:* Invoke-Command -cn $computerName,paulhig-3 -File c:\LinuxScript.ps1
    3:  "Test Complete!"

[DBG]: PS C:\>> stepin
At line:1 char:1
+ Write-Output "Running script on Linux!"
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[paulhig-3]:[DBG]: [Process:14072]: [Runspace5]: PS C:\Users\paulhi\Documents>

Notice that the debugger "stepin" command transitioned from local script debugging to debugging the remote session on computer "paulhig-3", as can be seen by the change in the debugger prompt.

You can also do this from the command line to drop directly into the debugger

Invoke-Command -cn localhost -Script  $scriptblock -RemoteDebug

These changes also remove an old behavior that was incompatible with this new step-in feature. Previously if a remote session running script hit a break point it would stop in the debugger and go to the "disconnected session" state. This was to allow the user to reconnect using Enter-PSSession and then interactively debug the remote session script. This behavior has been removed and now the user needs to attach a debugger using the newer Debug-Runspace cmdlet.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          Implementation for Invoke-Command step-in remote debugging

7ad5642

Minor updates to test.

Added It:Skip for Windows only test.

More CR changes

Fix for SSH remoting Ctrl+C operation

Fixed comment

Clean up PSObject base

Implementation for Invoke-Command step-in remote debugging

Remove GP cache changes

Added test

Fixed potential null reference error

Fixed ad hoc bug

Added debugger ready wait in debugger process loop

Stability improvements

Improve step out behavior

Added event raised when internal runsapce debug processing ends

Added consistent runspace clean up

Some clean up

Microsoft Contribution License Agreements (msftclas) added the cla-not-required label

Jan 19, 2017

PowerShellTeam added the Review - Needed The PR is being reviewed label

Jan 19, 2017

Ilya (iSazonov) reviewed

Jan 19, 2017

View reviewed changes

Comment thread

test/powershell/engine/Remoting/InvokeCommandRemoteDebug.Tests.ps1 Outdated

		return
		}

		$sb = [scriptblock]::Create(@'

Ilya (iSazonov) Jan 19, 2017

Copy link

Copy Markdown

Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The below code should be moved to else

if (!$IsWindows)  
        {  
            $originalDefaultParameterValues = $PSDefaultParameterValues.Clone()  
            $PSDefaultParameterValues["it:skip"] = $true  
} else {
    $sb = [scriptblock]::Create(@'
        "Hello!" 
'@)

Add-Type -TypeDefinition $typeDef -ReferencedAssemblies "System.Globalization","System.Management.Automation" 

...
}

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Ilya (iSazonov) Jan 20, 2017

Copy link

Copy Markdown

Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#Closed

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Ilya (iSazonov) reviewed

Jan 19, 2017

View reviewed changes

Comment thread

test/powershell/engine/Remoting/InvokeCommandRemoteDebug.Tests.ps1 Outdated

		if (!$IsWindows)
		{
		$global:PSDefaultParameterValues = $originalDefaultParameterValues
		return

Ilya (iSazonov) Jan 19, 2017

Copy link

Copy Markdown

Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Th same - 'return' should be removed and below code should be moved to else .

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Ilya (iSazonov) Jan 20, 2017

Copy link

Copy Markdown

Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#Closed

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Ilya (iSazonov) reviewed

Jan 19, 2017

View reviewed changes

Comment thread

test/powershell/engine/Remoting/InvokeCommandRemoteDebug.Tests.ps1 Outdated

		if ($remoteSession -ne $null) { Remove-PSSession $remoteSession -ErrorAction SilentlyContinue }
		}

		It "Verifies that asynchronous Invoke-Command -RemoteDebug is ignored" {

Ilya (iSazonov) Jan 19, 2017

Copy link

Copy Markdown

Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be better:

It "Verifies that asynchronous 'Invoke-Command -RemoteDebug' is ignored" {

It would be good to fix headers below similarly.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Ilya (iSazonov) Jan 20, 2017

Copy link

Copy Markdown

Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#Closed

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Ilya (iSazonov) reviewed

Jan 19, 2017

View reviewed changes

Comment thread

test/powershell/engine/Remoting/InvokeCommandRemoteDebug.Tests.ps1

		public override void StopProcessCommand() { }
		}
		}
		'@

Ilya (iSazonov) Jan 19, 2017 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we skip all test for non-Windows the above code should be moved in

if ($IsWindows) {
   # here
}

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Ilya (iSazonov) Jan 20, 2017

Copy link

Copy Markdown

Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#Closed

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Ilya (iSazonov) reviewed

Jan 19, 2017

View reviewed changes

Comment thread

test/powershell/engine/Remoting/InvokeCommandRemoteDebug.Tests.ps1

		##
		## PowerShell Invoke-Command -RemoteDebug Tests
		##

Ilya (iSazonov) Jan 19, 2017

Copy link

Copy Markdown

Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know whether this work on IoT and Windows Core Server? If not, then we need to change the 'skipping'.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Paul Higinbotham (PaulHigin) Jan 19, 2017

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test works on Windows Core Server. I don't know about IoT

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Mike Richmond (mirichmo) Mar 1, 2017

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can point you to an IoT VM (internally) so you can at least run a smoke test there. IoT and Nano Server are very similar environments, so I would expect it to work there as well.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Paul Higinbotham (PaulHigin) Mar 1, 2017

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't believe tests are currently run on IoT or Nano Server. So I feel this shouldn't block the PR merge.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Dongbo Wang (daxian-dbw) Mar 1, 2017

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment doesn't block the PR merge.
But I do think for features like this one, we should at least verify if it works on NanoServer, because it adds great value to customers and thus is very possible to be cherry-picked to the NanoServer in-box powershell.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

iSazonov reacted with thumbs up emoji

All reactions

👍 1 reaction

Ilya (iSazonov) reviewed

Jan 19, 2017

View reviewed changes

Comment thread

test/powershell/engine/Remoting/InvokeCommandRemoteDebug.Tests.ps1

		##
		## PowerShell Invoke-Command -RemoteDebug Tests
		##

Ilya (iSazonov) Jan 19, 2017

Copy link

Copy Markdown

Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know whether this work on IoT and Windows Core Server? If not, then we need to change the 'skipping'.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Paul Higinbotham (PaulHigin) Jan 19, 2017

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test works on Windows Core Server. I don't know about IoT

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Paul Higinbotham (PaulHigin) added 2 commits

January 19, 2017 09:15


          Minor test changes based on CR


          Added runspace event clean up to fix problems when reusing sessions

4396b0d

David Wilson (daviwil) mentioned this pull request

Jan 26, 2017

Debug: Should be able to Step-Into "Invoke-Command -Computername" #2697

Closed


          Merge branch 'master' of https://github.com/PaulHigin/PowerShell into…

… ICMRemoteDebug

Jason Shirk (lzybkr) reviewed

Jan 31, 2017

View reviewed changes

Comment thread

src/System.Management.Automation/engine/debugger/debugger.cs

		/// <summary>
		/// The runspace to process
		/// </summary>
		public Runspace Runspace

Jason Shirk (lzybkr) Jan 19, 2017

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

5 lines for a trivial property is too verbose - please use one line.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

src/System.Management.Automation/engine/debugger/debugger.cs Outdated

		/// When set to true this will cause PowerShell to handle this runspace debug session internally through its
		/// internal script debugger
		/// </summary>
		public bool HandleInternally

Jason Shirk (lzybkr) Jan 19, 2017

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this was an internal api, I wouldn't care about the name as much, but it's public, so can you think of a better term than Internally to prevent any ambiguity as to what Internally actually refers to?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Paul Higinbotham (PaulHigin) Feb 1, 2017

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed to "UseDefaultProcessing"

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

src/System.Management.Automation/engine/debugger/debugger.cs Outdated

		/// <summary>
		/// Event raised when a runspace debugger needs breakpoint processing
		/// </summary>
		public event EventHandler<ProcessRunspaceDebugEventArgs> ProcessRunspaceDebug;

Jason Shirk (lzybkr) Jan 19, 2017

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm finding Process confusing here - it's not the noun, but it seems like maybe RunspaceDebugProcessingStarted, RunspaceDebugProcessingCompleted, and RunspaceDebugProcessingCancelled might be clearer.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Paul Higinbotham (PaulHigin) Feb 1, 2017

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bit confusing. I'll change "ProcessRunspaceDebugEnd" to "RunspaceDebugProcessingCompleted" since that is much better.

The other two should probably be "StartRunspaceDebugProcessing" and "CancelRunspaceDebugProcessing".

The reason these two events exist is to give a host the chance to debug these remote sessions directly , rather than let PowerShell provide a default debug experience. For example VSCode may want to show multiple windows to debug multiple remote sessions simultaneously instead of using PowerShell default processing which debugs each session serially.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

src/System.Management.Automation/engine/debugger/debugger.cs Outdated

		/// <summary>
		/// Event raised when debugging session is over and runspace debuggers queued for processing should be released
		/// </summary>
		public event EventHandler<EventArgs> CancelProcessRunspaceDebug;

Jason Shirk (lzybkr) Jan 19, 2017

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment suggests this event is raised under normal processing but Cancel suggests termination that might not be normal.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

src/System.Management.Automation/engine/debugger/debugger.cs Outdated

		/// <param name="runspace">Runspace to debug</param>
		internal override void QueueRunspaceForDebug(Runspace runspace)
		{
		if (runspace == null) { throw new PSArgumentNullException("runspace"); }

Jason Shirk (lzybkr) Jan 19, 2017

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really need argument validation here? The method is internal, so it seems unnecessary - PSArgumentNullException or NullReferenceException are equally informative.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

test/powershell/engine/Remoting/InvokeCommandRemoteDebug.Tests.ps1

		public void PushRunspace(Runspace runspace) { }
		public void PopRunspace() { }
		public bool IsRunspacePushed { get { return false; } }
		public Runspace Runspace { get { return _runspace; } private set { _runspace = value; } }

Jason Shirk (lzybkr) Jan 19, 2017

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's just test code - but won't this return the wrong runspace if/when PushRunspace is used?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Paul Higinbotham (PaulHigin) Feb 1, 2017

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test host Push/Pop methods are not needed for the test. Only the Runspace property is needed to provide access to the host runspace.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Jason Shirk (lzybkr) Feb 1, 2017

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should avoid incorrect code where possible - it has a nasty habit of getting copied.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Paul Higinbotham (PaulHigin) Feb 8, 2017

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not incorrect code. The methods are simply unnecessary for the test.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Jason Shirk (lzybkr) Mar 1, 2017

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are trying to apply the same standard between test code and product code.

If this code were copied or reused beyond this test (and people definitely do copy our test code - maybe treating it like a sample) - what change would you suggest to avoid problems?

Maybe throw from push/pop - knowing that Runspace does not work as intended with push/pop?
At a minimum, I'd say you would add a comment pointing out the flaw when the code is used elsewhere.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

test/powershell/engine/Remoting/InvokeCommandRemoteDebug.Tests.ps1 Outdated

		if (!$IsWindows)
		{
		$originalDefaultParameterValues = $PSDefaultParameterValues.Clone()
		$PSDefaultParameterValues["it:skip"] = $true

Jason Shirk (lzybkr) Jan 19, 2017

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feature only works on Windows? Is that a temporary situation (I hope), then shouldn't this be Pending, not Skip?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

test/powershell/engine/Remoting/InvokeCommandRemoteDebug.Tests.ps1 Outdated


		It "Verifies that asynchronous 'Invoke-Command -RemoteDebug' is ignored" {

		$ps.AddCommand("Invoke-Command").AddParameter("Session", $remoteSession).AddParameter("ScriptBlock", $sb).AddParameter("RemoteDebug", $true).AddParameter("AsJob", $true)

Jason Shirk (lzybkr) Jan 19, 2017

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lines like this are hard to read if on one line. You can move method calls to the next line like this:

    $ps.AddCommand("Invoke-Command").
        AddParameter("Session", $remoteSession).
        AddParameter ...

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

src/System.Management.Automation/engine/debugger/debugger.cs Outdated

		#region Runspace Debug Processing

		/// <summary>
		/// RaiseProcessRunspaceDebugEvent

Jason Shirk (lzybkr) Jan 24, 2017

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use helpful xml doc comments or use a simple:

/// <summary/>

to silence the compiler warnings.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

src/System.Management.Automation/engine/debugger/debugger.cs Outdated

		_nestedRunningFrame = null;
		if (_lastActiveDebuggerAction == DebuggerResumeAction.StepOut)
		{
		CancelDebuggerProcessingAsNeeded(poppedDebugger);

Jason Shirk (lzybkr) Jan 24, 2017

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, I'm finding the verb used confusing. Here stepping out is normal - so do we really mean to "cancel" debugger processing?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Jason Shirk (lzybkr) Jan 24, 2017

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To elaborate, I expect potentially different cleanup code if something completes normally versus terminated (cancelled) for some other reason. Maybe they aren't different in this code, but that's how I'm thinking about it.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Paul Higinbotham (PaulHigin) Feb 1, 2017

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The CancelDebuggerProcessingAsNeeded() method cancels debugger processing only when stepping out of the current call stack, which means stepping out of the currently debugged remote session. This behavior was debated but we finally decided to step out of all remote sessions from the Invoke-Command, and stop in the local script that called Invoke-Command.

We decided to do this because stepping out of one remote session and into another results in confusion because you are debugging one remote session while one or more are running in parallel spewing output into your debug session console. So "step-out" at the top of the stack means to step out of the Invoke-Command remote debugging and back to the local script.

I'll add a comment to try to make this clear.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Paul Higinbotham (PaulHigin) Feb 1, 2017

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thinking about this some more I have changed my mind. Right now "step-out" aborts the Invoke-Command debugging and continues with local script debugging, which I thought was the right behavior for some reason. But this is different behavior from how "continue" and "quit" debugger commands work. I think "step-out" should be consistent and I'll remove this cancel code.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Paul Higinbotham (PaulHigin) added 3 commits

February 1, 2017 13:43


          Changes from Code Review

9db8fdf


          Missed one case from Code Review comments

984cffa


          Fixed step-out being ignored because debugger is disabled.

e492879

Ilya (iSazonov) reviewed

Feb 10, 2017

View reviewed changes

Comment thread

test/powershell/engine/Remoting/InvokeCommandRemoteDebug.Tests.ps1 Outdated

		if (-not (Get-Module TestRemoting -ErrorAction SilentlyContinue))
		{
		$remotingModule = Join-Path $PSScriptRoot "../Common/TestRemoting.psm1"
		Import-Module $remotingModule

Ilya (iSazonov) Feb 10, 2017

Copy link

Copy Markdown

Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to check Get-Module TestRemoting
This work well:

Import-Module $remotingModule  -ErrorAction SilentlyContinue

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Paul Higinbotham (PaulHigin) Feb 10, 2017

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

test/powershell/engine/Remoting/InvokeCommandRemoteDebug.Tests.ps1 Outdated


		It "Verifies that synchronous 'Invoke-Command -RemoteDebug' invokes debugger" {

		$ps.Commands.Clear()

Ilya (iSazonov) Feb 10, 2017

Copy link

Copy Markdown

Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If somebody add a new test with AddCommand this may break the existing tests.
It would be safer to move $ps.Commands.Clear() in AfterEach {}

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Paul Higinbotham (PaulHigin) Feb 10, 2017

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea. Thanks.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          CR test feedback

a343799

Ilya (iSazonov) commented Feb 11, 2017

Copy link

Copy Markdown

Collaborator

Paul Higinbotham (@PaulHigin) Whether to increase the number of files with similar tests?
If so, then perhaps we should move TestDebugger to TestRemoting.psm1 and merge DummyHost with TestHostCS in TestHostCS.psm1.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Paul Higinbotham (PaulHigin) commented Feb 13, 2017

Copy link

Copy Markdown

Contributor Author

Ilya (@iSazonov) I feel it is not necessary at this point. If in the future more tests rely on these classes then the code can be refactored.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Ilya (iSazonov) commented Feb 13, 2017

Copy link

Copy Markdown

Collaborator

Paul Higinbotham (@PaulHigin) Thanks for clarification! Clear.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Mike Richmond (mirichmo) commented Feb 17, 2017

Copy link

Copy Markdown

Member

Jason Shirk (@lzybkr) and Ilya (@iSazonov) - Do you have any additional comments, outstanding questions, or discussion points? Is this PR ready to go?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Ilya (iSazonov) commented Feb 17, 2017

Copy link

Copy Markdown

Collaborator

Mike Richmond (@mirichmo) With regard to the tests the minor question remains open about IoT #3015 (review)

PaulHigin The test works on Windows Core Server. I don't know about IoT

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Paul Higinbotham (PaulHigin) mentioned this pull request

Feb 22, 2017

Update PowerShell Console and ISE to allow debug step into Invoke-Command remote #2827

Closed

Paul Higinbotham (PaulHigin) assigned Bruce Payette (BrucePay) and Mike Richmond (mirichmo)

Feb 22, 2017

Paul Higinbotham (PaulHigin) requested a review from Bruce Payette (BrucePay)

February 24, 2017 19:47

Paul Higinbotham (PaulHigin) unassigned Bruce Payette (BrucePay)

Feb 24, 2017

Paul Higinbotham (PaulHigin) removed the request for review from Bruce Payette (BrucePay)

March 1, 2017 16:04

Paul Higinbotham (PaulHigin) assigned Dongbo Wang (daxian-dbw) and unassigned Mike Richmond (mirichmo)

Mar 1, 2017

Paul Higinbotham (PaulHigin) commented Mar 1, 2017

Copy link

Copy Markdown

Contributor Author

Dongbo Wang (@daxian-dbw) This PR has been languishing for weeks. Is there any reason why it cannot be merged? It seems to me that any minor issues can be addressed as they come up. Otherwise I am Ok abandoning this PR since this a debugging feature requested internally.
Thanks!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

David Wilson (daviwil) commented Mar 1, 2017

Copy link

Copy Markdown

Contributor

I think this should be merged, it adds value for all our users, not just the internal ones. This functionality will light up for users in VS Code once it's released in PowerShell Core.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Jason Shirk (lzybkr) commented Mar 1, 2017

Copy link

Copy Markdown

Contributor

Most of my feedback was addressed.

I won't block merging, but do think my feedback on the host test code should be addressed - I expanded on my thinking inline.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Dongbo Wang (daxian-dbw) mentioned this pull request

Mar 1, 2017

Add tests for NanoServer and IoT for the debugging support to step in "Invoke-Command -ComputerName" #3236

Closed

Dongbo Wang (daxian-dbw) commented Mar 1, 2017

Copy link

Copy Markdown

Member

I opened #3236 to capture 2 open comments in the review. Given that, I'm merging this PR.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Dongbo Wang (daxian-dbw) merged commit b4cb5e9 into PowerShell:master

Mar 1, 2017

Paul Higinbotham (PaulHigin) deleted the ICMRemoteDebug branch

March 6, 2017 19:04

Ilya (iSazonov) removed the Review - Needed The PR is being reviewed label

Mar 27, 2017

Thatgfsj (Thatgfsj) pushed a commit to Thatgfsj-contribs/PowerShell that referenced this pull request

Aug 6, 2026


          Implementation for Invoke-Command step-in remote debugging (PowerShel…

cbecaff

…l#3015)

These changes provide the ability to debug remote running scripts started with the Invoke-Command cmdlet. The design is event based and provides new public events that allow subscribers to be notified when an Invoke-Command remote session is ready for debugging. Since Invoke-Command allows running scripts on multiple targets at once (fan-out) the notification event is raised for each remote session as it becomes ready for debugging. The subscriber to these events will be a script debugger implementation (such as PowerShell console, ISE, or VSCode) and will handle all debugging details such as simultaneously debugging multiple remote sessions at once in separate windows.

But these changes also include an internal implementation which is used by default if host debuggers don't want to handle the debugging details. This internal implementation is what PowerShell console, ISE uses so they can have this new behavior without having to modify their debugger implementations. The internal implementation serializes each remote session of Invoke-Command so that they can be debugged one at a time. The remote session debugger is "pushed" onto the internal debugger stack so that debugging transitions to the remote session. Existing debugging commands work so that the "quit" debugging command will stop the current remote session script from running and allow the next remote session to be debugged. Similarly the "continue" debugging command allows the script to continue running outside step mode and again go to the next remote session for debugging. The "stepout" debugging command steps out of all Invoke-Command remote sessions and lets the script continue to run for each remote session in parallel as they are normally run.

The purpose of Invoke-Command step-in remote debugging is allow seamless debugging of a local script that calls Invoke-Command on remote targets. But there is also a new Invoke-Command "-RemoteDebug" parameter that lets you Invoke-Command on the command line and have it drop directly into the debugger.

An example from the PowerShell command line looks like this:
```
PS C:\> C:\TestICM.ps1
Entering debug mode. Use h or ? for help.

Hit Command breakpoint on 'Invoke-Command'

At C:\TestICM.ps1:2 char:1
+ Invoke-Command -cn $computerName,paulhig-3 -File c:\LinuxScript.ps1
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[DBG]: PS C:\>> list

    1:  $computerName = "localhost"
    2:* Invoke-Command -cn $computerName,paulhig-3 -File c:\LinuxScript.ps1
    3:  "Test Complete!"

[DBG]: PS C:\>> stepin
At line:1 char:1
+ Write-Output "Running script on Linux!"
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[paulhig-3]:[DBG]: [Process:14072]: [Runspace5]: PS C:\Users\paulhi\Documents>
```

Notice that the debugger "stepin" command transitioned from local script debugging to debugging the remote session on computer "paulhig-3", as can be seen by the change in the debugger prompt.

You can also do this from the command line to drop directly into the debugger
```
Invoke-Command -cn localhost -Script  $scriptblock -RemoteDebug
```

These changes also remove an old behavior that was incompatible with this new step-in feature. Previously if a remote session running script hit a break point it would stop in the debugger and go to the "disconnected session" state. This was to allow the user to reconnect using Enter-PSSession and then interactively debug the remote session script. This behavior has been removed and now the user needs to attach a debugger using the newer Debug-Runspace cmdlet.

Reviewers

Dongbo Wang (daxian-dbw)

daxian-dbw left review comments

Ilya (iSazonov)

iSazonov left review comments

+2 more reviewers

Jason Shirk (lzybkr)

lzybkr left review comments

Mike Richmond (mirichmo)

mirichmo left review comments

Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

Dongbo Wang (daxian-dbw)

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

9 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

Conversation

Paul Higinbotham (PaulHigin) commented Jan 19, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Ilya (iSazonov) Jan 19, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Ilya (iSazonov) Jan 19, 2017 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.