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

ILogger injects fine, but kernel.Get<ILogger>() throws null ref #19

Closed
jamesmanning opened this issue Apr 11, 2013 · 1 comment
Closed

Comments

@jamesmanning
Copy link
Contributor

This might be "by design", but off-hand it seemed a little harsh as an end-user experience.

steps:

  • create new console app (happens to be .NET 4.5, but that's not likely important)

  • Install-Package Ninject.Extensions.Logging.Log4net

    • the NLog and NLog2 ones are likely to do the same AFAICT
  • in Main, put these 2 lines and run:

        IKernel standardKernel = new StandardKernel();
        var logger = standardKernel.Get<ILogger>();
    

throws a null ref due to Target being null (since we're asking for the instance directly, instead of having it injected into a class) in LoggerFactoryBase.GetLogger

    public ILogger GetLogger(IContext context)
    {
        return this.GetLogger(context.Request.Target.Member.DeclaringType);
    }

I'm getting a "TypeLoadException: Inheritance security rules violated by type 'Ninject.Extensions.Logging.LoggerModuleBase'" locally (unrelated to this issue AFAICT), so I haven't been able to confirm this, but a failing test for this behavior should be:

    [Fact]
    public void CanManuallyGetILoggerInstance()
    {
        using (var kernel = this.CreateKernel())
        {
            var loggerClass = kernel.Get<ILogger>();
            loggerClass.Should().NotBeNull();
            loggerClass.Type.Should().Be(typeof(ILogger));
        }
    }

Certainly a user of ninject should have ILogger injected and never need to fetch ILogger directly (service locator anti-pattern), but in trying to use the ninject logging extension with a codebase that (unfortunately) has some places fetching instances manually, this null ref keeps it from being usable.

The least-code workaround AFAICT is rebinding ILogger (currently bound in LoggerModuleBase.Load) - here's what a modified LoggerModuleBase might look like (not sure if using the service type as a fallback is really desirable or not)

    public override void Load()
    {
        //this.Bind<ILogger>().ToMethod(context => context.Kernel.Get<ILoggerFactory>().GetLogger(context));
        this.Bind<ILogger>().ToMethod(context =>
        {
            var typeForLogger = context.Request.Target != null
                                    ? context.Request.Target.Member.DeclaringType
                                    : context.Request.Service;
            return context.Kernel.Get<ILoggerFactory>().GetLogger(typeForLogger);
        });
    }
@remogloor
Copy link
Member

The extension gets the logger name from the class it is injected into. By just getting a ILogger using Get this can't be evaluated. Use

kernel.Get<ILoggerFactory>().GetCurrentClassLogger()

instead.

https://github.com/ninject/ninject.extensions.logging/wiki/Using

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

No branches or pull requests

2 participants