A JsonRpc descriptor overriding CreateFormatter can not be wrapped.

[BertanAygun]

When ServiceJsonRpcDescriptor is used with copyFrom constructor, CreateFormatter override implementation of the source descriptor is lost because it is never called. This causes a problem in cases where VS/Gladstone wraps an existing ServiceJsonRpcDescriptor since they can't call in to wrapped descriptors protected CreateFormatter method.

Couple solutions I can think of:

• Store "copyFrom" descriptor instance in ServiceJsonRpcDescriptor and call "copyFrom.CreateFormatter" if it is not null.

  • This can cause long chains of descriptors calling it each other if not optimized, it can also cause incorrect behavior if settings such as multiplexing changes between copyFrom descriptor and cloned descriptor. (CanSendOutOfBandStreams test fails with this change)

• Make CreateFormatter public so wrappers can call it from their own override if needed.

  • This would be a binary breaking change unless we add a new public non-virtual method calling the protected one.

[AArnott]

Changing the descriptor type is intended to not call CreateFormatter on the original object, since switching the descriptor type may be specifically in order to change the behavior of these virtual methods.

Maybe we can apply a pattern that .NET uses in their HTTP stack: HttpMessageHandler.SendAsync is protected, yet folks want to be able to wrap this object and forward the call. They fix this by declaring a special DelegatingHandler-derived type whose SendAsync implementation calls the method on the inner method (which they can do, because it is protected internal, and both types are defined in the same assembly. The net effect of this is that by deriving from DelegatingHandler, you can just call this.SendAsync and get the behavior of the wrapped object.

So we could declare a DelegatingServiceRpcDescriptor that retains a reference to the wrapped descriptor and forwards certain calls to that instead of implementing them directly. Would that work for your scenario?

[BertanAygun]

In this case the situation is complicated by having to apply other options on the inner descriptor. For example if someone called WithMultiplexingOptions in the outer descriptor, we would have to handle and clone/set innerDescriptor accordingly as well. I think a DelegatingJsonRpcDescriptor like solution would still work given we handle such cases. We would likely have to make some properties to have protected internal setters though.

Another option could be to have a DelegatableJsonRpcDescriptor that exposes the method as a public, it would still help to hide it away in common cases if that's the concern but could help with my scenario.

In my case the inner descriptor sets a MessagePack formatter using a typeless formatter.

[AArnott]

Another option could be to have a DelegatableJsonRpcDescriptor that exposes the method as a public

How would that help you, given the descriptor you're wrapping doesn't derive from that new class, so the method still wouldn't be public?

We would likely have to make some properties to have protected internal setters though.

Descriptors are supposed to be immutable. If you want to be able to change properties on them, you'd need to copy them. But this seems doable. For example on your delegating-derived wrapper, you could override the WithMultiplexingOptions method to do this:

1. Forward the call to the inner descriptor
2. Return a new wrapper that wraps the result of the forwarded call.

[BertanAygun]

How would that help you, given the descriptor you're wrapping doesn't derive from that new class, so the method still wouldn't be public?

This was assuming we would make the method "protected internal" so that this new wrapper can then have a new public method but it feels very hacky.

I was working on a PR for this earlier, let me try the delegating handler solution with handling Multiplexing/ExceptionStrategy and see how that works.

[BertanAygun]

Submitted PR at https://github.com/microsoft/vs-servicehub/pull/170, doesnt look like I can tag a reviewer.