[API Proposal]: Event when a HttpResponseMessage is complete

[JamesNK]

Background and motivation

HttpClient and its middleware friends are based around passing the request message down through a nested HttpMessageHandler.SendAsync pipeline and returning the response message back.

The response being returned from SendAsync has historically been used to indicate that a request is complete. However, it only indicates the response header headers have been returned (apart from HttpClient and its option to read the response to completion before returning). This means that code that cares about the request being finished, such as counters, activities, and diagnostic source events, are reporting a HTTP request is finished even though the response body is still being read.

This becomes obvious in longer-running HTTP requests, such as gRPC streaming, where the response headers are returned almost immediately. Still, the response body periodically returns messages written by the server.

There should be an event that libraries and apps can subscribe to that indicates when the request is complete.

Potential users:

• Metrics built into HttpClient/HttpMessageInvoker can update counters based on when an HTTP request really ends. See [API Proposal]: HttpClient/HttpMessageInvoker metrics #84978
• Fix Activity lifetime. See HttpClient and Activity lifetime #37141
• Load balancing behavior based on the number of active requests has accurate information. See Support streaming calls in the SubchannelCallTracker  grpc/grpc-dotnet#2133

API Proposal

An event on HttpResponseMessage that reports when the overall request is considered complete.

namespace System.Net.Http;

public class HttpResponseMessage
{
    // Option 1: Cancellation token
    public CancellationToken Completed { get; }

    // Option 2: Register callback
    public void OnCompleted(Action<HttpResponseMessage> callback);
}

API options:

• CancellationToken might not be the right thing. I can't think of a scenario where someone would use it with an async method call that takes a token. I suggest it because it is similar to ASP.NET Core's HttpContext.RequestAborted token.
• HttpResponseMessage.OnCompleted(Action callback) may be preferable. It's like ASP.NET Core's HttpResponse.OnCompleted.

When these APIs are used, the registered callback is invoked if the response is complete.

The interesting discussion point here is: when is the overall request considered complete?

I think a request should be considered complete when both of these events are true (or the HttpResponseMessage is explicitly disposed):

1. The HttpResponseMessage.Content has been read to the end (or no content was returned at all),
2. And the HTTP request middleware pipeline is finished. That means the response has been returned from HttpClient.SendAsync or HttpMessageInvoker.SendAsync.

Edit: Potential problem with the idea above. The HttpResponseMessage returned for a request doesn't have to exit pipeline. A handler could decide to create a new one or retry the HTTP request which returns a new response. I added explicitly calling HttpResponseMessage.Dispose as another way to indicate the response is complete, but that doesn't seem like a guaranteed fix because someone might not dispose the original HttpResponseMessage.

API Usage

public void MyDelegatingHandler : DelegatingHandler
{
    public long CompletedRequests { get; private set; }

    public async Task<HttpResponseMessage> SendAsync(HttpRequestMessage request)
    {
        var response = await SendAsync(request);
        response.Completed.Register(() => CompletedRequests++);
        return response;
    }
}

Alternative Designs

No response

Risks

No response

[ghost]

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

Issue Details

---

Background and motivation

HttpClient and its middleware friends are based around passing the request message down through a nested HttpMessageHandler.SendAsync pipeline and returning the response message back.

The response being returned from SendAsync has historically been used to indicate that a request is complete. However, it only indicates the response header headers have been returned (apart from HttpClient and its option to read the response to completion before returning). This means that code that cares about the request being finished, such as counters, activities, and diagnostic source events, are reporting a HTTP request is finished even though the response body is still being read.

This becomes obvious in longer-running HTTP requests, such as gRPC streaming, where the response headers are returned almost immediately. Still, the response body periodically returns messages written by the server.

There should be an event that libraries and apps can subscribe to that indicates when the request is complete.

Potential users:

• Metrics built into HttpClient/HttpMessageInvoker can update counters based on when an HTTP request really ends. See [API Proposal]: HttpClient/HttpMessageInvoker metrics #84978
• Fix Activity lifetime. See HttpClient and Activity lifetime #37141
• Load balancing behavior based on the number of active requests has accurate information. See Support streaming calls in the SubchannelCallTracker  grpc/grpc-dotnet#2133

API Proposal

An event on HttpResponseMessage that reports when the overall request is considered complete.

namespace System.Net.Http;

public class HttpResponseMessage
{
    // Option 1: Cancellation token
    public CancellationToken Completed { get; }

    // Option 2: Register callback
    public OnCompleted(Action<HttpResponseMessage> callback);
}

If the response is already complete when these APIs are used, then the registered callback is immediately invoked.

The interesting discussion point here is: when is the overall request considered complete?

I think a request should be considered complete when both of these events are true:

1. The HttpResponseMessage.Content has been read to the end (or no content was returned at all),
2. And the HTTP request middleware pipeline is finished. That means the response has been returned from HttpClient.SendAsync or HttpMessageInvoker.SendAsync.

API Usage

public void MyDelegatingHandler : DelegatingHandler
{
    public long CompletedRequests { get; private set; }

    public async Task<HttpResponseMessage> SendAsync(HttpRequestMessage request)
    {
        var response = await SendAsync(request);
        response.Completed.Register(() => CompletedRequests++);
        return response;
    }
}

Alternative Designs

No response

Risks

No response

Author:	JamesNK
Assignees:	-
Labels:	api-suggestion, area-System.Net.Http
Milestone:	-

[JamesNK]

FYI @antonfirsov @noahfalk

[CarnaViire]

Just on a side note, there's another alternative approach used in System.Net.Quic -- a Task property: QuicStream.ReadsClosed and QuicStream.WritesClosed, and you can register continuations instead of subscribing to an event

[gfoidl]

Task has the downside that it's an allocatoin (IIRC 72 bytes). Should be ValueTask backed by IValueTaskSource (to avoid these allocations).

[antonfirsov]

1. The HttpResponseMessage.Content has been read to the end (or no content was returned at all),

My main concern with the proposal is with the code that would be responsible to watch this citeria. There are two options implementation-wise: to add a layer of indirection that wraps the response's HttpContent and Stream in a way similar to the current workaround or to code the EOF detection into all response stream implementations of individual handlers.

There are several concerns with this:

• The implementing handler is responsible for the creation of the response HttpContent and Stream, meaning that this feature won't work OOB for arbitrary external handlers (eg. WinHttpHandler), only for ones which explicitly implement support. Are we OK with this?
• Unless we want other teams to be involved, for mobile and wasm we would most likely implement this by adding an indirection. Are we OK paying the runtime costs on those platforms?
• For SocketsHttpHandler we can implement this by hooking EOF detection into all response stream implementations. This is the primary reason why I'm calling option (3) expensive/complex in Add request metrics to System.Net.Http #85447 (comment). Do we think the feature is worth the added complexity?

[antonfirsov]

Another, even more worrying concern:

or no content was returned at all

We don't have an abstraction for "no content". There is HttpContent being returned containing an empty stream. I don't see a way to detect this case from outside of the implementing handler. We could in theory require empty streams to return Stream.Length == 0, but that would throw for non-empty streams that don't have a length.

[karelz]

Triage: Not needed for Metrics anymore. Might be useful in general - moving to Future.