Skip to content

Enforce Get-Help offline dropback when page or internet is down #752

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
merged 7 commits into from
Sep 28, 2018
Merged

Enforce Get-Help offline dropback when page or internet is down #752

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
c6a31d2
Ensure local help is displayed in the console
Sep 27, 2018
f15a58a
[Style] Add newline in ShowHelpRequest.cs
Sep 27, 2018
706e0c6
Add iwr online help check, fall back to local help more intelligently
Sep 27, 2018
c5ce6b1
Use modern TLS version to get help
Sep 28, 2018
e8e13a7
Simplify try finally
Sep 28, 2018
01f3b60
Add 5s timeout to IWR ping of help page
Sep 28, 2018
d2b82f1
Module-qualify IWR call
Sep 28, 2018
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
3 changes: 2 additions & 1 deletion src/PowerShellEditorServices.Protocol/LanguageServer/ShowHelpRequest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
//

using Microsoft.PowerShell.EditorServices.Protocol.MessageProtocol;
using System;
using Microsoft.PowerShell.EditorServices.Protocol.MessageProtocol;

namespace Microsoft.PowerShell.EditorServices.Protocol.LanguageServer
{
Expand All @@ -16,6 +16,7 @@ public static readonly
RequestType<string, object, object, object> Type =
RequestType<string, object, object, object>.Create("powerShell/showOnlineHelp");
}

public class ShowHelpRequest
{
public static readonly
Expand Down
21 changes: 16 additions & 5 deletions src/PowerShellEditorServices.Protocol/Server/LanguageServer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,22 +254,33 @@ protected async Task HandleShowHelpRequest(
[String]$CommandName
)
try {
$null = Microsoft.PowerShell.Core\Get-Command $CommandName -ErrorAction Stop
$command = Microsoft.PowerShell.Core\Get-Command $CommandName -ErrorAction Stop
} catch [System.Management.Automation.CommandNotFoundException] {
$PSCmdlet.ThrowTerminatingError($PSItem)
}
try {
$null = Microsoft.PowerShell.Core\Get-Help $CommandName -Online
} catch [System.Management.Automation.PSInvalidOperationException] {
Microsoft.PowerShell.Core\Get-Help $CommandName -Full
}";
$helpUri = [Microsoft.PowerShell.Commands.GetHelpCodeMethods]::GetHelpUri($command)
# HEAD means we don't need the content itself back :)
$status = (Invoke-WebRequest -Method Head -Uri $helpUri -ErrorAction Stop).StatusCode
Copy link
Contributor

Choose a reason for hiding this comment

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

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

5s?

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

if ($status -lt 400)
{
$null = Microsoft.PowerShell.Core\Get-Help $CommandName -Online
return
}
} catch {
# Ignore - we want to drop out to Get-Help -Full
}
return Microsoft.PowerShell.Core\Get-Help $CommandName -Full
";

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
Copy link
Contributor

Choose a reason for hiding this comment

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

This actually fits nicely into the changes that PowerShell/vscode-powershell#884 morphed into.

// to VSCode to display in a help pop-up (or similar)
await editorSession.PowerShellContext.ExecuteCommand<PSObject>(checkHelpPSCommand, sendOutputToHost: true);
await requestContext.SendResult(null);
}
Expand Down