pamidur / aspect-injector

AOP framework for .NET (c#, vb, etc)
Apache License 2.0
742 stars 111 forks source link

Feature: Extend Universal attributes to support async aspects #230

Open fcastells opened 4 months ago

fcastells commented 4 months ago

Describe the solution you'd like Add the ability to execute async code before, after and on exception in Universal attributes

Additional context Currently we can apply a Universal attribute to an async method (most generically, a method that returns Task), but it's not possible to run async code around that method.

pamidur commented 3 months ago

Hi, I've seen your PR. It passes all the tests, but before I merge it. Could you please give me an example on how do you like to use those methods?

I'm just not sure if having two set of methods there is justified. Maybe one set of methods (async only) is enough.

fcastells commented 3 months ago

Hi, I'm already using this on my project actually (by copying the code for now). My use case is as follows: I have a test suite using Playwright for front-end testing. Each test is divided into steps. What I wanted is to automatically take a screenshot after each step, so I created an aspect that implements OnAfter and OnException, gets the instance, gets the page object from it and then needs to call await page.ScreenshotAsync(). Initially, I did page.ScreenshotAsync().GetAwaiter().GetResult(), but that's not the right way of doing it.

I agree with you that having only the async versions is the right way, but I didn't want to make a breaking change. This is up to you, but maybe you could mark the synchronous versions as obsolete and remove them in a future update, to give time to people to migrate the code.

pamidur commented 3 months ago

Thank you for clarification. I'll need yet another day to digest this PR.

fcastells commented 3 months ago

No worries. Let me know if you need further clarification or changes.

fcastells commented 2 months ago

Following up on this. Let me know if I can clarify or improve anything. Thanks.

kyro95 commented 2 months ago

Hi there, when the merge of the code is gonna issue? I need async aspects as well