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

throwConfigExceptions doesn't throw when using invalid extension types/assemblies #1508

Closed
jjbott opened this issue Jun 16, 2016 · 2 comments
Labels
bug Bug report / Bug fix up-for-grabs

Comments

@jjbott
Copy link

jjbott commented Jun 16, 2016

Type (choose one): Bug

NLog version: 4.3.5

Platform: .Net 4.5

Current NLog config (xml or C#, if relevant)

  <nlog throwConfigExceptions="true">
    <extensions>
      <add assembly="Gelf4NLog.Target" />
    </extensions>
    <targets>
      <target name="graylog" xsi:type="graylog" />
    </targets>
    <rules>
      <logger name="*" minlevel="Info" writeTo="graylog" />
    </rules>
  </nlog>

Note: In the above scenario, due to other problems, the assembly Gelf4NLog.Target can not be loaded (details to follow)

Expected Result: Since Gelf4NLog.Target can not be loaded, NLog configuration fails. Since throwConfigExceptions is set to true, the app should fail with an exception.

Current Result: No exceptions are thrown. The only hint that NLog had problems is in its internal log. In my case the internal log looks like:

2016-06-15 14:06:58.3440 Error Failed to add type 'Gelf4NLog.Target.GelfMessage'. Exception: System.IO.FileLoadException: Could not load file or assembly 'Newtonsoft.Json, Version=4.5.0.0, Culture=neutral, PublicKeyToken=30ad4fe6b2a6aeed' or one of its dependencies. The located assembly's manifest definition does not match the assembly reference. (Exception from HRESULT: 0x80131040)
File name: 'Newtonsoft.Json, Version=4.5.0.0, Culture=neutral, PublicKeyToken=30ad4fe6b2a6aeed'
   at System.ModuleHandle.ResolveType(RuntimeModule module, Int32 typeToken, IntPtr* typeInstArgs, Int32 typeInstCount, IntPtr* methodInstArgs, Int32 methodInstCount, ObjectHandleOnStack type)
   at System.ModuleHandle.ResolveTypeHandleInternal(RuntimeModule module, Int32 typeToken, RuntimeTypeHandle[] typeInstantiationContext, RuntimeTypeHandle[] methodInstantiationContext)
   at System.Reflection.RuntimeModule.ResolveType(Int32 metadataToken, Type[] genericTypeArguments, Type[] genericMethodArguments)
   at System.Reflection.CustomAttribute.FilterCustomAttributeRecord(CustomAttributeRecord caRecord, MetadataImport scope, Assembly& lastAptcaOkAssembly, RuntimeModule decoratedModule, MetadataToken decoratedToken, RuntimeType attributeFilterType, Boolean mustBeInheritable, Object[] attributes, IList derivedAttributes, RuntimeType& attributeType, IRuntimeMethodInfo& ctor, Boolean& ctorHasParameters, Boolean& isVarArg)
   at System.Reflection.CustomAttribute.GetCustomAttributes(RuntimeModule decoratedModule, Int32 decoratedMetadataToken, Int32 pcaCount, RuntimeType attributeFilterType, Boolean mustBeInheritable, IList derivedAttributes, Boolean isDecoratedTargetSecurityTransparent)
   at System.Reflection.CustomAttribute.GetCustomAttributes(RuntimeType type, RuntimeType caType, Boolean inherit)
   at System.RuntimeType.GetCustomAttributes(Type attributeType, Boolean inherit)
   at NLog.Config.Factory`2.RegisterType(Type type, String itemNamePrefix)
   at NLog.Config.Factory`2.ScanTypes(Type[] types, String prefix)

Workarounds: Well, fix the issue that is preventing the assembly from being loaded, for one. But I'd rather get notified when NLog's config step fails.

Unit Tests: I managed to create the following tests for similar issues around throwConfigExceptions, but I'm not sure how to test the case where the assembly exists but one of its dependencies doesn't. The weird try/catch stuff is because the referenced version of xUnit doesn't support ThrowsAny, and I personally don't care what the exception type is. You probably do. :)

       [Fact]
        public void ExtensionInvalidTypeTest()
        {
            Assert.NotNull(typeof(FooLayout));

            var configXml = @"
<nlog throwConfigExceptions='true'>
    <extensions>
        <add type='some_type_that_doesnt_exist'/>
    </extensions>
</nlog>";
            System.Exception e = null;
            try
            {
                CreateConfigurationFromString(configXml);
            }
            catch(System.Exception ex)
            {
                e = ex;
            }
            Assert.NotNull(e);
        }

        [Fact]
        public void ExtensionInvalidAssemblyTest()
        {
            Assert.NotNull(typeof(FooLayout));

            var configXml = @"
<nlog throwConfigExceptions='true'>
    <extensions>
        <add assembly='some_assembly_that_doesnt_exist'/>
    </extensions>
</nlog>";
            System.Exception e = null;
            try
            {
                CreateConfigurationFromString(configXml);
            }
            catch (System.Exception ex)
            {
                e = ex;
            }
            Assert.NotNull(e);
        }
@304NotModified
Copy link
Member

Thanks for the report! This is indeed a bug

@304NotModified 304NotModified added bug Bug report / Bug fix up-for-grabs labels Jun 16, 2016
@304NotModified
Copy link
Member

Xunit has .throws

Im on holiday, will look at this within 3 weeks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bug report / Bug fix up-for-grabs
Projects
None yet
Development

No branches or pull requests

2 participants