pamidur / aspect-injector

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

is aspect class do aspect your self?? #111

Closed kargencW closed 4 years ago

kargencW commented 4 years ago
[Aspect(Scope.PerInstance)]
    [Injection(typeof(AspectLogForMethodArountExceptionAndLog))]
    public class AspectLogForMethodArountExceptionAndLog : Attribute
    {
        private static readonly MethodInfo _asyncHandler = typeof(AspectLogForMethodArountExceptionAndLog).GetMethod(nameof(AspectLogForMethodArountExceptionAndLog.WrapAsync), BindingFlags.NonPublic | BindingFlags.Static);
        private static readonly MethodInfo _syncHandler = typeof(AspectLogForMethodArountExceptionAndLog).GetMethod(nameof(AspectLogForMethodArountExceptionAndLog.WrapSync), BindingFlags.NonPublic | BindingFlags.Static);
        private static readonly Type _voidTaskResult = Type.GetType("System.Threading.Tasks.VoidTaskResult");

        private ILogger logger;

        [Advice(Kind.Around)]
        public object Handle(
            [Argument(Source.Target)] Func<object[], object> target,
            [Argument(Source.Arguments)] object[] args,
            [Argument(Source.Name)] string name,
            [Argument(Source.ReturnType)] Type retType
            )
        {
            logger = PrncLogFactory.GetLogger();
            var operationName = $"{target.Method.DeclaringType.FullName}:{name}";

            if (typeof(Task).IsAssignableFrom(retType))
            {
                var syncResultType = retType.IsConstructedGenericType ? retType.GenericTypeArguments[0] : _voidTaskResult;
                var tgt = target;
               return _asyncHandler.MakeGenericMethod(syncResultType).Invoke(this, new object[] { tgt, args, operationName, logger }); 
            }
            else
            {
                retType = retType == typeof(void) ? typeof(object) : retType;
                return _syncHandler.MakeGenericMethod(retType).Invoke(this, new object[] { target, args, operationName, logger });
            }
        }

        private static T WrapSync<T>(Func<object[], object> target, object[] args, string name, ILogger logger)
        {
            try
            {
                logger.WriteLog(Microsoft.Extensions.Logging.LogLevel.Debug, null, $"Method `{name}` started.");
                var result = (T)target(args);
                logger.WriteLog(Microsoft.Extensions.Logging.LogLevel.Debug, null, $"Method `{name}` finished.");
                return result;
            }
            catch (Exception e)
            {
                logger.WriteLog(Microsoft.Extensions.Logging.LogLevel.Error, e, $"Method `{name}` throws {e.GetType()} exception.");
                throw;
            }
        }

        private static async Task<T> WrapAsync<T>(Func<object[], object> target, object[] args, string name, ILogger logger)
        {
            try
            {
                logger.WriteLog(Microsoft.Extensions.Logging.LogLevel.Debug, null, $"Method `{name}` started.");
                var result = await ((Task<T>)target(args)).ConfigureAwait(false);
                logger.WriteLog(Microsoft.Extensions.Logging.LogLevel.Debug, null, $"Method `{name}` finished.");
                return result;
            }
            catch (Exception e)
            {
                logger.WriteLog(Microsoft.Extensions.Logging.LogLevel.Error, e, $"Method `{name}` throws {e.GetType()} exception.");
                throw;
            }
        }
    }

this is my log aspect for logging, its actually logged injected class when throws ,but has another log second time same throws

First One : System.InvalidOperationException: An invalid request URI was provided. The request URI must either be an absolute URI or BaseAddress must be set. at System.Net.Http.HttpClient.PrepareRequestMessage(HttpRequestMessage request) at System.Net.Http.HttpClient.SendAsync(HttpRequestMessage request, HttpCompletionOption completionOption, CancellationToken cancellationToken) at System.Net.Http.HttpClient.GetAsync(Uri requestUri, HttpCompletionOption completionOption, CancellationToken cancellationToken)

Second One : System.Reflection.TargetInvocationException: Exception has been thrown by the target of an invocation. ---> System.InvalidOperationException: An invalid request URI was provided. The request URI must either be an absolute URI or BaseAddress must be set. at System.Net.Http.HttpClient.PrepareRequestMessage(HttpRequestMessage request) at System.Net.Http.HttpClient.SendAsync(HttpRequestMessage request, HttpCompletionOption completionOption, CancellationToken cancellationToken) at System.Net.Http.HttpClient.GetAsync(Uri requestUri, HttpCompletionOption completionOption, CancellationToken cancellationToken)xxxxxxxxx__a$_around_Get_100663318_o(DataSourceLoadOptions loadOptions)

pamidur commented 4 years ago

Hi, It is not enough info to reproduce and understand that, but my guess would be that you have two methods calling one another like this:

method1->method2

then you most probably applied logger to both of them like:

[Log]method1->[Log]method2

it in turn is converted into

log->method1->log->method2

then since exception is only logged, but not catched it goes all the way back to first log call like this:

log->method1->log->method2
______________________|
throws exception
log->method1->log->method2
_______________|
logged here 1st time, no catch
log->method1->log->method2
_______|
no catch
log->method1->log->method2
_|
logged here 2nd time
kargencW commented 4 years ago

second error "System.Reflection.TargetInvocationException" can we accept, its not a problem for code flow.

pamidur commented 4 years ago

I just wanted to suggest a bit another approach in case you want to apply [Log] to everything and forget about it. First you can catch TargetInvocationException and unwrap it. Second, you can wrap original exceptions in new exception class that contains information if this exception was logged or not.

[Aspect(Scope.PerInstance)]
    [Injection(typeof(Log))]
    public class Log : Attribute
    {
        private static readonly MethodInfo _asyncHandler = typeof(Log).GetMethod(nameof(Log.WrapAsync), BindingFlags.NonPublic | BindingFlags.Static);
        private static readonly MethodInfo _syncHandler = typeof(Log).GetMethod(nameof(Log.WrapSync), BindingFlags.NonPublic | BindingFlags.Static);
        private static readonly Type _voidTaskResult = Type.GetType("System.Threading.Tasks.VoidTaskResult");

        [Advice(Kind.Around)]
        public object Handle(
            [Argument(Source.Target)] Func<object[], object> target,
            [Argument(Source.Arguments)] object[] args,
            [Argument(Source.Name)] string name,
            [Argument(Source.ReturnType)] Type retType
            )
        {
            var operationName = $"{target.Method.DeclaringType.FullName}:{name}";

            if (typeof(Task).IsAssignableFrom(retType))
            {
                var syncResultType = retType.IsConstructedGenericType ? retType.GenericTypeArguments[0] : _voidTaskResult;
                var tgt = target;
                return InvokeWrapper(_asyncHandler.MakeGenericMethod(syncResultType), target, args, operationName);
            }
            else
            {
                retType = retType == typeof(void) ? typeof(object) : retType;
                return InvokeWrapper(_syncHandler.MakeGenericMethod(retType), target, args, operationName);
            }
        }

        private object InvokeWrapper(MethodInfo method, Func<object[], object> target, object[] args, string opname)
        {
            try
            {
                return method.Invoke(this, new object[] { target, args, opname });
            }
            catch (TargetInvocationException te)
            {
                throw te.InnerException;
            }
        }

        private static T WrapSync<T>(Func<object[], object> target, object[] args, string name)
        {
            try
            {
                Console.WriteLine($"Method `{name}` started.");
                var result = (T)target(args);
                Console.WriteLine($"Method `{name}` finished.");

                return result;
            }
            catch(LogException le)
            {
                if (!le.Logged)
                {
                    le.Logged = true;
                    Console.WriteLine($"Method `{name}` throws {le.GetBaseException().GetType()} exception.");
                }
                throw;
            }
            catch (Exception e)
            {
                Console.WriteLine($"Method `{name}` throws {e.GetType()} exception.");
                throw new LogException(e) { Logged = true };
            }
        }

        private static async Task<T> WrapAsync<T>(Func<object[], object> target, object[] args, string name)
        {
            try
            {
                Console.WriteLine($"Method `{name}` started.");
                var result = await ((Task<T>)target(args)).ConfigureAwait(false);
                Console.WriteLine($"Method `{name}` finished.");
                return result;
            }
            catch (LogException le)
            {
                if (!le.Logged)
                {
                    le.Logged = true;
                    Console.WriteLine($"Method `{name}` throws {le.GetBaseException().GetType()} exception.");
                }
                throw;
            }
            catch (Exception e)
            {
                Console.WriteLine($"Method `{name}` throws {e.GetType()} exception.");
                throw new LogException(e) { Logged = true };
            }
        }
    }

class LogException: Exception
    {
        private readonly Exception _original;

        public LogException(Exception original)
            : base(original.Message, original.InnerException)
        {
            _original = original;
        }

        public bool Logged { get; set; } = false;        

        public override IDictionary Data => _original.Data;

        public override string HelpLink { get => _original.HelpLink; set => _original.HelpLink = value; }

        public override string Message => _original.Message;

        public override string Source { get => _original.Source; set => _original.Source = value; }

        public override string StackTrace 
            => _original.StackTrace;

        public override bool Equals(object obj) 
            => _original.Equals(obj);

        public override Exception GetBaseException() 
            => _original;

        public override int GetHashCode() 
            => _original.GetHashCode();

        public override void GetObjectData(SerializationInfo info, StreamingContext context) 
            => _original.GetObjectData(info, context);

        public override string ToString()
            => _original.ToString();
    }