Support OnException for Async Methods

[swestner]

Currently OnException does not trigger if an exception is thrown from an await in an async method.

[Ralf1108]

Can you provide an example code or a unit test to show this?

[swestner]

Sure.

I have a project here that replicates the issue :

https://github.com/swestner/MethodBoundaryAspect.Test

I also have a fork of this project I have tried to replicate the issue in, but for some reason I get a serialization exception (something to the effect 'Task can not be serialized' when returning a task) when using the AssemblyResolver to invoke the async method.

https://github.com/swestner/MethodBoundaryAspect.Fody

PS : that avatar is contagious, makes me smile everytime

[Ralf1108]

ok thx for the test. I will have a look into it after I finished working on issue Suppressing Rethrow #2

[Ralf1108]

Feel free to use my avatar as desktop background for the daily morning smile :-)

[Ralf1108]

can you retry this with latest method boundary aspect?

[Ralf1108]

possible workaround: move async method body in normal method and apply aspect to this method

[swestner]

Retrying with the latest method boundary aspect didn't work. I updated the Nugets in https://github.com/swestner/MethodBoundaryAspect.Test and the test still failed.

Moving the body of the async method to a sync method isn't really an option for us. We have gone async all the way and there are too many calls to other libraries (both system and external) that we would need to force to sync and then back to async. It would be incredibly messy.

[mightypanda]

I second that, same problem here, huge library all async.. no joy for OnException() How did you work around this problem?

[marcells]

Currently we don't have a solution for it. It's quite complex, cause of the generated state machine by the c# compiler. Similar issues should occur for methods with yield return.

If you have an idea how to implement this feature, feel free to make a PR.

[keith-anders]

I just submitted a PR for this (#39). Apologies for the large PR, but it's a complex feature. I don't know why it's not showing up in the history of this issue.

[Ralf1108]

Can somebody verify that this is fixed with PR #39 ?

[keith-anders]

So there is one more issue I have to fix. Specifically, I've now discovered that it is throwing a compile-time exception when weaving an async method with an aspect that handles OnException but no aspect which handles OnEntry. My fault for not writing simple tests before complex ones, I suppose. I will submit a PR for that now.

However, upon looking at the test project provided by @swestner I'm curious what the actual expected behavior is. What I implemented in my prior PR is this: when an exception happens during the async code, let the state machine record the exception on the task and complete the task as faulted with the original exception, then run OnException on any appropriate aspects. If exceptions happen during an aspect's OnException handler, the first one to happen simply swallows the rest of the handlers, and no further action is taken.

The test project, on the other hand, says that the expected behavior is this: when an exception happens during the async code, run the OnException handlers. Take the first exception which happens during one of those OnException handlers and then tell the state machine to complete the task as faulted by the exception thrown by OnException, not the original exception. Only then is the task completed.

If that is indeed the desired behavior, then that's fine. I can implement that, though it will take a few more days. However, I don't know whether the test project was implemented that way because exceptions thrown during OnException are actually important to his use case or just because it was a convenient way of demonstrating that async OnException wasn't working properly. It would be great if I could get some clarification on whether the test project's specific behavior regarding exceptions thrown during OnException is actually required. In the meantime, I'll submit the PR for the OnEntry fix.

[Wsm2110]

This still is not 100% bulletproof though. Any chance of this having an update anytime soon? Having troubles compiling using 1.0.62

[keith-anders]

The sample project given by @swestner builds fine with 1.0.62 (though the test doesn't pass, pending my previous question). Do you have a repo that reproduces the issue you're seeing, @Wsm2110 ?

[Wsm2110]

Its a private repo managed by my company, so i guess ive to look into this myself. But just giving you the heads up it gives troubles with large complex async await projects.

I will keep you updated if i find anything worth mentioning

[plompp001]

We are also are having an issue with Async methods using MethodBoundaryAspect.Fody. We are getting an 'InvalidProgramException' on async (void) methods containing a try-catch. Please take a look at the sample project (https://github.com/plompp001/SampleFodyAsync). The issue shows up at RunAsyncAndGetError(). It seems like there needs to be a return statement when using a try-catch, otherwise the error occures.

Could you help us fixing this issue? Thank you in advance.

[timonsdad]

I'm hoping to use this weaver to do exception wrapping in my project. If I'm understanding correctly, that will require the behaviour that @swestner has demonstrated in his test repo (i.e. the state machine throws the exception thrown by the OnException method. If there's another way to accomplish this that I'm missing, please let me know.

In an ideal world, I'd volunteer to work on this, but before today, I knew nothing about IL weaving, and this doesn't really sound like a good "starter" issue. @keith-anders if you're still willing to work on it, that would be super appreciated. I realize this issue has been sitting here for almost two years, so I'll totally understand if you've moved on from this. If nobody is able to help out, I'll probably eventually look into doing it myself, I just don't think I have the time to commit to learn how to do this properly at the moment.

Thanks.