microsoft / vs-streamjsonrpc

The StreamJsonRpc library offers JSON-RPC 2.0 over any .NET Stream, WebSocket, or Pipe. With bonus support for request cancellation, client proxy generation, and more.
Other
730 stars 149 forks source link

StreamJsonRpc + system.text.json throws hasValue exception if using SingleObjectParameterDeserialization and default parameter #1028

Closed mmoraga closed 3 months ago

mmoraga commented 5 months ago

When using SystemTextJsonFormatter, given this method:

[JsonRpcMethod("FOO", UseSingleObjectParameterDeserialization = true)]
public void Foo(FooContract fooContract = null)

when no parameter is supplied, StreamJsonRpc replies with the following:

Sent: {"jsonrpc":"2.0","id":"1","error":{"code":-32000,"message":"value.HasValue","data":{"type":"Microsoft.Assumes\u002BInternalErrorException","message":"value.HasValue","stack":"   at Microsoft.Assumes.Fail(String message)\n   at Microsoft.Assumes.True(Boolean condition, String message)\n   at Microsoft.Assumes.NotNull[T](Nullable\u00601 value)\n   at StreamJsonRpc.SystemTextJsonFormatter.JsonRpcRequest.TryGetTypedArguments(ReadOnlySpan\u00601 parameters, Span\u00601 typedArguments)\n   at StreamJsonRpc.TargetMethod.TryGetArguments(JsonRpcRequest request, MethodSignature method, Span\u00601 arguments)\n   at StreamJsonRpc.TargetMethod..ctor(JsonRpcRequest request, List\u00601 candidateMethodTargets, SynchronizationContext fallbackSynchronizationContext)\n   at StreamJsonRpc.Reflection.RpcTargetInfo.TryGetTargetMethod(JsonRpcRequest request, TargetMethod\u0026 targetMethod)\n   at StreamJsonRpc.JsonRpc.DispatchIncomingRequestAsync(JsonRpcRequest request)","code":-2146233088,"inner":null}}}

I can work around this by using method overloading:

[JsonRpcMethod("FOO")]
public void Foo()
[JsonRpcMethod("FOO", UseSingleObjectParameterDeserialization = true)]
public void Foo(FooContract fooContract)

Since this works if using NewtonSoft formatter this should not trow imho.

AArnott commented 3 months ago

Thank you for the excellent details. I can repro the problem. I agree we should fix this.

mmoraga commented 3 months ago

thanks!