Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Header Propagation Cannot Be Used in HybridCache #5568

Open
1 task done
chrisoverzero opened this issue Oct 23, 2024 · 4 comments
Open
1 task done

Header Propagation Cannot Be Used in HybridCache #5568

chrisoverzero opened this issue Oct 23, 2024 · 4 comments
Assignees

Comments

@chrisoverzero
Copy link

Is there an existing issue for this?

  • 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
Copy link
Member

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
Copy link
Member

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
Copy link
Author

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
Copy link
Member

The GetOrCreateAsync is fine and expected.

@BrennanConroy BrennanConroy transferred this issue from dotnet/aspnetcore Oct 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants