Enforce Get-Help offline dropback when page or internet is down #752
Conversation
rjmholt
requested review from
TylerLeonhardt,
rkeithhill and
SeeminglyScience
|
I've ensured that the help API I use to get the URI is available back to PS v3. Any other considerations on that front? Maybe IWR isn't available??? |
|
Honestly I designed my addition to only check if it could launch the browser. This will definitely make that better for the scenario where you don't have internet, or the site is down. I'll try to review this evening or tomorrow morning. |
|
My only concern about my change is that it will be slower -- so will let everyone else decide :) |
|
Oh, according to the docs, iwr is available in v3 - https://docs.microsoft.com/en-us/powershell/module/microsoft.powershell.utility/invoke-webrequest?view=powershell-6 |
|
|
||
| if (string.IsNullOrEmpty(helpParams)) { helpParams = "Get-Help"; } | ||
|
|
||
| PSCommand checkHelpPSCommand = new PSCommand() | ||
| .AddScript(CheckHelpScript, useLocalScope: true) | ||
| .AddArgument(helpParams); | ||
|
|
||
| // TODO: Rather than print the help in the console, we should send the string back |
There was a problem hiding this comment.
This actually fits nicely into the changes that PowerShell/vscode-powershell#884 morphed into.
| [System.Net.ServicePointManager]::SecurityProtocol = [System.Net.SecurityProtocolType]::Tls12 | ||
|
|
||
| # HEAD means we don't need the content itself back :) | ||
| $status = (Invoke-WebRequest -Method Head -Uri $helpUri -ErrorAction Stop).StatusCode |
There was a problem hiding this comment.
If we're overly concerned about speed, we could include -TimeoutSec. However, as per the docs this could still take up to 15 seconds if DNS resolution is slow. If we're in an offline state, is the speed of the timeout that big of an issue?
There was a problem hiding this comment.
I would assume in an offline state we'd fail pretty quickly (testing it by unplugging my ethernet cable just then, offline is pretty quick).
But yeah, maybe we should add an explicit timeout. The lower it is the less of a performance hit we take when internet is down in a weird way, but the more likely we will accidentally get offline help for people with slow-but-working internet access.
There was a problem hiding this comment.
5 seconds seems reasonable. I've seen latency as high as 3 - 4 seconds on high speed, but at that point you're better off thinking that you're offline.
There was a problem hiding this comment.
Yeah. Just played with the timeout. Doesn't affect PS Core much either way, but Windows PowerShell seems to much prefer having a 5s timeout
|
Ok I've verified that the latest change works on my machine (Win 10) |
|
Wow while testing this I've discovered that you literally cannot start VSCode 1.27.2 without internet connection... |
| [System.Net.ServicePointManager]::SecurityProtocol = [System.Net.SecurityProtocolType]::Tls12 | ||
|
|
||
| # HEAD means we don't need the content itself back, just the response header | ||
| $status = (Invoke-WebRequest -Method Head -Uri $helpUri -TimeoutSec 5 -ErrorAction Stop).StatusCode |
There was a problem hiding this comment.
Invoke-WebRequest -> Microsoft.PowerShell.Utility\Invoke-WebRequest
I noticed it was hard to get the offline behaviour in @corbob's awesome addition to fire, so I made a minor edit where we test if the help page is accessible first.
@corbob GitHub won't let me add you as a reviewer, but if you've got time to review please do!