LiamMorrow / OrgnalR

SignalR backplane implemented through Orleans
Apache License 2.0
34 stars 7 forks source link

[FEATURE] Support latest version of Orleans #45

Closed bbehrens closed 1 year ago

bbehrens commented 1 year ago

Is your feature request related to a problem? Please describe. In this commit on June 6 https://github.com/dotnet/orleans/commit/cbee643f64c9db4306bc23b04a35364d28ee140d#diff-1fcbe03001be1589b2f0995d055a1cec02147c50ba2421171f9b895f3d96b65a they enabled the nullable annotation context. This creates compilation errors with the WriteStateIfDirty method.

I discovered this investigating this stack trace running OrgnalR in a project updated to the latest version of Orleans (7.2.1) Here's the stack trace we got

System.MissingMethodException: Method not found: 'System.Threading.Tasks.ValueTask Orleans.Runtime.GrainReference.InvokeAsync(Orleans.Serialization.Invocation.IInvokable)'.
   at OrleansCodeGen.OrgnalR.Backplane.GrainInterfaces.Proxy_IAnonymousMessageGrain.global::OrgnalR.Backplane.GrainInterfaces.IAnonymousMessageGrain.SubscribeToMessages(IAnonymousMessageObserver arg0, MessageHandle arg1)
   at OrgnalR.Backplane.GrainAdaptors.GrainMessageObservable.SubscribeToAllAsync(Func`3 messageCallback, Func`2 onSubscriptionEnd, MessageHandle since, CancellationToken cancellationToken)
   at OrgnalR.SignalR.OrgnalRHubLifetimeManager`1.CreateAsync(IActorProviderFactory actorProviderFactory, IMessageObservable messageObservable, IMessageObserver messageObserver, IMessageArgsSerializer messageArgsSerializer, ILogger`1 logger, CancellationToken cancellationToken)
   at OrgnalR.SignalR.SignalRExtensions.OrgnalRHubLifetimeManagerFactory`1.OnConnectedAsync(HubConnectionContext connection)
2023-07-21 19:39:28.822 +00:00 [Information] [bingodaubcity-server-api] [790c61fc-b067-4c12-9f9d-30df6b663856] [Serilog.AspNetCore.RequestLoggingMiddleware] HTTP "GET" "/Game" responded 101 in 81.2067 ms

Describe the solution you'd like I'd like for OrgnalR to work with the latest versions of Orleans

Describe alternatives you've considered

Additional context If you update the Orleans nugets in the OrgnalR project to the latest version, you'll get a few compiler errors related to the aforementioned nullable annotation context. If you pass in string.Empty for state instead of null the compilation issues go away and all the tests pass. I'm not sure if that'll fix my particular issue, but that's probably a fix you'd want to make anyway. I would submit a PR, but it's exceptionally trivial and I'm not certain of just putting in string.Empty is an acceptable fix for the situation.

LiamMorrow commented 1 year ago

Thanks @bbehrens , will have a look at this today and see what comes out of it!

LiamMorrow commented 1 year ago

So I think that that solution is fine, I've applied and and will test a release. That state is unused, so it is a bit strange they decided to make it non nullable, would be nice if it was generic but oh well.

I've published 2.3.0 and it is running without error in my example app. Let me know if you see any more issues :+1:

bbehrens commented 1 year ago

Awesome thanks so much for the quick turnaround Liam!

bbehrens commented 1 year ago

Liam that fixed our issue! We'll be looking at how we can add additional automated testing in our application to harden ourselves against future things like this. Will report back with anything we find that might be useful to this library

LiamMorrow commented 1 year ago

That'd be awesome thank you! My current strategy of booting up my example app is okay, but would be good to automate.

bbehrens commented 1 year ago

@LiamMorrow We ended up using the testing infrastructure from the SignalR.OrleansIntegration project https://github.com/Hahn-Software/SignalR.OrleansIntegration/tree/master/src/AspNetCore.SignalR.OrleansIntegration.TestUtils/Microsoft

I've been keeping an eye on the Orleans TestKit. They've got PRs merged in but the updated nuget hasn't been published yet. The tooling around Orleans is really having a difficult time upgrading to the new architecture