We have moved to GitHub Issues
Created by Shannon Deminick 06 Mar 2017, 16:38:22 Updated by Shannon Deminick 28 Mar 2017, 02:11:46
Subtask of: U4-9609
Sometimes when you run the health check you might get an error which is because of this code:
var useSsl = GlobalSettings.UseSSL || HealthCheckContext.HttpContext.Request.ServerVariables["SERVER_PORT"] == "443";
we need to first check if the ServerVariables collection contains that key - sometimes it doesn't.
Can repro an error but not sure it's linked to ServerVariables not containing the variable - ServerVariables is a NameValueCollection which should returns null if the key is not found - but the error comes when trying to read server variables in IIS.
Also in ExcessiveHeadersCheck when getting server variables.
In addition seeing some weird things with HealthCheckResolver - each health check runs its own request, but the resolver instanciate all checks, just to return one of them each time = sub-optimal, perfs-wise.
Ouch... we seem to have an ugly issue in ManyObjectsResolverBase with HttpContext.Current being captured in a weird way ;(
Root cause of the issue was that ManyObjectsResolverBase somehow captured the current HttpContext ''at the time the resolver was created'' and then many weird things could happen when the resolver's scope was ObjectLifetimeScope.HttpRequest, including reusing the same objects in many requests (thus leading to, I believe, the weird ServerVariables errors). Just set a breakpoint in HealthCheckResolver and marvel at the fact that the resolvers, which are meant to be per-request, are actually only created once.
The subtle thing is that some health checks trigger an application restart (trying to write files in App_Code or bin) and thus, various HttpContext would be captured during restarts, leading to a very confusing picture (ie it would not seem to consistently fail).
So... changing the title of this issue to reflect the real issue.
Fixes the ManyObjectResolverBase - review: now, a new set of health checks should be created each time the health checks run (can see eg with a breakpoint) and there should not be ServerVariables errors anymore.
Assiging to @Shandem for review as it touches Core's resolvers.
@zpqrtbnk makes sense, good find! I've added a comment in the PR, we just need to put a null check back in. Once you do that you can merge in the PR and close the issue.
Updated, back in review & assigned to you.
Backwards Compatible: True
Due in version: 7.5.12
Sprint: Sprint 55
Story Points: 1