Header Propagation Cannot Be Used in `HybridCache`

[chrisoverzero]

Is there an existing issue for this?

• [x] I have searched the existing issues

Describe the bug

1. Add a service depending on HttpClient to the DI container via AddHttpClient from "Microsoft.Extensions.Http". ("inner service")
2. Add a service depending on inner service and HybridCache to the DI container. ("outer service")
3. Call a method on inner service from outer service as the factory argument of GetOrCreateAsync on HybridCache.
4. Configure a header to be propagated from the incoming request to HttpClient. (Say, "etag".)

When the HttpClient-depending method is called from the HybridCache, an InvalidOperationException is thrown, saying that the headers collection has not been initialized. When the call is moved out of GetOrCreateAsync, it succeeds.

Expected Behavior

• Header propagation works within or without HybridCache.
• Requests are able to be attempted on cache misses.

Steps To Reproduce

• Run the project from this repo. Ideally, with a debugger.
• Make a GET request with an ETag: say, curl --include --url 'http://localhost:8080/widgets/1' --header "etag: W/'1'".
• You'll get an exception (because the remote URL is invalid), but note that the request was attempted.
• Comment out the direct call to _innerClient.GetWidgetAsync in CachingWidgetClient.
• Repeat the request.
• Note the exception. (Matching the one below, I hope!)

Exceptions (if any)

An exception of type 'System.InvalidOperationException' occurred in System.Private.CoreLib.dll but was not handled in user code: 'The HeaderPropagationValues.Headers property has not been initialized. Register the header propagation middleware by adding 'app.UseHeaderPropagation()' in the 'Configure(...)' method. Header propagation can only be used within the context of an HTTP request.'

.NET Version

9.0.100-rc.1.24452.12, 9.0.100-rc.2.24474.11

Anything else?

No response

[mgravell]

Firstly: we should probably move this to dotnet/extensions (where HybridCache resides now); not sure I have access to do that, but...

I'll look at the repro tomorrow; one part of me is thinking "but what when the ambient state differs between callers", but: we already allow that foot-shotgun in the regular delegate scenario, so it s perhaps moot to worry about that specifically here. When I see the repro I might have more context; I'm guessing it is the fact that we don't have the ambient http-request precisely because we fork the onward call and await it (as part of the stampede protection).

[mgravell]

Random thought: one option I was discussing the other day was a new flag to disable the stampede protection. If we did that: we would not be context switching - the primary execution flow could involve the callback. Would losing stampede protection be an acceptable trade for ambient http-request access?

[chrisoverzero]

we should probably move this to dotnet/extensions

Oops, sorry, I didn't realize the library had moved.

we already allow that foot-shotgun in the regular delegate scenario

I'm afraid I'll be no help there – I don't know what that means. 😅 Is GetOrCreateAsync not the expected entry point?

Would losing stampede protection be an acceptable trade for ambient http-request access?

For me, for the project in which I discovered this: yes.

[mgravell]

The GetOrCreateAsync is fine and expected.

[KalleOlaviNiemitalo]

HeaderPropagationValues uses AsyncLocal<T> here: https://github.com/dotnet/aspnetcore/blob/af22effae4069a5dfb9b0735859de48820104f5b/src/Middleware/HeaderPropagation/src/HeaderPropagationValues.cs#L13

As described in https://github.com/dotnet/extensions/issues/5648#issuecomment-2549477439, the stampede protection in HybridCache does not propagate the ExecutionContext to the thread pool work item, so the factory delegate cannot read any AsyncLocal<T>.Value set by the caller. Until HybridCache is fixed, I hope that the workaround I posted there helps with header propagation too.

[mgravell]

IMO the simplest solution here is to access the headers outside GetOrCreateAsync; because of stampede combination, without that we get into a complex "which request?" problem - although if it matters, then the value (or at least a token with the same granularity, such is user-id), should also probably be part of the key. We'll also look at making a stampede pass-thru here, which will remove the execution-context transition (i.e. "we don't switch thread/async, and async-local keeps working")

[KalleOlaviNiemitalo]

To access the headers outside GetOrCreateAsync, would the application have to read IDictionary<string, StringValues>? Headers from HeaderPropagationValues before the GetOrCreateAsync call and make the factory assign the dictionary back to HeaderPropagationValues.Headers? And perhaps assign null to HeaderPropagationValues.Headers at the end.

[TwentyFourMinutes]

I am trying to comprehend why one might be loosing stampede protection when the execution context is propagated to the factory method.

I do understand that you have no way of knowing/no guarantee which Execution Context is used within the factory method, when multiple requests occur simultaneously (and usually you do not really care explicitly -> just implicitly in this case), just like with the ConcurrentDictionary#GetOrAdd. But how/why does it differ with the DefaultHybridCache implementation?

Per se I have nothing against the idea of running the factory in a somewhat "encapsulated" manner, however with the aforementioned ConcurrentDictionary implementation in contrast, it feels like an extremely easy pit fall. Especially, when there is no remarks comment on the GetOrCreateAsync method. At least for me this is not just an implementation detail, but something that a developer should be aware of.

Currently, I am encountering a similar issue as the OP mentioned, just with ambient transactions and DI scopes. I have no strong opinion on GetOrCreateAsync propagating the Execution Context. For me its just the matter of making the correct decision on how I am solving the problem and what drawbacks I each solution has, while understanding what I am doing.

I'd be very glad if someone could go into detail! 😄

[mgravell]

(if someone could unassign me, that'd be awesome, thx)