Skip to content
This repository was archived by the owner on Dec 18, 2017. It is now read-only.

Also remove stack traces from compilation errors as part of #864 #2853

Closed
wants to merge 3 commits into from
Closed

Also remove stack traces from compilation errors as part of #864 #2853

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
624ce5a
Also remove stack traces from compilation errors as part of #864
JunTaoLuo Sep 28, 2015
b949e01
minor aesthetic changes
JunTaoLuo Sep 28, 2015
825fece
addressing PR suggestion by pranav
JunTaoLuo Sep 28, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 14 additions & 9 deletions src/Microsoft.Dnx.ApplicationHost/Program.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before we threw with ThrowEntryPointNotfoundException only if ex.FileName == applicationName now we will throw for any FileLoadException or FileNotFoundException - is it intentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I checked with @davidfowl about it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

{
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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not check the InnerException of any exception?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

{
throw ex.InnerException;
if (ex.InnerException is ICompilationException)
{
throw SuppressStackTrace(ex.InnerException);
}

ThrowEntryPointNotfoundException(
host,
applicationName,
ex.InnerException);
}

ThrowEntryPointNotfoundException(
host,
applicationName,
ex.InnerException);

return Task.FromResult(-1);
throw;
}

if (assembly == null)
Expand Down
1 change: 0 additions & 1 deletion src/Microsoft.Dnx.Host/RuntimeBootstrapper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
using System.Collections.Generic;
using System.IO;
using System.Reflection;
using System.Runtime.Versioning;
using System.Threading.Tasks;
using Microsoft.Dnx.Runtime;
using Microsoft.Dnx.Runtime.Common.CommandLine;
Expand Down