csharp-language-server-protocol
csharp-language-server-protocol copied to clipboard
MediatR v9.0.0 includes a breaking change.
This is probably an issue that is not going to affect many people, but I believe it is a legit dependency issue that will have to be resolved at some point:
Directory.Build.targets
includes MediatR
at version 8.1.0
.
When adding as a dependency via dotnet or nuget, (e.g. dotnet add package OmniSharp.Extensions.LanguageServer
), which is what I'd guess most folk are doing, it follows the Nuget Versioning rules and pulls in the lowest version >= the specified version - which resolves to 8.1.0
and works fine.
When adding via paket add OmniSharp.Extensions.LanguageServer
it follows the paket rules which identifies the latest matching version which is 9.0.0
and unfortunately this version has a breaking change. The issue is only detectable at runtime due to the use of reflection I believe. The problem is an attempt to invoke a method that no longer exists on the IMediator
interface, due to being moved to a separate interface that IMediator
implements.
Compare:
v8.1.0: https://github.com/jbogard/MediatR/blob/v8.1.0/src/MediatR/IMediator.cs
vs
v9.0.0: https://github.com/jbogard/MediatR/blob/v9.0.0/src/MediatR/IMediator.cs
The exception that's thrown is:
OmniSharp.Extensions.JsonRpc.DefaultRequestInvoker: Failed to handle request initialize 1 -
System.Reflection.TargetInvocationException: Exception has been thrown by the target of an invocation.
---> System.MissingMethodException: Method not found: 'System.Threading.Tasks.Task`1<System.__Canon> MediatR.IMediator.Send(MediatR.IRequest`1<System.__Canon>, System.Threading.CancellationToken)'.
--- End of inner exception stack trace ---
at System.RuntimeMethodHandle.InvokeMethod(Object target, Object[] arguments, Signature sig, Boolean constructor, Boolean wrapExceptions)
at System.Reflection.RuntimeMethodInfo.Invoke(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
at System.Reflection.MethodBase.Invoke(Object obj, Object[] parameters)
at OmniSharp.Extensions.JsonRpc.RequestRouterBase`1.HandleRequest(IMediator mediator, IHandlerDescriptor descriptor, Object params, CancellationToken token)
at OmniSharp.Extensions.JsonRpc.RequestRouterBase`1.<RouteRequest>g__InnerRoute|7_0(IServiceScopeFactory serviceScopeFactory, Request request, TDescriptor descriptor, Object params, CancellationToken token, ILogger logger)
at OmniSharp.Extensions.JsonRpc.RequestRouterBase`1.RouteRequest(IRequestDescriptor`1 descriptors, Request request, CancellationToken token)
at OmniSharp.Extensions.JsonRpc.DefaultRequestInvoker.<>c__DisplayClass10_0.<<RouteRequest>b__5>d.MoveNext()
So it seems that the issue is somewhere around https://github.com/OmniSharp/csharp-language-server-protocol/blob/005c87f1c41da3f1b2b35f32c98e3d3132df99a3/src/JsonRpc/RequestRouterBase.cs#L204
It seems that while making one of the SendRequest
methods on the RequestRouterBase
generic is working, the invocation isn't, because the invocation is attempting to invoke a method on IMediator
interface itself, and not resolving to the method which is now on ISender
as of MediatR 9.0.0
. Not sure exactly why - perhaps if the method was invoked directly, rather than via reflection, the compiler would have worked it all out correctly?
Proposed solutions:
1. Limit the version range
By updating Directory.Build.targets
to include MediatR
with a floating version of 8.*
. It should ensure both nuget
and paket
resolve the dependency as 8.1.0
. (When using floating versions, nuget resolves the highest matching version). This is a more technically correct dependency version spec because it clearly identifies the valid version range. [8.1.0,9.0.0)
is an alternative way of expressing the same thing I think.
2. Update to MediatR 9.0.0 properly
I'd hope that updating Directory.Build.targets
to include MediatR
with a floating version of 9.*
should very quickly flush out any issues through the test suite, which could be resolved by perhaps using Mediatr.ISender
in the SendRequest
methods (and anywhere else).
Workaround for Paket users
Adding:
nuget MediatR 8.1.0
to paket.dependencies
resolves the issue as a workaround for paket users.
Summary
I'm happy to contribute a PR to resolve this upstream and avoid using the workaround if you have a preference for either of the above proposals, or any other preferred approach?
Have setup two PRs, one for each option. Bumping the version to [9.0.0,10.0.0)
didn't seem to require any code changes to get all tests to pass locally. It seems that the issue was just attempting to use MediatR 9.0.0
(as restored by paket) using the OmniSharp.Extensions.JsonRpc
package built against MediatR 8.1.0
, which makes sense now that I think about it!
Mediatr was updated to 9.0.0 in https://github.com/OmniSharp/csharp-language-server-protocol/pull/1084