Runtime: [Json] System.Net.Http.Json Post method miss overloads for reading content

[kant2002]

Previously I use

CreatePersonResponseModel response = await Http.PostJsonAsync<CreatePersonResponseModel>("/persons", this.model);

To get information from the server, and CreatePersonResponseModel was type of result. and this.model was object or so.

Now

HttpResponseMessage response = await Http.PostAsJsonAsync<CreatePersonResponseModel>("/persons", this.model);

And there no way to read JSON content. This seems to be just moving bits around, and make utility classes slightly useful

Is it possible make public

private static async Task<T> GetFromJsonAsyncCore<T>(Task<HttpResponseMessage> taskResponse, JsonSerializerOptions options, CancellationToken cancellationToken);

this allow have

HttpResponseMessage response = await Http.PostAsJsonAsync("/persons", this.model).GetFromJsonAsyncCore<CreatePersonResponseModel>();

or even better.

public static Task<TResponse> PostAsJsonAsync<TValue, TResponse>(this HttpClient client, Uri? requestUri, TValue value, JsonSerializerOptions? options = null, CancellationToken cancellationToken = default(CancellationToken))

/cc @SteveSandersonMS or who else might be interested in this.

[davidsh]

@karelz @Jozkee

[stephentoub]

And there no way to read JSON content.

C# CreatePersonResponseModel model = await response.Content.ReadFromJsonAsync<CreatePersonResponseModel>();
?

[kant2002]

technically yes. But overall idea if I understand correctly is to GET/POST/PUT date in simple fashion. It was quite confusing migrate from Preview 2. Maybe worth mentioning in the upgrade guide if this is intended.

[Jozkee]

So the ask here is to make PostAsJsonAsync and PutAsJsonAsync methods to directly deserialize the HttpResponseMessage's content?

I think that when you send a Post request, the server does not necessarily will respond with a JSON so I don't know how common is that scenario.

Using the previous APIs (PostJsonAsync), what happens when the response's content-type is not application/json?

@rynowak @terrajobst

[davidsh]

I think that when you send a Post request, the server does not necessarily will respond with a JSON so I don't know how common is that scenario.

Servers rely on HTTP request headers to decide what format to send back as the response. The 'Accept' request header will specify something like 'application/json' when it sends the request to the server. Then the server knows what is suitable to send back to the client.

[kant2002]

For me this API appear half-backed.

Previous

• Previous API was to communicate with HTTP API which use JSON as in/out format. Symmetric, easy to remember.
• Assume no fancy content negotiation happens.
• For mixed scenarios, was suggested go directly with HttpClient. This make sense. At least this was explicitly stated in public announcements.

Current

• Assume that GET out data in JSON, but for POST/PUT only in data assumed to be JSON. Not symmetric in my opinion.
• I can upload files, and most likely in data require customization in the in part, and not out.

All of that can be just out of confusion how to properly use API and for what scenarios new API intended.

[ericstj]

I think that when you send a Post request, the server does not necessarily will respond with a JSON so I don't know how common is that scenario.

Using the previous APIs (PostJsonAsync), what happens when the response's content-type is not application/json?

I believe most of these convenience APIs assume the server will behave in a particular way and then throw when it doesn't. Same case could be true with a Get. If we can make these APIs be overloads (and not ambiguous) then it seems a reasonable request.

Why were they omitted? I remember discussion around this but can't recall the conclusion.

[scalablecory]

Many (most?) JSON APIs support some sort of error feedback that is not directly encoded in a successful response's object model -- either using e.g. "404" response with no content, or still JSON but with a different object model.

We discussed this briefly in API review and decided to leave it for future design. I think the challenge is to identify strong use cases for this.

[ericstj]

Many (most?) JSON APIs support some sort of error feedback that is not directly encoded in a successful response's object model -- either using e.g. "404" response with no content, or still JSON but with a different object model.

Doesn't that same concern apply to all Get methods we already expose?

[scalablecory]

Doesn't that same concern apply to all Get methods we already expose?

Yes. I would not recommend using them outside of very trivial APIs or in toy code.

99% of the time I'd use something that captures the data those methods throw away in the non-success paths:

```c#
async Task ProcessResponse(HttpResponseMessage response)
{
if(response.IsSuccessStatusCode)
{
return await response.Content.ReadAsAsync();
}

if(response.StatusCode == 404)
{
    return null;
}

if(response.StatusCode == Some500StatusWellDefinedByAPI)
{
    // might contain more specific error messages, a model dictionary of validation errors, etc.
    var errorInfo = await response.Content.ReadAsAsync<ErrorResponseObject>();
    throw CreateExceptionFromErrorInfo(errorInfo);
}

throw CreateUnknownError(response.StatusCode);

}
```

[ericstj]

So if the same concern applies to Get, and we have Get methods, then we can't apply that concern as a reason not to have the Post methods, right? Or am I missing something?

[rynowak]

A few thoughts from my POV...

---

Ultimately the proposal that was implemented was a combination of two things:

• The functionality available as part of the WebAPI client (using Newtonsoft.Json)
• The set of functionality that was in Blazor that we felt like could be easily agreed that it belongs in in a System. package

This feedback seems to be about something that was in Blazor that wasn't included in the proposal.

---

So why didn't we include this functionality in the proposal for dotnet/runtime? Well simply put we wanted to avoid asking for too many options/opinions, because all of this functionality is additive.

Example: right now you can write the thing being asked for as:

```C#
var response = await Http.PostAsJsonAsync("/persons", this.model);
response.EnsureSuccessStatusCode();
var person = await response.Content.ReadAsJsonAsync();

We could decide to add an accelerator for this in the future, but it would be additive:

```C#
var person = await Http.PostAsJsonAsync<CreatePersonResponseModel, CreatePersonResponseModel>("/persons", this.model); // TRequest, TResponse are the same in this case

I didn't propose this, because adding just the API listed in the example of above would feel incomplete. In short, we have added APIs that read, and APIs that write, but we didn't propose APIs that both read and write in a single method call.

You can add them, but there are more overloads/flavors/opinions that are necessary to do so.

Regardless of what accelerators we think need to exist, we should definitely document the basic recipes in detail. Users will always have use-cases that fall out of what we provide convenience APIs for.

---

The APIs that were proposed are really simple to describe and understand - let's look at fleshing this out more and see which ones seem like they are easy to use, describe and understand vs writing the 3 lines of code this takes without them. I'm going to describe all of these with POST. PUT is equally applicable.

There's a few classes of methods that could exist - classified by what they serialize, what they deserialize, and what they do with errors:

• Is there a serialized request payload or not? (TRequest)
• Is there a deserialized response payload or not? (TResponse) (in some cases its the same as TRequest)
• What do you do with errors? Do you throw? Do you attempt to read some different data type? (in case you're wondering, ASP.NET Core provides a standard error type that will be returned from most 400 responses).

Here's a table and some notes:

| TRequest | TResponse | TError | Notes |
|---------- |------------------------------ |-------- |----------------------------------------------------------------------------------------------------------- |
| Yes | No (void) | No | This is satisfied by the existing accelerator. |
| Yes | No (void) | Yes | It's somewhat rare that an API would return a value on failure but not on success |
| Yes | Yes (same as request) | No | This is a common case for "Create" APIs with ASP.NET Core that will return 201/202 and a response object. |
| Yes | Yes (same as request) | Yes | This is a common case for "Create" APIs with ASP.NET Core that will return 201/202 and a response object. |
| Yes | Yes (different from request) | No | This is a common case for general use. |
| Yes | Yes (different from request) | Yes | This is a common case for general use. |
| No | No (void) | No | Since there is no serialization, there's no need for an accelerator. |
| No | No (void) | Yes | It's somewhat bizzare that an API would return a value on failure but not on success |
| No | Yes | No | It's pretty rare to have an "empty" post in a REST API. |
| No | Yes | Yes | It's pretty rare to have an "empty" post in a REST API. |

So if we looked at the things that I think are pretty common in that table the API additions would look like (ignoring permutations like string vs Uri, JsonSerializerOptions, and CancelationTokens:

```C#
// Methods that throw when an error is encountered.
public static Task PostAsJsonAsync(this HttpClient client, string uri, TRequest request);
public static Task PostAsJsonAsync(this HttpClient client, string uri);

// Methods that attempt to deserialize an error.
// All of these really want discriminated unions.
public static Task<(TResponse, TError>) PostAsJsonAsync(this HttpClient client, string uri, TRequest request);
public static Task<(TResponse, TError)> PostAsJsonAsync(this HttpClient client, string uri);

Does functionality like this get classified as *too opinionated*? Similar to the code that @scalablecory provided:

```txt
- Send an HTTP POST Request
- Serialize this `TRequest` object into the message body
- If the response is a 2XX and JSON then:
  - Read the response body as a `TResponse`
- If the response is another status code and JSON then:
  - Read the response body as a `TError`
- Else throw

This assuming that I'm right about what's common and what's not. I did not add a method for the TRequest == TResponse case because that can be handled by the method with two generic parameters - adding it causes a conflict with the currently defined (and most general) method.

One way that I've seen this resolved in some other libraries (Microsoft.Rest) is that they use different method names for the ones that return HttpResponseMessage-likes, so that there's no ambiguity about who's responsible for serialization.

[ericstj]

@Jozkee @terrajobst can you please have a look here and consider @rynowak's analysis? This is precisely the type of feedback we're looking to consider with our preview of this package.

[rynowak]

If we want to talk about this more, I'm game. I also want to amend something I said (based on re-reading).

It's somewhat bizzare that an API would return a value on failure but not on success

I think I was wrong about this. Consider the case of a DELETE. I think there are probably plenty of cases where you return nothing on success, but have the potential to return an error object on failure.

[Jozkee]

As mentioned in https://github.com/dotnet/runtime/issues/32937#issuecomment-592616574, we might not need TError on methods that does not directly return a HttpResponsemessage, instead we should throw on responses that are not 2xx; that is how GetFromJsonAsync works.

[Jozkee]

As @rynowak suggested, this could be shaped as the following (extended to the remaining overloads):

public static Task<TResponse> PostAsJsonAsync<TRequest, TResponse>(this HttpClient client, string? requestUri, TRequest value, JsonSerializerOptions? options = null, CancellationToken cancellationToken = default);
public static Task<TResponse> PostAsJsonAsync<TRequest, TResponse>(this HttpClient client, string? requestUri, TRequest value, CancellationToken cancellationToken);
public static Task<TResponse> PostAsJsonAsync<TRequest, TResponse>(this HttpClient client, Uri? requestUri, TRequest value, JsonSerializerOptions? options = null, CancellationToken cancellationToken = default);
public static Task<TResponse> PostAsJsonAsync<TRequest, TResponse>(this HttpClient client, Uri? requestUri, TRequest value, CancellationToken cancellationToken);

The implementation is probably going to look something like this:

public static Task<TResponse> PostAsJsonAsync<TRequest, TResponse>(this HttpClient client, string? requestUri, TRequest value, JsonSerializerOptions? options = null, CancellationToken cancellationToken = default)
{
    return PostAsJsonAsyncCore<TRequest, TResponse>(client, requestUri, value, options, cancellationToken);
}

private static async Task<TResponse> PostAsJsonAsyncCore<TRequest, TResponse>(HttpClient client, string? requestUri, TRequest value, JsonSerializerOptions? options, CancellationToken cancellationToken)
{
    using (HttpResponseMessage response = await client.PostAsJsonAsync(requestUri, value, options, cancellationToken))
    {
        response.EnsureSuccessStatusCode();
        return await response.Content.ReadFromJsonAsync<TResponse>();
    }
}

• We will have to add the equivalet set of signatures for Put methods
• We will need to rename TValue to TRequest in the existing Post and Put methods
• and rename to TResponse in the Get methods.

All of this assuming that we agree on adding this set of APIs.

[Jozkee]

For what is worth, having this set of APIs could also provide an alternative for users from having to dispose the HttpResponseMessage themselves. https://github.com/dotnet/runtime/pull/33459#discussion_r391207191

[velocitytoo]

Yep, API is totally half baked. Look at .NET ServiceStack or any normal, popular Javascript HTTP library. They all will accept a request object that is completely different from the response object. For a GET request, the request object is serialized in a common format on the url string, again ServiceStack, Angular, etc. all work this way and all support it. For a POST, it is very common for the returned data schema to be different than the request schema, which means two completely separate .NET POCOs for the request and response. I would totally expect an extension method from System.Net.Http.Json for Post as:

HttpClient.PostAsJsonAsync(...)

but no such luck. Please add this ASAP, as we are having to add extension methods for the half baked solution of System.Net.Http.Json.

[SteveSandersonMS]

@velocitytoo You can send and receive different types:

var response = await Http.PostAsJsonAsync("some/url", myOutgoingValueOfAnyType);
var deserializedResult = response.Content.ReadFromJsonAsync<SomeOtherType>();