Skip to content

Commit

Permalink
Catch exceptions within completion handler
Browse files Browse the repository at this point in the history
We'll still see the same error in the log, but as a warning and not as
an exception bubbled up over LSP that causes some clients, like Kate, to
crash. We can't do anything about PowerShell's completion failing.
  • Loading branch information
andyleejordan committed Oct 24, 2022
1 parent d945e84 commit 0948131
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -69,16 +69,25 @@ public override async Task<CompletionList> Handle(CompletionParams request, Canc
int cursorColumn = request.Position.Character + 1;

ScriptFile scriptFile = _workspaceService.GetFile(request.TextDocument.Uri);
(bool isIncomplete, IReadOnlyList<CompletionItem> completionResults) = await GetCompletionsInFileAsync(
scriptFile,
cursorLine,
cursorColumn,
cancellationToken).ConfigureAwait(false);

// Treat completions trigged by space as incomplete so that `gci `
// and then typing `-` doesn't just filter the list of parameter values
// (typically files) returned by the space completion
return new CompletionList(completionResults, isIncomplete || request?.Context?.TriggerCharacter is " ");
try
{
(bool isIncomplete, IReadOnlyList<CompletionItem> completionResults) = await GetCompletionsInFileAsync(
scriptFile,
cursorLine,
cursorColumn,
cancellationToken).ConfigureAwait(false);

// Treat completions triggered by space as incomplete so that `gci `
// and then typing `-` doesn't just filter the list of parameter values
// (typically files) returned by the space completion
return new CompletionList(completionResults, isIncomplete || request?.Context?.TriggerCharacter is " ");
}
// We can't do anything about completions failing.
catch (Exception e)
{
_logger.LogWarning(e, "Exception occurred while running handling completion request");
return new CompletionList(isIncomplete: true);
}
}

// Handler for "completionItem/resolve". In VSCode this is fired when a completion item is highlighted in the completion list.
Expand Down
16 changes: 9 additions & 7 deletions test/PowerShellEditorServices.Test.E2E/LSPTestsFixures.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,9 @@ public class LSPTestsFixture : IAsyncLifetime
private const bool IsDebugAdapterTests = false;

public ILanguageClient PsesLanguageClient { get; private set; }
public List<Diagnostic> Diagnostics { get; set; }
internal List<PsesTelemetryEvent> TelemetryEvents { get; set; }
public List<LogMessageParams> Messages = new();
public List<Diagnostic> Diagnostics = new();
internal List<PsesTelemetryEvent> TelemetryEvents = new();
public ITestOutputHelper Output { get; set; }

protected PsesStdioProcess _psesProcess;
Expand All @@ -46,20 +47,21 @@ public async Task InitializeAsync()
_psesProcess = new PsesStdioProcess(factory, IsDebugAdapterTests);
await _psesProcess.Start().ConfigureAwait(false);

Diagnostics = new List<Diagnostic>();
TelemetryEvents = new List<PsesTelemetryEvent>();
DirectoryInfo testdir =
DirectoryInfo testDir =
Directory.CreateDirectory(Path.Combine(s_binDir, Path.GetRandomFileName()));

PsesLanguageClient = LanguageClient.PreInit(options =>
{
options
.WithInput(_psesProcess.OutputStream)
.WithOutput(_psesProcess.InputStream)
.WithWorkspaceFolder(DocumentUri.FromFileSystemPath(testdir.FullName), "testdir")
.WithWorkspaceFolder(DocumentUri.FromFileSystemPath(testDir.FullName), "testdir")
.WithInitializationOptions(new { EnableProfileLoading = false })
.OnPublishDiagnostics(diagnosticParams => Diagnostics.AddRange(diagnosticParams.Diagnostics.Where(d => d != null)))
.OnLogMessage(logMessageParams => Output?.WriteLine($"{logMessageParams.Type}: {logMessageParams.Message}"))
.OnLogMessage(logMessageParams => {
Output?.WriteLine($"{logMessageParams.Type}: {logMessageParams.Message}");
Messages.Add(logMessageParams);
})
.OnTelemetryEvent(telemetryEventParams => TelemetryEvents.Add(
new PsesTelemetryEvent
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ public class LanguageServerProtocolMessageTests : IClassFixture<LSPTestsFixture>
Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location);

private readonly ILanguageClient PsesLanguageClient;
private readonly List<LogMessageParams> Messages;
private readonly List<Diagnostic> Diagnostics;
private readonly List<PsesTelemetryEvent> TelemetryEvents;
private readonly string PwshExe;
Expand All @@ -46,6 +47,8 @@ public LanguageServerProtocolMessageTests(ITestOutputHelper output, LSPTestsFixt
{
data.Output = output;
PsesLanguageClient = data.PsesLanguageClient;
Messages = data.Messages;
Messages.Clear();
Diagnostics = data.Diagnostics;
Diagnostics.Clear();
TelemetryEvents = data.TelemetryEvents;
Expand Down Expand Up @@ -957,6 +960,36 @@ public async Task CanSendCompletionAndCompletionResolveRequestAsync()
Assert.Contains("Writes customized output to a host", updatedCompletionItem.Documentation.String);
}

// Regression test for https://github.com/PowerShell/PowerShellEditorServices/issues/1926
[SkippableFact]
public async Task CanRequestCompletionsAndHandleExceptions()
{
Skip.IfNot(VersionUtils.PSEdition == "Core", "This is a temporary bug in PowerShell 7, the fix is making its way upstream.");
string filePath = NewTestFile(@"
@() | ForEach-Object {
if ($false) {
return
}
@{key=$}
}");

Messages.Clear(); // On some systems there's a warning message about configuration items too.
CompletionList completionItems = await PsesLanguageClient.TextDocument.RequestCompletion(
new CompletionParams
{
TextDocument = new TextDocumentIdentifier
{
Uri = DocumentUri.FromFileSystemPath(filePath)
},
Position = new Position(line: 6, character: 11)
});

Assert.Empty(completionItems);
LogMessageParams message = Assert.Single(Messages);
Assert.Contains("Exception occurred while running handling completion request", message.Message);
}

[SkippableFact(Skip = "Completion for Expand-SlowArchive is flaky.")]
public async Task CanSendCompletionResolveWithModulePrefixRequestAsync()
{
Expand Down

0 comments on commit 0948131

Please sign in to comment.