dotnet / orleans

Cloud Native application framework for .NET
https://docs.microsoft.com/dotnet/orleans
MIT License
10.12k stars 2.04k forks source link

Microsoft.Orleans.Client 9.0 not usable with ASP.NET Core 8/.NET 8 #9238

Open fubar-coder opened 3 days ago

fubar-coder commented 3 days ago

Description

I get the following error after an upgrade of Orleans (to 9.0):

[2024-11-19 13:11:34 ERR]  Unexpected exception in static IISHttpServer.HandleRequest.
System.MissingMethodException: Method not found: 'System.Nullable`1<System.Net.Security.TlsCipherSuite> Microsoft.AspNetCore.Connections.Features.ITlsHandshakeFeature.get_NegotiatedCipherSuite()'.
   at Microsoft.AspNetCore.Server.IIS.Core.IISHttpServer.IISContextFactory`1.CreateHttpContext
   at Microsoft.AspNetCore.Server.IIS.Core.IISHttpServer.HandleRequest(IntPtr pInProcessHandler, IntPtr pvRequestContext)

Analysis

This happens, because Microsoft.Orleans.Client 9.0 references Microsoft.AspNetCore.Connections.Abstractions 9.0, but it's only compiled for the following TFMs:

Now let's assume that you have a web project targeting net8.0:

This results in using/copying the DLLs from Microsoft.AspNetCore.Connections.Abstractions 9.0. The best TFM from this package is netstandard2.1, which only contains a reduced ITlsHandshakeFeature without NegotiatedCipherSuite.

Solution

Variant 1

Microsoft.Orleans.Client should have an additional target framework net8.0, which references Microsoft.AspNetCore.Connections.Abstractions 8.0 instead of 9.0.

Variant 2

Let the Microsoft ASP.NET Core guys add an additional TF net8.0 to Microsoft.AspNetCore.Connections.Abstractions.

remcoros commented 3 days ago

This is not the first time something like this happened. See for example: https://github.com/dotnet/aspnetcore/issues/40255

I don't think it's correct to update all the extensions packages to 9.0 for all target frameworks. If a user targets net8.0 and still wants the 9.0 packages, they can directly reference that. I think orleans should not force clients the upgrade of all extensions packages if targeting an older tfm.

ReubenBond commented 3 days ago

@benjaminpetit perhaps we need to distinguish dependency versions based on TFM, and multi-target net8.0 & net9.0. Alternatively, we try to pick up only the latest ASP.NET 8.0 compatible bits.

fubar-coder commented 3 days ago

Huh, this seems to have happened before: dotnet/orleans#7608

remcoros commented 3 days ago

Huh, this seems to have happened before: #7608

ah yes, forgot about that one 👍

mgravell commented 9 hours ago

There's two ways of looking at this (I'm coming from the aspnetcore aspect, here; on the surface, yes I think this interface change was questionable, and the analysis is correct - the result of a #if'd member on an interface and dropping a TFM is that we effectively removed those members from the interface in .NET 8, if you're using the .NET 9 abstractions dll. However, I don't think we can realistically maintain all down-level TFMs in the list forever. Maybe .NET 8 is "different" as the LTS, and we could certainly re-add net8.0 in the next service release...

... but a large part of my brain is just saying "don't do this", i.e. don't use the 9.0 abstractions dll if you're using aspnetcore 8.0; ultimately, the abstractions are meant to describe the platform, so: anything new in 9.0 wouldn't actually exsit for you anyway!

(insert philosophical discussion about making changes to interfaces ever, but: that ship has sailed, so...)

I think the team (possibly @ReubenBond , @DeagleGross and myself) need to discuss this internally, and decide on either:

  1. Orleans 9.0 should target .net 9.0 and abstractions 9.0 (possibly with a new target that uses .net 8.0 and abstractions 8.0), or
  2. we add a net8.0 tfm back into abstractions 9.0 (perhaps using the "current major and last LTS major, if different" approach)

I'm leaning more towards option 1, but let's discuss!

(note: "2" would be adding ;($CurrentLtsTargetFramework) to the csproj)

remcoros commented 9 hours ago

I think both need to be done, (2) the missing 8.0 TFM of the abstractions package could affect other frameworks/libraries as well, so this feels like a 'bug'.

And for Orleans (1): I think a framework should not force its users to upgrade to non-LTS packages. So as long as Orleans (or any other framework) supports 8.0, it should target the accompanying 8.0 packages to avoid possible runtime incompatibilities like this.

mgravell commented 3 hours ago

It's an interesting support policy question; I'm not sure it makes sense for a net8 package to use the net9 abstractions; there are many cases where this is explicitly supported (especially intentionally OOB packages that re supplemental to the framework), but "abstractions" is a little weird and special, since it kinda reflects the in-box framework.

We'll figure that out internally, though. If I'm wrong and we absolutely want to support 8.0 applications using 9.0 "abstractions", then yes, we'd need to fix this via the ;($CurrentLtsTargetFramework) I proposed above.

ReubenBond commented 1 hour ago

PR here: https://github.com/dotnet/orleans/pull/9246