Benedicht / BestHTTP-Issues

Issue tracking repo for the Best HTTP and all related Unity plugins.
https://assetstore.unity.com/publishers/4137
12 stars 1 forks source link

SignalR Server Callable Async Client Functions Don't Return Values #162

Closed ShawkiVA closed 1 year ago

ShawkiVA commented 1 year ago

Hello - I've been unable to return a value from an async client function similar to the code located here (.NET client section): https://learn.microsoft.com/sv-se/aspnet/core/signalr/hubs?view=aspnetcore-7.0#client-results

Are there plans to support async client functions like these? In my test code, I am simply awaiting Task.Delay(1000) and returning a string, which never makes it back to the server

Benedicht commented 1 year ago

Just tried it out, it's working as expected with the json. There's a problem with the MessagePackProtocol i'm going to fix.

ShawkiVA commented 1 year ago

I think my explanation of the issue was unclear, as I'm currently working with json and still experiencing this issue.

I would like to be able to use an async function as a callback for HubConnection.On(), and then have that async function return a value back to the server. Here's the code I initially tried:

_signalRConnection.On("OnMessage", async () => { await Task.Delay(10000); return "response!"; }); This code would run, but the server would never get the response of "response!". If I changed it so that it was no longer asynchronous and just returned the string, the server WOULD receive the response with no issue.

Where I believe the issue lies:

I was able to trace the issue back to HubConnection.cs, within OnMessages() in the MessageTypes.Invocation case. A subscription's functionCallback will be called, return a value, and then immediately send that message back to the server. This works as expected for synchronous functions, but as far as I could tell, there is no place that accounts for awaiting async functions to get their results.

My fix:

I modified the Subscription class to include a new list for asyncFunctionCallbacks. These are handled very similarly to functionCallbacks, but the last parameter of the function is always Task<object>. Now, in HubConnection.OnMessages, I will run these asyncFunctionCallbacks on a thread, await their results, and then send the messages back to the server with the appropriate result.

^Not the most elegant fix, but it proved to me that it worked and could be a nice addition to the package. Please let me know if I can provide more detail - also I appreciate your quick response!

Benedicht commented 1 year ago

Thanks for the clarification!

Yes, the problem is that the return type in these cases is a Task, and the plugin tryies to send the task itself. You fix is fine too, i thought about using Task's ContinueWith and the use of a little reflection:

var result = callbackDesc.Callback(realArgs);

if (result is Task task && task.GetType() is Type taskType && taskType.IsGenericType)
{
    task.ContinueWith((t) =>
    {
        Exception error = null;
        try
        {
            if (t.IsCanceled || t.IsFaulted)
            {
                error = t.Exception.InnerException ?? new TaskCanceledException();
            }
            else
            {
                var prop = taskType.GetProperty("Result");
                var taskResult = prop.GetValue(t);

                SendMessage(new Message
                {
                    type = MessageTypes.Completion,
                    invocationId = message.invocationId,
                    result = taskResult
                });
            }
        }
        catch(Exception ex)
        {
            error = ex;
        }

        if (error != null)
        {
            SendMessage(new Message
            {
                type = MessageTypes.Completion,
                invocationId = message.invocationId,
                error = error.Message
            });
        }
    });
}
else
{
    SendMessage(new Message
    {
        type = MessageTypes.Completion,
        invocationId = message.invocationId,
        result = result
    });
}

Here's the full HubConnection.cs: https://gist.github.com/Benedicht/3398e08a1706ed1569053db990325a64#file-hubconnection-cs-L1101-L1149

ShawkiVA commented 1 year ago

Thank you! Your solution worked as expected for my use cases. Can I assume this will be included in the next release?

Benedicht commented 1 year ago

Yep, I think I'm going to include it.