Skip to content

Fix HTTP status from 409 to 429 for WebCmdlets to get retry interval from Retry-After header. #19622

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
daxian-dbw merged 3 commits into from
May 8, 2023
Merged

Fix HTTP status from 409 to 429 for WebCmdlets to get retry interval from Retry-After header. #19622

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3a1b998
Fix status code to get Retry-After from 409 to 429
mkht May 6, 2023
8848f05
Add tests for Retry-After in http 429 status
mkht May 8, 2023
4a1d067
Change success/failure judgment to verbose message instead of elapsed…
mkht May 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ...owerShell.Commands.Utility/commands/utility/WebCmdlet/Common/WebRequestPSCmdlet.Common.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1339,7 +1339,7 @@ internal virtual HttpResponseMessage GetResponse(HttpClient client, HttpRequestM

// If the status code is 429 get the retry interval from the Headers.
// Ignore broken header and its value.
if (response.StatusCode is HttpStatusCode.Conflict && response.Headers.TryGetValues(HttpKnownHeaderNames.RetryAfter, out IEnumerable<string> retryAfter))
if (response.StatusCode is HttpStatusCode.TooManyRequests && response.Headers.TryGetValues(HttpKnownHeaderNames.RetryAfter, out IEnumerable<string> retryAfter))
Copy link
Collaborator

@iSazonov iSazonov May 7, 2023

Choose a reason for hiding this comment

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

It would be great to cover this by a test.

Copy link
Contributor

@CarloToso CarloToso May 7, 2023

Choose a reason for hiding this comment

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

@iSazonov I can look into it

{
try
{
Expand Down
64 changes: 64 additions & 0 deletions test/powershell/Modules/Microsoft.PowerShell.Utility/WebCmdlets.Tests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2157,6 +2157,38 @@ Describe "Invoke-WebRequest tests" -Tags "Feature", "RequireAdminOnWindows" {
$jsonResult = $result.output.Content | ConvertFrom-Json
$jsonResult.SessionId | Should -BeExactly $sessionId
}

It "Invoke-WebRequest respects the Retry-After header value in 429 status" {
Copy link
Collaborator

@iSazonov iSazonov May 8, 2023

Choose a reason for hiding this comment

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

Can you confirm the tests fail before the change?

Copy link
Contributor Author

@mkht mkht May 8, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I've already tested it. Before the change (current master branch) these tests failed.

Executing script F:\PowerShell\test\powershell\Modules\Microsoft.PowerShell.Utility\WebCmdlets.Tests.ps1

  Describing Invoke-WebRequest tests
WARNING: This module is use-at-your-own-risk.
It exists to test PowerShell Core builds and is not supported by Microsoft.
Please report any issues at https://github.com/rjmholt/SelfSignedCertificate.
Certificate written to C:\Users\mkht\AppData\Local\Temp\ServerCert.pfx
WARNING: Parameter 'EmailAddress' is obsolete. The email name component is deprecated by the PKIX standard
Certificate written to C:\Users\mkht\AppData\Local\Temp\ClientCert.pfx

    Context Invoke-WebRequest retry tests
      [+] Invoke-WebRequest can retry - specified number of times - error 304 2.15s
      [+] Invoke-WebRequest can retry - specified number of times - error 400 3.07s
      [+] Invoke-WebRequest can retry - specified number of times - error 599 1.02s
      [+] Invoke-WebRequest can retry - specified number of times - error 404 2.04s
      [+] Invoke-WebRequest can retry - when retry count is higher than failure count 2.03s
      [+] Invoke-WebRequest should fail when failureCount is greater than MaximumRetryCount 1.04s
      [+] Invoke-WebRequest can retry with POST 1.02s
      [-] Invoke-WebRequest respects the Retry-After header value in 429 status 6.05s
        Expected the actual value to be less than 2.9, but got 6.0393409.
        2179:             $stopWatch.Elapsed.TotalSeconds | Should -BeLessThan 2.9
        at <ScriptBlock>, F:\PowerShell\test\powershell\Modules\Microsoft.PowerShell.Utility\WebCmdlets.Tests.ps1: line 2179
      [-] Invoke-WebRequest ignores the Retry-After header value NOT in 429 status 8.05s
        Expected the actual value to be less than 2.9, but got 8.0427231.
        2200:             $stopWatch.Elapsed.TotalSeconds | Should -BeLessThan 2.9
        at <ScriptBlock>, F:\PowerShell\test\powershell\Modules\Microsoft.PowerShell.Utility\WebCmdlets.Tests.ps1: line 2200

  Describing Invoke-RestMethod tests

    Context Invoke-RestMethod retry tests
      [+] Invoke-RestMethod can retry - specified number of times - error 304 2.04s
      [+] Invoke-RestMethod can retry with POST 1.02s
      [-] Invoke-RestMethod respects the Retry-After header value in 429 status 6.03s
        Expected the actual value to be less than 2.9, but got 6.0209983.
        4111:             $stopWatch.Elapsed.TotalSeconds | Should -BeLessThan 2.9
        at <ScriptBlock>, F:\PowerShell\test\powershell\Modules\Microsoft.PowerShell.Utility\WebCmdlets.Tests.ps1: line 4111
      [-] Invoke-RestMethod ignores the Retry-After header value NOT in 429 status 8.05s
        Expected the actual value to be less than 2.9, but got 8.0350285.
        4132:             $stopWatch.Elapsed.TotalSeconds | Should -BeLessThan 2.9
        at <ScriptBlock>, F:\PowerShell\test\powershell\Modules\Microsoft.PowerShell.Utility\WebCmdlets.Tests.ps1: line 4132

Tests completed in 128.49s
Tests Passed: 557, Failed: 4, Skipped: 4, Pending: 34, Inconclusive: 0


$Query = @{
statusCode = 429
reposnsephrase = 'Too Many Requests'
contenttype = 'application/json'
body = '{"message":"oops"}'
headers = '{"Retry-After":"1"}'
}
$uri = Get-WebListenerUrl -Test 'Response' -Query $Query
$verboseFile = Join-Path $TestDrive -ChildPath verbose.txt
$result = Invoke-WebRequest -Uri $uri -MaximumRetryCount 1 -RetryIntervalSec 3 -SkipHttpErrorCheck -Verbose 4>$verbosefile

$verboseFile | Should -FileContentMatch 'Retrying after interval of 1 seconds. Status code for previous attempt: TooManyRequests'
}

It "Invoke-WebRequest ignores the Retry-After header value NOT in 429 status" {

$Query = @{
statusCode = 409
reposnsephrase = 'Conflict'
contenttype = 'application/json'
body = '{"message":"oops"}'
headers = '{"Retry-After":"1"}'
}
$uri = Get-WebListenerUrl -Test 'Response' -Query $Query
$verboseFile = Join-Path $TestDrive -ChildPath verbose.txt
$result = Invoke-WebRequest -Uri $uri -MaximumRetryCount 1 -RetryIntervalSec 3 -SkipHttpErrorCheck -Verbose 4>$verbosefile

$verboseFile | Should -FileContentMatch 'Retrying after interval of 3 seconds. Status code for previous attempt: Conflict'
}
}

Context "Regex Parsing" {
Expand Down Expand Up @@ -4047,6 +4079,38 @@ Describe "Invoke-RestMethod tests" -Tags "Feature", "RequireAdminOnWindows" {
$result.output.failureResponsesSent | Should -Be 1
$result.output.sessionId | Should -BeExactly $sessionId
}

It "Invoke-RestMethod respects the Retry-After header value in 429 status" {

$Query = @{
statusCode = 429
reposnsephrase = 'Too Many Requests'
contenttype = 'application/json'
body = '{"message":"oops"}'
headers = '{"Retry-After":"1"}'
}
$uri = Get-WebListenerUrl -Test 'Response' -Query $Query
$verboseFile = Join-Path $TestDrive -ChildPath verbose.txt
$result = Invoke-RestMethod -Uri $uri -MaximumRetryCount 1 -RetryIntervalSec 3 -SkipHttpErrorCheck -Verbose 4>$verbosefile

$verboseFile | Should -FileContentMatch 'Retrying after interval of 1 seconds. Status code for previous attempt: TooManyRequests'
}

It "Invoke-RestMethod ignores the Retry-After header value NOT in 429 status" {

$Query = @{
statusCode = 409
reposnsephrase = 'Conflict'
contenttype = 'application/json'
body = '{"message":"oops"}'
headers = '{"Retry-After":"1"}'
}
$uri = Get-WebListenerUrl -Test 'Response' -Query $Query
$verboseFile = Join-Path $TestDrive -ChildPath verbose.txt
$result = Invoke-RestMethod -Uri $uri -MaximumRetryCount 1 -RetryIntervalSec 3 -SkipHttpErrorCheck -Verbose 4>$verbosefile

$verboseFile | Should -FileContentMatch 'Retrying after interval of 3 seconds. Status code for previous attempt: Conflict'
}
}
}

Expand Down