vescon / MethodBoundaryAspect.Fody

A Fody weaver which allows to decorate methods and hook into method start, method end and method exceptions.
MIT License
243 stars 75 forks source link

Decorating "async void" method throws weaver exception for aspect with OnException override #111

Open jowliwon opened 1 year ago

jowliwon commented 1 year ago

So, the thing is I have this injection script I run on all my source files so they can all be monitored and observed for issues. It works well, instead of just do all of this manually.

I have these put on class level, not on individual methods like the example bellow.

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace TraceLog.Debugger.TestObjects
{
    [ExceptionLogger]
    public class MainAwait
    {
        public MainAwait()
        {

        }

        private async void GetAsync()
        {
            await WaitForResponse();
        }

        private async Task<bool> WaitForResponse()
        {
            await Task.Delay(1);
            return true;
        }
    }
}

How ever as Im only interested in capturing Exceptions at this stage, I have the ExceptionLogger it self look like this

using MethodBoundaryAspect.Fody.Attributes;
using TraceLog.AttributeHandlers;
using TraceLog.Configuration;
using System;
using System.Collections.Generic;
using System.Text;

namespace TraceLog
{
    public class ExceptionLoggerAttribute : OnMethodBoundaryAspect
    {
        private readonly ExecutionTypes executionTypes = ExecutionTypes.Instance;

        public override void OnException(MethodExecutionArgs arg)
        {
                executionTypes.OnEntryActivity(arg);
                executionTypes.OnExceptionActivity(arg);            
        }
    }
}

This how ever will cause the weaver to drop error message on by that not compile.

Severity    Code    Description Project File    Line    Suppression State
Error       Fody: An unhandled exception occurred:
Exception:
Failed to execute weaver D:\Repos\tracelogger\packages\MethodBoundaryAspect.Fody.2.0.148\build\..\weaver\MethodBoundaryAspect.Fody.dll
Type:
System.Exception
StackTrace:
   at InnerWeaver.ExecuteWeavers() in C:\projects\fody\FodyIsolated\InnerWeaver.cs:line 221
   at InnerWeaver.Execute() in C:\projects\fody\FodyIsolated\InnerWeaver.cs:line 111
Source:
FodyIsolated
TargetSite:
Void ExecuteWeavers()
Async state machine for System.Void TraceLog.Debugger.TestObjects.MainAwait::GetAsync() did not catch exceptions in the expected way.
Type:
System.InvalidOperationException
StackTrace:
   at MethodBoundaryAspect.Fody.AsyncMethodWeaver.WeaveOnException(IList`1 allAspects, Instruction instructionCallStart, Instruction instructionCallEnd, Instruction instructionAfterCall, IPersistable returnValue)
   at MethodBoundaryAspect.Fody.MethodWeaver.Weave()
   at MethodBoundaryAspect.Fody.ModuleWeaver.WeaveMethod(ModuleDefinition module, MethodDefinition method, List`1 aspectInfos, MethodInfoCompileTimeWeaver methodInfoCompileTimeWeaver)
   at MethodBoundaryAspect.Fody.ModuleWeaver.WeaveType(ModuleDefinition module, TypeDefinition type, Collection`1 assemblyMethodBoundaryAspects)
   at MethodBoundaryAspect.Fody.ModuleWeaver.WeaveTypeAndNestedTypes(ModuleDefinition module, TypeDefinition type, Collection`1 assemblyMethodBoundaryAspects)
   at MethodBoundaryAspect.Fody.ModuleWeaver.Execute(ModuleDefinition module)
   at MethodBoundaryAspect.Fody.ModuleWeaver.Execute()
   at InnerWeaver.ExecuteWeavers() in C:\projects\fody\FodyIsolated\InnerWeaver.cs:line 185
Source:
MethodBoundaryAspect.Fody
TargetSite:
Void WeaveOnException(System.Collections.Generic.IList`1[MethodBoundaryAspect.Fody.AspectData], Mono.Cecil.Cil.Instruction, Mono.Cecil.Cil.Instruction, Mono.Cecil.Cil.Instruction, MethodBoundaryAspect.Fody.IPersistable) TraceLog.Debugger           

This only happens when dealing with OnException on class level, OnEntry and OnExit passes just fine. Is there anything that can be done to deal with an issue like this?

Ralf1108 commented 1 year ago

Does it compile if you provide empty overrides for OnEntry and OnExit methods?

jowliwon commented 1 year ago

Im afraid not. As soon as OnException is in the code it stops compiling, with or without OnEntry and OnExit.

Ralf1108 commented 1 year ago

Can you try to return a Task or an int from the GetAsync() method? Maybe there are problems with fire and forget async method weaving

jowliwon commented 1 year ago

That worked!

        private async Task GetAsync()
        {
            await WaitForResponse();
        }
        private async Task<bool> WaitForResponse()
        {
            await Task.Delay(1);
            return true;
        }
Ralf1108 commented 1 year ago

Great. Then this should also be reproducible if the attribute is just decorating the GetAsync() method if it returns void?

jowliwon commented 1 year ago

Yes, by having this on method level, its the same behavior. With and with out returning Task.

Ralf1108 commented 1 year ago

So then the bug is handling of void async methods. Wondering why this was not detected earlier.

So all in all you have a workaround and we can then create a test case for the issue

elmondir commented 1 year ago

any workaround for this issue?

jowliwon commented 1 year ago

any workaround for this issue?

Sort of. Have to return a task, just read above.