Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

How about Kind.Exception #153

Open
jerviscui opened this issue Apr 20, 2021 · 6 comments
Open

How about Kind.Exception #153

jerviscui opened this issue Apr 20, 2021 · 6 comments

Comments

@jerviscui
Copy link
Contributor

Now, if want to get Exception must be add a Kind.Around Advice.
It will create code like:

[Advice(Kind.Around, Targets = Target.Method)]
public object Handle([Argument(Source.Name)] string name, [Argument(Source.Target)] Func<object[], object> method, 
    [Argument(Source.Arguments)] object[] arguments)
{
    try
    {
        var result = method(arguments);
        return result;
    }
    catch (Exception e)
    {
        Console.WriteLine(e);
        throw;
    }
}

The rethrow Exception has waves method stack, just like:

at AspectInjectorTest.AsyncAroundTest.__a$_around_SyncTest_100663303_o()
at AspectInjectorTest.AsyncAroundTest.__a$_around_SyncTest_100663303_u(Object[] )
at AspectInjectorTest.ArroundAttribute.Handle()

Perhaps, let Kind.Exception Advice waves into target method,

public void Test()
{
    try{
        //origin method body
    }
    catch{
        //Kind.Exception Advice execute
    }    
}

and Task method,

private sealed class <AwaitTest>d__1 : IAsyncStateMachine
{
    private void MoveNext()
    {
        int num = <>1__state;
        try
        {
            //TaskAwaiter awaiter;            
        }
        catch (Exception exception)
        {
            //Kind.Exception Advice execute
            <>t__builder.SetException(exception);
            return;
        }
        ref AsyncTaskMethodBuilder reference = ref <>t__builder;
        //Aspect After
        __a$_after_state_machine(null);
        reference.SetResult();
    }
}

Just a thought.
Thankyou.

@pamidur
Copy link
Owner

pamidur commented Jun 11, 2021

Hi @jerviscui , thanks for the suggestion!
A few questions: is the only inconvenience with Kind.Around is the stacktrace? Or there some other concerns?

your suggestion is definitely doable , but sometimes ago there were talks about getting rid of everything except Kind.Around in favor of stability and robustness

@pamidur
Copy link
Owner

pamidur commented Jun 11, 2021

omg, just realized you posted in April, I'm so sorry))
I'm not ignoring this project, I was just away

@jerviscui
Copy link
Contributor Author

It doesn't matter.
This project is so cool. Thank you!

@pamidur
Copy link
Owner

pamidur commented Jun 17, 2021

Thank you!
If we want to implement Kind.Exception we need to answer these(and maybe more) questions:

  • Can we choose exceptions to cover? How this is done in an aspect?
  • Does this Kind allows to return a value as if method call was successful or exception is always re-thrown ?
  • Do we re-throw original exception or advice is allowed to substitute exceptions?

@devfinnd
Copy link

Since this Thread has been silent for quite a while I'd like to chime in and give some suggestions/opinions in hopes of this feature being released someday:

Choosing exceptions could likely be done by requiring the Attribute to implement some kind of Method/Interface through which the Types could be gathered. Alternatively with the upcoming generic Attributes there could be a required generic Parameter on the Attribute which could then be read in some form like triggers are currently.

Rethrow should imo be opt-in since Exception handling should be assumed to actually "handle" the Exception in some way

In general rethrowing should probably be something like how BackgroundWorker in Winforms goes about it by catching any exception and providing it to the _Completed Event as a Parameter Property.

Something like [Argument(Source.Exception)] Exception exception or [Argument(Source.Exception)] TException exception with a generic Constraint would imo look and feel the best

In an optimal world this would also not modify the stacktrace like Kind.Around does and instead preserve all information with the rethrow part actually rethrowing in the original method.

@pamidur
Copy link
Owner

pamidur commented May 30, 2022

Thank you for input, I assure you this feature is on the table we're looking into it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants