Add aspect exception wrapper (#148)

* Add aspect exception wrapper * Fix AspectCore.Extensions.Autofac.Sample * Fix samples * Fix issue 138 * update version * Fix AspectExceptionWrapper service
dotnetcore · Nov 25, 2018 · f6b67cf · f6b67cf
1 parent ee8aff7
commit f6b67cf
Show file tree

Hide file tree

Showing 29 changed files with 138 additions and 76 deletions.
diff --git a/build/version.props b/build/version.props
@@ -1,8 +1,8 @@
 <Project>
 	<PropertyGroup>
 		<VersionMajor>1</VersionMajor>
-		<VersionMinor>0</VersionMinor>
-		<VersionPatch>1</VersionPatch>
+		<VersionMinor>1</VersionMinor>
+		<VersionPatch>0</VersionPatch>
 		<VersionQuality></VersionQuality>
 		<VersionPrefix>$(VersionMajor).$(VersionMinor).$(VersionPatch)</VersionPrefix>
 	</PropertyGroup>

diff --git a/core/src/AspectCore.Abstractions/Configuration/IAspectConfiguration.cs b/core/src/AspectCore.Abstractions/Configuration/IAspectConfiguration.cs
@@ -11,5 +11,7 @@ public interface IAspectConfiguration
         InterceptorCollection Interceptors { get; }
 
         NonAspectPredicateCollection NonAspectPredicates { get; }
+
+        bool ThrowAspectException { get; set; }
     }
 }
diff --git a/core/src/AspectCore.Abstractions/DynamicProxy/AspectExceptionWrapper.cs b/core/src/AspectCore.Abstractions/DynamicProxy/AspectExceptionWrapper.cs
@@ -0,0 +1,31 @@
+using System;
+using AspectCore.Configuration;
+
+namespace AspectCore.DynamicProxy
+{
+    [NonAspect]
+    public class AspectExceptionWrapper : IAspectExceptionWrapper
+    {
+        private readonly IAspectConfiguration _configuration;
+
+        public AspectExceptionWrapper(IAspectConfiguration configuration)
+        {
+            _configuration = configuration;
+        }
+
+        public Exception Wrap(AspectContext aspectContext, Exception exception)
+        {
+            if (!_configuration.ThrowAspectException)
+            {
+                return exception;
+            }
+
+            if (exception is AspectInvocationException aspectInvocationException)
+            {
+                return aspectInvocationException;
+            }
+
+            return new AspectInvocationException(aspectContext, exception);
+        }
+    }
+}
diff --git a/core/src/AspectCore.Abstractions/DynamicProxy/AspectInvocationException.cs b/core/src/AspectCore.Abstractions/DynamicProxy/AspectInvocationException.cs
@@ -9,6 +9,7 @@ public class AspectInvocationException : Exception
         public AspectInvocationException(AspectContext aspectContext, Exception innerException)
             : base($"Exception has been thrown by the aspect of an invocation. ---> {innerException?.Message}.", innerException)
         {
+            AspectContext = aspectContext;
         }
     }
 }
diff --git a/core/src/AspectCore.Abstractions/DynamicProxy/IAspectExceptionWrapper.cs b/core/src/AspectCore.Abstractions/DynamicProxy/IAspectExceptionWrapper.cs
@@ -0,0 +1,9 @@
+using System;
+
+namespace AspectCore.DynamicProxy
+{
+    public interface IAspectExceptionWrapper
+    {
+        Exception Wrap(AspectContext aspectContext, Exception exception);
+    }
+}
diff --git a/core/src/AspectCore.Core/Configuration/AspectConfiguration.cs b/core/src/AspectCore.Core/Configuration/AspectConfiguration.cs
@@ -10,8 +10,11 @@ public sealed class AspectConfiguration : IAspectConfiguration
 
         public NonAspectPredicateCollection NonAspectPredicates { get; }
 
+        public bool ThrowAspectException { get; set; }
+
         public AspectConfiguration()
         {
+            ThrowAspectException = true;
             ValidationHandlers = new AspectValidationHandlerCollection().AddDefault(this);
             Interceptors = new InterceptorCollection();
             NonAspectPredicates = new NonAspectPredicateCollection().AddDefault();

diff --git a/core/src/AspectCore.Core/DynamicProxy/AspectActivator.cs b/core/src/AspectCore.Core/DynamicProxy/AspectActivator.cs
@@ -11,11 +11,13 @@ internal sealed class AspectActivator : IAspectActivator
     {
         private readonly IAspectContextFactory _aspectContextFactory;
         private readonly IAspectBuilderFactory _aspectBuilderFactory;
+        private readonly IAspectExceptionWrapper _aspectExceptionWrapper;
 
-        public AspectActivator(IAspectContextFactory aspectContextFactory, IAspectBuilderFactory aspectBuilderFactory)
+        public AspectActivator(IAspectContextFactory aspectContextFactory, IAspectBuilderFactory aspectBuilderFactory, IAspectExceptionWrapper aspectExceptionWrapper)
         {
             _aspectContextFactory = aspectContextFactory;
             _aspectBuilderFactory = aspectBuilderFactory;
+            _aspectExceptionWrapper = aspectExceptionWrapper;
         }
 
         public TResult Invoke<TResult>(AspectActivatorContext activatorContext)
@@ -26,18 +28,19 @@ public TResult Invoke<TResult>(AspectActivatorContext activatorContext)
                 var aspectBuilder = _aspectBuilderFactory.Create(context);
                 var task = aspectBuilder.Build()(context);
                 if (task.IsFaulted)
-                    throw context.InvocationException(task.Exception.InnerException);
+                    throw _aspectExceptionWrapper.Wrap(context, task.Exception.InnerException);
                 if (!task.IsCompleted)
                 {
                     // try to avoid potential deadlocks.
                     NoSyncContextScope.Run(task);
                     // task.GetAwaiter().GetResult();
                 }
-                return (TResult)context.ReturnValue;
+
+                return (TResult) context.ReturnValue;
             }
             catch (Exception ex)
             {
-                throw context.InvocationException(ex);
+                throw _aspectExceptionWrapper.Wrap(context, ex);
             }
             finally
             {
@@ -64,13 +67,13 @@ public async Task<TResult> InvokeTask<TResult>(AspectActivatorContext activatorC
                 }
                 else
                 {
-                    throw context.InvocationException(new InvalidCastException(
+                    throw _aspectExceptionWrapper.Wrap(context, new InvalidCastException(
                         $"Unable to cast object of type '{result.GetType()}' to type '{typeof(Task<TResult>)}'."));
                 }
             }
             catch (Exception ex)
             {
-                throw context.InvocationException(ex);
+                throw _aspectExceptionWrapper.Wrap(context, ex);
             }
             finally
             {
@@ -85,11 +88,11 @@ public async ValueTask<TResult> InvokeValueTask<TResult>(AspectActivatorContext
             {
                 var aspectBuilder = _aspectBuilderFactory.Create(context);
                 await aspectBuilder.Build()(context);
-                return await (ValueTask<TResult>)context.ReturnValue;
+                return await (ValueTask<TResult>) context.ReturnValue;
             }
             catch (Exception ex)
             {
-                throw context.InvocationException(ex);
+                throw _aspectExceptionWrapper.Wrap(context, ex);
             }
             finally
             {

diff --git a/core/src/AspectCore.Core/DynamicProxy/AspectActivatorFactory.cs b/core/src/AspectCore.Core/DynamicProxy/AspectActivatorFactory.cs
@@ -7,16 +7,18 @@ public sealed class AspectActivatorFactory : IAspectActivatorFactory
     {
         private readonly IAspectContextFactory _aspectContextFactory;
         private readonly IAspectBuilderFactory _aspectBuilderFactory;
+        private readonly IAspectExceptionWrapper _aspectExceptionWrapper;
 
-        public AspectActivatorFactory(IAspectContextFactory aspectContextFactory, IAspectBuilderFactory aspectBuilderFactory)
+        public AspectActivatorFactory(IAspectContextFactory aspectContextFactory, IAspectBuilderFactory aspectBuilderFactory, IAspectExceptionWrapper aspectExceptionWrapper)
         {
             _aspectContextFactory = aspectContextFactory ?? throw new ArgumentNullException(nameof(aspectContextFactory));
             _aspectBuilderFactory = aspectBuilderFactory ?? throw new ArgumentNullException(nameof(aspectBuilderFactory));
+            _aspectExceptionWrapper = aspectExceptionWrapper ?? throw new ArgumentNullException(nameof(aspectExceptionWrapper));
         }
 
         public IAspectActivator Create()
         {
-            return new AspectActivator(_aspectContextFactory, _aspectBuilderFactory);
+            return new AspectActivator(_aspectContextFactory, _aspectBuilderFactory, _aspectExceptionWrapper);
         }
     }
-}
+}
diff --git a/core/src/AspectCore.Core/DynamicProxy/Extensions/AspectContextRuntimeExtensions.cs b/core/src/AspectCore.Core/DynamicProxy/Extensions/AspectContextRuntimeExtensions.cs
@@ -13,32 +13,22 @@ public static class AspectContextRuntimeExtensions
 
         internal static readonly ConcurrentDictionary<MethodInfo, MethodReflector> reflectorTable = new ConcurrentDictionary<MethodInfo, MethodReflector>();
 
+        public static Task AwaitIfAsync(this AspectContext aspectContext)
+        {
+            return AwaitIfAsync(aspectContext, aspectContext.ReturnValue);
+        }
+
         public static async Task AwaitIfAsync(this AspectContext aspectContext, object returnValue)
         {
             if (returnValue == null)
             {
                 return;
             }
-            if (returnValue is Task task)
-            {
-                try
-                {
-                    await task;
-                }
-                catch (Exception ex)
-                {
-                    throw aspectContext.InvocationException(ex);
-                }
-            }
-        }
 
-        public static AspectInvocationException InvocationException(this AspectContext aspectContext, Exception exception)
-        {
-            if (exception is AspectInvocationException aspectInvocationException)
+            if (returnValue is Task task)
             {
-                return aspectInvocationException;
+                await task;
             }
-            return new AspectInvocationException(aspectContext, exception);
         }
 
         public static bool IsAsync(this AspectContext aspectContext)
@@ -47,33 +37,44 @@ public static bool IsAsync(this AspectContext aspectContext)
             {
                 throw new ArgumentNullException(nameof(aspectContext));
             }
+
             var isAsyncFromMetaData = isAsyncCache.GetOrAdd(aspectContext.ServiceMethod, IsAsyncFromMetaData);
             if (isAsyncFromMetaData)
             {
                 return true;
             }
+
             if (aspectContext.ReturnValue != null)
             {
                 return IsAsyncType(aspectContext.ReturnValue.GetType().GetTypeInfo());
             }
+
             return false;
         }
 
+        public static async Task<T> UnwrapAsyncReturnValue<T>(this AspectContext aspectContext)
+        {
+            return (T) await UnwrapAsyncReturnValue(aspectContext);
+        }
+
         public static Task<object> UnwrapAsyncReturnValue(this AspectContext aspectContext)
         {
             if (aspectContext == null)
             {
                 throw new ArgumentNullException(nameof(aspectContext));
             }
+
             if (!aspectContext.IsAsync())
             {
                 throw new AspectInvocationException(aspectContext, new InvalidOperationException("This operation only support asynchronous method."));
             }
+
             var returnValue = aspectContext.ReturnValue;
             if (returnValue == null)
             {
                 return null;
             }
+
             var returnTypeInfo = returnValue.GetType().GetTypeInfo();
             return Unwrap(returnValue, returnTypeInfo);
         }
@@ -90,7 +91,7 @@ private static async Task<object> Unwrap(object value, TypeInfo valueTypeInfo)
             else if (valueTypeInfo.IsValueTask())
             {
                 // Is there better solution to unwrap ?
-                result = (object)(await (dynamic)value);
+                result = (object) (await (dynamic) value);
             }
             else if (value is Task)
             {
@@ -111,6 +112,7 @@ private static async Task<object> Unwrap(object value, TypeInfo valueTypeInfo)
             {
                 return Unwrap(result, resultTypeInfo);
             }
+
             return result;
         }
 
@@ -120,13 +122,15 @@ private static bool IsAsyncFromMetaData(MethodInfo method)
             {
                 return true;
             }
+
             if (method.IsDefined(typeof(AsyncAspectAttribute), true))
             {
                 if (method.ReturnType == typeof(object))
                 {
                     return true;
                 }
             }
+
             return false;
         }
 
@@ -137,14 +141,17 @@ private static bool IsAsyncType(TypeInfo typeInfo)
             {
                 return true;
             }
+
             if (typeInfo.IsTaskWithResult())
             {
                 return true;
             }
+
             if (typeInfo.IsValueTask())
             {
                 return true;
             }
+
             return false;
         }
     }

diff --git a/core/src/AspectCore.Core/Injector/ServiceContainer.cs b/core/src/AspectCore.Core/Injector/ServiceContainer.cs
@@ -99,6 +99,8 @@ private void AddInternalServices()
                 Scopeds.AddType<IParameterInterceptorSelector, ParameterInterceptorSelector>();
             if (!Contains(typeof(IAspectCachingProvider)))
                 Singletons.AddType<IAspectCachingProvider, AspectCachingProvider>();
+            if (!Contains(typeof(IAspectExceptionWrapper)))
+                Singletons.AddType<IAspectExceptionWrapper, AspectExceptionWrapper>();
         }
 
         public int Count => _collection.Count;

diff --git a/core/src/AspectCore.Core/Injector/ServiceResolver.cs b/core/src/AspectCore.Core/Injector/ServiceResolver.cs
@@ -81,6 +81,7 @@ private void Dispose(bool disposing)
             {
                 if (disposing)
                 {
+                    disposedValue = true;
                     if (_root == null || _root == this)
                     {
                         foreach (var singleton in _resolvedSingletonServices.Where(x => x.Value != this))
@@ -95,7 +96,6 @@ private void Dispose(bool disposing)
                         disposable?.Dispose();
                     }
                 }
-                disposedValue = true;
             }
         }
 

diff --git a/core/src/AspectCore.Core/Utils/NoSyncContextScope.cs b/core/src/AspectCore.Core/Utils/NoSyncContextScope.cs
@@ -4,10 +4,10 @@
 
 namespace AspectCore.Core.Utils
 {
-    public static class NoSyncContextScope
+    internal static class NoSyncContextScope
     {
         // See: https://stackoverflow.com/questions/28305968/use-task-run-in-synchronous-method-to-avoid-deadlock-waiting-on-async-method
-        public static IDisposable Enter()
+        private static IDisposable Enter()
         {
             var context = SynchronizationContext.Current;
             SynchronizationContext.SetSynchronizationContext(null);

diff --git a/core/src/AspectCore.Core/Utils/ProxyGeneratorUtils.cs b/core/src/AspectCore.Core/Utils/ProxyGeneratorUtils.cs
@@ -1053,7 +1053,6 @@ public MethodConstantTable(TypeBuilder typeBuilder)
 
             public void AddMethod(string name, MethodInfo method)
             {
-                name = name.GetHashCode().ToString();
                 if (!_fields.ContainsKey(name))
                 {
                     var field = _nestedTypeBuilder.DefineField(name, typeof(MethodInfo), FieldAttributes.Static | FieldAttributes.InitOnly | FieldAttributes.Assembly);
@@ -1068,7 +1067,6 @@ public void AddMethod(string name, MethodInfo method)
 
             public void LoadMethod(ILGenerator ilGen, string name)
             {
-                name = name.GetHashCode().ToString();
                 if (_fields.TryGetValue(name, out FieldBuilder field))
                 {
                     ilGen.Emit(OpCodes.Ldsfld, field);

diff --git a/extras/sample/AspectCore.Extensions.Autofac.Sample/MethodExecuteLoggerInterceptor.cs b/extras/sample/AspectCore.Extensions.Autofac.Sample/MethodExecuteLoggerInterceptor.cs
@@ -1,24 +1,22 @@
 using System;
-using System.Collections.Generic;
 using System.Diagnostics;
-using System.Text;
 using System.Threading.Tasks;
 using AspectCore.DynamicProxy;
 
 namespace AspectCore.Extensions.Autofac.Sample
 {
     public class MethodExecuteLoggerInterceptor : AbstractInterceptor
     {
-        public async override Task Invoke(AspectContext context, AspectDelegate next)
+        public override async Task Invoke(AspectContext context, AspectDelegate next)
         {
-            Stopwatch stopwatch = Stopwatch.StartNew();
+            var stopwatch = Stopwatch.StartNew();
             await next(context);
             stopwatch.Stop();
             Console.WriteLine("Executed method {0}.{1}.{2} ({3}) in {4}ms",
-                context.ServiceMethod.DeclaringType.Namespace,
-                context.ServiceMethod.DeclaringType.Name,
-                context.ServiceMethod.Name,
-                context.ServiceMethod.DeclaringType.Assembly.GetName().Name,
+                context.ImplementationMethod.DeclaringType.Namespace,
+                context.ImplementationMethod.DeclaringType.Name,
+                context.ImplementationMethod.Name,
+                context.ImplementationMethod.DeclaringType.Assembly.GetName().Name,
                 stopwatch.ElapsedMilliseconds
             );
         }