radu-matei / websocket-manager

Real-Time library for ASP .NET Core
https://radu-matei.com/blog/real-time-aspnet-core/
MIT License
452 stars 183 forks source link

Client <-> Server return values and exceptions. #65

Closed Henry00IS closed 6 years ago

Henry00IS commented 6 years ago

Here is an example. Assume we have this method on the server:

image (sorry, the first argument is only in my library, bad screenshot on my part)
We call it on the client with parameters 7 and 3 to get result 10:

image
When we call it with parameters 0 and 3 we get the exception as expected:

image

Here is a flowchart of how I associate invocations and return values:

untitled diagram

There is a cancellation token in place so that if it has been waiting for a reply from the server for 60 seconds it will give up automatically and throws an exception. Please carefully review the code and feel free to come up with suggestions. This is likely going to be a breaking change. Server to Client is coming up soon...

Henry00IS commented 6 years ago

I have added Server to Client return values and exceptions as well. It appears I broke some of the example projects in the process according to Travis CI because I changed the .On() method to use Func with a return value. (Edit: I have not tested the .On() methods as I have my own system, should work though)

Henry00IS commented 6 years ago

I fixed that all the JSON types got obliterated. No more longs and JArrays.

danielohirsch commented 6 years ago

Hi! Any chance I can get your changes? I'd love to try to work on this pull...

radu-matei commented 6 years ago

Hi, @danielohirsch! You can follow this guide in order to checkout a pull request - https://help.github.com/articles/checking-out-pull-requests-locally/

Henry00IS commented 6 years ago

Hi, @danielohirsch! If you are as amazingly confused as me that these guys can checkout and magically contribute to pull requests, you can also just get it directly from the branch on my fork to try it out.

danielohirsch commented 6 years ago

Thanks Henry - I just ended up copying in all of your new and changed files... But I am having an issue with the web example... the values returned are undefined... not sure why - I traced it through the backend and it was putting the value through all the way - but when it gets to the js that handles it - when I trace through the js in chrome - the returned value is undefined... Were you able to get the web examples working?

Henry00IS commented 6 years ago

Ah, I haven't fixed the example projects yet. This will work with C# to C# communication. We may have to make a custom JavaScript include that can handle the more specific changes I proposed in this PR. You can't use return values there (which are optional anyway) but it's especially missing the $type variables in JSON that helps C# to keep track of bool/int/string[]/long/byte etc. so it's a lot more reflection friendly.

danielohirsch commented 6 years ago

Gotcha - I am digging through your changes... nice work! I am curious - is it possible to do a return value the other direction? (in the C# to C# world) - where the server sends a message to a client handler and the client returns a payload/value?

Henry00IS commented 6 years ago

Yeah that works already! The .On now takes a Func. I haven't tested the .On method as I am using reflection identical to the server but you should be able to write something like:

connection.On("Test", (args) => {
    return 5 + 5;
});

I mean... I am breaking so much in this PR. I might as well publish my client-side non-.On() solution that is so much better.

danielohirsch commented 6 years ago

I'd love to take a look at it.

Henry00IS commented 6 years ago

On the server inherit from WebSocketHandler and override the:

        public override async Task<object> OnInvokeMethodReceived(WebSocket socket, InvocationDescriptor invocationDescriptor)
        {
            // there must be a separator in the method name.
            if (!invocationDescriptor.MethodName.Contains('/')) throw new Exception("Invalid method name.");

            // find the controller and the method name.
            string[] names = invocationDescriptor.MethodName.Split('/');
            string controller = names[0].ToLower();
            string command = "ᐅ" + names[1];

            // use reflection to find the method in the desired controller.
            object self = null;
            MethodInfo method = null;
            switch (controller)
            {
                case "session": // add more to this list.
                    self = m_SessionController; // make some class containing your methods.
                    method = typeof(SessionController).GetMethod(command);
                    break;

                default:
                    throw new Exception($"Received command '{command}' for unknown controller '{controller}'.");
            }

            // if the method could not be found:
            if (method == null)
                throw new Exception($"Received unknown command '{command}' for controller '{controller}'.");

            // insert client as parameter. this is totally worth it but this won't compile for you.
            // List<object> args = invocationDescriptor.Arguments.ToList();
            // args.Insert(0, m_Clients[WebSocketConnectionManager.GetId(socket)]);

            // call the method asynchronously.
            return await Task.Run(() => method.Invoke(self, args.ToArray()));
        }

This will allow you to call methods in different classes "controllers":

        public int ᐅDoMath(/*WebSocketClient client,*/ int a, int b)
        {
            if (a == 0 || b == 0) throw new Exception("I am disappointed in you son.");

            return a + b;
        }

Using a simple prefix in the name:

        int value = await connection.SendAsync<int, int, int>("session/DoMath", 5, 5);

On the client inherit from Connection and override the:

        protected override async Task<object> Invoke(InvocationDescriptor invocationDescriptor)
        {
            // there must be a separator in the method name.
            if (!invocationDescriptor.MethodName.Contains('/')) return null;

            // find the controller and the method name.
            string[] names = invocationDescriptor.MethodName.Split('/');
            string controller = names[0].ToLower();
            string command = "ᐅ" + names[1];

            // use reflection to find the method in the desired controller.
            object self = null;
            MethodInfo method = null;
            switch (controller)
            {
                case "session": // add more to this list.
                    self = m_SessionController; // make some class containing your methods.
                    method = typeof(SessionController).GetMethod(command);
                    break;

                default:
                    //App.MainWindow.ViewModel.StatusBarMessage = $"Received command '{command}' for unknown controller '{controller}'.";
                    return null;
            }

            // if the method could not be found:
            if (method == null)
            {
                //App.MainWindow.ViewModel.StatusBarMessage = $"Received unknown command '{command}' for controller '{controller}'.";
                return null;
            }

            // call the method asynchronously.
            return await Task.Run(() => method.Invoke(self, invocationDescriptor.Arguments.ToArray()));
        }

This will allow you to call methods in different classes "controllers":

        public void ᐅOnSessionIdentifiers(string[] identifiers)
        {
            System.Windows.MessageBox.Show("Received " + identifiers.Count() + " identifiers!");
        }

Using a simple prefix in the name:

        await WebSocketManager.InvokeClientMethodOnlyAsync(client.Identifier, "session/OnSessionIdentifiers", GetSessionIdentifiers().ToArray());

(edit: you can return values too. just my example uses void here.)


I use the beautiful ᐅ character to prevent people calling methods they aren't supposed to as well as indicating that this method is called through reflection (as it has 0 references, don't want other programmers deleting them thinking it's unused code and git).

Henry00IS commented 6 years ago

I mean if everyone is okay with it I would do what @RobSchoenaker proposed here. And just remove .On() completely because it's a mess and no-one wants to bother with an object[] for the parameters. And replace it with some proper reflection logic just like the server and my post above.

Henry00IS commented 6 years ago

Also is anyone able to figure out why these lines:

int value = await connection.SendAsync<int, int, int>("session/DoMath", 5, 5);

Don't have type inference? Is it because it returns a Task that breaks it? It's sad that you have to specify all of the types now...

danielohirsch commented 6 years ago

It works for me... with a string var taskResult = _connection.SendAsync<string, string>("SayHello", "Test Client"); taskResult.Await(); var result = taskResult.Result;

in a static method - so calling Wait on the task instead of await

Henry00IS commented 6 years ago

I have added several method invocation strategies for the client (work in progress). The prefix is totally optional but I like ᐅ hehe. ;)

StringMethodInvocationStrategy

var strategy = new StringMethodInvocationStrategy();
var connection = new Connection(strategy);
strategy.On("Session/OnSessionIdentifiers", (args) =>
{
    System.Windows.MessageBox.Show("Received " + args.Length + " arguments!");
});

ControllerMethodInvocationStrategy

var strategy = new ControllerMethodInvocationStrategy("ᐅ", new TestController());
var connection = new Connection(strategy);

public class TestController
{
    // called as 'OnSessionIdentifiers'.
    public void ᐅOnSessionIdentifiers(WebSocket socket, string[] identifiers)
    {
        System.Windows.MessageBox.Show("Received " + identifiers.Count() + " identifiers!");
    }
}

DecoratedControllerMethodInvocationStrategy

var strategy = new DecoratedControllerMethodInvocationStrategy("ᐅ");
var connection = new Connection(strategy);
strategy.Register("Session", new TestController());
// can also set a custom separator instead of '/'.
// called as 'Session/OnSessionIdentifiers'.
Henry00IS commented 6 years ago

When you throw exceptions, because it's reflection that invokes methods, they will show up as user unhandled but they are handled. You can fix this behavior as described here.

RobSchoenaker commented 6 years ago

@Henry00IS can you please double check the failing build?

Henry00IS commented 6 years ago

Awesome! And yeah it's the example project(s) that have to be modified.

Henry00IS commented 6 years ago

Good news, found a legitimate excuse to fix up JavaScript for this new system (as I only work on this due to work). So will be making a small JavaScript include library that takes care of some overhead and by doing so fix the example projects.

Henry00IS commented 6 years ago

I think I covered all of it. The chat example uses all of the new features in one way or another.

Henry00IS commented 6 years ago

At this point we can merge it and hope that people will contribute to it to make it even better.

RobSchoenaker commented 6 years ago

As requested. Merged. I still need to merge my original changes too - too busy @ work 🙂

Henry00IS commented 6 years ago

This is now officially my favorite networking library. 😁 Can't wait to use this outside of work as well! ~ I think we just changed the world. Let's wait and see what happens.

danielohirsch commented 6 years ago

Nice job Henry

radu-matei commented 6 years ago

Suuuper excited about this merge! Congrats to @Henry00IS and @RobSchoenaker for the awesome work!

helmishariff commented 6 years ago

is there any sample in example project?

helmishariff commented 6 years ago

ok. I found it.

Henry00IS commented 6 years ago

For anyone reading this in the future, have a look at the ChatApplication sample.

helmishariff commented 6 years ago

from javascript I can use connection.methods. to return value but how to return value in c# client?

Henry00IS commented 6 years ago

When you create a new C# Connection, you pass a MethodInvocationStrategy in the constructor. It's identical to how you'd make a server method (depending on the invocation strategy you decide to use).

public int DoMath(int a, int b)
{
    if (a == 0 || b == 0) throw new Exception("That makes no sense.");
    return a + b;
}
helmishariff commented 6 years ago

I'm sorry. It's something like this?

_strategy.On("CallMethod", (arguments) => { return "test"; });

From server it will return value "test" when call InvokeClientMethodAsync.

Henry00IS commented 6 years ago

Sure, that's the StringMethodInvocationStrategy, if you prefer that then go for it. :) Just very annoying with the "arguments". If you wish to write methods like I did use the ControllerMethodInvocationStrategy.

Just make a class with a method like I posted above and pass it to the new ControllerMethodInvocationStrategy(myClass). It will use reflection and automagically call the methods that it finds in myClass.

int result = await InvokeClientMethodAsync<int, int, int>(clientid, "DoMath", 3, 5);

helmishariff commented 6 years ago

I tried using StringMethodInvocationStrategy but can't get return value. _ strategy.On("myMethod", (arguments) => { return "test"; }); string ret = await InvokeClientMethodAsync(wsId, "myMethod", obj);

But I can't get the ret value. I assume I should get value "test".

helmishariff commented 6 years ago

I tried using ControllerMethodInvocationStrategy but I get error.

ControllerMethodInvocationStrategy _ctrlStrategy = new ControllerMethodInvocationStrategy(InnerHandler);

public class InnerHandler { public string receiveMessages(string id, string msg) { return "success"; } }

helmishariff commented 6 years ago

I can call method using ControllerMethodInvocationStrategy but can't get return value. I get timeout: System.Threading.Tasks.TaskCanceledException: A task was canceled.

helmishariff commented 6 years ago

I tried but it freeze. Than I get timeout exception. No problem if I invoke the method on the client without return value. int result = await InvokeClientMethodAsync<int, int, int>(clientid, "DoMath", 3, 5);

From client sent to server I had no problem to get return value.