Pathoschild / FluentHttpClient

A modern async HTTP client for REST APIs. Its fluent interface lets you send an HTTP request and parse the response in one go.
MIT License
342 stars 52 forks source link

Error by using WithBody instear of WithBodyContent #88

Closed WebDucer closed 5 years ago

WebDucer commented 5 years ago

Hello.

We have the following code to get a token from an API with version 3.2:

client.PostAsync("token")
    .WithBodyContent(new FormUrlEncodedContent(new Dictionary<string,string>{
        {"grant_type","password"},
        {"username", "user"},
        {"password", "user password"}
    }))
    .As<AccessToken>();

This works fine. Now with version 3.3 WithBodyContent is obsolet an I tried to replace this with WithBody.

client.PostAsync("token")
    .WithBody(new FormUrlEncodedContent(new Dictionary<string,string>{
        {"grant_type","password"},
        {"username", "user"},
        {"password", "user password"}
    }))
    .As<AccessToken>();

But now I get BadRequest instead of the token. I think, FormUrlEncodedContent type is not handled properly.

Jericho commented 5 years ago

I too struggled a bit to figure out how to send FormUrlEncoded content using the new syntax in the latest version. The new syntax is not very discoverable but once you figure it out, it's pretty simple.

Try this:

client.PostAsync("token")
    .WithBody(bodyBuilder => bodyBuilder.FormUrlEncoded(new Dictionary<string,string>{
        {"grant_type","password"},
        {"username", "user"},
        {"password", "user password"}
    }))
    .As<AccessToken>();
WebDucer commented 5 years ago

@Jericho Hello. I tried this, but get the same BadRequest exception, as soon as I use .WithBody(builder => builder.bodyBuilder.FormUrlEncoded(parameters)) instead of .WithBodyContent(new FormUrlEncodedContent(parameters)).

Pathoschild commented 5 years ago

There's some bugs in the way form-url-encoded content is handled; I'll release a fix for those soon. In the meantime, passing a non-dictionary should work fine:

await client.PostAsync("token")
    .WithBody(builder => builder.FormUrlEncoded(new
    {
        grant_type = "password",
        username = "user",
        password = "user password"
    }))
    .As<Token>();
Pathoschild commented 5 years ago

The logic was technically correct, but it didn't support some intuitive ways of using the client. That's fixed in the upcoming 3.4:

Here's a preview build if you want to try it out. Feedback is welcome if you can think of other ways to make it more intuitive!

Jericho commented 5 years ago

@Pathoschild my suggestion to make it more intuitive would be to add extension methods to IRequest that would invoke the appropriate method against the body builder. Something similar to this:

public static IRequest WithFormUrlEncodedBody(this IRequest request, IEnumerable<KeyValuePair<string, object>> parameters)
{
    return request.WithBody(bodyBuilder => bodyBuilder.FormUrlEncoded(parameters));
}

Probably multiple overloaded extension methods would be required in order to handle various parameter types.

Pathoschild commented 5 years ago

The idea is for the body builder to support multiple formats and be a clean place for custom extensions, without cluttering the root level. Currently there's one WithBody method in root IntelliSense:

client.PostAsync("token").With
    ┌────────────────────────────┐
    │  WithArgument              │
    │  WithArguments             │
    │  WithBasicAuthentication   │
    │  WithBearerAuthentication  │
    │  WithBody                  │
    │  WithCancellationToken     │
    │  WithCustom                │
    │  WithHeader                │
    │  WithOptions               │
    │  WithRequestCoordinator    │
    └────────────────────────────┘

My concern is that adding those extensions would make the root IntelliSense more cluttered and harder to read as we add more formats:

client.PostAsync("token").With
    ┌────────────────────────────┐
    │  WithArgument              │
    │  WithArguments             │
    │  WithBasicAuthentication   │
    │  WithBearerAuthentication  │
    │  WithBody                  │
    │  WithCancellationToken     │
    │  WithCustom                │
    │  WithFormUrlEncodedBody    │
    │  WithHeader                │
    │  WithJsonBody              │
    │  WithOptions               │
    │  WithRequestCoordinator    │
    │  WithSoapBody              │
    │  WithXmlBody               │
    │  ...                       │
    └────────────────────────────┘

In contrast, with the builder approach:

client.PostAsync("token").WithBody(p => p.
    ┌────────────────────┐
    │  FormUrlEncoded    │
    │  Json              │
    │  Model             │
    │  Soap              │
    │  Xml               │
    │  ...               │
    └────────────────────┘
WebDucer commented 5 years ago

@Pathoschild I like the clean and short approach with only .WithBody. The current way with HTTPContent as paramater (if I already have the content) and body builder for more extensible way is the perfect way.

A good documentation, how to use and to extend would help to handle cutting ages by the developers itself.

I tried the preview build in my scenario and it works great (both, direct http content and dictionary in builder).

Pathoschild commented 5 years ago

@WebDucer Thanks! I improved the examples under 'HTTP body' in the readme; how does that look?

WebDucer commented 5 years ago

@Pathoschild Looks good for me.