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

Missing 'name' attribute on components/services element causes ArgumentNullException #18

Closed
dotnetjunkie opened this issue Dec 18, 2017 · 13 comments

Comments

@dotnetjunkie
Copy link

When using Autofac.Configuration v4.0.1, I found that it produces a very confusing exception when the 'name' attribute is omitted.

In case the name attribute on the element of is missing, a very generic ArgumentNullException is thrown from deep within Autofac.Configuration when ContainerBuilder.Build is called, instead of communicating the real cause.

Steps to reproduce:

The following XML "autofac.xml" configuration file:

<?xml version="1.0" encoding="utf-8" ?>
<autofac>
  <components name="0">
    <type>AutofacXmlConsole.Calculator, AutofacXmlConsole</type>
    <services  type="AutofacXmlConsole.ICalculator, AutofacXmlConsole" />
  </components>
</autofac>

With the following console application:

namespace AutofacXmlConsole
{
    using System;
    using Autofac;
    using Autofac.Configuration;
    using Microsoft.Extensions.Configuration;

    public interface ICalculator { }
    public class Calculator : ICalculator { }

    class Program
    {
        static void Main(string[] args)
        {
            var configBuilder = new ConfigurationBuilder();
            configBuilder.AddXmlFile("autofac.xml");
            var config = configBuilder.Build();

            var module = new ConfigurationModule(config);
            var builder = new ContainerBuilder();
            builder.RegisterModule(module);

            // Throws exception
            var container = builder.Build();

            container.Resolve<ICalculator>();
        }
    }
}

Running the code results in the following exception:

System.ArgumentNullException: 'Value cannot be null.'

With the following stack trace:

at System.RuntimeType.GetType(String typeName, Boolean throwOnError, Boolean ignoreCase, Boolean reflectionOnly, StackCrawlMark& stackMark)
at System.Type.GetType(String typeName)
at Autofac.Configuration.Core.ConfigurationExtensions.GetType(IConfiguration configuration, String key, Assembly defaultAssembly)
at Autofac.Configuration.Core.ComponentRegistrar.<EnumerateComponentServices>d__1.MoveNext()
at Autofac.Configuration.Core.ComponentRegistrar.RegisterComponentServices[TReflectionActivatorData,TSingleRegistrationStyle](IConfiguration component, IRegistrationBuilder`3 registrar, Assembly defaultAssembly)
at Autofac.Configuration.Core.ComponentRegistrar.RegisterConfiguredComponents(ContainerBuilder builder, IConfiguration configuration)at Autofac.Module.Configure(IComponentRegistry componentRegistry)
at Autofac.ContainerBuilder.Build(IComponentRegistry componentRegistry, Boolean excludeDefaultModules)
at Autofac.ContainerBuilder.Build(ContainerBuildOptions options)
at AutofacXmlConsole.Program.Main(String[] args)
@tillig
Copy link
Member

tillig commented Dec 18, 2017

Anything enumerable in XML needs a name attribute with a number. It's a requirement of Microsoft.Configuration. It's one of the reasons JSON is way easier to deal with.

Add a name attribute to the services element. You'll see that in the docs where there are XML examples are.

@tillig tillig closed this as completed Dec 18, 2017
@tillig tillig reopened this Dec 18, 2017
@tillig
Copy link
Member

tillig commented Dec 18, 2017

I will see if there's a way to add a better message. In the meantime, I'd save yourself some pain and switch to JSON.

@dotnetjunkie
Copy link
Author

dotnetjunkie commented Dec 18, 2017

Anything enumerable in XML needs a name attribute with a number. It's a requirement of Microsoft.Configuration.

I do understand (and of course I read the documentation). My sole point here was that the exception message is not very helpful.

I'd save yourself some pain and switch to JSON.

Won't do. I'm working on some examples for the book, and in this part we focus on XML. But I'll keep your advice in mind when working on the Autofac chapter. I might add a note or a warning.

save yourself some pain

What is it that makes working with the XML format much more painful than JSON? If it is Microsoft.Extensions.Configuration that is causing that pain, why did you choose to build on top of that library?

@tillig
Copy link
Member

tillig commented Dec 18, 2017

It's Microsoft.Extensions.Configuration that is the pain, and specifically around XML. We went with that because there is so much you get for free otherwise. Combine configs? Sure. Overrides for environments? No problem. Need to poke something into a deployed system? You can configure registrations via environment variables. There's a lot to be gained and the sacrifice is painful enumerable handling in XML. Seemed like a fair trade.

@dotnetjunkie
Copy link
Author

the sacrifice is painful enumerable handling in XML. Seemed like a fair trade.

Perhaps you should ditch XML completely? Your users will come to you for questions about this, even if things are caused by a crappy library you depend on.

@tillig
Copy link
Member

tillig commented Dec 18, 2017

Given it's Microsoft.Extensions.Configuration I don't actually know the format by the time it gets to me. It's already parsed into the internal configuration data structures. There's no special handling for any config format.

@blairconrad
Copy link
Contributor

I've been able to create a unit test to reproduce, and can improve (for some value of improve) the exception that's thrown. I can whip up a PR.

However, I wonder. Would we rather it Just Work(TM)? If Autofac doesn't care about the numbering imposed by the name attribute, we might be able to disregard the fact that it's missing and register the services anyhow. Preliminary tests suggest that it's possible, but I worry that it contravenes the spirit of the thing (or if a missing name would interfere with some of the other capabilities @tillig mentioned. (Sorry, my Microsoft.Extensions.Configuration knowledge is nil.)

@tillig
Copy link
Member

tillig commented Jun 6, 2018

Autofac doesn't care about the name but Microsoft.Extensions.Configuration very much does. The name indicates the order of the ordinal collection elements.

<?xml version="1.0" encoding="utf-8" ?>
<autofac>
  <components name="1">
    <type>A</type>
  </components>
  <components name="0">
    <type>B</type>
  </components>
  <components name="2">
    <type>C</type>
  </components>
</autofac>

The way MS config deserializes this, the order of the components will be B, A, C.

It's not pretty, but it's an unfortunate side effect of how ordinal collections have to work in order to get XML and JSON and INI and other systems working together.

For example, you could also have environment variables

COMPONENTS__0__TYPE = B
COMPONENTS__1__TYPE = A
COMPONENTS__2__TYPE = C

and that's the same as the XML.

Or you can use JSON, where the names get implicitly generated based on the array:

{
  "components": [
    { "type": "B" },
    { "type": "A" },
    { "type": "C" }
  ]
}

I'm happy for a PR to provide a nicer exception message but I very much don't want any custom parsing logic or things that add to/circumvent the way Microsoft.Extensions.Configuration handles things. Microsoft.Extensions.Configuration is new to folks, so I recognize there may be challenges in ramping up, but the point of this updated version of Autofac.Configuration is to leverage that existing abstraction and all the providers that are out there and get out of the business of any sort of manual parsing, knowing about XML/JSON/INI/whatever, etc.

@blairconrad
Copy link
Contributor

I'm happy for a PR to provide a nicer exception message but I very much don't want any custom parsing logic or things that add to/circumvent the way Microsoft.Extensions.Configuration handles things.

No problem. I see that you merged #21 (thanks), so I can rebase my "better exception" option and create a PR.

@blairconrad
Copy link
Contributor

@tillig
Copy link
Member

tillig commented Jun 20, 2018

I'm going to work on getting this released shortly, but I did post a deep dive on Microsoft.Extensions.Configuration blog post that covers some of the ins and outs of config, including ordinal collections and pain points with XML.

@tillig
Copy link
Member

tillig commented Jun 20, 2018

Oh, fair warning, it's 22 pages printed. I guess it's more a small book than a blog post. ;)

@blairconrad
Copy link
Contributor

Ha. I've added it to Pocket for a scan. I've enjoyed up to "And let’s finish up with command line parameters" so far.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants