async invocation with less repeated code & error handling

[ensemblebd]

Hi dsuryd!! So have been playing with this on one of my projects, and I ended up having to modify the VMSerializer.cs file to accommodate my needs.

Wanted to see if perhaps there is a better way, or if this is something for potential consideration via a PR?

When you have something like this in a BaseVM subclass:

public Action<int> doThings => async id => {
    await DoMyStuff(id)
};
// or
public async Task<Task> doThings(int id) {
    return await DoMyStuff(id);
}

Three problems present themselves.

1. It's not really async, it just has the markup. So it must be wrapped in Task.Run(async ()=> { .. }) That unleashes the VM's when you have multiple VM operations running at once. They otherwise run synchronously and block each other, despite being marked async and having an await operation inside AND returning typeof Task. Not sure why, above my head. But wrapping in Task.Run produces truly async execution.

2. However the Task.Run occurs too fast, and the BaseVM's ChangedProperties collects changes before the PushChanges is called asynchronously, causing data to not be sent back to frontend. So my action signatures must then become:

   public async Task<Task> doThings(int id) {
   return await Task.Run(async () => {
         Task.Delay(50); // let the deserializer thread finish execution before we attempt to respond (race condition).
         _ = await DoMyStuff(id);
   });
   }

Which brings up potential problem 3.

3. If an error occurs in an async operation, dot net core crashes completely. So my pattern becomes:

   public async Task<Task> doThings(int id) {
   return await Task.Run(async () => {
         Task.Delay(50);
   
         try {
            _ = await DoMyStuff(id);
         }catch{}
   });
   }

While this works well, I end up with a VM that has duplicate code in every method call, and the simplicity is lost by the ever expansive code.

So my proposed solution is: // DotNetifyLib.Core/BaseVM/VMSerializer.cs # 182

private bool InvokeIfMethod(object viewModel, string name, string newValue)
      {
         var methodInfo = viewModel.GetType().GetTypeInfo().GetMethods().FirstOrDefault(x => x.Name == name && x.GetParameters().Length <= 1);

         if (methodInfo != null)
         {
            var isAwaitable = methodInfo.ReturnType.GetMethod(nameof(Task.GetAwaiter)) != null;
            object result = null;
            object[] @params = new object[] { };
            if (methodInfo.GetParameters().Length > 0) {
                var arg = Command.ConvertParameter(newValue, methodInfo.GetParameters().First().ParameterType);
                @params = new object[] { arg };
            }

            if (isAwaitable) {
                // create async task to run asynchronously. Otherwise the .Invoke() operates on main thread (current), blocking everything else. Even if the underlying method is labeled "async" and awaits something.
                // this vastly improves client performance when client has multiple connections simultaneously.
                result = Task.Run(async () => {
                // prevent race condition for VM response when ChangedProperties is altered asynchronously (ie. let the calling thread (deserializer) complete before we execute the underlying action/task).
                    await Task.Delay(50); // todo?: consider making this a method System.Attribute property to customize? Depends how large the sent-in data is from front-end as to how long we should delay to accomodate deserialization. 

                    // additionally, we wrap in a try catch since async method failures can cause the entire apparatus to crash.
                    try {
                        await (Task)methodInfo.Invoke(viewModel, @params);
                    }
                    catch (Exception ex) {
                        // feed the error back to the VM, if so supported: (todo?: add virtual method to BaseVM to make use-case concrete?)
                        var mi_error = viewModel.GetType().GetTypeInfo().GetMethods().FirstOrDefault(x => x.Name == "OnActionError");
                        if (mi_error != null) {
                            mi_error.Invoke(viewModel, new object[] { ex });
                        }
                    }
                });
                if (result!=null && viewModel is BaseVM) {
                    (viewModel as BaseVM).AsyncCommands.Add(result as Task);
                }
            }
            else {
                // otherwise just invoke it. It's just a method. 
                result = methodInfo.Invoke(viewModel, @params);
                // perhaps the method still returns a task even if it's not asynchronous..
                    if (result != null && viewModel is BaseVM) {
                        (viewModel as BaseVM).AsyncCommands.Add(result as Task);
                   }
                }

            return true;
         }
         return false;
      }

Thus my actions can simply become (far cleaner, less repeated code, easier to read for regular developers):

public async Task<bool> doThings(int id) {
     _ = await DoMyStuff(id);
    return true;
}

// called when an action fails: 
public void OnActionError(Exception ex) {
    log.LogError(ex, ex.Message);
}

Wanted to get your thoughts on this when you get a chance, thank you!!

[dsuryd]

Hi! Perhaps I'm misunderstanding something, I'm puzzled with your statement that async methods in the BaseVM subclass aren't really async and therefore must be wrapped in Task.Run. Here's the callstack of a method from the SimpleList example that was turned into async:

As you can see, it's async all the way to the SignalR hub method.

But if you're referring specifically to the Action form, then you're right, it won't handle async properly and should be refactored into the former.

[ensemblebd]

Thank you! You are right for sure, boils down to the Action specifically. I hadn't fully tested comparatively to validate. Will go ahead and close this one out; can be solved by using non-action, no core modifications required. Thank you again :)