[API Proposal]: Expose overloads of HttpContent.LoadIntoBufferAsync taking a CancellationToken

[andrewhickman-aveva]

Background and motivation

HttpContent.LoadIntoBufferAsync and HttpContent.LoadIntoBufferAsync(long) currently forward to overloads which take a cancellation token, passing CancellationToken.None.

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

Lines 476 to 477 in 5535e31

	public Task LoadIntoBufferAsync(long maxBufferSize) =>
	LoadIntoBufferAsync(maxBufferSize, CancellationToken.None);

HttpClient also uses this API but passes a cancellation token:

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

Line 487 in 5535e31

response.Content.LoadIntoBuffer(_maxResponseContentBufferSize, cts.Token);

It would be useful for external consumers to be able to pass a cancellation token, similar to HttpContent.CopyToAsync(Stream, CancellationToken

API Proposal

 namespace System.Net.Http;
 
 public class HttpContent
 {
     public Task LoadIntoBufferAsync();
     public Task LoadIntoBufferAsync(long maxBufferSize);
+    public Task LoadIntoBufferAsync(CancellationToken cancellationToken);
+    public Task LoadIntoBufferAsync(long maxBufferSize, CancellationToken cancellationToken);
 }

API Usage

using var client = new HttpClient();
using var response = await client.GetAsync("https://example.com", HttpCompletionOption.ResponseHeadersRead);

using (var timeoutCts = new CancellationTokenSource(TimeSpan.FromSeconds(30))
{
    var maxBufferSize = 134217728L;
    await response.LoadIntoBufferAsync(maxBufferSize, timeoutCts.Token);
}

Alternative Designs

If the HttpContent has been created by HttpClient, it could internally track cancellation based on the token passed to SendAsync. However I don't think this is consistent with existing HttpContent methods like CopyToAsync.

Risks

None

[ManickaP]

@andrewhickman-aveva what is your motivation for filing this API proposal? Do you have any concrete use-case in mind? Does the current state limits you / your project? How impactfull it is?

I don't have anything against this API, I'm just trying to understand the need for this and assign priority accordingly.

[andrewhickman-aveva]

@andrewhickman-aveva what is your motivation for filing this API proposal? Do you have any concrete use-case in mind? Does the current state limits you / your project? How impactfull it is?

I don't have anything against this API, I'm just trying to understand the need for this and assign priority accordingly.

This is motivated by a memory dump we received where a particular task was stuck calling this method, when it should have timed out.

The reason we're using LoadIntoBufferAsync in the first place is to enable better retry behaviour. We're using PolicyHttpMessageHandler for retries, timeouts and circuit breaking behaviour. However the call to LoadIntoBufferAsync inside HttpClient runs outside the handlers and so wouldn't be retried. To work around this we defined a custom handler which runs within the scope of the retry policy and calls LoadIntoBufferAsync manually.

[ManickaP]

Triage: there were no concerns from our team about it. No reason not to expose this overloads. Putting in 9.0 time frame for the moment. I'll try to push it through API review. But as we don't have anyone else needing this, it might get push out 9.0.

[bartonjs]

Video

• We asked if they should use a more complicated pattern of showing them as optional parameters and doing EditorBrowsable-Never on the existing overloads, and decided that pure overloads was more locally consistent.

 namespace System.Net.Http;
 
 public partial class HttpContent
 {
     public Task LoadIntoBufferAsync(CancellationToken cancellationToken);
     public Task LoadIntoBufferAsync(long maxBufferSize, CancellationToken cancellationToken);
 }