Skip to content

Don't kill all code actions when a delegated server misbehaves #8214

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 2 commits into from
Feb 5, 2023
Merged

Don't kill all code actions when a delegated server misbehaves #8214

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a14dc1c
Don't kill all code actions when a delegated server misbehaves
davidwengier Feb 3, 2023
69da30f
Update tests
davidwengier Feb 3, 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
25 changes: 17 additions & 8 deletions src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/CodeActions/CodeActionEndpoint.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
using Microsoft.CodeAnalysis.Text;
using Microsoft.VisualStudio.LanguageServer.Protocol;
using Newtonsoft.Json.Linq;
using StreamJsonRpc;

namespace Microsoft.AspNetCore.Razor.LanguageServer.CodeActions;

Expand Down Expand Up @@ -86,7 +87,7 @@ public CodeActionEndpoint(
// HTML code actions aren't currently supported in VS Code:
// https://github.com/dotnet/razor/issues/8062
var delegatedCodeActions = _languageServerFeatureOptions.SupportsDelegatedCodeActions
? await GetDelegatedCodeActionsAsync(documentContext, razorCodeActionContext, cancellationToken).ConfigureAwait(false)
? await GetDelegatedCodeActionsAsync(documentContext, razorCodeActionContext, requestContext.Logger, cancellationToken).ConfigureAwait(false)
: ImmutableArray.Create<RazorVSInternalCodeAction>();

cancellationToken.ThrowIfCancellationRequested();
Expand Down Expand Up @@ -185,7 +186,7 @@ public RegistrationExtensionResult GetRegistration(VSInternalClientCapabilities
return context;
}

private async Task<ImmutableArray<RazorVSInternalCodeAction>> GetDelegatedCodeActionsAsync(VersionedDocumentContext documentContext, RazorCodeActionContext context, CancellationToken cancellationToken)
private async Task<ImmutableArray<RazorVSInternalCodeAction>> GetDelegatedCodeActionsAsync(VersionedDocumentContext documentContext, RazorCodeActionContext context, IRazorLogger logger, CancellationToken cancellationToken)
{
var languageKind = _documentMappingService.GetLanguageKind(context.CodeDocument, context.Location.AbsoluteIndex, rightAssociative: false);

Expand All @@ -195,7 +196,7 @@ private async Task<ImmutableArray<RazorVSInternalCodeAction>> GetDelegatedCodeAc
return ImmutableArray<RazorVSInternalCodeAction>.Empty;
}

var codeActions = await GetCodeActionsFromLanguageServerAsync(languageKind, documentContext, context, cancellationToken);
var codeActions = await GetCodeActionsFromLanguageServerAsync(languageKind, documentContext, context, logger, cancellationToken).ConfigureAwait(false);
if (codeActions is not [_, ..])
{
return ImmutableArray<RazorVSInternalCodeAction>.Empty;
Expand All @@ -212,7 +213,7 @@ private async Task<ImmutableArray<RazorVSInternalCodeAction>> GetDelegatedCodeAc
providers = _htmlCodeActionProviders;
}

return await FilterCodeActionsAsync(context, codeActions, providers, cancellationToken);
return await FilterCodeActionsAsync(context, codeActions, providers, cancellationToken).ConfigureAwait(false);
}

private RazorVSInternalCodeAction[] ExtractCSharpCodeActionNamesFromData(RazorVSInternalCodeAction[] codeActions)
Expand Down Expand Up @@ -263,11 +264,11 @@ private async Task<ImmutableArray<RazorVSInternalCodeAction>> FilterCodeActionsA
tasks.Add(provider.ProvideAsync(context, codeActions, cancellationToken));
}

return await ConsolidateCodeActionsFromProvidersAsync(tasks.ToImmutableArray(), cancellationToken);
return await ConsolidateCodeActionsFromProvidersAsync(tasks.ToImmutableArray(), cancellationToken).ConfigureAwait(false);
}

// Internal for testing
internal async Task<RazorVSInternalCodeAction[]> GetCodeActionsFromLanguageServerAsync(RazorLanguageKind languageKind, VersionedDocumentContext documentContext, RazorCodeActionContext context, CancellationToken cancellationToken)
internal async Task<RazorVSInternalCodeAction[]> GetCodeActionsFromLanguageServerAsync(RazorLanguageKind languageKind, VersionedDocumentContext documentContext, RazorCodeActionContext context, IRazorLogger logger, CancellationToken cancellationToken)
{
if (languageKind == RazorLanguageKind.CSharp)
{
Expand Down Expand Up @@ -298,7 +299,15 @@ internal async Task<RazorVSInternalCodeAction[]> GetCodeActionsFromLanguageServe
LanguageKind = languageKind
};

return await _languageServer.SendRequestAsync<DelegatedCodeActionParams, RazorVSInternalCodeAction[]>(RazorLanguageServerCustomMessageTargets.RazorProvideCodeActionsEndpoint, delegatedParams, cancellationToken);
try
{
return await _languageServer.SendRequestAsync<DelegatedCodeActionParams, RazorVSInternalCodeAction[]>(RazorLanguageServerCustomMessageTargets.RazorProvideCodeActionsEndpoint, delegatedParams, cancellationToken).ConfigureAwait(false);
}
catch (RemoteInvocationException e)
{
logger.LogException(e, "Error getting code actions from delegate language server for {languageKind}", languageKind);
Copy link
Member Author

@davidwengier davidwengier Feb 3, 2023

Choose a reason for hiding this comment

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

This means we'll get a prism bug if enough of these happens, which will be good to highlight for partner teams, but won't prevent us from showing Razor code actions either

return Array.Empty<RazorVSInternalCodeAction>();
}
}

private async Task<ImmutableArray<RazorVSInternalCodeAction>> GetRazorCodeActionsAsync(RazorCodeActionContext context, CancellationToken cancellationToken)
Expand All @@ -312,7 +321,7 @@ private async Task<ImmutableArray<RazorVSInternalCodeAction>> GetRazorCodeAction
tasks.Add(provider.ProvideAsync(context, cancellationToken));
}

return await ConsolidateCodeActionsFromProvidersAsync(tasks.ToImmutableArray(), cancellationToken);
return await ConsolidateCodeActionsFromProvidersAsync(tasks.ToImmutableArray(), cancellationToken).ConfigureAwait(false);
}

private static async Task<ImmutableArray<RazorVSInternalCodeAction>> ConsolidateCodeActionsFromProvidersAsync(
Expand Down
4 changes: 2 additions & 2 deletions ...test/Microsoft.AspNetCore.Razor.LanguageServer.Test/CodeActions/CodeActionEndpointTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -630,7 +630,7 @@ public async Task GetCSharpCodeActionsFromLanguageServerAsync_InvalidRangeMappin
Assert.NotNull(context);

// Act
var results = await codeActionEndpoint.GetCodeActionsFromLanguageServerAsync(RazorLanguageKind.CSharp, documentContext, context, default);
var results = await codeActionEndpoint.GetCodeActionsFromLanguageServerAsync(RazorLanguageKind.CSharp, documentContext, context, Logger, default);

// Assert
Assert.Empty(results);
Expand Down Expand Up @@ -675,7 +675,7 @@ public async Task GetCSharpCodeActionsFromLanguageServerAsync_ReturnsCodeActions
Assert.NotNull(context);

// Act
var results = await codeActionEndpoint.GetCodeActionsFromLanguageServerAsync(RazorLanguageKind.CSharp, documentContext, context, default);
var results = await codeActionEndpoint.GetCodeActionsFromLanguageServerAsync(RazorLanguageKind.CSharp, documentContext, context, Logger, default);

// Assert
var result = Assert.Single(results);
Expand Down