GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |||||||||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |||||||||||||||||||||||||||||||
GeezerVison is showing github.com — Fix HTTP status from 409 to 429 for WebCmdlets to get retry interval from Retry-After header. by mkht · Pull Request #19622 · PowerShell/PowerShell Conversation
Contributor
PR SummaryThe code that gets the appropriate retry interval from the Retry-After response header is incorrect and is executing with status code 409 when it should be executing with status code 429. This PR will correct that. The original code was implemented in PR #18717. PR Checklist
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
Contributor
Author
All reactions
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page.
Contributor
All reactions
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page.
Ilya (iSazonov)
approved these changes
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would be great to cover this by a test.
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ilya (@iSazonov) I can look into it
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
6 of 22 tasks
Contributor
Author
All reactions
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you confirm the tests fail before the change?
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've already tested it. Before the change (current 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
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
|
There was an error while loading. Please reload this page.
|
mkht Don't revert your changes, I'll close my PR. I think you should use the -Verbose redirect method from my PR instead of using |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
This PR has Quantification details
Why proper sizing of changes matters
Optimal pull request sizes drive a better predictable PR flow as they strike a
What can I do to optimize my changes
How to interpret the change counts in git diff output
Was this comment helpful? 👍 :ok_hand: :thumbsdown: (Email) |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
I followed CarloToso advice and changed the method of judging the test to using verbose messages. I confirmed that the test failed with the pre-modified code and succeeded with the modified code. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Thanks mkht for the contribution!
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
🎉 Handy links: |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
…from Retry-After header. (PowerShell#19622)
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
Retries in WebCmdlet does not respect Retry-After header in 429 statusInvoke-RestMethod and Invoke-WebRequest do not respect Retry-After header in 429 responses.
There was an error while loading. Please reload this page.