Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

No way to provide my own HttpClient or HttpMessageHandler #801

Open
alasdaircs opened this issue Jan 12, 2024 · 13 comments
Open

No way to provide my own HttpClient or HttpMessageHandler #801

alasdaircs opened this issue Jan 12, 2024 · 13 comments

Comments

@alasdaircs
Copy link

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 alasdaircs added the bug label Jan 12, 2024
@alasdaircs
Copy link
Author

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
Copy link
Owner

tmenier commented Jan 12, 2024

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.

@tmenier tmenier added enhancement and removed bug labels Jan 12, 2024
@lennartb-
Copy link

lennartb- commented Jan 17, 2024

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
Copy link
Owner

tmenier commented Jan 17, 2024

@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-
Copy link

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

@tmenier
Copy link
Owner

tmenier commented Jan 17, 2024

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

@tmenier
Copy link
Owner

tmenier commented Jan 17, 2024

@lennartb- Fixed & released: #803

@lennartb-
Copy link

lennartb- commented Jan 18, 2024

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-
Copy link

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
Copy link
Owner

tmenier commented Jan 18, 2024

@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
Copy link

daniloak commented May 15, 2024

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
Copy link

demian-floreani commented Jun 26, 2024

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
Copy link

daniloak commented Jun 26, 2024

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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants