tmenier / Flurl

Fluent URL builder and testable HTTP client for .NET
https://flurl.dev
MIT License
4.16k stars 385 forks source link

No way to provide my own HttpClient or HttpMessageHandler #801

Open alasdaircs opened 7 months ago

alasdaircs commented 7 months ago

I hope it's me being stupid, but I can't work out how to provide my own HttpMessageHandler. My use-case is I have created an implementation of HttpMessageHandler that sends and receives over Azure Relay - it's not doing conventional socket-based stuff directly. I can also see how I might want to use e.g. MockHttp which also replaces the stock HttpMessageHandler in the HttpClient.

I can see the IFlurlClientFactory appears to be what I want to implement, but the public API doesn't let me set the IFlurlClientFactory _factory member variable inside FlurlClientBuilder. In fact it's either a DefaultFlurlClientFactory or it's a SocketsHandlerFlurlClientFactory, and neither of them will work for me. Short of lots of nasty reflection, I think 4.0 has lost the ability that 3.x had to customise the creation of the HttpMessageHandler and that's a show-stopper for me.

alasdaircs commented 7 months ago

I'm working on a PR to fix this. Hopefully ready weekly next week. I'm adding a method to IFlurlClientBuilder to allow setting the _factory and fixing up other bits for consistency.

tmenier commented 7 months ago

This is not a bug, the change was intentional. You have a couple possible work-arounds:

  1. Implement your handler as a DelegatingHandler and use IFlurlClientBuilder.AddMiddleware. If you simply avoid calling base.SendAsync, then it's effectively the same behavior as if it were the inner-most handler.
  2. Use the FlurlClient constructor that takes an HttpClient arg. Then you can wire it up however you want.

I'll try and explain my reasons for the change. In 3.x and earlier, if you wanted to configure something that lives on HttpClientHandler, such as a certificate or proxy (very common scenarios), the answer was to create and register a custom factory. Now the way to do it is with a fluent one-liner via ConfigureInnerHandler. Not only is that easier, but factories were actually a little dangerous. Flurl needs to do some configuring of its own to that inner handler in order for some of its features to work (namely cookies and auto-redirects), and if you weren't careful you could inadvertently break those things.

I always had it in the back of my mind that I'd be taking away the ability to provide an entirely new inner handler (except via option 2 above), and I wondered if and when someone would have a legitimate use case to do that in Flurl. You could say that day arrived sooner than I expected. :) I'm willing to look at your PR and give it some thought, but I do still think the use cases are quite rare and I have the concern that making it "too easy" could steer some people in the wrong direction, if that makes sense.

lennartb- commented 7 months ago

Could you clarify how to use a custom FlurlClient? For our tests, we previously used the HttpClient created by a WebApplicationFactory, which we integrated with FlurlHttp.GlobalSettings.FlurlClientFactory = new TestServerFlurlClientFactory(httpClient) (where TestServerFlurlClientFactory did nothing except returning return new FlurlClient(httpClient)).

I can't derive from the migration guide how to replicate that behavior.

Edit: Having looked a bit more, it seems that the primary thing we used the Factory was to define a BaseAddress for all tests. I.e. we did (simplified) new Flurl.Url("/v0/someEndpoint).WithOAuthBearerToken("token").GetJsonAsync<SomeDto>() (the base address getting automatically prefixed)

I would have assumed that I simply need to configure the base address:

FlurlHttp.Clients.WithDefaults(builder => builder.ConfigureHttpClient(client => client.BaseAddress = httpClient.BaseAddress));

and could then use clientless operations:

"/v0/test".GetJsonAsync<SomeDto>()

However that fails, because the base address is not prefixed.

tmenier commented 7 months ago

@lennartb- I would expect what you did with FlurlHttp.Clients.WithDefaults... to work as you're expecting. You might be on to an unrelated bug here. If you can post that to a new issue, I can look into it as a high priority. Thanks.

lennartb- commented 7 months ago

Thanks, I'll try to repro it tomorrow in an isolated project and report back

tmenier commented 7 months ago

@lennartb- I just confirmed that you uncovered a bug. I'll get it logged and fixed. Thank you.

tmenier commented 7 months ago

@lennartb- Fixed & released: #803

lennartb- commented 7 months ago

Thank you, now the base address gets prefixed as expected. However we still can't seem to configure the default clients correctly, the base address doesnt seem to be enough. If we use the clientless pattern, all calls fail with e.g.

Flurl.Http.FlurlHttpException : Call failed. No connection could be made because the target machine actively refused it. (localhost:80): POST http://localhost/v0/OurEndpoint/resource

But, If I use a FlurlClient with the HttpClient ctor, the calls get through as normal:

var client = new FlurlClient(httpClient);
await client.Request("v0/OurEndpoint/resource").GetJsonAsync<OurDto>();

Now, this would be an okay workaround for us, but we'd still have to update our tests to use an explicit client instead of the (very comfortable) clientless extension methods.

I've compared both objects (client from the clientless builder and our manual client) and can't find any obvious differences., all properties seem to be equivalent.

Maybe it's related to the WebApplicationFactory? Our initialization for tests looks basically like this:

 var webApplicationFactory = new WebApplicationFactory<Startup>()
     .WithWebHostBuilder(
         builder =>
         {
             builder
                 .ClearConfigurationSources()               
                 .ConfigureTokenSigningKey("OurKey")              
                 .ConfigureTestServices(_ => {} ); //Some additional services
         });
var  httpClient = webApplicationFactory.CreateClient();

//...
lennartb- commented 7 months ago

Managed to get it to work, it feels a bit hacky but is not too bad:

 FlurlHttp
     .ConfigureClientForUrl(string.Empty)
     .ConfigureHttpClient(client => client.BaseAddress = webApplicationFactory.ClientOptions.BaseAddress)
     .AddMiddleware(() => new TestServerMessageHandler(webApplicationFactory.Server));

where TestServerMessageHandler is

 private class TestServerMessageHandler(Microsoft.AspNetCore.TestHost.TestServer testServer) : DelegatingHandler
 { 
     protected override Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
     {
         InnerHandler?.Dispose();
         InnerHandler = testServer.CreateHandler();
         return base.SendAsync(request, cancellationToken);
     }
 }

Disposing and replacing the InnerHandler with the "real" handler from the WebApplicationFactory was the key.

tmenier commented 7 months ago

@lennartb- Excellent, I'm glad to hear that worked! It probably would have taken me a bit of time to arrive at that as well, what exactly WebApplicationFactory does under the hood was still a bit of a mystery to me so I learned something here too. If you want to get more visibility of this technique, you might consider posting it here. (My answer there still holds but yours could start with "If you want to use the clientless pattern..." or something.)

I do have a longer-term goal of releasing a Flurl companion library for ASP.NET Core. It would likely include extensions for integration testing, MS's DI container, and Polly. Should make things like this easier. I wasn't planning on covering clientless in testing...but maybe.

daniloak commented 3 months ago

I tried @lennartb- solution, if I call more than one request I get This instance has already started one or more requests. Properties can only be modified before sending the first request

I ended up using .Request

demian-floreani commented 2 months ago

I tried @lennartb- solution, if I call more than one request I get This instance has already started one or more requests. Properties can only be modified before sending the first request

I ended up using .Request

as in _client.Request(url); ?

I am also getting this message when calling multiple requests

daniloak commented 2 months ago

I tried @lennartb- solution, if I call more than one request I get This instance has already started one or more requests. Properties can only be modified before sending the first request I ended up using .Request

as in _client.Request(url); ?

I am also getting this message when calling multiple requests

Yes

        _client = new FlurlClient(appFactory.CreateClient()).WithSettings(settings =>
        {
             ..... whatever settings if you need
        });

Later

_client.Request(url)