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

@SteveSandersonMS

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.

@karelz

@karelz @jozkee

And there no way to read JSON content.

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

?

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.

@rynowak

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

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.

For me this API appear half-backed.

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.

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.

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.

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?

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:

async Task<SuccessResponseObject?> ProcessResponse(HttpResponseMessage response)
{
    if(response.IsSuccessStatusCode)
    {
        return await response.Content.ReadAsAsync<SuccessResponseObject>();
    }

    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);
}

@jozkee

@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.

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.

As mentioned in #32937 (comment), 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.

@rynowak

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.

For what is worth, having this set of APIs could also provide an alternative for users from having to dispose the HttpResponseMessage themselves. #33459 (comment)

@velocitytoo You can send and receive different types:

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

I agree that this should be added; I was surprised to find out that no such overload exists already.

+1, think this would be greatly useful when using typed JSON request and response

and please also add support for PUT and PATCH when considering POST

Navigation Menu

Repository navigation

[System.Net.Http.Json] Post method miss overloads for reading content #34157

Previous

Current

2 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

[System.Net.Http.Json] Post method miss overloads for reading content #34157

Description

Activity

davidsh commented on Mar 26, 2020

stephentoub commented on Mar 26, 2020

kant2002 commented on Mar 26, 2020

jozkee commented on Mar 26, 2020

davidsh commented on Mar 26, 2020

kant2002 commented on Mar 27, 2020

Previous

Current

ericstj commented on Mar 31, 2020

scalablecory commented on Mar 31, 2020

ericstj commented on Apr 1, 2020

scalablecory commented on Apr 1, 2020

2 remaining items

ericstj commented on Apr 8, 2020

rynowak commented on Apr 8, 2020

jozkee commented on Apr 8, 2020

jozkee commented on Apr 8, 2020

jozkee commented on Apr 8, 2020

SteveSandersonMS commented on Sep 28, 2020

aradalvand commented on Oct 7, 2023

WeihanLi commented on May 20, 2024

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions