microsoft / vs-mef

Managed Extensibility Framework (MEF) implementation used by Visual Studio
MIT License
417 stars 87 forks source link

ImportMany failure at runtime instead of composition time #132

Open weltkante opened 5 years ago

weltkante commented 5 years ago

Consider the following scenario having ImportMany over mixed scoped/unscoped exports

public interface IMessageHandler { }

[Export(typeof(IMessageHandler))]
public class UniqueMessageHandler : IMessageHandler { }

[Export(typeof(IMessageHandler))]
[Shared]
public class GlobalMessageHandler : IMessageHandler { }

[Export(typeof(IMessageHandler))]
[Shared("somewhere")]
public class ScopedMessageHandler : IMessageHandler { }

[Export]
public class Helper1
{
    [ImportMany]
    public ICollection<Lazy<IMessageHandler>> Handlers { get; set; }
}

[Export]
public class Helper2
{
    [Import]
    public Lazy<Helper1> LazyHelper { get; set; }
}

Observed behavior:

Expected behavior:

PS: The original scenario was someone marking an export as scoped in an extension assembly, causing random application failures because the ImportMany was lazily resolved. Was kinda hard to root cause.

builder-main commented 4 months ago

I know your issue is a bit old now and not sure which version it corresponds to, but could this be related ? (using .NETFramework\v4.8\System.ComponentModel.Composition.xml)

https://stackoverflow.com/questions/78076711/mef-importmany-does-not-propagate-exceptions-and-silently-fail

weltkante commented 4 months ago

The scenario I outlined above is something that could be detected at composition time but isn't, and instead throws at runtime.

The issue you linked, on the contrary, never throws (according to its author) but he would like for it to be notified that an inner composition failed.

Basically its the opposite behavior, so not really related, no. What he would need is some feature that allows reporting exceptions during runtime construction back through the container.

AArnott commented 4 months ago

@weltkante: I think I agree with you on expected behavior. Alternatively it might fall out that we omit ScopedMessageHandler from the ImportMany collection and run with the remaining two. There's a design and some rules for composition that will probably dictate which of these directions the fix goes to.

That said, I doubt I'll be working on this right away.

weltkante commented 4 months ago

Oh, I was just responding to the above post. The original report was feedback on a weird edge case I was hitting when someone incorrectly annotated his extension export with a sharing boundary. Nothing thats blocking anything, just was hard to diagnose because the composition graph was reported as valid yet at runtime still failed. So any improvement here is just about improving the ability to diagnose error cases from unexpected scope annotations.

I don't really mind which of the two solutions you mentioned will eventually be picked, the "expected behavior" was just extrapolating the (already existing) failure of Helper1 from runtime to composition time. If its more straightforward to do it in a different way thats just as fine.