Also remove stack traces from compilation errors as part of #864 #2853
Conversation
| if (ex.InnerException is ICompilationException) | ||
| // Compilation exceptions either throw FileNotFound or FileLoad exceptions and may change | ||
| // from version to version. It's safest to catch both types of exceptions | ||
| if (ex is FileLoadException || ex is FileNotFoundException) |
There was a problem hiding this comment.
Why not check the InnerException of any exception?
There was a problem hiding this comment.
We could for compilation exceptions, but historically I've seen us only catching these two types of exceptions. e.g. 196e111.
Alternatively, we can catch FileNotFoundExceptions for ICompilationException and EntryPointNotFoundException then catch generic Exception only for ICompilationExceptions.
| applicationName, | ||
| ex.InnerException); | ||
| } | ||
| else |
There was a problem hiding this comment.
nit: this else is technically not needed...
JunTaoLuo
force-pushed
the
johluo/864-cleanup-compilation-errors
branch
from
c2533e8
to
b949e01
Compare
| @@ -229,19 +229,24 @@ private Task<int> ExecuteMain(DefaultHost host, string applicationName, string[] | |||
| { | |||
| assembly = host.GetEntryPoint(applicationName); | |||
| } | |||
| catch (FileNotFoundException ex) when (new AssemblyName(ex.FileName).Name == applicationName) | |||
| catch (Exception ex) | |||
There was a problem hiding this comment.
Before we threw with ThrowEntryPointNotfoundException only if ex.FileName == applicationName now we will throw for any FileLoadException or FileNotFoundException - is it intentional?
There was a problem hiding this comment.
Yes, I checked with @davidfowl about it.
There was a problem hiding this comment.
Couldn't you write this as
catch (Exception ex) when (ex is FileLoadException || ex is FileNotFoundException)`
{
if (ex.InnerException is ICompilationException)
{
throw SuppressStackTrace(ex.InnerException);
}
ThrowEntryPointNotfoundException(
host,
applicationName,
ex.InnerException);
}Wouldn't need the other throw.
|
|
2 similar comments
|
|
|
|
|
merged to dev |
#864 hide stack traces from compilation errors as well