NickCraver/StackExchange.Exceptional

Exception not logged if HttpContext is not passed as parameter

florinciubotariu opened this issue · 8 comments

Hello. I've installed Exceptional into an ASP.NET Core 2.2 application and I have some issues in some parts of the application when I want to log exceptions.

In the documentation of the Log method, it is stated that for non web requests, we should pass null for HttpContext. Also, it is stated that it should not be "forgotten for most web application usages, so it's not an optional parameter."

public static Error Log(
this Exception ex,
HttpContext context,
string category = null,
bool rollupPerServer = false,
Dictionary<string, string> customData = null,
string applicationName = null)
{
if (Statics.IsLoggingEnabled)
{
ExceptionalSettings settings = null;
try
{
settings = context.RequestServices.GetRequiredService<IOptions<ExceptionalSettings>>().Value;
// If we should be ignoring this exception, skip it entirely.
// Otherwise create the error itself, populating CustomData with what was passed-in.
var error = ex.GetErrorIfNotIgnored(settings, category, applicationName, rollupPerServer, customData);
if (error != null)
{
// Get everything from the HttpContext
error.SetProperties(context);
if (error.LogToStore())
{
return error;
}
}
}
catch (Exception e)
{
settings?.OnLogFailure?.Invoke(e);
Trace.WriteLine(e);
}
}
return null;
}

Looking at the code, I see that the HttpContext in mandatory for two operations:

  1. Get the Exceptional settings.
  2. Transfer data from HttpContext to Error.

The problem is that if you do not supply the HttpContext as parameter, a NullReferenceException is thrown because RequestServices is being accessed.

Is it possible to have the exception logged even if the HttpContext is not supplied? I'm thinking of:

  1. If you do not supply the context, then simply do not transfer data from it to Error.
  2. Get the Exceptional settings that were passed in the DI initializer (ExceptionalServiceCollection.AddExceptional)

I'm asking this because there are some special cases when you are in an ASP.NET Core application but you don't have access to the HttpContext (in my case, Custom JSON converter and a long running WebSocket connection)

I personally use ex.LogNoContext() - you should be able to use that if you also see ex.Log()

An excerpt from some of my (prod) code:

if (context != null)
{
    exception.Log(context, _categoryName, customData: customData);
}
else
{
    exception.LogNoContext(_categoryName, customData: customData);
}

However, I also remember reading that documentation about passing null to Log - @NickCraver is this still correct or should people now use the LogNoContext method?

Thank you @MattJeanes . Works without problems!

I did not see the LogNoContext method in the documentation from https://nickcraver.com/StackExchange.Exceptional but it seems some useful information can be found here.

Great to hear! I guess we'll wait and see what Nick has to say about how this should work

If you see the set properties method below, we intended to handle null context and log all the information available...that's a regression for sure. I'm fixing it up and adding tests right now.

Fixed in c36d68a - it should be up on MyGet to test shortly :) I'll update NuGet after a good Stack Overflow test run this week (updating several libraries there).

If this doesn't resolve you issue, please ping and I'll reopen!

That's what I figured. So when should we use LogNoContext vs Log with a null HttpContext?

There are 2 main cases where you'd use LogNoContext:

  1. When you know you don't have any ever (just for efficiency).
  2. When you're not even in a web project and don't want to reference anything with HttpContext.

For example, a user can use Exceptional.Shared in a console app with no knowledge or methods for HttpContext at all. .LogNoContext exists in a lower-level shared package and is designed for users not doing web at all. But it's generally usable whenever you don't want to reference or have a HttpContext for whatever reason (e.g. background thread).

Ah yes, they're in different packages. If I recall the LogNoContext was added because of console apps and to avoid the mess around the fact that HttpContext differed between AspNetCore and AspNet and depending on if you used the main StackExchange.Exceptional package or StackExchange.Exceptional.AspNetCore you'd get a Log extension method with the 2 variants of HttpContext