Remove existing implementation of StartupExceptionPage and use the on… #836
Conversation
| @@ -1,6 +1,7 @@ | |||
| <?xml version="1.0" encoding="utf-8"?> | |||
| <configuration> | |||
| <packageSources> | |||
| <add key="NufffGet" value="C:\temp\pagegeneratoroutput" /> | |||
There was a problem hiding this comment.
ignore this file. I will remove it before checkin
|
New screen shots, with and without details? |
|
What does this page look like? The diagnostics one? |
|
Here you go |
|
This startup page looks worse than the one we used to have 😄 . What happens when detailed errors aren't turned on? |
|
😄 |
|
You lost the detailed footer |
|
Good catch. Will fix. |
| } | ||
| else | ||
| { | ||
| model.ErrorDetails = new ExceptionDetails[] { }; |
There was a problem hiding this comment.
Alternatively, could you initialize this to an empty sequence in the ErrorModel type? No risk of accidental null refs.
|
🕐 |
| { | ||
| var exceptionDetailProvider = new ExceptionDetailsProvider( | ||
| hostingEnv.ContentRootFileProvider, | ||
| sourceCodeLineCount: 6); |
There was a problem hiding this comment.
Is the 6 a magic number or convention? And it is not configurable here.
There was a problem hiding this comment.
Nothing magical. It's just the same number we use everywhere. No need to make it configurable.
|
🆙 📅 |
| @@ -156,5 +156,8 @@ | |||
| <%$ include: ErrorPage.js %> | |||
| //--> | |||
| </script> | |||
| </body> | |||
| <footer> | |||
There was a problem hiding this comment.
@Eilon Footer here is after the <script> tag because our razor tool generator modifies the file in a way which generates incorrect .cs file (the escaping of the double quotes get messed up...snapshot below)...probably should file a separate issue for it?
|
🆙 📅 |
|
|
…e in Common [Fixes #831] Fix startup exception page to handle flattened exceptions
kichalla
force-pushed
the
kichalla/fix-startup-exception-page
branch
from
029a36d to
cba859a
Compare
…e in Common
[Fixes #831] Fix startup exception page to handle flattened exceptions
@Eilon @pranavkm