Write Premain errors to the response and write different status codes#10282
Conversation
jkotalik
changed the title
jkotalik
force-pushed
the
jkotalik/preMain
branch
from
a88f0b2 to
c2fe895
Compare
|
Some feedback I received from @shirhatti is that the error pages need work with regards to looking different than the default error page (they look too similar). Currently errors look like this: This does have the necessary information, but it isn't presented nicely. We can consider doing more work on this in either this PR or a separate PR. |
| std::unique_ptr<IN_PROCESS_APPLICATION, IAPPLICATION_DELETER> inProcessApplication; | ||
|
|
||
| // TODO not sure how easy it will be to untangle errors here | ||
| // ErrorContext errorContext; |
There was a problem hiding this comment.
Did you figure out if it's possible?
There was a problem hiding this comment.
It is possible, but it isn't an important scenario. There are a few errors that can occur here, but they are very rare because we already invoked hostfxr earlier. After main has started, we would return the developer exception page.
| USHORT m_subStatusCode; | ||
| std::string m_statusText; | ||
| std::vector<byte> m_ExceptionInfoContent; | ||
| std::string& m_ExceptionInfoContent; |
There was a problem hiding this comment.
I'd just like to publicly say "I'm not a fan of this!"
|
🆙 📅 |
BrennanConroy
left a comment
BrennanConroy
left a comment
There was a problem hiding this comment.
Looks good to me. Should anyone else be reviewing?
ghost
deleted the
amcasey
added
area-networking
7 participants
Fixes #10011 and will eventually fix #10008.
Here is so backstory behind how ANCM writes error pages, and my initial thoughts on how to design dynamic vs static responses.
When ANCM hits a startup exception in the shim or in the request handler, an error "Application" is created, which serves static HTML or the Hosting Startup Error page. This needs to be expanded now to serve dynamic content from hostfxr or potentially from ANCM. Along with checking if disableStartupErrorPage is set, writing text/plain vs text/html, status codes, substatus codes, etc, having a single application to handle multiple failure points has become unmanageable.
I think we should probably redesign how we server error responses. My initial idea was to create an ENUM or key off of HRESULT to know what type of error occurred, and change the type of application/handler to create. An example of this is what I did with hostfxr errors, where I create a new handler and application. I think the best approach is to create a unique application/handler per type of error which just serves a single type of failure. Then, per each failure type, we can add more dynamic data (like stack trace, hresult, doc link, etc.) when running in development. There may be a good amount of duplication, but trying to cram everything into the same class is getting worse and worse (see ServerApplication.cpp).
If this plan looks good, I'll go ahead and follow through. I just want to confirm with someone before I do a decently size refactoring.