fix(dotnet): add AOT-safe RequestId serialization for StreamJsonRpc#783
fix(dotnet): add AOT-safe RequestId serialization for StreamJsonRpc#783PureWeen wants to merge 1 commit intogithub:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes a NativeAOT/System.Text.Json source-generation gap in the .NET SDK that can crash the StreamJsonRpc connection when CancellationToken triggers and $/cancelRequest must serialize a StreamJsonRpc.RequestId.
Changes:
- Extend the SDK’s
JsonSerializerOptionsconfiguration with an AOT-safeIJsonTypeInfoResolver+ customJsonConverter<RequestId>. - Add a new test suite to validate
RequestIdround-tripping and type-info resolution under the SDK’s configured serializer options.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
dotnet/src/Client.cs |
Adds RequestIdTypeInfoResolver + RequestIdJsonConverter and wires it into the serializer options resolver chain before MakeReadOnly(). |
dotnet/test/SerializationTests.cs |
Adds new unit tests verifying RequestId serialization/deserialization and type info resolution with the SDK’s serializer options. |
You can also share your feedback on Copilot code review. Take the survey.
When a CancellationToken fires during a JSON-RPC call, StreamJsonRpc's StandardCancellationStrategy.CancelOutboundRequest() attempts to serialize RequestId. The SDK's SystemTextJsonFormatter is configured with AOT-safe source-generated contexts, but none of them include StreamJsonRpc.RequestId. This causes a NotSupportedException that is unobserved by the JSON-RPC reader, silently killing the connection and leaving sessions permanently stuck at IsProcessing=true. The fix adds a StreamJsonRpcTypeInfoResolver — a reflection-based fallback IJsonTypeInfoResolver — as the last entry in the TypeInfoResolverChain. This ensures RequestId (and any other StreamJsonRpc-internal types) can be serialized when cancellation fires, without disrupting the AOT-safe source-generated path for all SDK-owned types. The reflection fallback is only reached for types not covered by the five source-generated contexts, so the AOT/trimming suppressions (IL2026, IL3050) are targeted and justified. Fixes: PureWeen/PolyPilot#319 Affected versions: 0.1.30+ (introduced by NativeAOT work in PR github#81) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
PureWeen
force-pushed
the
fix/dotnet-requestid-aot-serialization
branch
from
d512e5a to
9d6a63b
Compare
|
Hmm... why can't this instead be done just by adding EDIT: Nevermind, I see the PR comment: "RequestId couldn't be added via [JsonSerializable] because it has [JsonConverter(typeof(RequestIdSTJsonConverter))] and that converter is internal to StreamJsonRpc (SYSLIB1220). The converter also can't be used directly (CS0122)" Can you file an issue on StreamJsonRpc about that? Such converters need to be public for this very reason. |
|
Problem
When
CancellationTokenfires during a JSON-RPC operation, StreamJsonRpc'sStandardCancellationStrategysends a$/cancelRequestnotification containing aRequestId. The SDK's AOT serialization setup (source-generated contexts +MakeReadOnly()) has no type info forRequestId, causing aNotSupportedExceptionthat silently kills the JSON-RPC connection.Fixes: PureWeen/PolyPilot#319
Root Cause
Commit 4e7319e locked serialization to source-generated contexts only.
RequestIdcouldn't be added via[JsonSerializable]because it has[JsonConverter(typeof(RequestIdSTJsonConverter))]and that converter isinternalto StreamJsonRpc (SYSLIB1220). The converter also can't be used directly (CS0122).Solution
Add AOT-safe
RequestIdTypeInfoResolver+RequestIdJsonConverter:RequestIdJsonConverter— hand-written converter usingRequestId's public API (Number,String, constructors). UsesTryGetInt64()with string fallback for overflow/decimal safety.RequestIdTypeInfoResolver—IJsonTypeInfoResolverthat returns type info viaJsonMetadataServices.CreateValueInfo<T>()(the same API the source generator uses internally). Zero reflection.This follows the existing converter pattern in the SDK (see
PermissionRequestResultKind.Converterin Types.cs).Tests
RequestId_CanBeSerializedAndDeserialized_WithSdkOptions— round-trips long, string, and null RequestIdsSerializerOptions_CanResolveRequestIdTypeInfo— verifies the resolver chain produces type infoRequestId_NumericEdgeCases_RoundTrip—[Theory]covering 0, -1,long.MaxValueAll 5 tests fail without the fix (
NotSupportedException) and pass with it.