rootPathProvider.GetRootPath returns null in FaviconStartup.LocateIconOnFileSystem #624

Closed
dragan opened this Issue May 20, 2012 · 26 comments

Comments

Projects
None yet
5 participants

dragan commented May 20, 2012

I'm not sure if you get the same result under .NET, but under Mono when running a simple test: https://gist.github.com/2757624

A ArgumentNullException is being thrown. I did a little investigating and sure enough rootPathProvider.GetRootPath() is returning null. Let me know if there's anything else that is needed. Below is the exception information.

System.ArgumentNullException : Argument cannot be null.
Parameter name: path
Stack Trace:
at System.IO.Path.Validate (System.String path, System.String parameterName) [0x00000] in <filename unknown>:0 
at System.IO.Path.Validate (System.String path) [0x00000] in <filename unknown>:0 
at System.IO.Directory.EnumerateCheck (System.String path, System.String searchPattern, SearchOption searchOption) [0x00000] in <filename unknown>:0 
at System.IO.Directory.EnumerateFiles (System.String path, System.String searchPattern, SearchOption searchOption) [0x00000] in <filename unknown>:0 
at Nancy.Bootstrapper.FavIconStartup.<LocateIconOnFileSystem>m__D (System.String extension) [0x00000] in <filename unknown>:0 
at System.Linq.Enumerable+<CreateSelectManyIterator>c__Iterator29`2[System.String,System.String].MoveNext () [0x00000] in <filename unknown>:0 
at System.Collections.Generic.List`1[System.String].AddEnumerable (IEnumerable`1 enumerable) [0x00000] in <filename unknown>:0 
at System.Collections.Generic.List`1[System.String]..ctor (IEnumerable`1 collection) [0x00000] in <filename unknown>:0 
at System.Linq.Enumerable.ToArray[String] (IEnumerable`1 source) [0x00000] in <filename unknown>:0 
at Nancy.Bootstrapper.FavIconStartup.LocateIconOnFileSystem () [0x00000] in <filename unknown>:0 
at Nancy.Bootstrapper.FavIconStartup.ScanForFavIcon () [0x00000] in <filename unknown>:0 
at Nancy.Bootstrapper.FavIconStartup.get_FavIcon () [0x00000] in <filename unknown>:0 
at Nancy.Bootstrapper.NancyBootstrapperBase`1[TinyIoC.TinyIoCContainer].get_FavIcon () [0x00000] in <filename unknown>:0 
at Nancy.Bootstrapper.NancyBootstrapperBase`1[TinyIoC.TinyIoCContainer].Initialise () [0x00000] in <filename unknown>:0 
at Nancy.Testing.Browser..ctor (INancyBootstrapper bootstrapper) [0x00000] in <filename unknown>:0 
at NancySandbox.Tests.Modules.RootModuleTests+when_requesting_root.should_return_status_ok () [0x00000] in <filename unknown>:0 
at (wrapper managed-to-native) System.Reflection.MonoMethod:InternalInvoke (System.Reflection.MonoMethod,object,object[],System.Exception&)
at System.Reflection.MonoMethod.Invoke (System.Object obj, BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture) [0x00000] in <filename unknown>:0

dragan commented May 21, 2012

I was wondering if you have to provide a RootPathProvider when your tests are in another assembly than the modules when using the Browser object? My bootstrapper just inherits off of DefaultNancyBootstrapper and figured this was handled for me being the super-duper-happy-path after all? ;-)

Owner

thecodejunkie commented Oct 9, 2012

@dragan think you could perhaps try this on the latest build (0.12.1) ? Somehow this report slipped through the cracks and I would like to make sure it's still happening in the latest drop before digging into it :)

dragan commented Oct 10, 2012

@thecodejunkie Sure, give me a bit, pretty busy at the moment with the conference next week.

Owner

thecodejunkie commented Oct 16, 2012

@dragan sure thing! Good luck with MonkeySpace!

alexy commented Oct 26, 2012

This is a problem for me on mono and Nancy.Hosting.Self 0.12.1.

Owner

thecodejunkie commented Oct 30, 2012

@dragan @alexy could either of you download the Nancy solution and stick a breakpoint in there? Having issues with my VM that has Mono setup =/

dragan commented Nov 12, 2012

@thecodejunkie Just tried this with v0.13.0 and it's still happening.

Owner

thecodejunkie commented Nov 13, 2012

@dragan is that using the same test as you mention in your initial post? https://gist.github.com/2757624 also what does your Bootstrapper class look like that you create and use in the test? Does it happen is you use DefaultNancyBootstrapper or the ConfigurableBootstrapper ?

dragan commented Nov 21, 2012

@thecodejunkie Yes and I'm using the ConfigurableBootstrapper.

I've ran into this issue, running Nancy 0.15.3.0 (downloaded from NuGet) on top of ASP.NET on .NET 4.
Like @dragan speculated it does indeed happen when the module is in a different assembly.
I explicitly specified the RootPathProvider as per the recommendation above and it seems to work now:

var browser = new Browser(x => x.RootPathProvider(new DefaultRootPathProvider()));

One curious thing - I managed to get the test to run without the code above by inserting some breakpoints and waiting between them but this only worked once and seems unreproducible without breakpoints and with Thread.Sleep instead of waiting. Possible race conditions perhaps?

Owner

grumpydev commented Feb 7, 2013

Shouldn't be a race condition, we are completely sync and single threaded on startup.

I've reproduced the bug running with the Nancy solution forked from this repo, once again on top of ASP.NET on .NET 4 and the solution to manually provide a RootPathProvider still works.
I don't know if this affects other versions of .NET, Mono, non ASP.NET hosting etc but this bug seems pretty confirmed. Will try to push a fix to you guys in the near future.

Owner

grumpydev commented Feb 7, 2013

I'd love to know what provider.GetType(); is in that case .. I wonder if it's picking up a bad fake or something.

Nope, the code ends up in AspNetRootSourceProvider as expected of an ASP.net hosted app. It fails on GetRootPath:

    public class AspNetRootSourceProvider : IRootPathProvider
    {
        public string GetRootPath()
        {
            return HostingEnvironment.MapPath("~/");
        }
    }

HostingEnviroment seems to be uninitialized with everything being null or some other default value.

SO says it's because, when testing, we aren't actually running under IIS so HostingEnvirmonet is uninitialized.

Browser should probably initialize the RootPathProvider to DefaultPathProvider by default, allowing the bootstrapper to later change it if need be.

Owner

grumpydev commented Feb 7, 2013

Ah, that makes sense then - we have spoken in the past about making a more "intelligent" root path provider for testing, one that actually tries to search for the correct path, but given how test runners copy files all over the shop I'm not sure it will actually work.

@grumpydev Well, this is a bit above my league (I'm still getting the hang of the codebase) but in the meanwhile, I'll put a warning about this in the Wiki page on testing.

MichaelAz added a commit to MichaelAz/Nancy that referenced this issue Feb 7, 2013

This commit fixes issue #624
Encapsulates the "hotfix" for #624 as a bootstrapper for testing
purposes, "TestingBootstrapper"
Owner

thecodejunkie commented Feb 8, 2013

Today I submitted #963 #963 which
changed so that the bootstrappers returned an IRootPathProvider instance
instread of a type. A long with this change, we made the
ConfigurableBootstrapper return the DefaultRootPathProvider as default
https://github.com/NancyFx/Nancy/pull/963/files#L1R282

On Thu, Feb 7, 2013 at 2:29 PM, MichaelAz notifications@github.com wrote:

@grumpydev https://github.com/grumpydev Well, this is a bit above my
league (I'm still getting the hang of the codebase) but in the meanwhile,
I'll put a warning about this in the Wiki page on testing.


Reply to this email directly or view it on GitHubhttps://github.com/NancyFx/Nancy/issues/624#issuecomment-13235621.

@thecodejunkie I've tested this bug with the latest version of master and this doesn't fix the bug.
#963 only returns DefaultRootPathProvider if providerType is null

var providerType = AppDomainAssemblyTypeScanner
    .TypesOf<IRootPathProvider>(ScanMode.ExcludeNancy)
    .SingleOrDefault();

if (providerType == null)
{
    providerType = typeof(DefaultRootPathProvider);
}

but in the scenario that triggers this bug providerType still hits AspNetRootSourceProvider. This is understandable, seeing as the testing assembly and the modules being tested are not in the same assembly but are in the same AppDomain.

I, personally, see two solutions:

  1. Somehow tell the bootstrapper we're doing testing so it excludes assemblies like Nancy.Hosting.* for example.
  2. Write a TestingBootstrapper that defaults to sensible values for stuff like GetRootPathProvider and SafeGetNancyEngineInstance

I'm personally in favor of option two.

Owner

thecodejunkie commented Feb 10, 2013

I thought you said this was only happening while running under a
Nancy.Testing.Browser ? The ConfigurableBootstrapper overrides the
RootPathProvider property and explicitly return a DefaultRootPathProvider
instance

On Sun, Feb 10, 2013 at 9:24 PM, MichaelAz notifications@github.com wrote:

@thecodejunkie https://github.com/thecodejunkie I've tested this bug
with the latest version of master and this doesn't fix the bug.
#963 #963 only returns
DefaultRootPathProvider if providerType is null

var providerType = AppDomainAssemblyTypeScanner
.TypesOf(ScanMode.ExcludeNancy)
.SingleOrDefault();
if (providerType == null){
providerType = typeof(DefaultRootPathProvider);}

but in the scenario that triggers this bug providerType still hits
AspNetRootSourceProvider. This is understandable, seeing as the testing
assembly and the modules being tested are not in the same assembly but are
in the same AppDomain.

I, personally, see two solutions:

  1. Somehow tell the bootstrapper we're doing testing so it excludes
    assemblies like Nancy.Hosting.* for example.
  2. Write a TestingBootstrapper that defaults to sensible values for stuff
    like GetRootPathProvider and SafeGetNancyEngineInstance

I'm personally in favor of option two.


Reply to this email directly or view it on GitHubhttps://github.com/NancyFx/Nancy/issues/624#issuecomment-13360295..

Right, it seems I was testing against the wrong bootstrapper (DefaultNancyBootstrapper insted of ConfigurableBootstrapper). I'll change the wiki page on testing to reflect the need to use ConfigurableBootstrapper.

Owner

thecodejunkie commented Feb 11, 2013

So if you switch to ConfigurableBootstrapper the problem is now gone? I.e you used to have it when using that bootstrapper, but the change has fixed it?

The testing was done against DefaultNancyBootstrapper as per the instructions on the wiki (which I've now changed to use ConfigurableBootstrapper). But yes, the problem is now gone.

Owner

thecodejunkie commented Feb 11, 2013

Na I think we should leave the docs as is + update the FavIcon scanner to be able to handle the error

Owner

thecodejunkie commented Feb 11, 2013

@MichaelAz @dragan could you verify that my last commit fixed it when you use the DefaultNancyBootstrapper ?

@thecodejunkie seems to work now. Might be worth to check this on Mono, but it's resolved as far as I can see.

@grumpydev grumpydev closed this Feb 12, 2013

swallowcamel pushed a commit to swallowcamel/nancywiki that referenced this issue Apr 1, 2016

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