-
-
Notifications
You must be signed in to change notification settings - Fork 239
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
Update TypeSourceSelector.cs #230
Conversation
dotnet/SqlClient#1930 > I had the same issue here. After changing to GetExportedTypes() seems to fix the issue
Hi @peterhel! 👋🏻 Thanks for this contribution 🙏🏻 Unfortunately, it looks like there's a syntax error and build is failing 😅 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tried to apply a fix since it was only a missing closing parenthesis!
Co-authored-by: 121GWJolt ("Jolt") <121GWJolt@users.noreply.github.com>
There's another problem with this change; it does no longer scan private/internal types, which is a massive behavior change. Is there anything else we can do to mitigate this? I don't have enough time to fully read through the thread you linked 😅 |
@khellang When I was doing assembly scanning I used the same mechanism as shown here dotnet/SqlClient#1930 (comment) public static Type[] GetLoadableTypes(this Assembly assembly)
{
try
{
return assembly.GetTypes();
}
catch (ReflectionTypeLoadException ex)
{
return ex.Types.Where(t => t is not null).ToArray()!;
}
}
private IImplementationTypeSelector InternalFromAssemblies(IEnumerable<Assembly> assemblies)
{
return AddSelector(assemblies.SelectMany(asm => asm.GetLoadableTypes()));
} |
Thanks @CZEMacLeod ! That looks like a decent workaround. Let's ship it! 😄 |
Checking with my code (which was in vb.net) and removing the logging I had included, I think it actually includes one more catch public static Type[] GetLoadableTypes(this Assembly assembly)
{
try
{
return assembly.GetTypes();
}
catch (ReflectionTypeLoadException ex)
{
return ex.Types.Where(t => t is not null).ToArray()!;
}
catch
{
return [];
}
} HTH |
I've converted the full version of my original with logging (updated to use MEL instead of L4N) to c# below. I don't know if it adds much, but it might be useful for some users with debugging. using Microsoft.Extensions.Logging;
namespace System.Reflection;
public static class AssemblyTypeExtensions
{
public static Type[] GetLoadableTypes(this Assembly assembly) =>
assembly.GetLoadableTypes(Microsoft.Extensions.Logging.Abstractions.NullLogger.Instance);
public static Type[] GetLoadableTypes(this Assembly assembly, ILogger logger)
{
try
{
return assembly.GetTypes();
}
catch (ReflectionTypeLoadException ex)
{
using var scope = logger.BeginScope("Loading types from {assemblyName}", assembly.FullName);
logger.LogWarning(ex, "Issue loading types from {assemblyName}", assembly.FullName);
foreach (var le in ex.LoaderExceptions.Where(e => e is not null).OfType<Exception>())
{
if (le is TypeLoadException tle)
{
logger.LogWarning(tle, "TypeLoadException for {typeName} in {assemblyName}", tle.TypeName, assembly.FullName);
}
else
{
logger.LogWarning(le, "{exceptionType}: {message}", le.GetType().Name, le.Message);
}
}
return ex.Types.Where(t => t is not null).ToArray()!;
}
catch (Exception ex)
{
logger.LogWarning(ex, "Failed to load types from {assemblyName}", assembly.FullName);
return [];
}
}
} |
I don't know if your implementation is smart in caching the types of assemblies, but if you scan for multiple interfaces and classes and call this multiple times per assembly, it is probably worth caching the results as the exception handling adds some overhead. |
Thanks for the details! It pulls the types once per call to |
dotnet/SqlClient#1930
Hey! Got into trouble with .NET 8. If you look through the thread above you can see why I made the proposed change.