Skip to content

Conversation

@CyrusNajmabadi
Copy link
Member

@CyrusNajmabadi CyrusNajmabadi commented Apr 8, 2025

Dependent on #78056

@ghost ghost added Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead labels Apr 8, 2025
@CyrusNajmabadi
Copy link
Member Author

@dibarbet @matteo-prosperi this is ready for review.

[property: JsonPropertyName("response"), JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)] string? Response,
[property: JsonPropertyName("extensionWasUnloaded"), JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingDefault)] bool ExtensionWasUnloaded,
[property: JsonPropertyName("extensionException"), JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)]
ExtensionException? ExtensionException);
Copy link
Member Author

Choose a reason for hiding this comment

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

@matteo-prosperi up to you what you will do with this info on the gladstone side.


// Report any exceptions the extension itself caused while registering and getting the message names.
if (handlerNames.ExtensionException is not null)
context.Logger.LogException(handlerNames.ExtensionException);
Copy link
Member Author

Choose a reason for hiding this comment

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

@dibarbet i'm still logging this here (as well as passing back to Gladstone). is that ok with you ?

internal readonly record struct ExtensionMessageResponse(
[property: JsonPropertyName("response")] string Response);
[property: JsonPropertyName("response"), JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)] string? Response,
[property: JsonPropertyName("extensionWasUnloaded"), JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingDefault)] bool ExtensionWasUnloaded,
Copy link
Member Author

Choose a reason for hiding this comment

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

New field to represent a supported failure mode that does not return a respone and was not an exception itself throwing an exception.

@CyrusNajmabadi CyrusNajmabadi marked this pull request as ready for review April 8, 2025 19:21
@CyrusNajmabadi CyrusNajmabadi requested a review from a team as a code owner April 8, 2025 19:21
@CyrusNajmabadi CyrusNajmabadi enabled auto-merge April 8, 2025 20:21
@CyrusNajmabadi CyrusNajmabadi disabled auto-merge April 8, 2025 20:21
namespace Microsoft.CodeAnalysis.LanguageServer.Handler.Extensions;

internal sealed record class ExtensionException(
[property: JsonPropertyName("typeName")] string TypeName,
Copy link
Member

Choose a reason for hiding this comment

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

I'm fine with this - however I would have naively started with serializing System.Exception - I assume we don't want that here? I know the exception type may not exist on the other side, but that is something jsonrpc already has to deal with when we throw exceptions for example.

@CyrusNajmabadi CyrusNajmabadi merged commit 08759c1 into dotnet:main Apr 8, 2025
25 checks passed
@CyrusNajmabadi CyrusNajmabadi deleted the gladstoneAPI branch April 8, 2025 21:04
@dotnet-policy-service dotnet-policy-service bot added this to the Next milestone Apr 8, 2025
@RikkiGibson RikkiGibson modified the milestones: Next, 18.0 P1 Aug 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead VSCode

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants