Skip to content
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

Allow configuring or detecting current UI framework to avoid exceptions during dependency registrations #2395

Closed
JesperTreetop opened this issue Apr 27, 2020 · 7 comments · Fixed by #2507

Comments

@JesperTreetop
Copy link

JesperTreetop commented Apr 27, 2020

Is your feature request related to a problem? Please describe.

When I start up my ReactiveUI WPF project in the debugger and have exceptions set to always break on first throw (which is very helpful sometimes), I always see a number of FileNotFoundExceptions thrown in assembly loading, because of DependencyResolverMixins.InitializeReactiveUI making sure that all dependencies for all UI frameworks are registered. (This line is where the throwing originates.)

Describe the solution you'd like

For registrations to not happen which would always knowably result in exceptions being thrown. At most half of them could be loaded at the same time (WPF, WinForms and Drawing) - the registration path has to always go down 3-5 blind alleys every launch for every ReactiveUI application. Not having all UI frameworks ReactiveUI supports loaded in every application is the opposite of exceptional (and it's the result of vexing exceptions being handled down the stack), so if possible the exceptions should be avoided.

Describe alternatives you've considered

I'm seeing this because I have exceptions break on first throw and I could turn it off. I have a habit of having it enabled since WPF is spectacular about swallowing exceptions, and I'd rather know. This issue is mainly about developer ergonomics for me. There are performance implications to both throwing the exceptions and whatever other work happens on the road to those exceptions being thrown, but that's not my primary issue.

Describe suggestions on how to achieve the feature

For ReactiveUI to detect which assemblies are loaded by that time (AppDomain.CurrentDomain.GetAssemblies()) and opt into the namespaces to register based on that. Maybe there's a timing problem where the assemblies may not yet have loaded. In that case, being able to somehow provide a list of which UI frameworks would be good.

Additional context
Stepping past these exceptions takes 8-12 seconds, about as much as disabling and re-enabling first-throw exception breaking. Over a day of stopping and starting my program it really adds up.

I would have worked on a solution but I don't have domain knowledge about how this works for all the UI frameworks - this seems like an internals thing that should be idiomatic to the codebase.

@glennawatson
Copy link
Contributor

This isn't a bug. It's scanning for DLL from common rxui nuget package. There are plans to make the init more obvious in the future. So essentially we scan each common DLL we have in the framework since we don't know which nuget packages are installed. Going to close for now but there is a issue for tracking init changes you may want to track.

@JesperTreetop
Copy link
Author

But at the place where the scanning happens, isn't it possible to check which assemblies are loaded and only hook up the ones that are present?

@glennawatson
Copy link
Contributor

They wouldn't necessarily be loaded since assemblies are lazy loaded.

@JesperTreetop
Copy link
Author

JesperTreetop commented Apr 27, 2020

If you're referring to #1737, being able to call an init method per framework as per the latest comment sounds great. I don't know how long-lead this work is since it's been open for a while now.

If it would be accepted, I wouldn't mind writing the pull request for a method which you could call before RxApp is statically constructed, which would initialize the namespaces array to only the ones that would work by listing the frameworks you want (and if the method wasn't called, it would behave the same as today); I understand if this would be considered contrary to the larger init effort though.

@glennawatson
Copy link
Contributor

There's been a bit of work done on init but that being said if you have a way of making the exception happen a pr would be accepted since it'll avoid some support time for us from people being confused while we are doing the init work.

@JesperTreetop
Copy link
Author

The way of making the exception happen is:

  1. In Visual Studio, go to Debug → Windows → Exception Settings.
  2. Check the box next to Common Language Runtime Exceptions under "Break When Thrown" - ie break directly when the exception is thrown, and not just if it remains unhandled. (You can right-click → Restore Defaults to restore the default list of such exceptions.)
  3. Start literally any ReactiveUI application that doesn't have references to all the assemblies for all the frameworks, and step through the FileNotFoundExceptions that happens when Type.GetType tries to find the type for registrations, tries to load the assembly for each framework and can't do so.

Most people don't run with Break When Thrown all the time and it's certainly not the default. I do it often when working with WPF code because I've seen my fair share of latent bugs, where code I've written in WPF UI code has thrown, but that exception has been swallowed by WPF in the interest of keeping the app going, like during bindings, type conversions and so on. Those bugs are very hard to find otherwise, and in some cases ended up being shipped.

@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 23, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants