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

Engine should not crash on encountering assembly that cannot be examined #798

Open
ChrisMaddock opened this issue Jul 18, 2020 · 5 comments
Labels
Bug V4 All issues related to V4 - use -label:V4 to get non-V4 issues
Milestone

Comments

@ChrisMaddock
Copy link
Member

See #794.

Currently, if the engine is passed an assembly that it cannot examine, the engine throws an exception. This was the correct behaviour at the time of implementation, however our handling of invalid assemblies has since improved.

We should instead suppress the crash, and create some form of NotRunnableFrameworkDriver for the assembly. That will allow the engine and various runners to produce more user-friendly error messages for these cases.

@CharliePoole
Copy link
Member

Interesting. Our DriverService already does what you suggest in the face of a BadImageFormatException.

But this exception comes in the code that initially examines the assembly in order to select a runner and the DriverService isn't in play until we have a runner that needs to create a driver.

I'm pretty sure this worked in the past but we have added more pre-checks to the code before we actually try to load the assembly and probably that's what broke it.

What I'm not getting is why this doesn't happen for Run as well as Explore.

@ChrisMaddock
Copy link
Member Author

I think I does happen for run as well. 🙂

@CharliePoole
Copy link
Member

Ah. That would make sense. I think we should go through all the "pre-discovery" we do with Cecil and try to handle errors in a more friendly way.

In terms of the sequence of things, I think we would want to implement #718 first, if you decide to do it. It's still sitting as an "idea" here although I have a vague memory you said something about going ahead with it somewhere else.

@ChrisMaddock
Copy link
Member Author

Yes, I think we must have had that conversation elsewhere. Please do go ahead with it, if you're still willing! 😀

@CharliePoole
Copy link
Member

I'll try to work on it over the weekend. Considering that I'm not employed and sheltering in place at home, it's amazing how many things I have going on. 😄

@CharliePoole CharliePoole added this to the 4.0 milestone Mar 9, 2022
@CharliePoole CharliePoole modified the milestones: 4.0, 4.0.0-beta.1 Dec 29, 2024
@CharliePoole CharliePoole added the V4 All issues related to V4 - use -label:V4 to get non-V4 issues label Feb 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug V4 All issues related to V4 - use -label:V4 to get non-V4 issues
Projects
None yet
Development

No branches or pull requests

2 participants