HttpContextAccessor.HttpContext is null when using TestServer and .net 2.2.2 #7975
Comments
|
I'm having similar issues, though forcing |
|
@brandonpollett thank you for isolating the repro. I'm also hitting this issue. |
|
@skolima Can you tell me what you mean by forcing the test project to a version of |
|
@parekhkb exactly this - that did solve the problem in my case. |
|
I think we run into the same problem in our project. This is our code: In our in TestProject we are using: and in controller project: Before we were using: and |
|
I have exactly the same issue -> integration tests are broken because of null at HttpContextAccessor.HttpContext. |
|
Looks like there is indeed a regression here. The problem lies in the nested HTTP calls. When you make a call back to the application using the HttpClient produced by This behavior does not appear to occur in 2.2.0. After a brief binary search it appears to have been introduces in 2.2.2. I do see a change to HttpContextAccessor in 2.2.2 (#6036 by @JunTaoLuo), which definitely seems suspiciously relevant :). I think something in that PR regressed this TestServer scenario where there are nested HTTP calls. Repro details below. cc @Tratcher @davidfowl @JunTaoLuo The following program repros the issue class Program
{
public static async Task Main(string[] args)
{
HttpClient client = null;
var builder = WebHost.CreateDefaultBuilder()
.Configure(app =>
{
app.Use(async (context, next) =>
{
var accessor = context.RequestServices.GetRequiredService<IHttpContextAccessor>();
if (context.Request.Path.StartsWithSegments("/inner"))
{
if (accessor.HttpContext == null)
{
throw new System.Exception("Invalid During Nested Call!");
}
}
else if (context.Request.Path.StartsWithSegments("/outer"))
{
var nestedResp = await client.GetAsync("/inner");
if (accessor.HttpContext == null)
{
Console.WriteLine("HttpContextAccessor NULL after nested!");
}
else
{
Console.WriteLine("HttpContextAccessor NON-NULL after nested!");
}
}
else
{
await next();
}
});
})
.ConfigureServices(services =>
{
services.AddHttpContextAccessor();
});
Console.WriteLine($"Runtime Path: {typeof(string).Assembly.Location}");
Console.WriteLine($"ASP.NET Path: {typeof(HttpContext).Assembly.Location}");
var server = new TestServer(builder);
client = server.CreateClient();
var resp = await client.GetAsync("/outer");
resp.EnsureSuccessStatusCode();
}
}If I build this and then run it against 2.2.0, I get: If I run against 2.2.2, I get: |
|
Nested HttpClient's are one of the least realistic things that you can do with TestServer. You end up with two HttpContextAccessor async local's on the same thread. You'd never get that with a real server. I don't think we should try to change HttpContextAccessor to address this. We may be able to work around it in TestServer. At some point in this flow you need to split the execution context so the nested call can run in isolation. |
Yeah, I came to a similar conclusion. I think we need to abandon the ExecutionContext when we "send" the request in |
|
Perhaps as simple as this change? master...anurse/spike-suppress-flow (just for illustrative purposes) |
|
It would break anyone trying to intentionally hold an |
|
Can you apply that around the recursive call in your repro above? |
|
I can do one better and get a full-on workaround by adding an HttpMessageHandler that abandons the ExecutionContext for you. Then you just have to wrap the ClientHandler up in that and it seems to work: My main concern is the unintended consequences of abandoning the EC here. Perhaps this would live best as a known-issue workaround rather than a fix. |
|
Just use |
💯 agree. Just want to make sure we have an escape hatch because it will break some code (arguably wrong code ;)). An option to "not abandon the EC" seems reasonable to me.
Returning a value from Ok, I'll throw this in to preview 6 as low pri. We'll see what we can pull out here. This seems like another area I might be able to prove I can still write code ;P. @brandonpollett if you want a workaround for now, see my comment above. |
|
Setting up a recursive HttpClient call is so manual and rare that I think the workaround is adequate, there's no need to change the default experience. At most we'd provide an opt-in solution, not an opt-out one. |
I don't agree with that, ideally the |
|
TestServer walks a fine line between emulating real server behavior and providing useful test functionality. E.g. It doesn't covert application exceptions to 500's like a real server because it can flow the exception directly to the caller. Note this client/server pairing doesn't exist in real servers, and there's no first class way to set up this recursive scenario in the test server. Before we start suppressing the execution context flow for this specialized scenario we need to make sure we're not compromising more useful, mainstream scenarios. What else would we loose? |
The thing we lose is fairly simple, it's the flow of ExecutionContext between the "client" and the "server". That means
Of course there is. We literally give you the ability to create A user trying to flow To me, disabling EC flow by default seems like the best option here and in 3.0 it can be relatively easily justified as a potential breaking change. We can annoucement it, do it in an early preview and provide an opt-out switch. |
This workaround unblocked us. Our scenario is this: We have multiple microservices. We use JWT Bearer auth and Open ID Connect using our own IdentityServer4 IdP. In order for our tests to work we need to set up two TestServers: the microservice we are testing and the IdP. Then we delegate the JWT verification from our microservice to our IdP. public class ApiServiceFactory : WebApplicationFactory<Api.Startup>
{
private WebApplicationFactory<Identity.Startup> _identityServerFactory;
protected override void ConfigureWebHost(IWebHostBuilder builder)
{
var backChannelHandler = _identityServerFactory.CreateHandler();
builder.ConfigureServices(
s => s.AddSingleton<IPostConfigureOptions<IdentityServerAuthenticationOptions>>(
new PostConfigureOptions<IdentityServerAuthenticationOptions>(
"AuthScheme",
opt => { opt.JwtBackChannelHandler = new SuppressExecutionContextHandler(backChannelHandler); })));
}
}This regression is unfortunate, but glad we're unblocked now. I'm not fully understanding the reasoning why this is an edge case. It seems like a common case for anyone testing with authentication. Thanks for the workaround @anurse . |
|
Thanks @parekhkb, that's a more concrete example and it's not even recursive, it's multi-tier. It makes sense we'd need to prevent cross contamination between the server instances. |
|
Awesome, we'll schedule this fix for 3.0 |
|
I found the first use of trying to preserve an async local across the client server boundary (https://github.com/aspnet/AspNetCore/blob/f5ff181222d6f20f07e6b3d707dc24cb95966cc2/src/Mvc/test/Mvc.FunctionalTests/JsonOutputFormatterTestBase.cs#L160-L173). MVC is currently doing it to verify the Activity gets written to the response (I know because just broke all of these tests with a PR 😄) |
|
Lol :). Well I think we'd still provide an option to "enable" flowing the local. It would just restrict your ability to use "nested" calls back through the test server. |
|
I have the same symptom, I'm attempting to write integration tests using the TestServer. My service uses authentication and to fake the authentication, I add the following to my TestServer host builder configuration: When I attempt to use HttpContextAccessor.HttpContext it returns null. Not sure how to workaround this problem, other than to replace the services that call it with test doubles (reducing the value of doing integration tests). Any suggestions? |
|
The workaround provided by @anurse is here #7975 (comment) |
|
I'll look at making this change and adding a setting to turn it back off. Technically a breaking change which will need some announcement. |
analogrelay
added
the
enhancement
fixes #7975 There is a 'PreserveExecutionContext' property to turn the old behavior back on. Also I had to modify where IHttpApplication.CreateContext is called since that's what sets the IHttpContextAccessor, which depends on AsyncLocals!
fixes #7975 There is a 'PreserveExecutionContext' property to turn the old behavior back on. Also I had to modify where IHttpApplication.CreateContext is called since that's what sets the IHttpContextAccessor, which depends on AsyncLocals!
fixes #7975 There is a 'PreserveExecutionContext' property to turn the old behavior back on. Also I had to modify where IHttpApplication.CreateContext is called since that's what sets the IHttpContextAccessor, which depends on AsyncLocals!
|
I have same issue here #6080 (comment) |
|
After a lot of hours, I found a workaround for my case. That (#7975 (comment)) other doesn't work because in |
ghost
locked as resolved and limited conversation to collaborators
amcasey
added
area-networking
Describe the bug
When using .net core 2.2.2 along with the
TestServer(https://github.com/aspnet/AspNetCore/blob/master/src/Hosting/TestHost/src/TestServer.cs),HttpContextAccessor.HttpContextwill return null in certain situations. We're experiencing this is the repro: https://github.com/Microsoft/fhir-server/If you have 2.2.1 installed, this behavior is not exhibited. It appears related to the use of authentication and calls that are made on the JWT backchannel handler.
To Reproduce
Steps to reproduce the behavior:
Expected behavior
The tests should pass while using 2.2.2 and 2.2.1
Additional context
When the unit tests pass the output of
dotnet --infois:When failing the output of
dotnet--info is:The text was updated successfully, but these errors were encountered: