Diagnostics: Manual exception logging fails for self hosted applications.

[amanda-tarafa]

Seen while writing integration tests for Google.Cloud.Diagnostics.AspNet using a self hosted Web API application, will probably happen as well for self hosted Asp.Net MVC applications.

Google.Cloud.Diagnostics.AspNet.GoogleExceptionLogger.Log uses HtppContext.Current to access the current context in case none is passed as a parameter. For Asp.Net self hosted applications HttpContext.Current is always null because the part of the pipeline that sets it is IIS dependant. We then use HttpContext.Current to create an HttpContextWrapper which checks that the HttpContext it receives is not null, failing always for self hosted applications.

Two possible solutions I can think of:

• Forfeit logging context information when manualling logging exceptions for self hosted Asp.Net applications (exception information will be logged, just not HTTP context information). This can be easily achieved by creating an EmptyHttpContextWrapper and using that whenever HtppContext.Current is null.

• Add an overload for Google.Cloud.Diagnostics.AspNet.GoogleExceptionLogger.Create that takes a function returning the HttpActionContext which is available for each controller action execution withing the controller and contains a reference both to the HttpRequestMessage and HttpResponseMessage. We would also need to build an HttpActionContextWrapper. Usage would look something like this:

    public class MyController : ApiController
    {
        private readonly IExceptionLogger _exceptionLogger;
        public MyController()
        {
            _exceptionLogger = GoogleExceptionLogger.Create(
                                             ProjectId, Service, Version, () => this.ActionContext);
        }

        [HttpGet]
        public string Index(string id)
        {
            try
            {
                // Action code goes here.
            }
            catch (Exception ex)
            {
                _exceptionLogger.Log(ex);
            }
        }
    }

• Boths solutions mentioned above are not mutually exclusive so we can do both.

This won't happen for Asp.Net Core because Google.Cloud.Diagnostics.AspNetCore.GoogleExceptionLogger.Log uses IHttpContextAccessor to get the current HttpContext which is the correct way to do this in Asp.Net Core.

@iantalarico @jskeet let me know your thougths on this.

[jskeet]

Hmm. When you say "self-hosted" - what does this, and is there some way we can make the HTTP context available? I don't know much about this area.

[iantalarico]

@jskeet Self hosted is basically just running a mini stripped down webserver as a console app. I don't think this is a common occurrence but I may be wrong.

@amanda-tarafa Good catch Amanda!

I think the use case for this is relatively small (and we haven't seen any issues around this). Ideally doing both of the two mentioned solutions would be good.

Being that I think this is a corner case and the usage of the ASP.NET library is very small I'd vote to do the first one (as it seems easy). If the second one isn't too much work we do that too.

What do you think?

[amanda-tarafa]

Self-hosted means hosted outside of IIS, for .Net this is done using OWIN: You can see here:

Open Web Interface for .NET (OWIN) defines an abstraction between .NET web servers and web applications. OWIN decouples the web application from the server, which makes OWIN ideal for self-hosting a web application in your own process, outside of IIS.

More info on owin.org and AspNetKatana (MS implementation of OWIN)

There's lot of info if you search "HttpContext.Current self hosted Wep Api" or similar. Most reliable answer here (although with not a lot of detail). Also I've found a lot of "workarounds" for accessing info contained within HttpContext.Current. Some of those are OWIN specific, but we cannot use those because we don't know whether the applications using error logging will be OWIN or not, although we could provide (possibly, not sure, and they might even work on IIS) some OWIN Middlewares for clients to use if they are using OWIN. This I'd need to look into further. For accessing info contained on HttpContext.Current.Request or HttpContext.Current.Response (which is what we need) most "workarounds" rely on ApiController.ActionContext.Request and ApiController.ActionContext.Response but ApiController.ActionContext is only accessible from within the controller, there's no static Current property that we can use. That's why we would need something like the solution on my second bullet above. I'm also not very familiar with differences between the IIS/ASP.Net and OWIN pipelines so there might be things I'm missing. But from what I've been reading and looking at in the last few hours, we shouldn't be using HttpContext.Current in a library that might be used from a self hosted ASP.Net Web Api application.

[amanda-tarafa]

@iantalarico I think we can do both with little effort. I'll submit a PR with the first solution first, since that at least stops it from failing, and then another one with the second solution.

[amanda-tarafa]

Downgraded to P2 via #2296, this doesn't fail now, only there's no way for clients to log http context information alongside error information for Web Api self hosted Asp.Net apps.

[amanda-tarafa]

Closing via #2304