[API Proposal]: Accept HttpCompletionOptions to more places

[huoyaoyuan]

Background and motivation

Currently, HttpClient uses HttpCompletionOption.ResponseContentRead as the default option:

runtime/src/libraries/System.Net.Http/src/System/Net/Http/HttpClient.cs

Line 22 in 0d20f9a

private const HttpCompletionOption DefaultCompletionOption = HttpCompletionOption.ResponseContentRead;

This can simplify some logic of consumers, for example cancellation and duplex buffering can be handled at once.

When ResponseHeadersRead is desired, for example passing the content stream to some parser or image control that supports incremental loading, there are only a limited members available. Importantly, the most-convenient GetStreamAsync method doesn't accept HttpCompletionOption. The streaming json helpers are explicitly specifying ResponseHeadersRead:

runtime/src/libraries/System.Net.Http.Json/src/System/Net/Http/Json/HttpClientJsonExtensions.Get.cs

Lines 18 to 19 in 0d20f9a

	private static readonly Func<HttpClient, Uri?, CancellationToken, Task<HttpResponseMessage>> s_getAsync =
	static (client, uri, cancellation) => client.GetAsync(uri, HttpCompletionOption.ResponseHeadersRead, cancellation);

API Proposal

namespace System.Net.Http;

public class HttpClient
{
    public Task<Stream> GetStreamAsync([StringSyntax("Uri")] string? requestUri);
    public Task<Stream> GetStreamAsync([StringSyntax("Uri")] string? requestUri, CancellationToken cancellationToken);
+    public Task<Stream> GetStreamAsync([StringSyntax("Uri")] string? requestUri, HttpCompletionOption completionOption);
+    public Task<Stream> GetStreamAsync([StringSyntax("Uri")] string? requestUri, HttpCompletionOption completionOption, CancellationToken cancellationToken);
    public Task<Stream> GetStreamAsync(Uri? requestUri);
    public Task<Stream> GetStreamAsync(Uri? requestUri, CancellationToken cancellationToken);
+    public Task<Stream> GetStreamAsync(Uri? requestUri, HttpCompletionOption completionOption);
+    public Task<Stream> GetStreamAsync(Uri? requestUri, HttpCompletionOption completionOption, CancellationToken cancellationToken);

+    public HttpCompletionOptions DefaultCompletionOptions { get; set; }
}

API Usage

using var httpClient = httpClientFactory.GetClient();
var imaTeapot = httpClient.GetStreamAsync("https://http.cat/images/418.jpg", HttpCompletionOption.ResponseHeadersRead);
imageControl.SetSourceStream(imaTeapot);

Alternative Designs

Are the overloads redundant? Can they be combined with CancellationToken cancellationToken = default?

Risks

Allow specifying DefaultCompletionOptions may have unintended effects for callers having assumptions for ResponseContentRead. We can choose to only expose the completion options on individual methods. The CanSeek property for content stream is changed.

This also increases the risk of using ResponseHeadersRead inappropriately. Per testing, if the HttpClient is disposed before the stream is read, the response stream will be effectively "detached" and become empty. I didn't find explicit mentions about this at https://learn.microsoft.com/dotnet/fundamentals/runtime-libraries/system-net-http-httpclient#buffering-and-request-lifetime.

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

[MihaZupan]

Maybe there's a misunderstanding of the current behavior?
GetStreamAsync already implies ResponseHeadersRead. That's the only option it does right now. There'd be no need to do

httpClient.GetStreamAsync("...", HttpCompletionOption.ResponseHeadersRead);
// instead of just
httpClient.GetStreamAsync("...");

or are you asking for an option to get the response stream that has already been buffered and is seekable?

public HttpCompletionOptions DefaultCompletionOptions { get; set; }
Allow specifying DefaultCompletionOptions may have unintended effects for callers having assumptions for ResponseContentRead. We can choose to only expose the completion options on individual methods.

Yes, this has the potential to break a lot of other places where HttpClient is used, in unexpected ways.
I don't think we should add it. Using ResponseHeadersRead comes with other considerations for the caller - see warning banner in docs. IMO using this should be an explicit choice by the caller and not implied by the client instance.

The option for example also impacts whether you can re-read the content multiple times.
E.g.

using var response = await client.GetAsync(...);
await DoSomethingAsync(await response.Content.ReadAsStreamAsync());
return response.Content.ReadAsStringAsync(); // Will this work?

[huoyaoyuan]

GetStreamAsync already implies ResponseHeadersRead.

Hmm, I chose to GetAsync with ResponseHeadersRead instead of GetStreamAsync years ago. Maybe I was calling GetAsync with Content.ReadAsStreamAsync instead of GetStreamAsync then? I didn't remember the exact case.

or are you asking for an option to get the response stream that has already been buffered and is seekable?

Nope, I do want ResponseHeadersRead more.

The only concern is that the default of GetStreamAsync is not the same with GetAsync. Maybe adding documentation is sufficient.

[MihaZupan]

I see this in GetStreamAsync remarks:

This operation doesn't block. The returned Task<Stream> object will complete after the response headers are read. This method does not read nor buffer the response body.

Did you have something else in mind?

[huoyaoyuan]

Did you have something else in mind?

Not immediately. I'd check for more cases tomorrow, or close this.

[huoyaoyuan]

I can't find more cases about it. Since content buffering and HttpCompletionOption are HttpClient-only, there's no need to look at boarder surface area. Closing.