From b9eb36203b358dfb01771701296ba629d101939a Mon Sep 17 00:00:00 2001 From: dotnetjunkie Date: Fri, 8 Jan 2021 14:34:49 +0100 Subject: [PATCH] Improved type initialization error handling by calling cctor on expression tree build and moving logic to Registration. Also moved tests to new class. #812 --- .../InstanceProducerTests.cs | 100 ---------- .../TypeInitializationErrorTests.cs | 187 ++++++++++++++++++ src/SimpleInjector/InstanceProducer.cs | 36 +--- src/SimpleInjector/Registration.cs | 38 +++- 4 files changed, 226 insertions(+), 135 deletions(-) create mode 100644 src/SimpleInjector.Tests.Unit/TypeInitializationErrorTests.cs diff --git a/src/SimpleInjector.Tests.Unit/InstanceProducerTests.cs b/src/SimpleInjector.Tests.Unit/InstanceProducerTests.cs index f03041df9..5804e5d5b 100644 --- a/src/SimpleInjector.Tests.Unit/InstanceProducerTests.cs +++ b/src/SimpleInjector.Tests.Unit/InstanceProducerTests.cs @@ -356,98 +356,6 @@ public void GetInstance_ForSingletonValueType_CanBeResolved() Assert.AreEqual(expectedValue, actualValue); } - // #812 - [TestMethod] - public void GetInstance_ResolvingATypeWithTypeInitializationException_ThrowsExpressiveException() - { - // Act - var container = new Container(); - - container.Register(); - - // Act - Action action = () => container.GetInstance(); - - // Assert - AssertThat.ThrowsWithExceptionMessageContains( - "The type initializer for TopLevelClassWithFailingTypeInitializer threw an " + - "exception. .", - action); - } - - // #812 - [TestMethod] - public void GetInstance_ResolvingANestedTypeWithTypeInitializationException_ThrowsExpressiveException() - { - // Act - var container = new Container(); - - container.Register(); - - // Act - Action action = () => container.GetInstance(); - - // Assert - AssertThat.ThrowsWithExceptionMessageContains( - "The type initializer for InstanceProducerTests.NestedClassWithFailingTypeInitializer " + - "threw an exception. .", - action); - } - - // #812 - [TestMethod] - public void GetInstance_ResolvingAGenericNestedTypeWithTypeInitializationException_ThrowsExpressiveException() - { - // Act - var container = new Container(); - - container.Register(typeof(NestedClassWithFailingTypeInitializer<>)); - - // Act - Action action = () => container.GetInstance>(); - - // Assert - AssertThat.ThrowsWithExceptionMessageContains( - "The type initializer for InstanceProducerTests" + - ".NestedClassWithFailingTypeInitializer threw an exception. .", - action); - } - - // #812 - [TestMethod] - public void GetInstance_ResolvingASingletonWithTypeInitializationException_ThrowsExpressiveException() - { - // Act - var container = new Container(); - - container.RegisterSingleton(); - - // Act - Action action = () => container.GetInstance(); - - // Assert - AssertThat.ThrowsWithExceptionMessageContains( - "The type initializer for InstanceProducerTests.NestedClassWithFailingTypeInitializer " + - "threw an exception. .", - action); - } - - public class NestedClassWithFailingTypeInitializer - { - static NestedClassWithFailingTypeInitializer() - { - throw new Exception("."); - } - } - - public class NestedClassWithFailingTypeInitializer - { - static NestedClassWithFailingTypeInitializer() - { - throw new Exception("."); - } - } - public class OneAndTwo : IOne, ITwo { } @@ -473,12 +381,4 @@ public NodeFactory(IEnumerable nodes) } } } - - public class TopLevelClassWithFailingTypeInitializer - { - static TopLevelClassWithFailingTypeInitializer() - { - throw new Exception("."); - } - } } \ No newline at end of file diff --git a/src/SimpleInjector.Tests.Unit/TypeInitializationErrorTests.cs b/src/SimpleInjector.Tests.Unit/TypeInitializationErrorTests.cs new file mode 100644 index 000000000..5f74a9480 --- /dev/null +++ b/src/SimpleInjector.Tests.Unit/TypeInitializationErrorTests.cs @@ -0,0 +1,187 @@ +namespace SimpleInjector.Tests.Unit +{ + using System; + using Microsoft.VisualStudio.TestTools.UnitTesting; + + public class TopLevelClassWithFailingTypeInitializer + { + static TopLevelClassWithFailingTypeInitializer() + { + throw new Exception("."); + } + } + + // #812 + [TestClass] + public class TypeInitializationErrorTests + { + [TestMethod] + public void GetInstance_ResolvingATypeWithTypeInitializationException_ThrowsExpressiveException() + { + // Act + var container = new Container(); + + container.Register(); + + // Act + Action action = () => container.GetInstance(); + + // Assert + AssertThat.ThrowsWithExceptionMessageContains( + "The type initializer for TopLevelClassWithFailingTypeInitializer threw an " + + "exception. .", + action); + } + + [TestMethod] + public void GetInstance_ResolvingANestedTypeWithTypeInitializationException_ThrowsExpressiveException() + { + // Act + var container = new Container(); + + container.Register(); + + // Act + Action action = () => container.GetInstance(); + + // Assert + AssertThat.ThrowsWithExceptionMessageContains( + "The type initializer for " + + $"{nameof(TypeInitializationErrorTests)}.NestedClassWithFailingTypeInitializer " + + "threw an exception. .", + action); + } + + [TestMethod] + public void GetInstance_ResolvingAGenericNestedTypeWithTypeInitializationException_ThrowsExpressiveException() + { + // Act + var container = new Container(); + + container.Register(typeof(NestedClassWithFailingTypeInitializer<>)); + + // Act + Action action = () => container.GetInstance>(); + + // Assert + AssertThat.ThrowsWithExceptionMessageContains( + "The type initializer for " + + $"{nameof(TypeInitializationErrorTests)}.NestedClassWithFailingTypeInitializer " + + "threw an exception. .", + action); + } + + [TestMethod] + public void GetInstance_ResolvingASingletonWithTypeInitializationException_ThrowsExpressiveException() + { + // Act + var container = new Container(); + + container.RegisterSingleton(); + + // Act + Action action = () => container.GetInstance(); + + // Assert + AssertThat.ThrowsWithExceptionMessageContains( + "The type initializer for " + + $"{nameof(TypeInitializationErrorTests)}.NestedClassWithFailingTypeInitializer " + + "threw an exception. .", + action); + } + + [TestMethod] + public void GetInstance_ResolvingADecoratedInstanceWithInitializationException_ThrowsExpressiveException() + { + // Act + var container = new Container(); + + container.Register(); + container.RegisterDecorator(); + + // Act + Action action = () => container.GetInstance(); + + // Assert + AssertThat.ThrowsWithExceptionMessageContains( + "The type initializer for " + + $"{nameof(TypeInitializationErrorTests)}.NestedClassWithFailingTypeInitializer " + + "threw an exception. .", + action); + } + + [TestMethod] + public void GetInstance_ResolvingADecoratorWithTypeInitializationException_ThrowsExpressiveException() + { + // Act + var container = new Container(); + + container.Register, RealCommandHandler>(); + container.RegisterDecorator(typeof(ICommandHandler<>), typeof(DecoratorWithFailingTypeInitializer<>)); + + // Act + Action action = () => container.GetInstance>(); + + // Assert + AssertThat.ThrowsWithExceptionMessageContains( + "The type initializer for " + + $"{nameof(TypeInitializationErrorTests)}.DecoratorWithFailingTypeInitializer " + + "threw an exception. .", + action); + } + + [TestMethod] + public void GetInstance_ResolvingANestedDecoratorWithTypeInitializationException_ThrowsExpressiveException() + { + // Act + var container = new Container(); + + container.Register, RealCommandHandler>(); + container.RegisterDecorator(typeof(ICommandHandler<>), typeof(CommandHandlerDecorator<>)); + container.RegisterDecorator(typeof(ICommandHandler<>), typeof(DecoratorWithFailingTypeInitializer<>)); + container.RegisterDecorator(typeof(ICommandHandler<>), typeof(CommandHandlerDecorator<>)); + + // Act + Action action = () => container.GetInstance>(); + + // Assert + AssertThat.ThrowsWithExceptionMessageContains( + "The type initializer for " + + $"{nameof(TypeInitializationErrorTests)}.DecoratorWithFailingTypeInitializer " + + "threw an exception. .", + action); + } + + public class NestedClassWithFailingTypeInitializer : INonGenericService + { + static NestedClassWithFailingTypeInitializer() + { + throw new Exception("."); + } + + public void DoSomething() + { + } + } + + public class NestedClassWithFailingTypeInitializer + { + static NestedClassWithFailingTypeInitializer() + { + throw new Exception("."); + } + } + + public class DecoratorWithFailingTypeInitializer : ICommandHandler + { + static DecoratorWithFailingTypeInitializer() + { + throw new Exception("."); + } + + public DecoratorWithFailingTypeInitializer(ICommandHandler decoratee) + { + } + } + } +} \ No newline at end of file diff --git a/src/SimpleInjector/InstanceProducer.cs b/src/SimpleInjector/InstanceProducer.cs index d413183f3..d9ea5e604 100644 --- a/src/SimpleInjector/InstanceProducer.cs +++ b/src/SimpleInjector/InstanceProducer.cs @@ -329,10 +329,6 @@ public Expression BuildExpression() throw; } - catch (TypeInitializationException ex) - { - throw this.MakeMoreExpressiveTypeInitializationException(ex); - } catch (Exception ex) { if (this.MustWrapThrownException(ex)) @@ -615,16 +611,9 @@ private object BuildAndReplaceInstanceCreatorAndCreateFirstInstance() { this.instanceCreator = this.BuildInstanceCreator(); - try - { - var instance = this.instanceCreator(); - this.InstanceSuccessfullyCreated = true; - return instance; - } - catch (TypeInitializationException ex) - { - throw this.MakeMoreExpressiveTypeInitializationException(ex); - } + var instance = this.instanceCreator(); + this.InstanceSuccessfullyCreated = true; + return instance; } // Prevents any recursive calls from taking place. @@ -686,25 +675,6 @@ private static bool ShouldBeRegisteredAsAnExternalProducer(Registration registra return !(registration is ExpressionRegistration); } - private Exception MakeMoreExpressiveTypeInitializationException( - TypeInitializationException ex) - { - Type implementationType = this.Registration.ImplementationType; - - // #812 The exceptions thrown by the runtime in case of a type initialization error are - // unproductive. Here we throw a more expressive exception. This is done by appending the - // inner exception's message to the exception message and replacing the type name with a - // 'friendly type name', which is especially useful in the case of generic types. - return new ActivationException( - ex.Message - // When the type in question is nested, the exception will contain just the simple - // type name, while for non-nested types, the full name is used (don't ask why). - .Replace($"'{implementationType.FullName}'", implementationType.ToFriendlyName()) - .Replace($"'{implementationType.Name}'", implementationType.ToFriendlyName()) + - " " + ex.InnerException?.Message, - ex); - } - [ExcludeFromCodeCoverage] internal sealed class InstanceProducerDebugView { diff --git a/src/SimpleInjector/Registration.cs b/src/SimpleInjector/Registration.cs index 40c2557a4..01aaa9fbc 100644 --- a/src/SimpleInjector/Registration.cs +++ b/src/SimpleInjector/Registration.cs @@ -8,6 +8,7 @@ namespace SimpleInjector using System.Linq; using System.Linq.Expressions; using System.Reflection; + using System.Runtime.CompilerServices; using SimpleInjector.Advanced; using SimpleInjector.Diagnostics; using SimpleInjector.Internals; @@ -21,9 +22,9 @@ namespace SimpleInjector /// service type. s returned from the /// BuildExpression method can be /// intercepted by any event registered with , have - /// initializers + /// initializers /// applied, and the caching particular to its lifestyle have been applied. Interception using the - /// Container.ExpressionBuilt will not + /// Container.ExpressionBuilt will not /// be applied in the Registration, but will be applied in . /// /// See the documentation for an example. @@ -340,6 +341,8 @@ private Action BuildInstanceInitializer() private Expression BuildNewExpression() { + this.EnsureImplementationTypeInitialized(); + ConstructorInfo constructor = this.Container.Options.SelectConstructor(this.ImplementationType); ParameterDictionary parameters = this.BuildConstructorParameters(constructor); @@ -353,6 +356,37 @@ private Expression BuildNewExpression() return expression; } + // Implements #812. + // The exceptions thrown by the runtime in case of a type initialization error are unproductive. + // Using this method we can throw a more expressive exception. This is done by appending the inner + // exception's message to the exception message and replacing the type name with a friendly type name, + // which is especially useful in the case of generic types. Unfortunately, we can only reliably do + // this by triggering type initialization, which means we are slightly changing the behavior + // of Simple Injector. Type initialiation now runs earlier in the pipeline, and could even potentially + // run in cases where it would normally not run (in case the type is a stand-in where the user + // replaces the type by altering the expression tree). This, however, is such a rare scenario that I + // consider this to be a fair trade off. + private void EnsureImplementationTypeInitialized() + { + try + { + // The Class Constructor of a type is guaranteed to be to be called just once. + RuntimeHelpers.RunClassConstructor(this.ImplementationType.TypeHandle); + } + catch (TypeInitializationException ex) + { + Type type = this.ImplementationType; + + // When the type in question is nested, the exception will contain just the simple + // type name, while for non-nested types, CLR uses the full name (because of... reasons?). + string message = ex.Message + .Replace($"'{type.FullName}'", type.ToFriendlyName()) + .Replace($"'{type.Name}'", type.ToFriendlyName()); + + throw new ActivationException(message + " " + ex.InnerException?.Message, ex); + } + } + private Expression BuildInvocationExpression(Func instanceCreator) { Expression expression = Expression.Invoke(Expression.Constant(instanceCreator));