Document examples of error handling with the different async interception options

[DaEkstrim]

I suggest that in the README.md, some examples on exception handling (such as logging an exception to Trace) are added, because I think that when using the ProcessingAsyncInterceptor option, this is not possible, and thus it would help the package users to understand better which option to use.

[JSkimming]

@DaEkstrim I've just been looking at this and it's relatively complicated. So I've given a stab at helper base class to simplify exception handling.

Have a look at TestSimpleAsyncInterceptor in the #20. Any feedback is appreciated.

[DaEkstrim]

@JSkimming Thank you for looking into this. I should have been clearer in my original request. What I meant to say is that the documentation on the front page should make it clear that exception handling through ProcessingAsyncInterceptor<TState> cannot be done, in which case the developer should either implement the IAsyncInterceptor or now use the SimpleAsyncInterceptor

As you said, yes it's a complex problem to solve as the execution flow varies vastly between void/T Task and Task<T>, however the SimpleAsyncInterceptor might help cut down the implementation required for IAsyncInterceptor drastically, so when I've got a moment, I'll switch my interceptors to use your new implementation rather than my abstract and let you know how it goes and whether it caters for most scenarios.

[JSkimming]

@DaEkstrim I've further enhanced PR #20 to the pointy where I'm happy with the solution.

SimpleAsyncInterceptor required a derived class to implement two methods.

1. Task InterceptAsync()
2. Task<TResult> InterceptAsync<TResult>()

There needs to be two because the returned task is used to replace the return value of the IInvocation.

Since these are both Task returning methods, you can write then using the async/await pattern.

The base class SimpleAsyncInterceptor does the work of executing synchronous methods in an asynchronous way, allowing the derived class to treat all invocations the same way.

An simple example could be as follows:

public class ExceptionHandlingInterceptor : SimpleAsyncInterceptor
{
    protected override async Task InterceptAsync(IInvocation invocation, Func<IInvocation, Task> proceed)
    {
        try
        {
            await proceed(invocation).ConfigureAwait(false);
        }
        catch (Exception e)
        {
            // Add custom handling of the exception here or, re-throw.
            throw;
        }
    }

    protected override async Task<T> InterceptAsync<T>(IInvocation invocation, Func<IInvocation, Task<T>> proceed)
    {
        try
        {
            return await proceed(invocation).ConfigureAwait(false);
        }
        catch (Exception e)
        {
            // Add custom handling of the exception here, or re-throw.
            throw;
        }
    }
}

What do you think?

[JSkimming]

Also, I'm note sure about the name SimpleAsyncInterceptor but it's all I got so far. Any ideas?

[JSkimming]

@DaEkstrim I've just pushed a version 1.4 to NuGet.

I've also updated the documentation on how best to perform exception handling.

[DaEkstrim]

@JSkimming Apologies for taking long to get back to you. I haven't yet got a chance to test your new updated version, due to personal commitments, however I have a task on my JIRA so it's definitely on my next things to do.

With regards to the name, SimpleAsyncInterceptor might not convey the concern of the type at first look, but so do 99% of Microsoft's type names, so I wouldn't worry too much about it. If its backed by documentation you can name it whatever your heart desires. 👍 You may keep the issue closed. I shall update it as soon as I have made progress on implementing your new version.

Edit: I just saw that you changed the name to AsyncInterceptorBase which is much better at explaining itself. Good choice!

[JSkimming]

@DaEkstrim Yes, having now given it greater consideration, I think this could be the default way in which people intercept invocations. It's why I placed it as option 1.

I could go back and update the other two abstract classes, but that would have been a breaking change and it wouldn't have enhanced things in any way, so I decided to leave them as is.

I'd love to hear how things work out for you. It's great to know my library is getting use out there in the wild 😃

[DaEkstrim]

@JSkimming So I finally got some spare time to convert my code to use version 1.4.0, using Option 1: AsyncInterceptorBase, however while reviewing the type and its base types, I found none which derive from IInterceptor which is what the Castle.Core.DynamicProxy requires to operate and in fact when I tried to use my class which derives from AsyncInterceptorBase the proxy thew the following error:

InvalidCastException: Unable to cast object of type 'SwitchX.Implementation.DependencyInjection.InvocationLogXInterceptor' to type 'Castle.DynamicProxy.IInterceptor'

Am I missing something?

Edit: Alrite, so I downloaded the repository to look into how the tests are being executed, and I found that it is using a custom extension method CreateInterfaceProxyWithTargetInterface to convert the IAsyncInterceptor to IInterceptor. But for this to work it needs the interceptor to be instantiated while being registered through the extension method, which presents me with a bit of a problem, because I let the IoC container instantiate the interceptors for me and register them with the proxy while instantiating the service which needs to be intercepted. This helps in the cases where I need to inject services into the interceptors (such as loggers, transaction unit of work handlers, and so on). So bottom line, is that I can't use Option 1 for such cases. I will look into trying to create a generic AsyncDeterminiationInterceptor to go around the problem and have it accept a factory for the interceptor instead of an instance.

[JSkimming]

@DaEkstrim AsyncInterceptorBase isn't intended to implement IInterceptor but IAsyncInterceptor.

How did you do it previously? Presumably you either implemented IAsyncInterceptor directly which is now option 2, or you derived from ProcessingAsyncInterceptor or AsyncTimingInterceptor, which themselves implement IAsyncInterceptor, which is now option 3.

This library works by requiring you to provide an implementation of IAsyncInterceptor with one of the extension methods in ProxyGeneratorExtensions that do take IAsyncInterceptor. The readme states:

Create a class that extends the abstract base class AsyncInterceptorBase, then register it for interception in the same was as IInterceptor using the ProxyGenerator extension methods, e.g.

All these extension methods are intended to reflect exactly the methods on ProxyGenerator. It's possible there are newer methods on ProxyGenerator that are not reflected on ProxyGeneratorExtensions, and you just happen to trying to use one.

Can you share some sample code?

[DaEkstrim]

@JSkimming Thanks for the response. Yes after downloading the source code and studying it, I found that it was the extension methods that were taking care of wrapping the IAsyncInterceptor in the AsyncDeterminiationInterceptor which DOES implement the IInterceptor. Which is fine if the proxies are generated during design time. However I'm creating proxies for interfaces during the instantiation of the classes from the IoC. I do that by registering the interceptors with the IoC itself during the registration of the service. (I'm using Autofac in this case)

registration.EnableInterfaceInterceptors().InterceptedBy(interceptors.ToArray());

Where registration is the service registration builder and the interceptors.ToArray() is a collection of Type containing types of classes which derive from IInterceptor. This way the IoC container creates the interceptor(s) itself whenever the service is registered to be intercepted. This has the advantage that I can use constructor injection inside the interceptors themselves (such as injecting the UnitOfWork implementation for transactional interceptors).

This means that I cannot use the extension methods provided to instantiate the interceptors, since the container's instantiation logic is (and should be) out of my reach.

So what I did was take your implementation of AsyncDeterminiationInterceptor and make it generic.

    /// <summary>
    /// Intercepts method invocations and determines if is an asynchronous method.
    /// </summary>
    public class AsyncDeterminationInterceptor<T> : IInterceptor where T : IAsyncInterceptor
    {
        private static readonly MethodInfo HandleAsyncMethodInfo =
            typeof(AsyncDeterminationInterceptor)
                    .GetMethod(nameof(HandleAsyncWithResult), BindingFlags.Static | BindingFlags.NonPublic);

        private static readonly ConcurrentDictionary<Type, GenericAsyncHandler> GenericAsyncHandlers =
            new ConcurrentDictionary<Type, GenericAsyncHandler>();

        private readonly IAsyncInterceptor _asyncInterceptor;

        /// <summary>
        /// Initializes a new instance of the <see cref="AsyncDeterminationInterceptor"/> class.
        /// </summary>
        public AsyncDeterminationInterceptor(T asyncInterceptor)
        {
            _asyncInterceptor = asyncInterceptor;
        }

And register both the interceptor and the determination wrapper with the IoC, like this

 builder.RegisterType<UnitOfWorkXInterceptor>();
 builder.RegisterType<AsyncDeterminationInterceptor<UnitOfWorkXInterceptor>>();

where UnitOfWorkXInterceptor derives from AsyncInterceptorBase,

public class UnitOfWorkXInterceptor : AsyncInterceptorBase { ... }

This way the UnitOfWorkXInterceptor has become a constructor requirement for AsyncDeterminationInterceptor<UnitOfWorkXInterceptor>, and since they are both registered in the container, the container will know how to instantiate the dependency graph to create the interceptors required to generate a proxy to intercept a particular service without leaking concerns of how the proxy should be generated (that is leaving it as it was intended by the Castle team)

I know this is a lot, so if you want me to clarify anything which I wasn't clear enough on, let me know and I'll elaborate.

[DaEkstrim]

@JSkimming I actually found a simpler solution for the generic AsyncDeterminiationInterceptor without having to modify your implementation and use the implementation that is provided from the package.

public class AsyncDeterminationGenericInterceptor<T> : AsyncDeterminationInterceptor where T : IAsyncInterceptor
{
    public AsyncDeterminationGenericInterceptor(T interceptor) : base(interceptor)
    {
    }
}

This way I achieved the same thing as I stated in the above post AND I get to keep your implementation as is.

[JSkimming]

@DaEkstrim I see. That's great, glad you got it working.