HttpClient.Send (synchronous API) is not intercepted

[Timovzl]

Describe the bug

The package correctly intercepts HttpClient.SendAsync() (and all the extension methods that invoke it, such as GetAsync() or PostAsync()), adding the Authorization header and such.

However, the synchronous API is not properly intercepted. HttpClient.Send() does not cause the necessary headers to be added.

Presumably, this is because this API was added in .NET 5. A new target would be needed.

To Reproduce

In .NET 5, create an HttpClient configured with this package's message handler, which normally signs any request made. Construct an HttpRequestMessage and send it using HttpClient.Send(). After sending, confirm the absence of headers on the HttpRequestMessage. Compare this to HttpClient.SendAsync(), which causes the headers to be added as expected.

Expected behavior

On send, the necessary headers are expected to be added to the HttpRequestMessage, such as the Authorization header.

Environment

.NET 5.

Additional context

It would probably make sense to include .NET 5 as an additional target platform, so that the synchrous API can be intercepted overridden.

There are significant use cases for the HttpClient's synchronous API. The original proposal contains extensive explanations and discussions about it. It was finally added in .NET 5.

Stephen Toub explains:

The key is [sync-over-async] ends up requiring many more threads in order to sustain throughput. And it can take a long time for the thread pool to ramp up to the necessary level, while in the interim the system can essentially be deadlocked. That means either the app-level dev needs to explicitly set a high min thread count to force the ramp up, or we need to make the pool way more aggressive at thread injection, which harms other cases.

My own experience confirms that thread pool exhaustion happens extremely quickly with sync-over-async - much more so than one might expect. This limits the number of simultaneous operations to a fraction of what is achievable with a purely synchronous API.

In a .NET 5 preview blog, Stephen Toub further summarizes the addition of a synchronous API:

While HttpClient was designed for asynchronous usage, we have found situations where developers are unable to utilize asynchrony, such as when implementing an interface method that’s only synchronous, or being called from a native operation that requires a response synchronously, yet the need to download data is ubiquitous. In these cases, forcing the developer to perform “sync over async” (meaning performing an asynchronous operation and then blocking waiting for it to complete) performs and scales worse than if a synchronous operation were used in the first place. As such, .NET 5 sees limited new synchronous surface area added to HttpClient and its supporting types. dotnet/runtime does itself have use for this in a few places. For example, on Linux when the X509Certificates support needs to download a certificate as part of chain building, it is generally on a code path that needs to be synchronous all the way back to an OpenSSL callback; previously this would use HttpClient.GetByteArrayAsync and then block waiting for it to complete, but that was shown to cause noticeable scalability problems for some users…

[github-actions]

Hi there and welcome to this repository!

A maintainer will be with you shortly, but first and foremost I would like to thank you for taking the time to report this issue. Quality is of the highest priority for us, and we would never release anything with known defects. We aim to do our best but unfortunately you are here because you encountered something we didn't expect. Lets see if we can figure out what went wrong and provide a remedy for it.

[FantasticFiasco]

Hi @Timovzl, I'll look into the issue tonight.

[Timovzl]

@FantasticFiasco I have created a PR to help speed things along. I did not have time to change the commit message to adhere to your guidelines 100%, so feel free to use the PR as you see fit. If you prefer to reject it and use it for inspiration only, perfect. I hope it's of help.

[FantasticFiasco]

Release v2.1.1 is now live, thank you so much for your contribution!

[Timovzl]

My pleasure. I'm glad you are providing this library, as AWS signing turned out to be much more complex than anticipated.

I just realized that with this addition, the package provides something unique: Truly synchronous access to AWS resources over HTTPS, without resorting to sync-over-async.

Unique because, for example, all available ElasticSearch client packages use HttpClient, but none of them have accounted for its (relatively new) synchronous APIs. As such, using such a package synchronously requires sync-over-async, which does not scale at all. That leaves manual use of the HttpClient as the only way to achieve proper synchronous operation. That leaves you with the problem of handling AWS signing. This package resolves the problem. :)

[FantasticFiasco]

If you're happy, I'm happy 😀

It was after all a feature you yourself implemented, which to many open source project is rare and extremely valuable!

I might update README.md to mention the support for synchronous requests, just to highlight it.