Allow async method for HeaderPropagationOptions.ValueFilter

[wu-yafeng]

Background and Motivation

Consider following code:

builder.Services.AddHttpClient()
    .AddHeaderPropagation(options =>
    {
        options.Headers.Add("Authorization", context =>
        {
            // await here
            var access_token = context.HttpContext.GetTokenAsync("access_token").GetAwaiter().GetResult();

            return access_token ?? StringValues.Empty;
        });
    });

Proposed API

Add property AsyncValueFilter for HeaderPropagationEntry.

namespace Microsoft.AspNetCore.HeaderPropagation;

public class HeaderPropagationEntry
{
+    public Func<HeaderPropagationContext, Task<StringValues>>? AsyncValueFilter { get; }
}

public sealed class HeaderPropagationEntryCollection : Collection<HeaderPropagationEntry>
{
+   public void Add(string headerName, Func<HeaderPropagationContext, Task<StringValues>> valueFilter);
}

Usage Examples

builder.Services.AddHttpClient()
    .AddHeaderPropagation(options =>
    {
        options.Headers.Add("Authorization", async context =>
        {
            // await here
            var access_token = await context.HttpContext.GetTokenAsync("access_token");

            return access_token ?? StringValues.Empty;
        });
    });

[gfoidl]

Should it be a ValueTask instead the Task?
Or do we expect that it's always truly asynchronous?

[wu-yafeng]

Should it be a ValueTask instead the Task? Or do we expect that it's always truly asynchronous?

Most of AspNetCore API such as JwtBearerEvents.OnMessageReceived returns Task. Do these APIs need to maintain design consistency?

[PaulusParssinen]

async2 may also effect considerations wrt. Task vs. ValueTask here?

[gfoidl]

Most of AspNetCore API such as JwtBearerEvents.OnMessageReceived returns Task.

At least that API was there long before ValueTask came into play, so it couldn't be changed w/o breaking something.
I'd prefer perf-betterness over design consistency here, especially it's only Task vs. ValueTask, the rest of the design remains consistent.
If in your example GetTokenAsync can return synchronous, then the cost for the Task allocation could be avoided.

---

For "async" / "runtime async": at the moment one shouldn't bother and speculate about the impact of async2 (see also dotnet/runtime#118334). async2 is and will be modeled around the current usage patterns anyway, and may open new possibilities (in the future). But for now Task vs. VT should be based on the same as outlined in Should every new asynchronous API return ValueTask / ValueTask?.

[wu-yafeng]

Most scenario returns synchronous. ValueTask is much better.