PowerShell / PowerShell

PowerShell for every system!
https://microsoft.com/PowerShell
MIT License
43.55k stars 7.06k forks source link

Proposed command `Invoke-Process` - to solve common problems with Start-Process -Wait and stream redirection #21570

Open jwittner opened 2 weeks ago

jwittner commented 2 weeks ago

Summary of the new feature / enhancement

Right now it's very easy to shoot yourself in the foot if you want to start a process, redirect its output, and wait on it. For example, If you don't consume each stream in its own thread, if enough data is pushed to the streams, the process will hang. It'd be great to support this functionality right inside this module instead of having to share out my teams extensions to the Process noun when I see others run into this issue.

I've attached my solution as a proposed example implementation. It's built on System.Diagnostics.Process and ThreadJobs. Generally it makes invoking a process act a lot more like running a powershell command. Standard output is written to the PS output stream immediately line by line. Error output is accumulated until the exit code can be compared to the set of success codes - if it's a success then the error is written to the verbose stream, otherwise to the error stream.

I've started poking through the csharp code that implements the other *-Process commands and it seems like this would be straightforward to add and I'd be happy to make those contributions if I can convince the team here of the value of such a command.

Thanks for considering!

Proposed technical implementation details (optional)

<#
.Synopsis
    Runs a process and waits for it, redirecting output and errors as appropriate. Supports $LASTEXITCODE.
.Description
    The process's standard output is redirected and is read and written to the Output stream line by line.
    The process's error output is redirected and if not discarded it will be written to the Error stream
    if an error code is reported by the process, otherwise to the Verbose stream.

    The global $LASTEXITCODE is assigned the exit code of the process.

    Note:   PowerShell does not always wait for processes to finish if invoked directly before
            moving onto other commands. Using Invoke-Process makes processes act more like
            cmdlets in their behavior and output.

    Note:   Starts a background thread job to read the error stream. There are edge cases where not
            reading from all redirected output streams simultaneously will result in process deadlocks.
            Ensure the ThrottleLimit is set as necessary for your work load.
#>
function Invoke-Process {
    [Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSUseUsingScopeModifierInNewRunspaces', '', Justification = 'Reports false positives when using ArgumentList, see https://github.com/PowerShell/PSScriptAnalyzer/issues/1504')]
    [CmdletBinding()]
    param(
        [Parameter(Mandatory = $true, Position = 0)]
        # Name of the process to invoke.
        [string] $Name,
        [Parameter(Position = 1)]
        # The arguments to pass to the process.
        [string] $Arguments,
        # The working directory the process is started in.
        [ValidateNotNullOrEmpty()]
        [ValidateScript({ if((-not $_) -or (Test-Path $_ -PathType Container)) { return $true } throw "WorkingDirectory $_ must exist."})]
        [string] $WorkingDirectory = $PWD,
        # The codes that represent success for this process.
        [int[]] $SuccessCodes = @(0),
        # Should the process output stream be discarded?
        [switch] $DiscardOutput,
        # Should the process error stream be discarded?
        [switch] $DiscardError,
        # If specified, will set the throttle limit for thread jobs when starting it's own.
        [int] $ThrottleLimit
    )

    # Resolve to full path
    $WorkingDirectory = $ExecutionContext.SessionState.Path.GetUnresolvedProviderPathFromPSPath($WorkingDirectory)

    $command = Get-Command -Name $Name -ErrorAction Ignore
    if (-not $command) {
        throw "$Name must be available in the path"
    }

    if ($command.CommandType -ne [System.Management.Automation.CommandTypes]::Application ) {
        throw "$Name does not refer to an application"
    }

    $process = New-Object System.Diagnostics.Process
    $process.StartInfo.Filename = $command.Source
    $process.StartInfo.Arguments = $Arguments
    $process.StartInfo.UseShellExecute = $false
    $process.StartInfo.CreateNoWindow = $true
    $process.StartInfo.WorkingDirectory = $WorkingDirectory
    $process.StartInfo.WindowStyle = [System.Diagnostics.ProcessWindowStyle]::Hidden
    $process.StartInfo.RedirectStandardOutput = $true
    $process.StartInfo.RedirectStandardError = $true

    Write-Verbose "Invoking process '$($process.StartInfo.FileName) $($process.StartInfo.Arguments)'"

    try { $process.Start() > $null }
    catch { throw }

    $readErrorStreamThreadJobArgs = @{
        Name         = "$($process.StartInfo.FileName) $($process.StartInfo.Arguments) Error"
        ScriptBlock  = {
            param($process, $discard)

            [string]$errorOutput = ''
            while (-not $process.StandardError.EndOfStream) {
                $line = $process.StandardError.ReadLine()
                if (-not $discard) { $errorOutput += ($line + "`r`n"); }
            }

            if (-not $discard) {
                Write-Output $errorOutput
            }
        }
        ArgumentList = @($process, $DiscardError)
    }

    if ($PSBoundParameters.ContainsKey('ThrottleLimit')) {
        $readErrorStreamThreadJobArgs['ThrottleLimit'] = $ThrottleLimit
    }

    $errorJob = Start-ThreadJob @readErrorStreamThreadJobArgs

    while (-not $process.StandardOutput.EndOfStream) {
        $line = $process.StandardOutput.ReadLine()
        if (-not $DiscardOutput) { Write-Output $line }
    }

    [string]$errorOutput = $errorJob | Receive-Job -AutoRemove -Wait | ForEach-Object { $_.Trim() }
    $process.WaitForExit()

    $global:LASTEXITCODE = $process.ExitCode
    if ($process.ExitCode -in $SuccessCodes) {
        if ($errorOutput.Length -gt 0) {
            Write-Verbose "Begin output for process '$Name $Arguments':`r`n$errorOutput"
            Write-Verbose "End output for process '$Name $Arguments'"
        }
    }
    else {
        $errorString = "Error invoking '$Name $Arguments', exited with code $($process.ExitCode)."
        if ($errorOutput.Length -gt 0) {
            $errorString += " Error Logs:`r`n$errorOutput"
        }

        Write-Error $errorString
    }
}
vexx32 commented 2 weeks ago

I will say once you start getting into a C# cmdlet you need to manually put in wait / halt points by overriding StopProcessing and having something checking for Ctrl+C when you're doing waiting like this, which will complicate the code somewhat. It's very doable, though; you can refer to the Start-Sleep command for some clues on how that works for a cmdlet.

Also, iirc $process.WaitForExit() is not always fully reliable and there are other ways to check for a process stopping -- I think hooking into its Exiting event is one of the more reliable ones?

I think it's also worth noting that if you're doing redirection like this you can't use -Verb RunAs (or set UseShellExecute = true), as that breaks the redirection.

So I would encourage you to take a look at actually building a cmdlet to see the full scope of how it needs to work and tie in, and also the problems you're going to run into (e.g., is there a different way to handle redirection while starting a process as admin? I vaguely recall you might be able to do something interesting with named pipes, but I've no idea how that works in practice;; does Ctrl+C correctly cancel the command and the running process?).

Also, I'm not clear here why this wouldn't just be an enhancement to Start-Process, ultimately. Possibly if the behaviour is different enough you could put it behind a switch to enable certain parts of it, but I'm not sure even that's warranted.

jborean93 commented 2 weeks ago

Also, I'm not clear here why this wouldn't just be an enhancement to Start-Process, ultimately

To me the difference would how Start-Process starts and optionally outputs the process object whereas running an inline process runs and outputs the stdout/stderr. Trying to overload Start-Process with more parameters to retrieve the output doesn't make too much sense from a UX perspective because as you've said things like -Verb just doesn't really apply.

What I would love to see is basically what a normal exe invocation does in PowerShell but exposed as a cmdlet. This cmdlet would then provide the normal behaviour but also allow you to:

Basically it's the normal invocation but a more pwsh friendly way of controlling how that works which doesn't involve either global settings or vars that people don't really know about.

I agree that this should be a C# cmdlet, there are too many problems when it comes to events, stopping, etc that makes this unviable as a script function.

rhubarb-geek-nz commented 2 weeks ago

hi, interesting idea. I would second the idea of making it C#. A couple of further observations

I see no problem with this being a PSGallery module rather than having to be in PowerShell itself, then it can be used on any version of PowerShell.

mklement0 commented 2 weeks ago

See also:

SteveL-MSFT commented 1 week ago

I see no problem with this being a PSGallery module rather than having to be in PowerShell itself, then it can be used on any version of PowerShell.

Agree, there's nothing proposed that doesn't allow this cmdlet to be in a separate module published to PSGallery. If there is sufficient feedback and usage, we can decide separately whether it NEEDS to be part of PS7.

jwittner commented 1 week ago

Thanks everyone for the great feedback and ideas! Publishing this as a separate module to the PSGallery seems like a great way to make it available quickly, gather early feedback on the cmdlet, and how useful it might be more broadly.

I considered doing this before posting this proposal, but didn't want to step on the built in *-Process commands or any plans this group might have already had to improve the use cases this serves. Internally I've published this to our feeds as the 'Process' module, but this is already on the PSGallery. Any suggestions for good alternative names?

If we go this route I'd want to make it an OSS project. I can do that under the Microsoft org (I'm a Microsoft FTE), but perhaps I could work with someone on this group to add the module to the official PowerShell org? Is owning this module long term something the PowerShell org would be willing to take over? Would be great to have more folks/organizational power behind it than just me in my corner. :)

Does anyone see problems with going this route?

Thanks again for the responsive and thoughtful feedback!

SteveL-MSFT commented 1 week ago

@jwittner I would suggest just having a repo under your GitHub account for now. Depending on the popularity/feedback, we can find a more formal home for it later. Thanks for taking this up!