Fix: Handle ReflectionTypeLoadException when loading assemblies#405
Fix: Handle ReflectionTypeLoadException when loading assemblies#405numandina wants to merge 1 commit intoIvanMurzak:mainfrom
Conversation
Assemblies with missing dependencies now get skipped instead of crashing the MCP plugin build. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This pull request adds robust error handling for assembly loading to prevent crashes when Unity projects contain assemblies with missing optional dependencies. The fix implements a defensive filtering approach that logs warnings for problematic assemblies instead of crashing the entire plugin.
Key Changes:
- Introduced
GetSafeAssemblies()method that wraps assembly type loading in try-catch blocks - Catches
ReflectionTypeLoadExceptionspecifically for assemblies with missing dependencies - Skips dynamic assemblies which cannot be safely reflected
| protected virtual IEnumerable<Assembly> GetSafeAssemblies() | ||
| { | ||
| foreach (var assembly in AppDomain.CurrentDomain.GetAssemblies()) | ||
| { | ||
| if (assembly.IsDynamic) | ||
| continue; | ||
|
|
||
| try | ||
| { | ||
| assembly.GetTypes(); | ||
| yield return assembly; | ||
| } | ||
| catch (ReflectionTypeLoadException) | ||
| { | ||
| _logger.LogWarning("Skipping assembly with missing dependencies: {assembly}", assembly.GetName().Name); | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| _logger.LogWarning("Skipping assembly {assembly}: {error}", assembly.GetName().Name, ex.Message); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The new GetSafeAssemblies() method lacks test coverage. Given that the repository has comprehensive test coverage (including tests for McpPlugin, ConnectionManager, and various tools), this new error handling logic should be tested. Consider adding a test that verifies assemblies with missing dependencies are properly skipped and logged, and that valid assemblies are correctly returned.
|
Hi @numandina |
|
@IvanMurzak Right, the terminal saw red when executing scripts and skirted around the errors, but I was content with the MCP just building and being able to use simple commands. Good luck and hope to see a fix soon. As for optional DLLs, my project has one that references other DLLs that don't need to be installed (building construction library looking for platform specific or software specific libraries). The libraries vary by user but the main DLL looks at them all.
|
|
I am working on the fix in this pull request:
|
|
@IvanMurzak awesome, thank you. Will gladly test once it's implemented. Currently testing the try catch applied to 3 other scripts (any gettypes call) locally. Looking forward to the caching implementation. |
|
@numandina the fix was added to Thanks again for the report, please await for the next release to try it out. Feel free to chat here if for any reason it won't work anyway. |
Plugin crashed during build when my project contained assemblies that reference optional DLLs not present on my machine. Calling
GetTypes()on these assemblies throws aReflectionTypeLoadException.Added
GetSafeAssemblies()method that wrapsGetTypes()in a try-catch and skips problematic assemblies with a warning log instead of crashing.Assemblies with missing dependencies now get skipped instead.
NOTE: This works well for my case which was external DLLs with optional dependencies. But it skips those DLLs entirely. A more sophisticated approach could extract partial types of the DLL if it's relevant.