Aspnetcore: [3.1.x - SignalR] Duplex Stream using IAsyncEnumerable fail to Bind argument while it works with ChannelReader, or on 5.x with both

[tebeco]

Describe the bug

This bug seems to occur when running on 3.1.x
My initial attempt to migrate the repro code on this repository was on main branch and I _think_ I could not reproduce, this might require double check
I pushed 2 branches with code "relatively up to date" on releases/3.1 and the old main while it was BEFORE the move to 6.0 here

When trying to do a sort of Duplex streaming using IAsyncEnumerable + HubConnection inside a Hub class, the HubInvocationBindier<THub> does not seems to detect that the targeted "second" Hub as a parameter (the IAsyncEnumerable).
The failure is happening when "planets are aligned" :

• All things described later use IAsyncEnumerable (not ChannelReader)
• EnableDetailedError is set to true
• the runtime is the LTS 3.1.x
• The streaming is happening within a Hub already streaming
• The "action" invoqued is both:
  
  
  
  • accepting Stream from Client
  
  
  • returning Stream to Client
  
  
  • This "action" is also using a HubConnection that
  
  
  • Stream to another Hub
  
  
  • accept Stream from that other Hub
  
  

AppA is like a dotnet new worker -n AppA
AppB is like a dotnet new webapi -n AppB
AppC is like a dotnet new webapi -n AppC

A stream is supposed to be generated on AppA => [1, 2, 3, 4]
It's streamed to AppB
AppB also has a HubConnection client streaming to AppC
For debug purpose I added some helper to "Log" / "Multiply" / "Enumerate Back" the stream values by 10x

     ==> AppB
               ==> AppC
         AppB <==
AppA <==

To Reproduce

git clone --recursive https://github.com/tebeco/aspnetcore repro31
cd repro31
git checkout -b repro-bug-iae-signalr-duplex-on-31 origin/repro-bug-iae-signalr-duplex-on-31
./restore.cmd
./build.cmd
cd ./src/SignalR
./startvs.cmd

Change startup to be multiple project :

• These sample have both implementation for IAsyncEnumerable and ChannelReader
• Make sure to check the "streaming startup code" in the Worker BackgroundService :
• //*/ will toggle IAsyncEnumerable
• /*/ will toggle ChannelReader
  
  

Press F5
It should throw a System.IO.InvalidDataException 'Invocation provides 1 argument(s) but target expects 0.'

For the code targetting 5.0:

git clone --recursive https://github.com/tebeco/aspnetcore repro50
cd repro50
git checkout -b no-repro-on-50 origin/no-repro-on-50
./restore.cmd
./build.cmd
cd ./src/SignalR
./startvs.cmd

Exceptions (if any)

System.IO.InvalidDataException: 'Invocation provides 1 argument(s) but target expects 0.'

StackTrace :

Microsoft.AspNetCore.SignalR.Protocols.Json.dll!Microsoft.AspNetCore.SignalR.Protocol.JsonHubProtocol.BindTypes(ref System.Text.Json.Utf8JsonReader reader, System.Collections.Generic.IReadOnlyList<System.Type> paramTypes) Line 731  C#
Microsoft.AspNetCore.SignalR.Protocols.Json.dll!Microsoft.AspNetCore.SignalR.Protocol.JsonHubProtocol.ParseMessage(System.Buffers.ReadOnlySequence<byte> input, Microsoft.AspNetCore.SignalR.IInvocationBinder binder) Line 270 C#
Microsoft.AspNetCore.SignalR.Protocols.Json.dll!Microsoft.AspNetCore.SignalR.Protocol.JsonHubProtocol.TryParseMessage(ref System.Buffers.ReadOnlySequence<byte> input, Microsoft.AspNetCore.SignalR.IInvocationBinder binder, out Microsoft.AspNetCore.SignalR.Protocol.HubMessage message) Line 91 C#
Microsoft.AspNetCore.SignalR.Core.dll!Microsoft.AspNetCore.SignalR.HubConnectionHandler<AppC.Hubs.AppCHub>.DispatchMessagesAsync(Microsoft.AspNetCore.SignalR.HubConnectionContext connection) Line 269 C#
[Resuming Async Method] 
[Async Call Stack]  
[Async] Microsoft.AspNetCore.SignalR.Core.dll!Microsoft.AspNetCore.SignalR.HubConnectionHandler<AppC.Hubs.AppCHub>.RunHubAsync(Microsoft.AspNetCore.SignalR.HubConnectionContext connection) Line 147   C#
[Async] Microsoft.AspNetCore.SignalR.Core.dll!Microsoft.AspNetCore.SignalR.HubConnectionHandler<AppC.Hubs.AppCHub>.OnConnectedAsync(Microsoft.AspNetCore.Connections.ConnectionContext connection) Line 119 C#
[Async] Microsoft.AspNetCore.Http.Connections.dll!Microsoft.AspNetCore.Http.Connections.Internal.HttpConnectionContext.ExecuteApplication(Microsoft.AspNetCore.Connections.ConnectionDelegate connectionDelegate) Line 534  C#
[Async] System.Private.CoreLib.dll!System.Threading.Tasks.TaskFactory.ContinueWhenAny   Unknown
[Async] Microsoft.AspNetCore.Http.Connections.dll!Microsoft.AspNetCore.Http.Connections.Internal.HttpConnectionDispatcher.DoPersistentConnection(Microsoft.AspNetCore.Connections.ConnectionDelegate connectionDelegate, Microsoft.AspNetCore.Http.Connections.Internal.Transports.IHttpTransport transport, Microsoft.AspNetCore.Http.HttpContext context, Microsoft.AspNetCore.Http.Connections.Internal.HttpConnectionContext connection) Line 272   C#
[Async] Microsoft.AspNetCore.Http.Connections.dll!Microsoft.AspNetCore.Http.Connections.Internal.HttpConnectionDispatcher.ExecuteAsync(Microsoft.AspNetCore.Http.HttpContext context, Microsoft.AspNetCore.Connections.ConnectionDelegate connectionDelegate, Microsoft.AspNetCore.Http.Connections.HttpConnectionDispatcherOptions options, Microsoft.AspNetCore.Http.Connections.Internal.ConnectionLogScope logScope) Line 173   C#
[Async] Microsoft.AspNetCore.Http.Connections.dll!Microsoft.AspNetCore.Http.Connections.Internal.HttpConnectionDispatcher.ExecuteAsync(Microsoft.AspNetCore.Http.HttpContext context, Microsoft.AspNetCore.Http.Connections.HttpConnectionDispatcherOptions options, Microsoft.AspNetCore.Connections.ConnectionDelegate connectionDelegate) Line 82    C#
[Async] Microsoft.AspNetCore.Routing.dll!Microsoft.AspNetCore.Routing.EndpointMiddleware.Invoke.__AwaitRequestTask|6_0(Microsoft.AspNetCore.Http.Endpoint endpoint, System.Threading.Tasks.Task requestTask, Microsoft.Extensions.Logging.ILogger logger) Line 80   C#
[Async] Microsoft.AspNetCore.Server.Kestrel.Core.dll!Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.HttpProtocol.ProcessRequests<Microsoft.AspNetCore.Hosting.HostingApplication.Context>(Microsoft.AspNetCore.Hosting.Server.IHttpApplication<Microsoft.AspNetCore.Hosting.HostingApplication.Context> application) Line 623   C#
[Async] Microsoft.AspNetCore.Server.Kestrel.Core.dll!Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.HttpProtocol.ProcessRequestsAsync<Microsoft.AspNetCore.Hosting.HostingApplication.Context>(Microsoft.AspNetCore.Hosting.Server.IHttpApplication<Microsoft.AspNetCore.Hosting.HostingApplication.Context> application) Line 534  C#
[Async] Microsoft.AspNetCore.Server.Kestrel.Core.dll!Microsoft.AspNetCore.Server.Kestrel.Core.Internal.HttpConnection.ProcessRequestsAsync<Microsoft.AspNetCore.Hosting.HostingApplication.Context>(Microsoft.AspNetCore.Hosting.Server.IHttpApplication<Microsoft.AspNetCore.Hosting.HostingApplication.Context> httpApplication) Line 101 C#
[Async] Microsoft.AspNetCore.Server.Kestrel.Core.dll!Microsoft.AspNetCore.Server.Kestrel.Https.Internal.HttpsConnectionMiddleware.InnerOnConnectionAsync(Microsoft.AspNetCore.Connections.ConnectionContext context) Line 260   C#
[Async] Microsoft.AspNetCore.Server.Kestrel.Core.dll!Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure.KestrelConnection.ExecuteAsync() Line 197 C#

Further technical details

• ASP.NET Core version : 3.1.7 / 3.1.8 (to be released ?)
• Include the output of dotnet --info :

 C:\dev\github\tebeco\aspnetcore\src\SignalR   repro-bug-iae-signalr-duplex-on-31 ≣ +0 ~1 -0 ! 
[15:24]❯ dotnet --info

Host (useful for support):
  Version: 5.0.0-preview.7.20364.11
  Commit:  53976d38b1

.NET SDKs installed:
  2.1.808 [C:\Program Files\dotnet\sdk]
  2.2.402 [C:\Program Files\dotnet\sdk]
  3.1.202 [C:\Program Files\dotnet\sdk]
  3.1.302 [C:\Program Files\dotnet\sdk]
  3.1.400 [C:\Program Files\dotnet\sdk]
  3.1.401 [C:\Program Files\dotnet\sdk]
  5.0.100-preview.7.20366.6 [C:\Program Files\dotnet\sdk]

.NET runtimes installed:
  Microsoft.AspNetCore.All 2.1.20 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.2.7 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.App 2.1.20 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.2.7 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 3.1.4 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 3.1.6 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 3.1.7 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 5.0.0-preview.7.20365.19 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 2.1.20 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.2.7 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 3.1.4 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 3.1.6 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 3.1.7 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 5.0.0-preview.7.20364.11 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.WindowsDesktop.App 3.1.4 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 3.1.6 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 3.1.7 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 5.0.0-preview.7.20366.1 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]

To install additional .NET runtimes or SDKs:
  https://aka.ms/dotnet-download

• The IDE (VS / VS Code/ VS4Mac) you're running on, and it's version
• VsCode Insider
• Vs2019 Enterprise Preview 16.8 Preview 1

[BrennanConroy]

Looks like the same issue that was fixed in https://github.com/dotnet/aspnetcore/pull/24926 which is why you can't see it happening in 5.0.

You can workaround the issue on 3.1 by "fixing" the IAsyncEnumerable, e.g.

public IAsyncEnumerable<int> EnumerableDuplexAsync(IAsyncEnumerable<int> stream)
{
    var fixedStream = Fix(stream);
    return HubConnection.StreamAsync<int>(nameof(EnumerableDuplexAsync), fixedStream);

    static async IAsyncEnumerable<int> Fix(IAsyncEnumerable<int> stream)
    {
        await foreach (var item in stream)
        {
            yield return item;
        }
    }
}

[tebeco]

thx for the tips / fix / workaround ;)
I probably would not have tried this TBH

I let your decide what to do with this:
https://github.com/dotnet/aspnetcore/pull/24926#pullrequestreview-467924562
Do you want me to see if doing a PR on release/3.1 is possible ?
Should it be a branch merge ? or just applying the same patch ?

I honestly do not care as I prefer relying on ChannelReader :D
Someone went on about it on Gitter about this thinking it was ASR related so I took a look at it too.
This was a bit hard to get a repro and/or explanation from Gitter text only, so I tried to make one because I was verrrrry curious about it :D

[tebeco]

also can you enlighten me on how this fix it ?
I do confirme this works with this patch applied, though I'm not sure my brain has accepted it yet :D

--                return type.GetGenericTypeDefinition() == typeof(IAsyncEnumerable<>);
++                if (type.GetGenericTypeDefinition() == typeof(IAsyncEnumerable<>))
++                {
++                    return true;
++                }

[BrennanConroy]

Before we were assuming if the type was generic then it must be an IAsyncEnumerable<> or some non-streaming type which is what that check was doing. But that was a wrong assumption since you can have a generic type that implements IAsyncEnumerable<> instead of being an IAsyncEnumerable directly. We then fallback to the logic below that statement that checks all interface types for IAsyncEnumerable<>. If you take a look at the added test case then you can see the structure of a type that failed the check before.

Do you want me to see if doing a PR on release/3.1 is possible ?

I mean it's possible, it's just the servicing bar is unlikely to be met. https://github.com/dotnet/aspnetcore/blob/master/docs/Servicing.md#servicing-bar

[tebeco]

oO I was too focus on the semantic on the equality itself rather that the fact that the check was ALWAYS returning before, and now only if true.
Thx

[tebeco]

thx a lot ;)
too bad I can't "👍" on the "merge" event on github