Skip to content

Display startup errors in ANCM #8518

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

Merged
merged 23 commits into from
Apr 18, 2019
Merged

Display startup errors in ANCM #8518

merged 23 commits into from
Apr 18, 2019

Conversation

pakrym
Copy link
Contributor

@pakrym pakrym commented Mar 14, 2019

No description provided.

@Tratcher
Copy link
Member

👀

@jkotalik
Copy link
Contributor

I'll be taking this over eventually, but we still need some downstream dependencies.

@jkotalik jkotalik added this to the 3.0.0-preview4 milestone Mar 18, 2019
@jkotalik
Copy link
Contributor

I have a prototype of this using replicating the hosting exception page. Currently looks like:

image

As @pakrym mentioned, this unfortunately brings in some dependencies that will be present in every inproc app. Here are the usings I have for the startup hook.

using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Reflection;
using System.Text;
using System.Text.Encodings.Web;
using Microsoft.AspNetCore.Server.IIS;
using Microsoft.Extensions.FileProviders;
using Microsoft.Extensions.Primitives;
using Microsoft.Extensions.StackTrace.Sources;

@Tratcher
Copy link
Member

Hosting already has all of those dependencies, no? You wouldn't be adding anything new.

@jkotalik jkotalik added the blocked The work on this issue is blocked due to some dependency label Mar 25, 2019
@jkotalik
Copy link
Contributor

jkotalik commented Mar 25, 2019

@jkotalik jkotalik self-assigned this Mar 26, 2019
@jkotalik jkotalik force-pushed the pakrym/error-logging-ancm branch from bcbb8af to feed9f0 Compare March 26, 2019 18:01
@jkotalik jkotalik force-pushed the pakrym/error-logging-ancm branch 2 times, most recently from 149aa61 to 7cafe21 Compare April 15, 2019 22:59
@jkotalik jkotalik removed the blocked The work on this issue is blocked due to some dependency label Apr 16, 2019
@jkotalik
Copy link
Contributor

This is now working. Probably a bit of cleanup around the edges (like using host properties rather than environment variables), but overall it looks good.

Here is what the exception page looks like:
image

And showing the .NET Runtime event log:

image

@jkotalik jkotalik marked this pull request as ready for review April 16, 2019 01:43
@jkotalik jkotalik requested a review from analogrelay as a code owner April 16, 2019 01:43
@jkotalik jkotalik requested a review from BrennanConroy April 16, 2019 01:43
{
}

ServerErrorHandler(IHttpContext& pContext, USHORT statusCode, USHORT subStatusCode, std::string statusText, HRESULT hr, HINSTANCE module, bool disableStartupPage, int page) noexcept
Copy link
Contributor

Choose a reason for hiding this comment

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

There are two scenarios here. One which loads from a resource file (page number passed in) and the other is passed in via the startup hook. I have a 3rd constructor which takes both.


AppDomain.CurrentDomain.UnhandledException += (sender, eventArgs) =>
{
var exception = (Exception)eventArgs.ExceptionObject;
Copy link
Contributor

Choose a reason for hiding this comment

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

@jkotalik
Copy link
Contributor

I'm going to try verifying this change E2E ASAP. I'm confident in the change, but I would like to make sure this works as part of the SDK.

@jkotalik jkotalik force-pushed the pakrym/error-logging-ancm branch from c399ee2 to 6ea8496 Compare April 18, 2019 19:12
@jkotalik jkotalik merged commit 56c064b into master Apr 18, 2019
@jkotalik jkotalik deleted the pakrym/error-logging-ancm branch April 18, 2019 20:48
@amcasey amcasey added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-runtime labels Jun 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants