Support for other formatters/serializers

[AndreaCuneo]

I'd like to use Flurl.Http, in particular for testing, but we're currently using MessagePack formatters in both payloads (send and receive).

Is there a way to use FlurlClient with a deserializer that depends on Content-Type response header and a choosen serializer that correctly set the Content-Type in the request?

If not, is there a way I can hack my way around using IFlurlClient extensions? I was thinking about using the same pattern of ReceiveJson but is there a way to store in the FlurlRequest.Settings some custom serializers?

This is how I use HttpClient right now.

MessagePackFormatter _msgPackFormatter = new MessagePackFormatter();
JsonMediaTypeFormatter _jsonFormatter = new JsonMediaTypeFormatter();
MediaTypeFormatterCollection _formatters = new MediaTypeFormatterCollection();
_formatters.Add(_jsonFormatter);
_formatters.Add(_msgPackFormatter);

using (var client = new HttpClient())
using (var req = new HttpRequestMessage(HttpMethod.Post, url))
{
    client.DefaultRequestHeaders.Accept.Add(new MediaTypeWithQualityHeaderValue(MessagePackFormatter.DefaultMediaType.MediaType, 1.0));
   client.DefaultRequestHeaders.Accept.Add(new MediaTypeWithQualityHeaderValue(JsonMediaTypeFormatter.DefaultMediaType.MediaType, 0.9));
   
   req.Content = new ObjectContent(body, _msgPackFormatter);
   using (var res = await _client.SendAsync(req, HttpCompletionOption.ResponseContentRead, ctk))
   {
       return await res.Content.ReadAsAsync<MyResult>(_formatters, ctk);
   }
}

[tmenier]

Have you read the extensibility section of the docs? Extension methods are the way to go here. I don't think Settings needs to play a role at all. Flurl methods like SendAsync and PostAsync take a raw HttpRequestMessage and return a raw HttpResponseMessage, so those make good building blocks for extensions like PostMessagePackAsync or whatever you want to build. If you need help with the implementation details I'd say ask on Stack Overflow. I can answer (unless someone beats me to it), and it'll get better visibility for others who might want to do something similar.

Just a note on automatically picking a serializer based on headers, etc. I've seen other libraries like RestSharp do that sort of thing but I intentionally decided to require being explicit with Flurl. You know what you want, so just tell it. I find it leaves less room for bugs. (How does Flurl know to add a parameter to the query string rather than form data? Because you called SetQueryParam!) Just a philosophical difference. So doing what you want via IFlurlClient extensions isn't a "hack", it's following Flurl's patterns exactly as intended.

[AndreaCuneo]

I've read it. My reference to "hack" was related to the fact that I cannot really make and extension like the PutJsonAsync(). The underlying implementation of SendJsonAsync takes info out of the HttpCall to get access to the JsonSerializer present in the Settings. I didn't found a way to extend setting or to set another serializer in the FluentClient settings.

Currently I've something like this

public static Task<HttpResponseMessage> PostMsgPackAsync<T>(this IFlurlRequest request, T data, IFormatterResolver formatterResolver, CancellationToken cancellationToken = default, HttpCompletionOption completionOption = HttpCompletionOption.ResponseContentRead)
        {
            var content = new ObjectContent<T>(data, new MessagePackMediaTypeFormatter(formatterResolver));
            return request.SendAsync(HttpMethod.Post, content, cancellationToken, completionOption);
}

The problem with the above is the param IFormatterResolver that I've to pass on each call.

Regarding the "You know what you want, so just tell it." isn't always true in particular with scrapers. This is another requirements of mine. Also, unfortunately, APIs I've to deal with don't really respect the Accept header in case of errors. For example if result is 200 the payload in indeed with the expected Content-Type but if there is a 401 or 403 or 500 they return either plaintext, html or empty. And while it's a violation of the Accept header the client sent, it's quite common out there: they do at least set the Content-Type correctly on the response even if I was not supposed to accept it :-/

I've got another issue with the ReceiveJson pattern since it extend Task<HttpResponseMessage> but I actually know the response format after I handle the response code: often if is a 404 it's "correct" to return null but actually the Deserialize call doesn't like an empty stream as the response neither have a Content-Type header nor a payload (Content-Lengh: 0). But this is another issue.

Aside the above, I currently managed to hack my way using extensions. My definition of "hack" is only related to the fact that for actually extending the library from ser/des point of view I'd really need to extend settings but I didn't find any extension points there. Did I missed them? I'd have liked to have an object bag in the Settings so that I can store Formatters, other serializers, etc. there for my extensions to use. And having a way to access them from the HttpResponseMessage: I tried to emulate the pattern in ReceiveJson extension but it uses internal methods.

[tmenier]

The problem with the above is the param IFormatterResolver that I've to pass on each call.

What determines which IFormatterResolver to use? Is there just one instance globally? One of several depending on a response header? If passing an instance on each call is less than I ideal, then I need a better understanding of the requirements.

unfortunately, APIs I've to deal with don't really respect the Accept header in case of errors.

This is why Flurl favors a try/catch pattern to deal with error responses. For successful calls, you can get all the way to a deserialized body without ever dealing with the raw HttpResponseMessage. But if an error response is returned, you can get at the raw message in the catch block via FlurlHttpException.Call.Response, or just get the body via GetResponseStringAsync().

[AndreaCuneo]

The problem with the above is the param IFormatterResolver that I've to pass on each call.

What determines which IFormatterResolver to use? Is there just one instance globally? One of several depending on a response header? If passing an instance on each call is less than I ideal, then I need a better understanding of the requirements.

You can think of IFormatterResolver as Newtonsof t.Json SerializerSettings. It has a global, sane-default configuration that should work fine in best cases but that can be customized per-api (per-baseurl or per-host depending on versioning strategies in models).

The ideal would be to follow the same flow and capabilities of JsonSerializer as in having (logically) a new MsgPackSerializer in the Settings that can be either configured Globally, per-client or per-request as JsonSerializer is now. That way it can be set as JsonSerializer with different settings if needed. Differently from JsonSeriazer that is provided out of the box by you, this would require an extension to be called either on the static configuration or on the client or on the request (as is now).

client.Configure(s => s.ConfigureMsgPackSerializer(formatter));

As far as possible implementations, I've see you use a neat solution for merging the settings, wiring Settings properties setter and getters to a dictionary of objects. Exposing that underlining dictionary so that extensions devs can add their stuff and making public the extension for reaching the HttpCall instance from the HttpReq/ResMessage would make sense for you?

unfortunately, APIs I've to deal with don't really respect the Accept header in case of errors.

This is why Flurl favors a try/catch pattern to deal with error responses. For successful calls, you can get all the way to a deserialized body without ever dealing with the raw HttpResponseMessage. But if an error response is returned, you can get at the raw message in the catch block via FlurlHttpException.Call.Response, or just get the body via GetResponseStringAsync().

I'm not a fan of using exception handling for stuff that are not exceptions as in you know it will happen and you have all means to check before triggering an expensive exception. Also using 404 as "null" when checking if a resource exists is a positive flow, not an error flow and handing it with a catch in all places where a remote call is done is counterintuitive for the future reader aka "the unlucky mainteiner of that code" :D

But that is is a separate topic from the "serializers" issue I'm facing :) As long as I can add new serializers without having to pass the serializer (or its settings) to the extension method as params but making use of the tiered settings capabilieties, I can use your great library the way I need (and the awesome testing feature).

The main problem I've is that while the extension point of this library is via extensions, those don't have a way to make use of the tiered settings capability nor they can access HttpCall or process the response as ReceiveJson does because it uses internal methods.

[tmenier]

There's some good reasoning here for improvements to the extensibility model. I might create a new issue (this one's starting to read like a novel), but I think some of this is worthy of landing in a future release. Hopefully you can work around the shortcomings for now.

Also, although we may agree to disagree on the try/catch pattern, I think #354 could be a useful enhancement for you.

[tmenier]

It's been a while since I've looked at this issue. Many things were raised over the course of the conversation and I think there might be some ideas worth implementing but I'd like to summarize where we're at and get to the bottom of what's really needed:

1. If you write a custom DoSomethingAsync method per the extension pattern, you're extending IFlurlRequest, which has a Settings property, so I think the suggestion of somehow exposing Settings and HttpCall (which just contains request & response info, and response doesn't exist yet) is already covered. Unless I'm missing something?

2. If you write a custom ReceiveXXX method, you're extending IFlurlResponse (as of #354), which does NOT contain the corresponding request or settings. I'm open to adding one or the other to IFlurlResponse if it's useful here.

3. Adding a custom properties bag to Settings, or just exposing the one that backs the existing settings, is a possibility. Although this idea is different in a lot of ways from #433, both relate to passing "context" around, so it might be smart to have a cohesive plan for implementing and documenting both so knowing which to use when isn't confusing. (More of a note to self.)

4. In general I think adding custom serializers to Settings might be useful only if you're developing a library that extends Flurl for other content types, and you want users to be able to replace the default implementation at the application level. In your PostMsgPackAsync example, I don't think there's a great reason for using Settings to provide the formatter/serializer to the extension method, or for passing it in on every call. If MsgPackSerializer is cheap to create, why not just new one up inside PostMsgPackAsync? Or keep a static instance in the same class? Do you really need to support different implementations of a MessagePack serializer at the application level?

5. Objections to the try/catch pattern that were mentioned should be resolved by #354.

[AndreaCuneo]

First, thanks for looking after this.

1. It has the Settings property exposed indeed, but there is no way to use the tiered (override) Settings by storing custom objects into it.
2. I think what would be best is to expose the HttpCall so that extension developers can mimik what ReceiveJson does. Making GetHttpCall() extension public would suffice, in addition to 3.
3. I think the support for Settings differs from #433 in a lot of ways. First the #433 is about HttpxxxMessage, that is not Flurl. Second it doesn't cover the tiered nature of Settings, where you can set and override Settings from Global to Client to Request.
4. The main point about non using a static MsgPackSerializer is just that different 'hosts' could have different settings for the MsgPackSerializer. The same that would go for Json and why is possible to currently have different JsonSerializer on different FlurlClient/FlurlRequests. So an application communicating with two different APIs might need to set different serializers at Client level. Marking FlurlHttpSettings Get and Set methods as public would be enough. All other tiered and reset capabilities would be fine.
5. #534 is grand, thanks!