Skip to content

Commit ad15657

Browse files
HowardWoloskyHowardWolosky
authored
Fix handling of -WhatIf in core functions (#213)
WhatIf support has now been simiplified in all API's that eventually call into `Invoke-GHRestMethod`. There's now a single check at the top of that function which checks if it should continue or not. If it shouldn't, it early returns in order to avoid any code that might access uninitialized content from outside of the ShouldProcess blocks later on. Resolves #197
1 parent ff6ab5c commit ad15657

File tree

1 file changed

+82
-89
lines changed

1 file changed

+82
-89
lines changed

‎GitHubCore.ps1‎

Lines changed: 82 additions & 89 deletions
Original file line numberDiff line numberDiff line change
@@ -193,130 +193,123 @@ function Invoke-GHRestMethod
193193
$headers.Add("Content-Type", "application/json; charset=UTF-8")
194194
}
195195

196+
if (-not $PSCmdlet.ShouldProcess($url, "Invoke-WebRequest"))
197+
{
198+
return
199+
}
200+
196201
try
197202
{
198203
Write-Log -Message $Description -Level Verbose
199204
Write-Log -Message "Accessing [$Method] $url [Timeout = $(Get-GitHubConfiguration -Name WebRequestTimeoutSec))]" -Level Verbose
200205

206+
$result = $null
201207
$NoStatus = Resolve-ParameterWithDefaultConfigurationValue -Name NoStatus -ConfigValueName DefaultNoStatus
202208
if ($NoStatus)
203209
{
204-
if ($PSCmdlet.ShouldProcess($url, "Invoke-WebRequest"))
210+
$params = @{}
211+
$params.Add("Uri", $url)
212+
$params.Add("Method", $Method)
213+
$params.Add("Headers", $headers)
214+
$params.Add("UseDefaultCredentials", $true)
215+
$params.Add("UseBasicParsing", $true)
216+
$params.Add("TimeoutSec", (Get-GitHubConfiguration -Name WebRequestTimeoutSec))
217+
218+
if ($Method -in $ValidBodyContainingRequestMethods -and (-not [String]::IsNullOrEmpty($Body)))
219+
{
220+
$bodyAsBytes = [System.Text.Encoding]::UTF8.GetBytes($Body)
221+
$params.Add("Body", $bodyAsBytes)
222+
Write-Log -Message "Request includes a body." -Level Verbose
223+
if (Get-GitHubConfiguration -Name LogRequestBody)
224+
{
225+
Write-Log -Message $Body -Level Verbose
226+
}
227+
}
228+
229+
[Net.ServicePointManager]::SecurityProtocol=[Net.SecurityProtocolType]::Tls12
230+
$result = Invoke-WebRequest @params
231+
if ($Method -eq 'Delete')
205232
{
233+
Write-Log -Message "Successfully removed." -Level Verbose
234+
}
235+
}
236+
else
237+
{
238+
$jobName = "Invoke-GHRestMethod-" + (Get-Date).ToFileTime().ToString()
239+
240+
[scriptblock]$scriptBlock = {
241+
param($Url, $Method, $Headers, $Body, $ValidBodyContainingRequestMethods, $TimeoutSec, $LogRequestBody, $ScriptRootPath)
242+
243+
# We need to "dot invoke" Helpers.ps1 and GitHubConfiguration.ps1 within
244+
# the context of this script block since we're running in a different
245+
# PowerShell process and need access to Get-HttpWebResponseContent and
246+
# config values referenced within Write-Log.
247+
. (Join-Path -Path $ScriptRootPath -ChildPath 'Helpers.ps1')
248+
. (Join-Path -Path $ScriptRootPath -ChildPath 'GitHubConfiguration.ps1')
249+
206250
$params = @{}
207-
$params.Add("Uri", $url)
251+
$params.Add("Uri", $Url)
208252
$params.Add("Method", $Method)
209-
$params.Add("Headers", $headers)
253+
$params.Add("Headers", $Headers)
210254
$params.Add("UseDefaultCredentials", $true)
211255
$params.Add("UseBasicParsing", $true)
212-
$params.Add("TimeoutSec", (Get-GitHubConfiguration -Name WebRequestTimeoutSec))
256+
$params.Add("TimeoutSec", $TimeoutSec)
213257

214258
if ($Method -in $ValidBodyContainingRequestMethods -and (-not [String]::IsNullOrEmpty($Body)))
215259
{
216260
$bodyAsBytes = [System.Text.Encoding]::UTF8.GetBytes($Body)
217261
$params.Add("Body", $bodyAsBytes)
218262
Write-Log -Message "Request includes a body." -Level Verbose
219-
if (Get-GitHubConfiguration -Name LogRequestBody)
263+
if ($LogRequestBody)
220264
{
221265
Write-Log -Message $Body -Level Verbose
222266
}
223267
}
224268

225-
[Net.ServicePointManager]::SecurityProtocol=[Net.SecurityProtocolType]::Tls12
226-
$result = Invoke-WebRequest @params
227-
if ($Method -eq 'Delete')
269+
try
228270
{
229-
Write-Log -Message "Successfully removed." -Level Verbose
271+
[Net.ServicePointManager]::SecurityProtocol=[Net.SecurityProtocolType]::Tls12
272+
Invoke-WebRequest @params
230273
}
231-
}
232-
}
233-
else
234-
{
235-
$jobName = "Invoke-GHRestMethod-" + (Get-Date).ToFileTime().ToString()
236-
237-
if ($PSCmdlet.ShouldProcess($jobName, "Start-Job"))
238-
{
239-
[scriptblock]$scriptBlock = {
240-
param($Url, $Method, $Headers, $Body, $ValidBodyContainingRequestMethods, $TimeoutSec, $LogRequestBody, $ScriptRootPath)
241-
242-
# We need to "dot invoke" Helpers.ps1 and GitHubConfiguration.ps1 within
243-
# the context of this script block since we're running in a different
244-
# PowerShell process and need access to Get-HttpWebResponseContent and
245-
# config values referenced within Write-Log.
246-
. (Join-Path -Path $ScriptRootPath -ChildPath 'Helpers.ps1')
247-
. (Join-Path -Path $ScriptRootPath -ChildPath 'GitHubConfiguration.ps1')
248-
249-
$params = @{}
250-
$params.Add("Uri", $Url)
251-
$params.Add("Method", $Method)
252-
$params.Add("Headers", $Headers)
253-
$params.Add("UseDefaultCredentials", $true)
254-
$params.Add("UseBasicParsing", $true)
255-
$params.Add("TimeoutSec", $TimeoutSec)
256-
257-
if ($Method -in $ValidBodyContainingRequestMethods -and (-not [String]::IsNullOrEmpty($Body)))
258-
{
259-
$bodyAsBytes = [System.Text.Encoding]::UTF8.GetBytes($Body)
260-
$params.Add("Body", $bodyAsBytes)
261-
Write-Log -Message "Request includes a body." -Level Verbose
262-
if ($LogRequestBody)
263-
{
264-
Write-Log -Message $Body -Level Verbose
265-
}
266-
}
267-
274+
catch [System.Net.WebException]
275+
{
276+
# We need to access certain headers in the exception handling,
277+
# but the actual *values* of the headers of a WebException don't get serialized
278+
# when the RemoteException wraps it. To work around that, we'll extract the
279+
# information that we actually care about *now*, and then we'll throw our own exception
280+
# that is just a JSON object with the data that we'll later extract for processing in
281+
# the main catch.
282+
$ex = @{}
283+
$ex.Message = $_.Exception.Message
284+
$ex.StatusCode = $_.Exception.Response.StatusCode
285+
$ex.StatusDescription = $_.Exception.Response.StatusDescription
286+
$ex.InnerMessage = $_.ErrorDetails.Message
268287
try
269288
{
270-
[Net.ServicePointManager]::SecurityProtocol=[Net.SecurityProtocolType]::Tls12
271-
Invoke-WebRequest @params
289+
$ex.RawContent = Get-HttpWebResponseContent -WebResponse $_.Exception.Response
272290
}
273-
catch [System.Net.WebException]
291+
catch
274292
{
275-
# We need to access certain headers in the exception handling,
276-
# but the actual *values* of the headers of a WebException don't get serialized
277-
# when the RemoteException wraps it. To work around that, we'll extract the
278-
# information that we actually care about *now*, and then we'll throw our own exception
279-
# that is just a JSON object with the data that we'll later extract for processing in
280-
# the main catch.
281-
$ex = @{}
282-
$ex.Message = $_.Exception.Message
283-
$ex.StatusCode = $_.Exception.Response.StatusCode
284-
$ex.StatusDescription = $_.Exception.Response.StatusDescription
285-
$ex.InnerMessage = $_.ErrorDetails.Message
286-
try
287-
{
288-
$ex.RawContent = Get-HttpWebResponseContent -WebResponse $_.Exception.Response
289-
}
290-
catch
291-
{
292-
Write-Log -Message "Unable to retrieve the raw HTTP Web Response:" -Exception $_ -Level Warning
293-
}
294-
295-
throw (ConvertTo-Json -InputObject $ex -Depth 20)
293+
Write-Log -Message "Unable to retrieve the raw HTTP Web Response:" -Exception $_ -Level Warning
296294
}
297-
}
298295

299-
$null = Start-Job -Name $jobName -ScriptBlock $scriptBlock -Arg @(
300-
$url,
301-
$Method,
302-
$headers,
303-
$Body,
304-
$ValidBodyContainingRequestMethods,
305-
(Get-GitHubConfiguration -Name WebRequestTimeoutSec),
306-
(Get-GitHubConfiguration -Name LogRequestBody),
307-
$PSScriptRoot)
308-
309-
if ($PSCmdlet.ShouldProcess($jobName, "Wait-JobWithAnimation"))
310-
{
311-
Wait-JobWithAnimation -Name $jobName -Description $Description
312-
}
313-
314-
if ($PSCmdlet.ShouldProcess($jobName, "Receive-Job"))
315-
{
316-
$result = Receive-Job $jobName -AutoRemoveJob -Wait -ErrorAction SilentlyContinue -ErrorVariable remoteErrors
296+
throw (ConvertTo-Json -InputObject $ex -Depth 20)
317297
}
318298
}
319299

300+
$null = Start-Job -Name $jobName -ScriptBlock $scriptBlock -Arg @(
301+
$url,
302+
$Method,
303+
$headers,
304+
$Body,
305+
$ValidBodyContainingRequestMethods,
306+
(Get-GitHubConfiguration -Name WebRequestTimeoutSec),
307+
(Get-GitHubConfiguration -Name LogRequestBody),
308+
$PSScriptRoot)
309+
310+
Wait-JobWithAnimation -Name $jobName -Description $Description
311+
$result = Receive-Job $jobName -AutoRemoveJob -Wait -ErrorAction SilentlyContinue -ErrorVariable remoteErrors
312+
320313
if ($remoteErrors.Count -gt 0)
321314
{
322315
throw $remoteErrors[0].Exception

0 commit comments

Comments
 (0)