Runtime: Improve diagnostics for missing resources

[diverdan92]

When porting existing WPF or WinForms applications to target .NET Core that contain EmbeddedResources, the logical name is not fully defined and results in missing resources.

@ericstj @dsplaisted fyi from our debugging session last week.

[danmosemsft]

In what cases -- do you have a repro case? There is a bit convoluted mechanism for picking these names.

This is all done in the build though -- any reason I shouldn't move this to MSBuild?

[nguerrera]

RootNamespace is defaulted to project name in sdk. If you had an empty one or a different one before the port, then you need to keep it in your csproj.

[nguerrera]

Please supply a repro showing expected and actual behavior. If it's the root namespace default as I suspect, I don't think we can do anything about it.

[ericstj]

I think this bug belongs in the SDK or MSBuild repo, it's all about the build tooling for this stuff. I think the gist here is the globbing and how that interacts with folks when they are porting their stuff. If they manually organized a project into folders without using those folders to represent a namespace then the generated .resources will have the wrong name.

[nguerrera]

If that's what the issue is, then I think this should go to docs, not sdk or msbuild.

We can't say that it "is not set properly", or that it is "not fully defined". I will grant that it is certainly a pain point in migration.

The algorithm for picking the names is the same as always. If you are using default RootNamespace + default globs, then you get the corresponding names from those defaults. If you go from File -> New classic project, add a folder, add a resource to it, then do the same for an sdk project, you will get the same names in both projects: (RootNamespace=ProjectNameByDefault).(FolderName).(Filename).resources.

If you were not using the folder names in your resource names before, then you had one of LogicalName, Link, or MainfestResourceName metadata (possibly others) to override that behavior. In the new world, you can still do that with item updates or by disabling globs and copying over your old explicit includes wholesale. We cannot guess these overrides, you have to carry them over.

[danmosemsft]

Are there any exception messages commonly seen in working through these difficulties, that we can improve? That at least is within BCL control. Eg., "did not find stream named X, only streams named Y and Z" might be pretty helpful?

[nguerrera]

Yes, we don't have the Y and Z part, but it would be helpful. A link to a document describing how names are assigned and how to control them would be a nice touch as well.

[nguerrera]

I _think_ this is the common failure case: https://github.com/dotnet/coreclr/blob/791332b101524034ca9a69ab7669d5a87713447b/src/System.Private.CoreLib/src/System/Resources/ManifestBasedResourceGroveler.cs#L512

[tarekgh]

Will move this to the doc repo.

[tarekgh]

Issue moved to dotnet/docs dotnet/runtime#17842 via ZenHub

[ericstj]

So I happened to be looking at this code yesterday and noticed that there was another path where it tries to determine the name from the source file.

It checks the EmbeddedResource for DependentUpon metadata:
https://github.com/Microsoft/msbuild/blob/0263a588f3db16abe4ae4a051213f4809e8b55c2/src/Tasks/CreateManifestResourceName.cs#L143-L165

Then it uses that file to determine the name of the resources:
https://github.com/Microsoft/msbuild/blob/0263a588f3db16abe4ae4a051213f4809e8b55c2/src/Tasks/CreateCSharpManifestResourceName.cs#L109-L130

I noticed that the SDK globs don't set the DependentUpon metadata that the task is looking for:
https://github.com/dotnet/sdk/blob/f0a1246be1ebe11a88097ac3dd5736f0ad15b855/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.DefaultItems.props#L30

Now this isn't always present but there are some cases where it is. I think the SDK might consider addressing this (or have the WinForms SDK do it). I would imagine this is covered by https://github.com/dotnet/sdk/issues/1441.

[nguerrera]

Ah, yes, I've seen that before and been terrified by it. :(

The POR is that we will not be inferring DependentUpon in globs and instead VS will do the inference and draw the tree accordingly. This does break the case where the first class in your .cs that is depended upon by a .resx does not have a matching class name to file name. You can see this working in VS 16 already. The reason we are not doing this with SDK magic is that it can be quite expensive to do this in msbuild evaluation. cc @davkean @drewnoakes @davidwengier

I think that this case where the build is inspecting DependentUpon was overlooked. I also think it is reasonable to take a break here and suggest either naming your files to match (common case of first class name == file names without extension is unaffected), or setting metadata to force the resource name you want.

If we had good diagnostics discussed above, this could be called out in linked documentation.

[nguerrera]

Will move this to the doc repo.

Do we not want to have an issue here to track better error messages as we started to discuss? These could link to the docs.

[nguerrera]

Above I oversimplified (first class name == file name case, there is also the namespace that may not match rootnamespace + folders). It doesn't change my position to document how things work + have better error message in corefx pointing to documentation.

[mjrousos]

Thanks, @nguerrera. In case it's useful, here's some context on how I ran into this recently helping to port a WinForms app:

• The app included a Windows Form in a sub-directory (Views\Form1.cs) but the form's namespace didn't include the sub-directory.
• The old .NET Framework app had added an icon to the Form using the form's property window. This caused an EmbeddedResource dependent upon the form to be added to the csproj and the designer-generated C# to load the icon from that resource.
• The developer then moved to use an SDK-style project and simply copied C# and resx files over, thinking they would automatically be included by the SDK-style project.
• The app failed with the exception pasted below because without the <DependentUpon> metadata in the project file, the path where the icon's resx file ended up embedded didn't match where the designer.cs thought it would be (the resources file was embedded based on the resx file's location rather than based on the form's name).
• The developer who was initially doing the port was confused about why they were seeing that exception and what should be done about it.

System.Resources.MissingManifestResourceException: 'Could not find any resources appropriate for the specified culture or the neutral culture.  Make sure "MyApp.Form1.resources" was correctly embedded or linked into assembly "MyApp" at compile time, or that all the satellite assemblies required are loadable and fully signed.'

I understand that this issue is hard to make work without the DependentUpon attribute, but the exception message should be improved to help developers better understand what's going on. Perhaps we can link from the exception message to some documentation that talks about how resources are embedded and calls out <DependentUpon> as something that influences resource paths that so that they understand it should be brought over if they're porting apps that used it previously.

[davidwengier]

Posted a pretty simple repro in https://github.com/dotnet/project-system/issues/4807#issuecomment-491111821

[danmosemsft]

I want to make this task concrete. At this point, how about we just create a new fwlink and add it to these

  <data name="MissingManifestResource_NoNeutralAsm" xml:space="preserve">
    <value>Could not find any resources appropriate for the specified culture or the neutral culture.  Make sure "{0}" was correctly embedded or linked into assembly "{1}" at compile time, or that all the satellite assemblies required are loadable and fully signed.</value>
  </data>
  <data name="MissingManifestResource_NoNeutralDisk" xml:space="preserve">
    <value>Could not find any resources appropriate for the specified culture (or the neutral culture) on disk.</value>
  </data>

We could perhaps point it to https://docs.microsoft.com/en-us/dotnet/api/system.resources.missingmanifestresourceexception?view=netcore-3.0 initially. Then the docs issue would be to make a better topic, or extend that one.

[nguerrera]

@danmosemsft I like that. Simple win.

[tarekgh]

I'll follow with the doc team and look if we can get fwlink from now even if the doc is not fully baked.

[danmosemsft]

Thanks @tarekgh, it's easy to create a fwlink now (I can dig up how if you need it) and point at something reasonable like the page mentioned, then change later (that's the beauty of fwlinks of course). That way we can edit the string now and close this so it's done.

[tarekgh]

@danmosemsft yes that is what I meant. I just want to get the doc team aware so we can track the fwlink always pointing to the right place.

[dsplaisted]

Consider an aka.ms link instead of a fwlink, as then you can choose a meaningful name for the link instead of just having an arbitrary ID.

[tarekgh]

The PR is here https://github.com/dotnet/coreclr/pull/24552

[davkean]

Note that from a .NET tooling perspective we still want to address the underlying issue despite the error message change. We're tracking that via https://github.com/dotnet/project-system/issues/4807.

[tarekgh]

I have opened and linked the PR for adding the link to the exception message. On the PR, @jkotas has a good point that why we need to treat MissingManifestResourceException different than any other exceptions and add a link to the exception message. I agree with @jkotas here because the link would not be that useful. If we add all the needed information/documentation to the exception doc, I believe this should be enough. Whoever gets this exception and want to diagnose more will just consult the documentation of this exception without having explicit link in the exception message.

If you agree with that, I would close this issue and we already have a doc issue tracking getting all needed information to the docs.

[nguerrera]

I commented on that issue, that I don't feel strongly enough to push back, but I personally think it's fine to add links to commonly misunderstood errors.

What do you think about instead of adding links, making the error more detailed like: "Manifest resource Foo.Bar not found among resources A,B,C in assembly Z". This makes it easier to understand that the resource isn't missing, but actually got the wrong name. I don't feel strongly about this either.

[danmosemsft]

One principle that I think is important is to make sure that ToString() includes all useful information.

Reason: the ReflectionTypeLoadException used to say "Consult the LoaderExceptions property for more information" which was always maddening if you were looking at a test log where it was long gone. We fixed that one to write it out in the ToString().

[tarekgh]

@danmosemsft what exactly you are suggesting to add to the ToString for MissingManifestResourceException? I am not sure there is anything specific we can say more than what we have in the exception message. but I may be missing something here.

[danmosemsft]

@tarekgh I was responding to @nguerrera suggestion of "Manifest resource Foo.Bar not found among resources A,B,C in assembly Z"." . That being in the message, it would be fine.

[verbedr]

A small side note.
Now that EF6.3 supports standard 2.1 this is causing a problem with the migrations and their dependencies. I can manually update the designer code so it works. But perhaps you could contact someone of EF team to point this out so that it can be documented. @bricelam?

[bricelam]

cc @divega

What’s the issue? I thought we already resolved this by making resource names the same as they were on .NET Framework projects...

[divega]

@verbedr any chance you were referring to https://github.com/aspnet/EntityFramework6/issues/1225?

[verbedr]

@divega I think this is a different issue. The ticket is referring to an error at compile time. I experienced the problem at migration time, but the problem is still compiler related. It happens due to how the compiler names the resources when embedding it in the dll. The full stack version renamed the resource to match the first class name in the depended cs file. So the resource was not named using the file name. The new .net core compile now only relies on the file name because the concept of depended upon is just a cosmetic feature of the IDE. And when the ResourceManager resolves the resources for a class using the class type it fails.

So existing migrations had some designer generated code that looks like this.

    [GeneratedCode("EntityFramework.Migrations", "6.2.0-61023")]
    public sealed partial class Migration1 : IMigrationMetadata
    {
        private readonly ResourceManager Resources = new ResourceManager(typeof(Migration1));

        string IMigrationMetadata.Id
        {
            get { return "201812191001443_Migration1"; }
        }

With a depended resource file named 201812191001443_Migration1.resx

When running this migration scripts it will throw the exception mentioned in this thread (System.Resources.MissingManifestResourceException)

My first work around before I realised what the root cause was, was to rename the files and drop the date/time marker. But this gave problems when I tried to add/rescaffole a migration. When I digged deeper and found this thread I solved the issue by changing the designer generated code like this

    [GeneratedCode("EntityFramework.Migrations", "6.3.0-preview8-19405-04")]
    public sealed partial class Migration1 : IMigrationMetadata
    {
        const string _id = "201901071203440_Migration1";
        private readonly ResourceManager Resources = new ResourceManager($"{typeof(Refactoring).Namespace}.{_id}", typeof(Refactoring).Assembly);

        string IMigrationMetadata.Id
        {
            get { return _id; }
        }

To give more context check what Nick wrote https://github.com/dotnet/corefx/issues/34190#issuecomment-455322419

++++++ update
I need to learn to read the complete thread! So to come back, yes the issue you mentioned is related.
++++++ update2
Just got vs2019 Preview 3.0 which pulled in preview 9 and you can forget my remarks now. Sorry to bother you guys and have created the confusion.

[divega]

@verbedr thanks for the detailed explanation. Although I am still not sure why the new naming convention for resources wouldn't work. With https://github.com/microsoft/msbuild/issues/4695 fixed, we expect that the issue with files in subfolders will be gone.

@bricelam can you please take a look at the above comment to see if you think there is something new we should track?

[bricelam]

I believe all the issues mentioned are resolved in the daily builds