From d0a95d25125d3b01993357eb2abb1508ec362fe1 Mon Sep 17 00:00:00 2001 From: Howard Wolosky Date: Sun, 28 Jun 2020 16:42:42 -0700 Subject: [PATCH 1/3] Reimagining status for the module Status for commands was originally added to this module based on my experience with other REST API's where individual commands could easily take 10-20 seconds. Practical usage has shown that most GitHub requests in reality take under one second. The additional work that PowerShell has to do in order to display progress to the user can easily make the overall command take 4-6 times longer than its actual execution time. Therefore, status is being ripped out of this module (for the most part). `Invoke-GHRestMethod` and `Invoke-SendTelemetryEvent` no longer have bifurcating execution paths based on the value of `$NoStatus`. Everything runs synchronously now on the command prompt. * `DefaultNoStatus` has been deprecated. Its value will be ignored. * The `NoStatus` switch has not been removed from the module commands in order to avoid a breaking change. It may be removed in a future update. * `Invoke-GHRestMethod -ExtendedResult` has been updated to include the next page's number and the total number of pages for the REST request. * A new configuration value has been added: `MultiRequestProgressThreshold` `Invoke-GHRestMethodMultipleResult` will display a ProgressBar to the user tracking the number of remaining requests for the overall execution of the requested command based on this threshold value. It will only display the progress bar if the number of requets needed meets or exceeds this threshold value. This defaults to 10, and can be disabled with a value of 0. Practical usage has shown that this adds less than a second of additional time to the overall execution of a multi-request command (quite different than the previous status). * `Wait-JobWithAnimation` has been removed since it's no longer used. Fixes #247 --- GitHubConfiguration.ps1 | 15 ++ GitHubCore.ps1 | 265 ++++++++++++--------------------- Helpers.ps1 | 136 ----------------- Telemetry.ps1 | 201 +++---------------------- Tests/Common.ps1 | 2 +- Tests/GitHubContents.tests.ps1 | 1 - 6 files changed, 130 insertions(+), 490 deletions(-) diff --git a/GitHubConfiguration.ps1 b/GitHubConfiguration.ps1 index 0320f399..28ab414d 100644 --- a/GitHubConfiguration.ps1 +++ b/GitHubConfiguration.ps1 @@ -78,6 +78,9 @@ function Set-GitHubConfiguration .PARAMETER DefaultNoStatus Control if the -NoStatus switch should be passed-in by default to all methods. + The -NoStatus switch has been deprecated. Commands in this module no longer display status + on the console, and therefore passing in -NoStatus to a command has no effect. + Therefore, the value of this configuration setting has no impact on command execution. .PARAMETER DefaultOwnerName The owner name that should be used with a command that takes OwnerName as a parameter @@ -132,6 +135,14 @@ function Set-GitHubConfiguration .PARAMETER LogTimeAsUtc If specified, all times logged will be logged as UTC instead of the local timezone. + .PARAMETER MultiRequestProgressThreshold + Some commands may require sending multiple requests to GitHub. In some situations, + getting the entirety of the request might take 70+ requests occurring over 20+ seconds. + A progress bar will be shown (displaying which sub-request is being executed) if the number + of requests required to complete this command is greater than or equal to this configuration + value. + Set to 0 to disable this feature. + .PARAMETER RetryDelaySeconds The number of seconds to wait before retrying a command again after receiving a 202 response. @@ -212,6 +223,8 @@ function Set-GitHubConfiguration [switch] $LogTimeAsUtc, + [int] $MultiRequestProgressThreshold, + [int] $RetryDelaySeconds, [switch] $SuppressNoTokenWarning, @@ -296,6 +309,7 @@ function Get-GitHubConfiguration 'LogProcessId', 'LogRequestBody', 'LogTimeAsUtc', + 'MultiRequestProgressThreshold', 'RetryDelaySeconds', 'SuppressNoTokenWarning', 'SuppressTelemetryReminder', @@ -640,6 +654,7 @@ function Import-GitHubConfiguration 'logProcessId' = $false 'logRequestBody' = $false 'logTimeAsUtc' = $false + 'multiRequestProgressThreshold' = 10 'retryDelaySeconds' = 30 'suppressNoTokenWarning' = $false 'suppressTelemetryReminder' = $false diff --git a/GitHubCore.ps1 b/GitHubCore.ps1 index 09068429..3281a780 100644 --- a/GitHubCore.ps1 +++ b/GitHubCore.ps1 @@ -84,29 +84,28 @@ function Invoke-GHRestMethod no bucket value will be used. .PARAMETER NoStatus - If this switch is specified, long-running commands will run on the main thread - with no commandline status update. When not specified, those commands run in - the background, enabling the command prompt to provide status information. + Deprecated switch after v0.14.0. Kept here for the time being to reduce module churn. + Specifying it does nothing. It used to control whether or not the web request would have + a corresponding progress UI. .OUTPUTS [PSCustomObject] - The result of the REST operation, in whatever form it comes in. .EXAMPLE - Invoke-GHRestMethod -UriFragment "applications/" -Method Get -Description "Get first 10 applications" + Invoke-GHRestMethod -UriFragment "users/octocat" -Method Get -Description "Get information on the octocat user" - Gets the first 10 applications for the connected dev account. + Gets the user information for Octocat. .EXAMPLE - Invoke-GHRestMethod -UriFragment "applications/0ABCDEF12345/submissions/1234567890123456789/" -Method Delete -Description "Delete Submission" -NoStatus + Invoke-GHRestMethod -UriFragment "user" -Method Get -Description "Get current user" - Deletes the specified submission, but the request happens in the foreground and there is - no additional status shown to the user until a response is returned from the REST request. + Gets information about the current authenticated user. .NOTES This wraps Invoke-WebRequest as opposed to Invoke-RestMethod because we want access to the headers that are returned in the response, and Invoke-RestMethod drops those headers. #> - [CmdletBinding(SupportsShouldProcess)] + [CmdletBinding()] param( [Parameter(Mandatory)] [string] $UriFragment, @@ -195,12 +194,6 @@ function Invoke-GHRestMethod $headers.Add("Content-Type", "application/json; charset=UTF-8") } - if (-not $PSCmdlet.ShouldProcess($url, "Invoke-WebRequest")) - { - return - } - - $NoStatus = Resolve-ParameterWithDefaultConfigurationValue -Name NoStatus -ConfigValueName DefaultNoStatus $originalSecurityProtocol = [Net.ServicePointManager]::SecurityProtocol try @@ -209,136 +202,34 @@ function Invoke-GHRestMethod Write-Log -Message "Accessing [$Method] $url [Timeout = $(Get-GitHubConfiguration -Name WebRequestTimeoutSec))]" -Level Verbose $result = $null - if ($NoStatus) + $params = @{} + $params.Add("Uri", $url) + $params.Add("Method", $Method) + $params.Add("Headers", $headers) + $params.Add("UseDefaultCredentials", $true) + $params.Add("UseBasicParsing", $true) + $params.Add("TimeoutSec", (Get-GitHubConfiguration -Name WebRequestTimeoutSec)) + + if ($Method -in $ValidBodyContainingRequestMethods -and (-not [String]::IsNullOrEmpty($Body))) { - $params = @{} - $params.Add("Uri", $url) - $params.Add("Method", $Method) - $params.Add("Headers", $headers) - $params.Add("UseDefaultCredentials", $true) - $params.Add("UseBasicParsing", $true) - $params.Add("TimeoutSec", (Get-GitHubConfiguration -Name WebRequestTimeoutSec)) - - if ($Method -in $ValidBodyContainingRequestMethods -and (-not [String]::IsNullOrEmpty($Body))) - { - $bodyAsBytes = [System.Text.Encoding]::UTF8.GetBytes($Body) - $params.Add("Body", $bodyAsBytes) - Write-Log -Message "Request includes a body." -Level Verbose - if (Get-GitHubConfiguration -Name LogRequestBody) - { - Write-Log -Message $Body -Level Verbose - } - } - - # Disable Progress Bar in function scope during Invoke-WebRequest - $ProgressPreference = 'SilentlyContinue' - - [Net.ServicePointManager]::SecurityProtocol=[Net.SecurityProtocolType]::Tls12 - $result = Invoke-WebRequest @params - - if ($Method -eq 'Delete') + $bodyAsBytes = [System.Text.Encoding]::UTF8.GetBytes($Body) + $params.Add("Body", $bodyAsBytes) + Write-Log -Message "Request includes a body." -Level Verbose + if (Get-GitHubConfiguration -Name LogRequestBody) { - Write-Log -Message "Successfully removed." -Level Verbose + Write-Log -Message $Body -Level Verbose } } - else - { - $jobName = "Invoke-GHRestMethod-" + (Get-Date).ToFileTime().ToString() - - [scriptblock]$scriptBlock = { - param( - $Url, - $Method, - $Headers, - $Body, - $ValidBodyContainingRequestMethods, - $TimeoutSec, - $LogRequestBody, - $ScriptRootPath) - - # We need to "dot invoke" Helpers.ps1 and GitHubConfiguration.ps1 within - # the context of this script block since we're running in a different - # PowerShell process and need access to Get-HttpWebResponseContent and - # config values referenced within Write-Log. - . (Join-Path -Path $ScriptRootPath -ChildPath 'Helpers.ps1') - . (Join-Path -Path $ScriptRootPath -ChildPath 'GitHubConfiguration.ps1') - - $params = @{} - $params.Add("Uri", $Url) - $params.Add("Method", $Method) - $params.Add("Headers", $Headers) - $params.Add("UseDefaultCredentials", $true) - $params.Add("UseBasicParsing", $true) - $params.Add("TimeoutSec", $TimeoutSec) - - if ($Method -in $ValidBodyContainingRequestMethods -and (-not [String]::IsNullOrEmpty($Body))) - { - $bodyAsBytes = [System.Text.Encoding]::UTF8.GetBytes($Body) - $params.Add("Body", $bodyAsBytes) - Write-Log -Message "Request includes a body." -Level Verbose - if ($LogRequestBody) - { - Write-Log -Message $Body -Level Verbose - } - } - - try - { - # Disable Progress Bar in function scope during Invoke-WebRequest - $ProgressPreference = 'SilentlyContinue' - - [Net.ServicePointManager]::SecurityProtocol=[Net.SecurityProtocolType]::Tls12 - Invoke-WebRequest @params - } - catch [System.Net.WebException] - { - # We need to access certain headers in the exception handling, - # but the actual *values* of the headers of a WebException don't get serialized - # when the RemoteException wraps it. To work around that, we'll extract the - # information that we actually care about *now*, and then we'll throw our own exception - # that is just a JSON object with the data that we'll later extract for processing in - # the main catch. - $ex = @{} - $ex.Message = $_.Exception.Message - $ex.StatusCode = $_.Exception.Response.StatusCode - $ex.StatusDescription = $_.Exception.Response.StatusDescription - $ex.InnerMessage = $_.ErrorDetails.Message - try - { - $ex.RawContent = Get-HttpWebResponseContent -WebResponse $_.Exception.Response - } - catch - { - Write-Log -Message "Unable to retrieve the raw HTTP Web Response:" -Exception $_ -Level Warning - } - $jsonConversionDepth = 20 # Seems like it should be more than sufficient - throw (ConvertTo-Json -InputObject $ex -Depth $jsonConversionDepth) - } - } - - $null = Start-Job -Name $jobName -ScriptBlock $scriptBlock -Arg @( - $url, - $Method, - $headers, - $Body, - $ValidBodyContainingRequestMethods, - (Get-GitHubConfiguration -Name WebRequestTimeoutSec), - (Get-GitHubConfiguration -Name LogRequestBody), - $PSScriptRoot) + # Disable Progress Bar in function scope during Invoke-WebRequest + $ProgressPreference = 'SilentlyContinue' - Wait-JobWithAnimation -Name $jobName -Description $Description - $result = Receive-Job $jobName -AutoRemoveJob -Wait -ErrorAction SilentlyContinue -ErrorVariable remoteErrors + [Net.ServicePointManager]::SecurityProtocol=[Net.SecurityProtocolType]::Tls12 + $result = Invoke-WebRequest @params - if ($remoteErrors.Count -gt 0) - { - throw $remoteErrors[0].Exception - } - - if ($Method -eq 'Delete') - { - Write-Log -Message "Successfully removed." -Level Verbose - } + if ($Method -eq 'Delete') + { + Write-Log -Message "Successfully removed." -Level Verbose } # Record the telemetry for this event. @@ -346,7 +237,7 @@ function Invoke-GHRestMethod if (-not [String]::IsNullOrEmpty($TelemetryEventName)) { $telemetryMetrics = @{ 'Duration' = $stopwatch.Elapsed.TotalSeconds } - Set-TelemetryEvent -EventName $TelemetryEventName -Properties $localTelemetryProperties -Metrics $telemetryMetrics -NoStatus:$NoStatus + Set-TelemetryEvent -EventName $TelemetryEventName -Properties $localTelemetryProperties -Metrics $telemetryMetrics } $finalResult = $result.Content @@ -379,11 +270,26 @@ function Invoke-GHRestMethod $links = $result.Headers['Link'] -split ',' $nextLink = $null + $nextPageNumber = 1 + $numPages = 1 + $since = 0 foreach ($link in $links) { - if ($link -match '<(.*)>; rel="next"') + if ($link -match '<(.*page=(\d+)[^\d]*)>; rel="next"') { - $nextLink = $matches[1] + $nextLink = $Matches[1] + $nextPageNumber = [int]$Matches[2] + } + elseif ($link -match '<(.*since=(\d+)[^\d]*)>; rel="next"') + { + # Special case scenario for the users endpoint. + $nextLink = $Matches[1] + $since = [int]$Matches[2] + $numPages = 0 # Signifies an unknown number of pages. + } + elseif ($link -match '<.*page=(\d+)[^\d]+rel="last"') + { + $numPages = [int]$Matches[1] } } @@ -417,6 +323,9 @@ function Invoke-GHRestMethod 'statusCode' = $result.StatusCode 'requestId' = $result.Headers['X-GitHub-Request-Id'] 'nextLink' = $nextLink + 'nextPageNumber' = $nextPageNumber + 'numPages' = $numPages + 'since' = $since 'link' = $result.Headers['Link'] 'lastModified' = $result.Headers['Last-Modified'] 'ifNoneMatch' = $result.Headers['If-None-Match'] @@ -436,9 +345,6 @@ function Invoke-GHRestMethod } catch { - # We only know how to handle WebExceptions, which will either come in "pure" - # when running with -NoStatus, or will come in as a RemoteException when running - # normally (since it's coming from the asynchronous Job). $ex = $null $message = $null $statusCode = $null @@ -468,32 +374,10 @@ function Invoke-GHRestMethod $requestId = $ex.Response.Headers['X-GitHub-Request-Id'] } } - elseif (($_.Exception -is [System.Management.Automation.RemoteException]) -and - ($_.Exception.SerializedRemoteException.PSObject.TypeNames[0] -eq 'Deserialized.System.Management.Automation.RuntimeException')) - { - $ex = $_.Exception - try - { - $deserialized = $ex.Message | ConvertFrom-Json - $message = $deserialized.Message - $statusCode = $deserialized.StatusCode - $statusDescription = $deserialized.StatusDescription - $innerMessage = $deserialized.InnerMessage - $requestId = $deserialized['X-GitHub-Request-Id'] - $rawContent = $deserialized.RawContent - } - catch [System.ArgumentException] - { - # Will be thrown if $ex.Message isn't JSON content - Write-Log -Exception $_ -Level Error - Set-TelemetryException -Exception $ex -ErrorBucket $errorBucket -Properties $localTelemetryProperties -NoStatus:$NoStatus - throw - } - } else { Write-Log -Exception $_ -Level Error - Set-TelemetryException -Exception $_.Exception -ErrorBucket $errorBucket -Properties $localTelemetryProperties -NoStatus:$NoStatus + Set-TelemetryException -Exception $_.Exception -ErrorBucket $errorBucket -Properties $localTelemetryProperties throw } @@ -558,7 +442,7 @@ function Invoke-GHRestMethod $newLineOutput = ($output -join [Environment]::NewLine) Write-Log -Message $newLineOutput -Level Error - Set-TelemetryException -Exception $ex -ErrorBucket $errorBucket -Properties $localTelemetryProperties -NoStatus:$NoStatus + Set-TelemetryException -Exception $ex -ErrorBucket $errorBucket -Properties $localTelemetryProperties throw $newLineOutput } finally @@ -643,8 +527,7 @@ function Invoke-GHRestMethodMultipleResult but the request happens in the foreground and there is no additional status shown to the user until a response is returned from the REST request. #> - [CmdletBinding(SupportsShouldProcess)] - [Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSShouldProcess", "", Justification="Methods called within here make use of PSShouldProcess, and the switch is passed on to them inherently.")] + [CmdletBinding()] [Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSReviewUnusedParameter", "", Justification="One or more parameters (like NoStatus) are only referenced by helper methods which get access to it from the stack via Get-Variable -Scope 1.")] [OutputType([Object[]])] param( @@ -683,10 +566,14 @@ function Invoke-GHRestMethodMultipleResult $currentDescription = $Description $nextLink = $UriFragment + $multiRequestProgressThreshold = Get-GitHubConfiguration -Name 'MultiRequestProgressThreshold' + $iteration = 0 + $progressId = $null try { do { + $iteration++ $params = @{ 'UriFragment' = $nextLink 'Method' = 'Get' @@ -706,7 +593,35 @@ function Invoke-GHRestMethodMultipleResult } $nextLink = $result.nextLink - $currentDescription = "$Description (getting additional results)" + $status = [String]::Empty + $percentComplete = 0 + if ($result.numPages -eq 0) + { + # numPages == 0 is a special case for when the total number of pages is simply unknown. + # This can happen with getting all GitHub users. + $status = "Getting additional results [page $iteration of (unknown)]" + $percentComplete = 10 # No idea what percentage to use in this scenario + } + else + { + $status = "Getting additional results [page $($result.nextPageNumber)/$($result.numPages)])" + $percentComplete = (($result.nextPageNumber / $result.numPages) * 100) + } + + $currentDescription = "$Description ($status)" + if (($multiRequestProgressThreshold -gt 0) -and + (($result.numPages -ge $multiRequestProgressThreshold) -or ($result.numPages -eq 0))) + { + $progressId = 1 + $progressParams = @{ + 'Activity' = $Description + 'Status' = $status + 'PercentComplete' = $percentComplete + 'Id' = $progressId + } + + Write-Progress @progressParams + } } until ($SinglePage -or ([String]::IsNullOrWhiteSpace($nextLink))) @@ -724,6 +639,14 @@ function Invoke-GHRestMethodMultipleResult { throw } + finally + { + # Ensure that we complete the progress bar once the command is done, regardless of outcome. + if ($null -ne $progressId) + { + Write-Progress -Activity $Description -Id $progressId -Completed + } + } } filter Split-GitHubUri diff --git a/Helpers.ps1 b/Helpers.ps1 index 17fa80e2..482c5875 100644 --- a/Helpers.ps1 +++ b/Helpers.ps1 @@ -1,142 +1,6 @@ # Copyright (c) Microsoft Corporation. All rights reserved. # Licensed under the MIT License. -function Wait-JobWithAnimation -{ -<# - .SYNOPSIS - Waits for a background job to complete by showing a cursor and elapsed time. - - .DESCRIPTION - Waits for a background job to complete by showing a cursor and elapsed time. - - The Git repo for this module can be found here: http://aka.ms/PowerShellForGitHub - - .PARAMETER Name - The name of the job(s) that we are waiting to complete. - - .PARAMETER Description - The text displayed next to the spinning cursor, explaining what the job is doing. - - .PARAMETER StopAllOnAnyFailure - Will call Stop-Job on any jobs still Running if any of the specified jobs entered - the Failed state. - - .EXAMPLE - Wait-JobWithAnimation Job1 - Waits for a job named "Job1" to exit the "Running" state. While waiting, shows - a waiting cursor and the elapsed time. - - .NOTES - This is not a stand-in replacement for Wait-Job. It does not provide the full - set of configuration options that Wait-Job does. -#> - [CmdletBinding()] - Param( - [Parameter(Mandatory)] - [string[]] $Name, - - [string] $Description = "", - - [switch] $StopAllOnAnyFailure - ) - - [System.Collections.ArrayList]$runningJobs = $Name - $allJobsCompleted = $true - $hasFailedJob = $false - - $animationFrames = '|','/','-','\' - $framesPerSecond = 9 - - $progressId = 1 - $iteration = 0 - [int] $seconds = 0 - $secondWord = 'seconds' - - while ($runningJobs.Count -gt 0) - { - # We'll run into issues if we try to modify the same collection we're iterating over - $jobsToCheck = $runningJobs.ToArray() - foreach ($jobName in $jobsToCheck) - { - $state = (Get-Job -Name $jobName).state - if ($state -ne 'Running') - { - $runningJobs.Remove($jobName) - - if ($state -ne 'Completed') - { - $allJobsCompleted = $false - } - - if ($state -eq 'Failed') - { - $hasFailedJob = $true - if ($StopAllOnAnyFailure) - { - break - } - } - } - } - - if ($hasFailedJob -and $StopAllOnAnyFailure) - { - foreach ($jobName in $runningJobs) - { - Stop-Job -Name $jobName - } - - $runingJobs.Clear() - } - - $seconds = [int]($iteration / $framesPerSecond) - $secondWord = 'seconds' - if (1 -eq $seconds) - { - $secondWord = 'second' - } - - $animationFrameNumber = $iteration % $($animationFrames.Length) - $progressParams = @{ - 'Activity' = $Description - 'Status' = "$($animationFrames[$animationFrameNumber]) Elapsed: $seconds $secondWord" - 'PercentComplete' = $(($animationFrameNumber / $animationFrames.Length) * 100) - 'Id' = $progressId - } - - Write-Progress @progressParams - Start-Sleep -Milliseconds ([int](1000 / $framesPerSecond)) - $iteration++ - } - - # Ensure that we complete the progress bar once the command is done, regardless of outcome. - Write-Progress -Activity $Description -Id $progressId -Completed - - # We'll wrap the description (if provided) in brackets for display purposes. - if (-not [string]::IsNullOrWhiteSpace($Description)) - { - $Description = "[$Description]" - } - - if ($allJobsCompleted) - { - Write-InteractiveHost "`rDONE - Operation took $seconds $secondWord $Description" -NoNewline -f Green - - # We forcibly set Verbose to false here since we don't need it printed to the screen, since we just did above -- we just need to log it. - Write-Log -Message "DONE - Operation took $seconds $secondWord $Description" -Level Verbose -Verbose:$false - } - else - { - Write-InteractiveHost "`rDONE (FAILED) - Operation took $seconds $secondWord $Description" -NoNewline -f Red - - # We forcibly set Verbose to false here since we don't need it printed to the screen, since we just did above -- we just need to log it. - Write-Log -Message "DONE (FAILED) - Operation took $seconds $secondWord $Description" -Level Verbose -Verbose:$false - } - - Write-InteractiveHost "" -} - function Get-SHA512Hash { <# diff --git a/Telemetry.ps1 b/Telemetry.ps1 index 16efb7c5..2ddffbc1 100644 --- a/Telemetry.ps1 +++ b/Telemetry.ps1 @@ -130,11 +130,6 @@ function Invoke-SendTelemetryEvent .PARAMETER TelemetryEvent The raw object representing the event data to send to Application Insights. - .PARAMETER NoStatus - If this switch is specified, long-running commands will run on the main thread - with no commandline status update. When not specified, those commands run in - the background, enabling the command prompt to provide status information. - .OUTPUTS [PSCustomObject] - The result of the REST operation, in whatever form it comes in. @@ -144,18 +139,12 @@ function Invoke-SendTelemetryEvent Invoke-* methods share a common base code. Leaving this as-is to make this file easier to share out with other PowerShell projects. #> - [CmdletBinding(SupportsShouldProcess)] - [Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSAvoidGlobalVars", "", Justification="We use global variables sparingly and intentionally for module configuration, and employ a consistent naming convention.")] + [CmdletBinding()] param( [Parameter(Mandatory)] - [PSCustomObject] $TelemetryEvent, - - [switch] $NoStatus + [PSCustomObject] $TelemetryEvent ) - # Temporarily forcing NoStatus to always be true to see if it improves user experience. - $NoStatus = $true - $jsonConversionDepth = 20 # Seems like it should be more than sufficient $uri = 'https://dc.services.visualstudio.com/v2/track' $method = 'POST' @@ -168,116 +157,22 @@ function Invoke-SendTelemetryEvent { Write-Log -Message "Sending telemetry event data to $uri [Timeout = $(Get-GitHubConfiguration -Name WebRequestTimeoutSec))]" -Level Verbose - if ($NoStatus) - { - if ($PSCmdlet.ShouldProcess($url, "Invoke-WebRequest")) - { - $params = @{} - $params.Add("Uri", $uri) - $params.Add("Method", $method) - $params.Add("Headers", $headers) - $params.Add("UseDefaultCredentials", $true) - $params.Add("UseBasicParsing", $true) - $params.Add("TimeoutSec", (Get-GitHubConfiguration -Name WebRequestTimeoutSec)) - $params.Add("Body", $bodyAsBytes) - - # Disable Progress Bar in function scope during Invoke-WebRequest - $ProgressPreference = 'SilentlyContinue' - - $result = Invoke-WebRequest @params - } - } - else - { - $jobName = "Invoke-SendTelemetryEvent-" + (Get-Date).ToFileTime().ToString() - - if ($PSCmdlet.ShouldProcess($jobName, "Start-Job")) - { - [scriptblock]$scriptBlock = { - param($Uri, $Method, $Headers, $BodyAsBytes, $TimeoutSec, $ScriptRootPath) - - # We need to "dot invoke" Helpers.ps1 and GitHubConfiguration.ps1 within - # the context of this script block since we're running in a different - # PowerShell process and need access to Get-HttpWebResponseContent and - # config values referenced within Write-Log. - . (Join-Path -Path $ScriptRootPath -ChildPath 'Helpers.ps1') - . (Join-Path -Path $ScriptRootPath -ChildPath 'GitHubConfiguration.ps1') - - $params = @{} - $params.Add("Uri", $Uri) - $params.Add("Method", $Method) - $params.Add("Headers", $Headers) - $params.Add("UseDefaultCredentials", $true) - $params.Add("UseBasicParsing", $true) - $params.Add("TimeoutSec", $TimeoutSec) - $params.Add("Body", $BodyAsBytes) - - try - { - # Disable Progress Bar in function scope during Invoke-WebRequest - $ProgressPreference = 'SilentlyContinue' - - Invoke-WebRequest @params - } - catch [System.Net.WebException] - { - # We need to access certain headers in the exception handling, - # but the actual *values* of the headers of a WebException don't get serialized - # when the RemoteException wraps it. To work around that, we'll extract the - # information that we actually care about *now*, and then we'll throw our own exception - # that is just a JSON object with the data that we'll later extract for processing in - # the main catch. - $ex = @{} - $ex.Message = $_.Exception.Message - $ex.StatusCode = $_.Exception.Response.StatusCode - $ex.StatusDescription = $_.Exception.Response.StatusDescription - $ex.InnerMessage = $_.ErrorDetails.Message - try - { - $ex.RawContent = Get-HttpWebResponseContent -WebResponse $_.Exception.Response - } - catch - { - Write-Log -Message "Unable to retrieve the raw HTTP Web Response:" -Exception $_ -Level Warning - } - - $jsonConversionDepth = 20 # Seems like it should be more than sufficient - throw (ConvertTo-Json -InputObject $ex -Depth $jsonConversionDepth) - } - } - - $null = Start-Job -Name $jobName -ScriptBlock $scriptBlock -Arg @( - $uri, - $method, - $headers, - $bodyAsBytes, - (Get-GitHubConfiguration -Name WebRequestTimeoutSec), - $PSScriptRoot) + $params = @{} + $params.Add("Uri", $uri) + $params.Add("Method", $method) + $params.Add("Headers", $headers) + $params.Add("UseDefaultCredentials", $true) + $params.Add("UseBasicParsing", $true) + $params.Add("TimeoutSec", (Get-GitHubConfiguration -Name WebRequestTimeoutSec)) + $params.Add("Body", $bodyAsBytes) - if ($PSCmdlet.ShouldProcess($jobName, "Wait-JobWithAnimation")) - { - $description = 'Sending telemetry data' - Wait-JobWithAnimation -Name $jobName -Description $Description - } - - if ($PSCmdlet.ShouldProcess($jobName, "Receive-Job")) - { - $result = Receive-Job $jobName -AutoRemoveJob -Wait -ErrorAction SilentlyContinue -ErrorVariable remoteErrors - } - } + # Disable Progress Bar in function scope during Invoke-WebRequest + $ProgressPreference = 'SilentlyContinue' - if ($remoteErrors.Count -gt 0) - { - throw $remoteErrors[0].Exception - } - } - - return $result + return Invoke-WebRequest @params } catch { - # We only know how to handle WebExceptions, which will either come in "pure" when running with -NoStatus, - # or will come in as a RemoteException when running normally (since it's coming from the asynchronous Job). $ex = $null $message = $null $statusCode = $null @@ -301,27 +196,6 @@ function Invoke-SendTelemetryEvent Write-Log -Message "Unable to retrieve the raw HTTP Web Response:" -Exception $_ -Level Warning } } - elseif (($_.Exception -is [System.Management.Automation.RemoteException]) -and - ($_.Exception.SerializedRemoteException.PSObject.TypeNames[0] -eq 'Deserialized.System.Management.Automation.RuntimeException')) - { - $ex = $_.Exception - try - { - $deserialized = $ex.Message | ConvertFrom-Json - $message = $deserialized.Message - $statusCode = $deserialized.StatusCode - $statusDescription = $deserialized.StatusDescription - $innerMessage = $deserialized.InnerMessage - $rawContent = $deserialized.RawContent - } - catch [System.ArgumentException] - { - # Will be thrown if $ex.Message isn't the JSON content we prepared - # in the System.Net.WebException handler earlier in $scriptBlock. - Write-Log -Exception $_ -Level Error - throw - } - } else { Write-Log -Exception $_ -Level Error @@ -402,32 +276,16 @@ function Set-TelemetryEvent A collection of name/value pair metrics (string/double) that should be associated with this event. - .PARAMETER NoStatus - If this switch is specified, long-running commands will run on the main thread - with no commandline status update. When not specified, those commands run in - the background, enabling the command prompt to provide status information. - .EXAMPLE Set-TelemetryEvent "zFooTest1" - Posts a "zFooTest1" event with the default set of properties and metrics. If the telemetry - client needs to be created to accomplish this, and the required assemblies are not available - on the local machine, the download status will be presented at the command prompt. + Posts a "zFooTest1" event with the default set of properties and metrics. .EXAMPLE Set-TelemetryEvent "zFooTest1" @{"Prop1" = "Value1"} Posts a "zFooTest1" event with the default set of properties and metrics along with an - additional property named "Prop1" with a value of "Value1". If the telemetry client - needs to be created to accomplish this, and the required assemblies are not available - on the local machine, the download status will be presented at the command prompt. - - .EXAMPLE - Set-TelemetryEvent "zFooTest1" -NoStatus - - Posts a "zFooTest1" event with the default set of properties and metrics. If the telemetry - client needs to be created to accomplish this, and the required assemblies are not available - on the local machine, the command prompt will appear to hang while they are downloaded. + additional property named "Prop1" with a value of "Value1". .NOTES Because of the short-running nature of this module, we always "flush" the events as soon @@ -441,9 +299,7 @@ function Set-TelemetryEvent [hashtable] $Properties = @{}, - [hashtable] $Metrics = @{}, - - [switch] $NoStatus + [hashtable] $Metrics = @{} ) if (Get-GitHubConfiguration -Name DisableTelemetry) @@ -478,7 +334,7 @@ function Set-TelemetryEvent Add-Member -InputObject $telemetryEvent.data.baseData -Name 'measurements' -Value ([PSCustomObject] $measurements) -MemberType NoteProperty -Force } - $null = Invoke-SendTelemetryEvent -TelemetryEvent $telemetryEvent -NoStatus:$NoStatus + $null = Invoke-SendTelemetryEvent -TelemetryEvent $telemetryEvent } catch { @@ -521,26 +377,11 @@ function Set-TelemetryException prevents that automatic flushing (helpful in the scenario where the exception occurred when trying to do the actual Flush). - .PARAMETER NoStatus - If this switch is specified, long-running commands will run on the main thread - with no commandline status update. When not specified, those commands run in - the background, enabling the command prompt to provide status information. - .EXAMPLE Set-TelemetryException $_ Used within the context of a catch statement, this will post the exception that just - occurred, along with a default set of properties. If the telemetry client needs to be - created to accomplish this, and the required assemblies are not available on the local - machine, the download status will be presented at the command prompt. - - .EXAMPLE - Set-TelemetryException $_ -NoStatus - - Used within the context of a catch statement, this will post the exception that just - occurred, along with a default set of properties. If the telemetry client needs to be - created to accomplish this, and the required assemblies are not available on the local - machine, the command prompt will appear to hang while they are downloaded. + occurred, along with a default set of properties. .NOTES Because of the short-running nature of this module, we always "flush" the events as soon @@ -554,9 +395,7 @@ function Set-TelemetryException [string] $ErrorBucket, - [hashtable] $Properties = @{}, - - [switch] $NoStatus + [hashtable] $Properties = @{} ) if (Get-GitHubConfiguration -Name DisableTelemetry) @@ -627,7 +466,7 @@ function Set-TelemetryException } Add-Member -InputObject $telemetryEvent.data.baseData -Name 'exceptions' -Value @($exceptionData) -MemberType NoteProperty -Force - $null = Invoke-SendTelemetryEvent -TelemetryEvent $telemetryEvent -NoStatus:$NoStatus + $null = Invoke-SendTelemetryEvent -TelemetryEvent $telemetryEvent } catch { diff --git a/Tests/Common.ps1 b/Tests/Common.ps1 index 2e5d5512..054eb7e8 100644 --- a/Tests/Common.ps1 +++ b/Tests/Common.ps1 @@ -98,7 +98,7 @@ function Initialize-CommonTestSetup Reset-GitHubConfiguration Set-GitHubConfiguration -DisableTelemetry # We don't want UT's to impact telemetry Set-GitHubConfiguration -LogRequestBody # Make it easier to debug UT failures - Set-GitHubConfiguration -DefaultNoStatus # Status corrupts the raw CI logs for Linux and Mac, and makes runs take slightly longer. + Set-GitHubConfiguration -MultiRequestProgressThreshold 0 # Status corrupts the raw CI logs for Linux and Mac, and makes runs take slightly longer. Set-GitHubConfiguration -DisableUpdateCheck # The update check is unnecessary during tests. } diff --git a/Tests/GitHubContents.tests.ps1 b/Tests/GitHubContents.tests.ps1 index ba1ef8d4..c1b9661e 100644 --- a/Tests/GitHubContents.tests.ps1 +++ b/Tests/GitHubContents.tests.ps1 @@ -358,7 +358,6 @@ try CommitterEmail = $committerEmail authorName = $authorName authorEmail = $authorEmail - NoStatus = $true } $result = Set-GitHubContent @setGitHubContentParms From 4576caed5f5658545101fd608a87feb08f807411 Mon Sep 17 00:00:00 2001 From: Howard Wolosky Date: Mon, 29 Jun 2020 15:05:34 -0700 Subject: [PATCH 2/3] CBH update --- GitHubConfiguration.ps1 | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/GitHubConfiguration.ps1 b/GitHubConfiguration.ps1 index 28ab414d..7c50f858 100644 --- a/GitHubConfiguration.ps1 +++ b/GitHubConfiguration.ps1 @@ -79,8 +79,9 @@ function Set-GitHubConfiguration .PARAMETER DefaultNoStatus Control if the -NoStatus switch should be passed-in by default to all methods. The -NoStatus switch has been deprecated. Commands in this module no longer display status - on the console, and therefore passing in -NoStatus to a command has no effect. - Therefore, the value of this configuration setting has no impact on command execution. + on the console, thus passing in -NoStatus to a command no longer has any impact. + Therefore, the value of this configuration setting also no longer has any impact on + command execution. .PARAMETER DefaultOwnerName The owner name that should be used with a command that takes OwnerName as a parameter From 7881d7d3af740430479855a2206eccb8669d4adf Mon Sep 17 00:00:00 2001 From: Howard Wolosky Date: Tue, 30 Jun 2020 12:25:35 -0700 Subject: [PATCH 3/3] Add back -WhatIf support to core methods until #254 is in. --- GitHubCore.ps1 | 14 ++++++++++++-- Telemetry.ps1 | 7 ++++++- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/GitHubCore.ps1 b/GitHubCore.ps1 index 3281a780..019ef6ac 100644 --- a/GitHubCore.ps1 +++ b/GitHubCore.ps1 @@ -105,7 +105,7 @@ function Invoke-GHRestMethod This wraps Invoke-WebRequest as opposed to Invoke-RestMethod because we want access to the headers that are returned in the response, and Invoke-RestMethod drops those headers. #> - [CmdletBinding()] + [CmdletBinding(SupportsShouldProcess)] param( [Parameter(Mandatory)] [string] $UriFragment, @@ -194,6 +194,11 @@ function Invoke-GHRestMethod $headers.Add("Content-Type", "application/json; charset=UTF-8") } + if (-not $PSCmdlet.ShouldProcess($url, "Invoke-WebRequest")) + { + return + } + $originalSecurityProtocol = [Net.ServicePointManager]::SecurityProtocol try @@ -527,7 +532,7 @@ function Invoke-GHRestMethodMultipleResult but the request happens in the foreground and there is no additional status shown to the user until a response is returned from the REST request. #> - [CmdletBinding()] + [CmdletBinding(SupportsShouldProcess)] [Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSReviewUnusedParameter", "", Justification="One or more parameters (like NoStatus) are only referenced by helper methods which get access to it from the stack via Get-Variable -Scope 1.")] [OutputType([Object[]])] param( @@ -552,6 +557,11 @@ function Invoke-GHRestMethodMultipleResult [switch] $NoStatus ) + if (-not $PSCmdlet.ShouldProcess($UriFragment, "Invoke-GHRestMethod")) + { + return + } + $AccessToken = Get-AccessToken -AccessToken $AccessToken $stopwatch = [System.Diagnostics.Stopwatch]::StartNew() diff --git a/Telemetry.ps1 b/Telemetry.ps1 index 2ddffbc1..857bc373 100644 --- a/Telemetry.ps1 +++ b/Telemetry.ps1 @@ -139,7 +139,7 @@ function Invoke-SendTelemetryEvent Invoke-* methods share a common base code. Leaving this as-is to make this file easier to share out with other PowerShell projects. #> - [CmdletBinding()] + [CmdletBinding(SupportsShouldProcess)] param( [Parameter(Mandatory)] [PSCustomObject] $TelemetryEvent @@ -153,6 +153,11 @@ function Invoke-SendTelemetryEvent $body = ConvertTo-Json -InputObject $TelemetryEvent -Depth $jsonConversionDepth -Compress $bodyAsBytes = [System.Text.Encoding]::UTF8.GetBytes($body) + if (-not $PSCmdlet.ShouldProcess($uri, "Invoke-WebRequest")) + { + return + } + try { Write-Log -Message "Sending telemetry event data to $uri [Timeout = $(Get-GitHubConfiguration -Name WebRequestTimeoutSec))]" -Level Verbose