Throw ArgumentNullException when context is null

[stijnherreman]

It took me a while to figure out why context-less logging wasn't working with the ASP.NET Core extension methods. The ArgumentNullException points users at the error, instead of having the code enter catch { ... } block.

[NickCraver]

Finally catching up tonight - I really don't want to throw here. I believe this isn't the correct behavior.

If we think about the most likely case for what's happening, it's probably a background thread. Instead of logging errors for a user, suddenly we've gone from being silent (but not breaking) to crashing the application. That's pretty bad behavior for an error handler. Right now we should be logging that context was null (and alerting the user to such) in the catch block. There's an event there to listen to if a user wants to.

[stijnherreman]

In the case of a background thread, is there a scenario possible where it only sometimes passes null instead of failing 100% of the time? In that case I can see the current implementation being desired behaviour.

But if consuming code either always passes a non-null context or always passes null, then I don't see a problem with crashing since it would be detected during development, assuming the developer runs their code at least once.

[NickCraver]

@stijnherreman in short: this would take Stack Overflow offline. Cases where you expect to have an HttpContext but can also be on a background thread in some cases absolutely happens. Lots of cache, async (e.g. .ConfigureAwait(false)) and parallel threading cases can cause this. Overall, this isn’t a behavior change that I can take. An exception handler should never be the cause on an exception. We aren’t doing our job if we are.

[stijnherreman]

OK, then I agree that the change is incorrect. Either way, thanks for reviewing it.