5

I am using NATS mechanism to publish events between different processes, when I subscribe to a topic and supply async action that handles the topic if an exception has been thrown from the action that handles the topic I can't catch it because I am not do await for the invocation.

The problem is that the "connection.subscribeAsync" argument is the following:

delegate void EventHandler<TEventArgs>(object sender, TEventArgs e);

I use Custom Attribute the find out if the action is async action or sync

and if it is async I use reflection to build Task from Action;

but at this point, its looks like I have to choose between two bad options,

The first is is use GetAwaiter().GetResults();

and the second is to do the method - async void and wrap the invocation with try and catch so I will catch exceptions when they occur before the synchronization Context.

I would love to hear any suggestions to deal with these issues.

code snaps:

async void MsgEvent(object sender, EncodedMessageEventArgs eArgs)
{
    try
    {
        if (IsAsyncAction(request.Action))
        {
            await (Task)request.Action.GetMethodInfo()
                    .Invoke(request.Action, new object[] { eArgs });

        }
        .
        .
        .

    }
    catch (Exception e)
    {
        _logger.LogError(e.Message);
    }
}

var subscription = connection.SubscribeAsync(request.Topic, MsgEvent);

    /// <summary>
    /// Expresses interest in the given <paramref name="subject" /> to the NATS Server, and begins delivering
    /// messages to the given event handler.
    /// </summary>
    /// <remarks>The <see cref="T:NATS.Client.IAsyncSubscription" /> returned will start delivering messages
    /// to the event handler as soon as they are received. The caller does not have to invoke
    /// <see cref="M:NATS.Client.IAsyncSubscription.Start" />.</remarks>
    /// <param name="subject">The subject on which to listen for messages.
    /// The subject can have wildcards (partial: <c>*</c>, full: <c>&gt;</c>).</param>
    /// <param name="handler">The <see cref="T:System.EventHandler`1" /> invoked when messages are received
    /// on the returned <see cref="T:NATS.Client.IAsyncSubscription" />.</param>
    /// <returns>An <see cref="T:NATS.Client.IAsyncSubscription" /> to use to read any messages received
    /// from the NATS Server on the given <paramref name="subject" />.</returns>
    /// <seealso cref="P:NATS.Client.ISubscription.Subject" />
    IAsyncSubscription SubscribeAsync(string subject, EventHandler<EncodedMessageEventArgs> handler);

nive-2510
  • 63
  • 3

1 Answers1

3

The problem is ...

Yup. The core problem is that the library requires a synchronous handler (void return type) and doesn't understand / properly deal with asynchronous handlers (Task return type).

First, I would ask the library authors to permit asynchronous handlers.

I use Custom Attribute the find out if the action is async action or sync

This could be a bit dangerous. If you mean you have your own attribute type, then sure, that would work, as long as you remember to put it on all your handlers. A code analyzer would be helpful there. If you mean you are detecting the compiler attributes for async methods, then I would recommend against that; it would break if the compiler attribute names ever changed, or if the handler itself is not async but invokes an async method.

looks like I have to choose between two bad options

Yes. Choosing between bad options is where you always end up when you're forced to do sync-over-async.

The first is is use GetAwaiter().GetResults();

This is one approach (the "direct blocking hack"). See my article on brownfield async for some other approaches.

and the second is to do the method - async void and wrap the invocation with try and catch so I will catch exceptions when they occur before the synchronization Context.

That won't work. The other approach is to add a top-level try/catch within each async void method. You can't wrap the invocation because the exception has already been caught by the async void state machine and passed to the synchronization context. It has to be caught within the async void method instead.

It may be possible to define an extension SubscribeAsync method that takes a Func<EncodedMessageEventArgs, Task> handler; you can provide the top-level try/catch there in an async void lambda and pass that to the original SubscribeAsync.

Stephen Cleary
  • 437,863
  • 77
  • 675
  • 810
  • Thanks for your answer @Stephen Cleary First I use .Net attribute that is stored in the custom attribute list as the following: `private bool IsAsyncAction(Action> action) { return action.Method.GetCustomAttribute(typeof(AsyncStateMachineAttribute)) != null; }` So the async keyword inserts this attribute and I don't have to remember to mark each function. – nive-2510 Aug 15 '22 at 14:52
  • as you can see, in the case of async action I create a task from the actions so if an exception will occur, I will be able to catch it. So I do a walkaround but it's still inside the async void function and I don't want to insert this code smell into our code base. – nive-2510 Aug 15 '22 at 14:53
  • @nive-2510: Yes, I see the try/catch does go inside the async void, so that is correct. I'm still not wild about depending on the compiler state machine attribute, though, for the reasons mentioned in my answer. – Stephen Cleary Aug 15 '22 at 15:59