Skip to content

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.

10 participants