Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[API Proposal]: Streaming HTTP POST API for the System.Net.Http.Json extensions #101564

Open
IEvangelist opened this issue Apr 25, 2024 · 5 comments
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Text.Json untriaged New issue has not been triaged by the area owner

Comments

@IEvangelist
Copy link
Member

Background and motivation

As a follow up to the related API proposal #87577, this aims to include Post* APIs to the HttpClientJsonExtensions class as extensions methods.

API Proposal

namespace System.Net.Http.Json;

public static partial class HttpClientJsonExtensions
{
    [RequiresUnreferencedCode(HttpContentJsonExtensions.SerializationUnreferencedCodeMessage)]
    [RequiresDynamicCode(HttpContentJsonExtensions.SerializationDynamicCodeMessage)]
    public static IAsyncEnumerable<TBodyValue?> PostAsJsonAsyncEnumerable<TBodyValue, TResponseValue>(
        this HttpClient client,
        [StringSyntax(StringSyntaxAttribute.Uri)] string? requestUri,
        TBodyValue value,
        JsonSerializerOptions? bodyOptions = null,
        JsonSerializerOptions? responseOptions = null,
        CancellationToken cancellationToken = default) { }

    [RequiresUnreferencedCode(HttpContentJsonExtensions.SerializationUnreferencedCodeMessage)]
    [RequiresDynamicCode(HttpContentJsonExtensions.SerializationDynamicCodeMessage)]
    public static IAsyncEnumerable<TBodyValue?> PostAsJsonAsyncEnumerable<TBodyValue, TResponseValue>(
        this HttpClient client,
        Uri? requestUri,
        TBodyValue value,
        JsonSerializerOptions? bodyOptions = null,
        JsonSerializerOptions? responseOptions = null,
        CancellationToken cancellationToken = default) { }

    [RequiresUnreferencedCode(HttpContentJsonExtensions.SerializationUnreferencedCodeMessage)]
    [RequiresDynamicCode(HttpContentJsonExtensions.SerializationDynamicCodeMessage)]
    public static Task<HttpResponseMessage> PostAsJsonAsyncEnumerable<TValue>(
        this HttpClient client,
        [StringSyntax(StringSyntaxAttribute.Uri)] string? requestUri,
        TBodyValue value,
        CancellationToken cancellationToken) { }

    [RequiresUnreferencedCode(HttpContentJsonExtensions.SerializationUnreferencedCodeMessage)]
    [RequiresDynamicCode(HttpContentJsonExtensions.SerializationDynamicCodeMessage)]
    public static IAsyncEnumerable<TBodyValue?> PostAsJsonAsyncEnumerable<TBodyValue, TResponseValue>(
        this HttpClient client,
        Uri? requestUri,
        TBodyValue value,
        CancellationToken cancellationToken) { }

    public static IAsyncEnumerable<TBodyValue?> PostAsJsonAsyncEnumerable<TBodyValue, TResponseValue>(
        this HttpClient client,
        [StringSyntax(StringSyntaxAttribute.Uri)] string? requestUri,
        TBodyValue value,
        JsonTypeInfo<TBodyValue> jsonBodyTypeInfo,
        JsonTypeInfo<TResponseValue> jsonResponseTypeInfo,
        CancellationToken cancellationToken = default) { }

    public static IAsyncEnumerable<TBodyValue?> PostAsJsonAsyncEnumerable<TBodyValue, TResponseValue>(
        this HttpClient client,
        Uri? requestUri,
        TBodyValue value,
        JsonTypeInfo<TBodyValue> jsonBodyTypeInfo,
        JsonTypeInfo<TResponseValue> jsonResponseTypeInfo,
        CancellationToken cancellationToken = default) { }

API Usage

var client = httpClientFactory.Create("openai");

var prompt = new Prompt();

await foreach (var response in client.PostAsJsonAsyncEnumerable<Prompt, ChatResponse>(
    requestUri: "/openai/chat",
    bodyValue: prompt,
    bodyJsonTypeInfo: GeneratedSerializerContext.Default.Prompt,
    responseJsonTypeInfo: GeneratedSerializerContext.Default.ChatResponse))
{
    // TODO: handle each response
}

public record Prompt /* Omitted for brevity */;
public record ChatResponse /* Omitted for brevity */;

[JsonSerializable(typeof(Prompt))]
[JsonSerializable(typeof(ChatResponse))]
internal sealed partial class GeneratedSerializerContext
    : JsonSerializerContext
{
}

Alternative Designs

No response

Risks

No response

@IEvangelist IEvangelist added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Apr 25, 2024
@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Apr 25, 2024
Copy link
Contributor

Tagging subscribers to this area: @dotnet/area-system-text-json, @gregsdennis
See info in area-owners.md if you want to be subscribed.

@eiriktsarpalis
Copy link
Member

There's no precedent in us using two generic parameters for the request and response types, the existing POST methods simply return an HttpResponseMessage. I wasn't involved in that design, but I'm guessing it was done to keep the APIs simple.

If I'm honest, I think the existing HttpContent.ReadFromJsonAsAsyncEnumerable methods are the ideally suited workaround which work for any request type:

RequestModel model = ...;
HttpResponseMessage response = await client.PostAsJsonAsync("/openai/chat", model);
await foreach (ResponseModel item in response.ReadFromJsonAsAsyncEnumerable<ResponseModel>())
{
    ...
}

@MihaZupan
Copy link
Member

On the other hand, that sample demonstrates how easy it is to use it wrong.
The PostAsJsonAsync call will buffer the whole response, meaning you won't actually get streaming semantics.

This would preserve the goal of streaming the response, but also with the difference that you now don't enforce the timeout/response size limits.

var request = new HttpRequestMessage(HttpMethod.Post, "/openai/chat")
{
    Content = JsonContent.Create(model)
};
using HttpResponseMessage response = await client.SendAsync(request, HttpCompletionOption.ResponseHeadersRead);
await foreach (ResponseModel item in response.ReadFromJsonAsAsyncEnumerable<ResponseModel>())
{
    ...
}

cc: @dotnet/ncl

@JohnGalt1717
Copy link

JohnGalt1717 commented Apr 25, 2024

Does this help? It's my shot at it, and it appears to not buffer and stream.

public static async IAsyncEnumerable<TResponse> PostFromJsonAsAsyncEnumerable<TRequest, TResponse>(
    this HttpClient client,
    string requestUri,
    TRequest request,
    [EnumeratorCancellation] CancellationToken cancellationToken = default) 
{
  var requestMessage = new HttpRequestMessage(HttpMethod.Post, requestUri);
  
  using var ms = new MemoryStream();
  await JsonSerializer.SerializeAsync(ms, request, JsonSerializerSettings.Options, cancellationToken: cancellationToken);
  ms.Position = 0;
  
  using var requestContent = new StreamContent(ms);
  requestMessage.Content = requestContent;
  
  requestContent.Headers.ContentType = new System.Net.Http.Headers.MediaTypeHeaderValue("application/json");
  requestMessage.Headers.Add("Accept", "application/json");
  
  using var response = await client.SendAsync(requestMessage, HttpCompletionOption.ResponseHeadersRead, cancellationToken);
  
  response.EnsureSuccessStatusCode();

  var items = response.Content.ReadFromJsonAsAsyncEnumerable<TResponse>(cancellationToken);

  await foreach (var item in items)
  {
	if (cancellationToken.IsCancellationRequested)
		throw new TaskCanceledException();
	if (item is null)
		continue;

	yield return item;
  }
}

@eiriktsarpalis
Copy link
Member

The PostAsJsonAsync call will buffer the whole response, meaning you won't actually get streaming semantics.

Perhaps then the answer is to expose overloads to the existing HttpClientJsonExtensions methods accepting HttpCompletionOptions?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Text.Json untriaged New issue has not been triaged by the area owner
Projects
None yet
Development

No branches or pull requests

4 participants